Skip to content

Commit 865b609

Browse files
bors[bot]philberty
andauthored
Merge #992
992: Cleanup bad unused code warnings r=philberty a=philberty This patchset contains 4 distinct fixes: When a constant is declared after where it is used the code-generation pass falls back to a query compilation of the HIR::Item this did not contain a check to verify if it was already compiled and results in duplicate CONST_DECLS being generated if query compilation was used. We were using a zero precision integer to contain unit-type expressions this results in VAR_DECLS being lost in the GENERIC graph which does not allow us to perform any static analysis upon the DECL. This changes the unit type to use an empty struct and for initialization of a VAR_DECL we can simply pass an empty constructor and let GCC optimize this code for us. Update our DEAD_CODE scan to take into account modules of items and also respect if structures are prefixed with an underscore we can ignore generating an unused warning. Remove our AST scan for unused code and reuse GCC TREE_USED to track wether VAR_DECL, PARM_DECL, CONST_DECL are actually used or not. We reuse the GCC walk_tree functions to have this as nice separate lint. Fixes #676 Co-authored-by: Philip Herron <philip.herron@embecosm.com>
2 parents 366c533 + 7820ff8 commit 865b609

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

71 files changed

+298
-276
lines changed

gcc/rust/Make-lang.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ GRS_OBJS = \
101101
rust/rust-autoderef.o \
102102
rust/rust-substitution-mapper.o \
103103
rust/rust-lint-marklive.o \
104+
rust/rust-lint-unused-var.o \
104105
rust/rust-hir-type-check-path.o \
105106
rust/rust-compile-intrinsic.o \
106107
rust/rust-compile-pattern.o \

gcc/rust/backend/rust-compile-context.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,11 @@ class Context
307307
return mangler.mangle_item (ty, path);
308308
}
309309

310+
std::vector<tree> &get_type_decls () { return type_decls; }
311+
std::vector<::Bvariable *> &get_var_decls () { return var_decls; }
312+
std::vector<tree> &get_const_decls () { return const_decls; }
313+
std::vector<tree> &get_func_decls () { return func_decls; }
314+
310315
private:
311316
::Backend *backend;
312317
Resolver::Resolver *resolver;

gcc/rust/backend/rust-compile-expr.cc

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1339,18 +1339,22 @@ CompileExpr::visit (HIR::IdentifierExpr &expr)
13391339
Bvariable *var = nullptr;
13401340
if (ctx->lookup_const_decl (ref, &translated))
13411341
{
1342+
TREE_USED (translated) = 1;
13421343
return;
13431344
}
13441345
else if (ctx->lookup_function_decl (ref, &fn))
13451346
{
1347+
TREE_USED (fn) = 1;
13461348
translated = address_expression (fn, expr.get_locus ());
13471349
}
13481350
else if (ctx->lookup_var_decl (ref, &var))
13491351
{
1352+
// TREE_USED is setup in the gcc abstraction here
13501353
translated = ctx->get_backend ()->var_expression (var, expr.get_locus ());
13511354
}
13521355
else if (ctx->lookup_pattern_binding (ref, &translated))
13531356
{
1357+
TREE_USED (translated) = 1;
13541358
return;
13551359
}
13561360
else
@@ -1371,6 +1375,11 @@ CompileExpr::visit (HIR::IdentifierExpr &expr)
13711375
else
13721376
translated = CompileItem::compile (resolved_item, ctx, lookup, true,
13731377
expr.get_locus ());
1378+
1379+
if (translated != error_mark_node)
1380+
{
1381+
TREE_USED (translated) = 1;
1382+
}
13741383
}
13751384
}
13761385

gcc/rust/backend/rust-compile-item.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ CompileItem::visit (HIR::StaticItem &var)
7373
void
7474
CompileItem::visit (HIR::ConstantItem &constant)
7575
{
76+
if (ctx->lookup_const_decl (constant.get_mappings ().get_hirid (),
77+
&reference))
78+
return;
79+
7680
// resolve the type
7781
TyTy::BaseType *resolved_type = nullptr;
7882
bool ok

gcc/rust/backend/rust-compile-resolve-path.cc

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
#include "rust-hir-trait-resolve.h"
2424
#include "rust-hir-path-probe.h"
2525

26+
#include "print-tree.h"
27+
2628
namespace Rust {
2729
namespace Compile {
2830

@@ -117,12 +119,18 @@ ResolvePathRef::resolve (const HIR::PathIdentSegment &final_segment,
117119
// might be a constant
118120
tree constant_expr;
119121
if (ctx->lookup_const_decl (ref, &constant_expr))
120-
return constant_expr;
122+
{
123+
TREE_USED (constant_expr) = 1;
124+
return constant_expr;
125+
}
121126

122127
// this might be a variable reference or a function reference
123128
Bvariable *var = nullptr;
124129
if (ctx->lookup_var_decl (ref, &var))
125-
return ctx->get_backend ()->var_expression (var, expr_locus);
130+
{
131+
// TREE_USED is setup in the gcc abstraction here
132+
return ctx->get_backend ()->var_expression (var, expr_locus);
133+
}
126134

127135
// it might be a function call
128136
if (lookup->get_kind () == TyTy::TypeKind::FNDEF)
@@ -131,13 +139,19 @@ ResolvePathRef::resolve (const HIR::PathIdentSegment &final_segment,
131139
tree fn = NULL_TREE;
132140
if (ctx->lookup_function_decl (fntype->get_ty_ref (), &fn))
133141
{
142+
TREE_USED (fn) = 1;
134143
return address_expression (fn, expr_locus);
135144
}
136145
}
137146

138147
// let the query system figure it out
139-
return query_compile (ref, lookup, final_segment, mappings, expr_locus,
140-
is_qualified_path);
148+
tree resolved_item = query_compile (ref, lookup, final_segment, mappings,
149+
expr_locus, is_qualified_path);
150+
if (resolved_item != error_mark_node)
151+
{
152+
TREE_USED (resolved_item) = 1;
153+
}
154+
return resolved_item;
141155
}
142156

143157
tree

gcc/rust/backend/rust-compile-stmt.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ class CompileStmt : public HIRCompileBase, public HIR::HIRStmtVisitor
8181
bool ok = ctx->get_tyctx ()->lookup_type (
8282
stmt.get_init_expr ()->get_mappings ().get_hirid (), &actual);
8383
rust_assert (ok);
84+
tree stmt_type = TyTyResolveCompile::compile (ctx, ty);
8485

8586
Location lvalue_locus = stmt.get_pattern ()->get_locus ();
8687
Location rvalue_locus = stmt.get_init_expr ()->get_locus ();
@@ -90,8 +91,14 @@ class CompileStmt : public HIRCompileBase, public HIR::HIRStmtVisitor
9091
auto fnctx = ctx->peek_fn ();
9192
if (ty->is_unit ())
9293
{
93-
// FIXME this feels wrong
9494
ctx->add_statement (init);
95+
96+
auto unit_type_init_expr
97+
= ctx->get_backend ()->constructor_expression (stmt_type, false, {},
98+
-1, rvalue_locus);
99+
auto s = ctx->get_backend ()->init_statement (fnctx.fndecl, var,
100+
unit_type_init_expr);
101+
ctx->add_statement (s);
95102
}
96103
else
97104
{

gcc/rust/backend/rust-compile-tyty.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,10 @@ class TyTyCompile : public TyTy::TyVisitor
5454

5555
void visit (TyTy::TupleType &type) override
5656
{
57-
if (type.num_fields () == 0)
58-
translated = backend->unit_type ();
59-
else
60-
gcc_unreachable ();
57+
// this interface is only for unit-type the -type interface takes into
58+
// account the context
59+
rust_assert (type.num_fields () == 0);
60+
translated = backend->unit_type ();
6161
}
6262

6363
void visit (TyTy::ArrayType &) override { gcc_unreachable (); }

gcc/rust/lang.opt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,18 @@ Wall
3838
Rust
3939
; Documented in c.opt
4040

41+
Wunused-variable
42+
Rust Var(warn_unused_variable) Init(1) Warning
43+
; documented in common.opt
44+
45+
Wunused-const-variable
46+
Rust Warning Alias(Wunused-const-variable=, 2, 0)
47+
Warn when a const variable is unused.
48+
49+
Wunused-const-variable=
50+
Rust Joined RejectNegative UInteger Var(warn_unused_const_variable) Init(1) Warning LangEnabledBy(Rust,Wunused-variable, 1, 0) IntegerRange(0, 2)
51+
Warn when a const variable is unused.
52+
4153
Wunused-result
4254
Rust Var(warn_unused_result) Init(1) Warning
4355
Warn if a caller of a function, marked with attribute warn_unused_result, does not use its return value.

gcc/rust/lint/rust-lint-marklive.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,12 @@ class MarkLive : public MarkLiveBase
262262
stct.get_struct_base ()->base_struct->accept_vis (*this);
263263
}
264264

265+
void visit (HIR::Module &module) override
266+
{
267+
for (auto &item : module.get_items ())
268+
item->accept_vis (*this);
269+
}
270+
265271
private:
266272
std::vector<HirId> worklist;
267273
std::set<HirId> liveSymbols;

gcc/rust/lint/rust-lint-scan-deadcode.h

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,11 @@ class ScanDeadcode : public MarkLiveBase
8181
HirId hirId = stct.get_mappings ().get_hirid ();
8282
if (should_warn (hirId))
8383
{
84-
rust_warning_at (stct.get_locus (), 0,
85-
"struct is never constructed: %<%s%>",
86-
stct.get_identifier ().c_str ());
84+
bool name_starts_underscore = stct.get_identifier ().at (0) == '_';
85+
if (!name_starts_underscore)
86+
rust_warning_at (stct.get_locus (), 0,
87+
"struct is never constructed: %<%s%>",
88+
stct.get_identifier ().c_str ());
8789
}
8890
else
8991
{
@@ -124,6 +126,12 @@ class ScanDeadcode : public MarkLiveBase
124126
}
125127
}
126128

129+
void visit (HIR::Module &mod) override
130+
{
131+
for (auto &item : mod.get_items ())
132+
item->accept_vis (*this);
133+
}
134+
127135
private:
128136
std::set<HirId> live_symbols;
129137
Resolver::Resolver *resolver;

0 commit comments

Comments
 (0)