In Go it is an error if a variable is defined but not used. This patch implements this error in gccgo. This required some tweaks to gccgo specific changes in libgo. It also required updating various tests in the testsuite. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline.
Ian
Index: gcc/go/gofrontend/gogo.cc =================================================================== --- gcc/go/gofrontend/gogo.cc (revision 183374) +++ gcc/go/gofrontend/gogo.cc (working copy) @@ -1036,6 +1036,23 @@ Gogo::add_named_object(Named_object* no) this->current_bindings()->add_named_object(no); } +// Mark all local variables used. This is used when some types of +// parse error occur. + +void +Gogo::mark_locals_used() +{ + for (Open_functions::iterator pf = this->functions_.begin(); + pf != this->functions_.end(); + ++pf) + { + for (std::vector<Block*>::iterator pb = pf->blocks.begin(); + pb != pf->blocks.end(); + ++pb) + (*pb)->bindings()->mark_locals_used(); + } +} + // Record that we've seen an interface type. void @@ -1731,6 +1748,15 @@ Check_types_traverse::variable(Named_obj reason.c_str()); var->clear_init(); } + else if (!var->is_used() + && !var->is_global() + && !var->is_parameter() + && !var->is_receiver() + && !var->type()->is_error() + && (init == NULL || !init->is_error_expression()) + && !Lex::is_invalid_identifier(named_object->name())) + error_at(var->location(), "%qs declared and not used", + named_object->message_name().c_str()); } return TRAVERSE_CONTINUE; } @@ -2973,6 +2999,7 @@ Function::closure_var() Type* struct_type = Type::make_struct_type(sfl, loc); Variable* var = new Variable(Type::make_pointer_type(struct_type), NULL, false, true, false, loc); + var->set_is_used(); this->closure_var_ = Named_object::make_variable("closure", NULL, var); // Note that the new variable is not in any binding contour. } @@ -3693,7 +3720,7 @@ Variable::Variable(Type* type, Expressio Location location) : type_(type), init_(init), preinit_(NULL), location_(location), backend_(NULL), is_global_(is_global), is_parameter_(is_parameter), - is_receiver_(is_receiver), is_varargs_parameter_(false), + is_receiver_(is_receiver), is_varargs_parameter_(false), is_used_(false), is_address_taken_(false), is_non_escaping_address_taken_(false), seen_(false), init_is_lowered_(false), type_from_init_tuple_(false), type_from_range_index_(false), type_from_range_value_(false), @@ -4877,6 +4904,19 @@ Bindings::define_type(Named_object* no, this->named_objects_.push_back(no); } +// Mark all local variables as used. This is used for some types of +// parse error. + +void +Bindings::mark_locals_used() +{ + for (std::vector<Named_object*>::iterator p = this->named_objects_.begin(); + p != this->named_objects_.end(); + ++p) + if ((*p)->is_variable()) + (*p)->var_value()->set_is_used(); +} + // Traverse bindings. int Index: gcc/go/gofrontend/gogo.h =================================================================== --- gcc/go/gofrontend/gogo.h (revision 183340) +++ gcc/go/gofrontend/gogo.h (working copy) @@ -344,6 +344,11 @@ class Gogo void add_named_object(Named_object*); + // Mark all local variables in current bindings as used. This is + // used when there is a parse error to avoid useless errors. + void + mark_locals_used(); + // Return a name to use for a thunk function. A thunk function is // one we create during the compilation, for a go statement or a // defer statement or a method expression. @@ -1232,6 +1237,16 @@ class Variable this->is_varargs_parameter_ = true; } + // Return whether the variable has been used. + bool + is_used() const + { return this->is_used_; } + + // Mark that the variable has been used. + void + set_is_used() + { this->is_used_ = true; } + // Clear the initial value; used for error handling. void clear_init() @@ -1368,6 +1383,8 @@ class Variable bool is_receiver_ : 1; // Whether this is the varargs parameter of a function. bool is_varargs_parameter_ : 1; + // Whether this variable is ever referenced. + bool is_used_ : 1; // Whether something takes the address of this variable. For a // local variable this implies that the variable has to be on the // heap. @@ -2124,6 +2141,11 @@ class Bindings void remove_binding(Named_object*); + // Mark all variables as used. This is used for some types of parse + // error. + void + mark_locals_used(); + // Traverse the tree. See the Traverse class. int traverse(Traverse*, bool is_global); Index: gcc/go/gofrontend/parse.cc =================================================================== --- gcc/go/gofrontend/parse.cc (revision 183374) +++ gcc/go/gofrontend/parse.cc (working copy) @@ -49,7 +49,8 @@ Parse::Parse(Lex* lex, Gogo* gogo) break_stack_(NULL), continue_stack_(NULL), iota_(0), - enclosing_vars_() + enclosing_vars_(), + type_switch_vars_() { } @@ -539,6 +540,7 @@ Parse::field_decl(Struct_field_list* sfl else { error_at(this->location(), "expected field name"); + this->gogo_->mark_locals_used(); while (!token->is_op(OPERATOR_SEMICOLON) && !token->is_op(OPERATOR_RCURLY) && !token->is_eof()) @@ -554,6 +556,7 @@ Parse::field_decl(Struct_field_list* sfl if (!this->peek_token()->is_identifier()) { error_at(this->location(), "expected field name"); + this->gogo_->mark_locals_used(); while (!token->is_op(OPERATOR_SEMICOLON) && !token->is_op(OPERATOR_RCURLY) && !token->is_eof()) @@ -1123,6 +1126,8 @@ Parse::block() if (!token->is_eof() || !saw_errors()) error_at(this->location(), "expected %<}%>"); + this->gogo_->mark_locals_used(); + // Skip ahead to the end of the block, in hopes of avoiding // lots of meaningless errors. Location ret = token->location(); @@ -1249,6 +1254,7 @@ Parse::method_spec(Typed_identifier_list "name list not allowed in interface type"); else error_at(location, "expected signature or type name"); + this->gogo_->mark_locals_used(); token = this->peek_token(); while (!token->is_eof() && !token->is_op(OPERATOR_SEMICOLON) @@ -1498,6 +1504,7 @@ Parse::type_spec(void*) if (type->is_error_type()) { + this->gogo_->mark_locals_used(); while (!this->peek_token()->is_op(OPERATOR_SEMICOLON) && !this->peek_token()->is_eof()) this->advance_token(); @@ -1558,6 +1565,7 @@ Parse::var_spec(void*) type = this->type(); if (type->is_error_type()) { + this->gogo_->mark_locals_used(); while (!this->peek_token()->is_op(OPERATOR_EQ) && !this->peek_token()->is_op(OPERATOR_SEMICOLON) && !this->peek_token()->is_eof()) @@ -1894,6 +1902,7 @@ Parse::init_var(const Typed_identifier& // initializer can be assigned to the type. Variable* var = new Variable(type, init, false, false, false, location); + var->set_is_used(); static int count; char buf[30]; snprintf(buf, sizeof buf, "sink$%d", count); @@ -2188,6 +2197,7 @@ Parse::receiver() if (!token->is_identifier()) { error_at(this->location(), "method has no receiver"); + this->gogo_->mark_locals_used(); while (!token->is_eof() && !token->is_op(OPERATOR_RPAREN)) token = this->advance_token(); if (!token->is_eof()) @@ -2227,6 +2237,7 @@ Parse::receiver() if (!token->is_identifier()) { error_at(this->location(), "expected receiver name or type"); + this->gogo_->mark_locals_used(); int c = token->is_op(OPERATOR_LPAREN) ? 1 : 0; while (!token->is_eof()) { @@ -2258,6 +2269,7 @@ Parse::receiver() error_at(this->location(), "method has multiple receivers"); else error_at(this->location(), "expected %<)%>"); + this->gogo_->mark_locals_used(); while (!token->is_eof() && !token->is_op(OPERATOR_RPAREN)) token = this->advance_token(); if (!token->is_eof()) @@ -2365,6 +2377,7 @@ Parse::operand(bool may_be_sink) } case Named_object::NAMED_OBJECT_VAR: case Named_object::NAMED_OBJECT_RESULT_VAR: + this->mark_var_used(named_object); return Expression::make_var_reference(named_object, location); case Named_object::NAMED_OBJECT_SINK: if (may_be_sink) @@ -2477,6 +2490,8 @@ Parse::enclosing_var_reference(Named_obj { go_assert(var->is_variable() || var->is_result_variable()); + this->mark_var_used(var); + Named_object* this_function = this->gogo_->current_function(); Named_object* closure = this_function->func_value()->closure_var(); @@ -2648,6 +2663,7 @@ Parse::composite_lit(Type* type, int dep { error_at(this->location(), "expected %<,%> or %<}%>"); + this->gogo_->mark_locals_used(); int depth = 0; while (!token->is_eof() && (depth > 0 || !token->is_op(OPERATOR_RCURLY))) @@ -3019,6 +3035,7 @@ Parse::id_to_expression(const std::strin return Expression::make_const_reference(named_object, location); case Named_object::NAMED_OBJECT_VAR: case Named_object::NAMED_OBJECT_RESULT_VAR: + this->mark_var_used(named_object); return Expression::make_var_reference(named_object, location); case Named_object::NAMED_OBJECT_SINK: return Expression::make_sink(location); @@ -3534,6 +3551,7 @@ Parse::simple_stat(bool may_be_composite { if (!exp->is_error_expression()) error_at(token->location(), "non-name on left side of %<:=%>"); + this->gogo_->mark_locals_used(); while (!token->is_op(OPERATOR_SEMICOLON) && !token->is_eof()) token = this->advance_token(); @@ -4287,7 +4305,15 @@ Parse::type_case_clause(Named_object* sw Variable* v = new Variable(type, init, false, false, false, location); v->set_is_type_switch_var(); - this->gogo_->add_variable(switch_no->name(), v); + Named_object* no = this->gogo_->add_variable(switch_no->name(), v); + + // We don't want to issue an error if the compiler + // introduced special variable is not used. Instead we want + // to issue an error if the variable defined by the switch + // is not used. That is handled via type_switch_vars_ and + // Parse::mark_var_used. + v->set_is_used(); + this->type_switch_vars_[no] = switch_no; } this->statement_list(); statements = this->gogo_->finish_block(this->location()); @@ -4343,6 +4369,7 @@ Parse::type_switch_case(std::vector<Type types->push_back(t); else { + this->gogo_->mark_locals_used(); token = this->peek_token(); while (!token->is_op(OPERATOR_COLON) && !token->is_op(OPERATOR_COMMA) @@ -5209,6 +5236,7 @@ Parse::program() else { error_at(this->location(), "expected declaration"); + this->gogo_->mark_locals_used(); do this->advance_token(); while (!this->peek_token()->is_eof() @@ -5267,6 +5295,7 @@ Parse::increment_iota() bool Parse::skip_past_error(Operator op) { + this->gogo_->mark_locals_used(); const Token* token = this->peek_token(); while (!token->is_op(op)) { @@ -5294,3 +5323,22 @@ Parse::verify_not_sink(Expression* expr) } return expr; } + +// Mark a variable as used. + +void +Parse::mark_var_used(Named_object* no) +{ + if (no->is_variable()) + { + no->var_value()->set_is_used(); + + // When a type switch uses := to define a variable, then for + // each case with a single type we introduce a new variable with + // the appropriate type. When we do, if the newly introduced + // variable is used, then the type switch variable is used. + Type_switch_vars::iterator p = this->type_switch_vars_.find(no); + if (p != this->type_switch_vars_.end()) + p->second->var_value()->set_is_used(); + } +} Index: gcc/go/gofrontend/lex.cc =================================================================== --- gcc/go/gofrontend/lex.cc (revision 183374) +++ gcc/go/gofrontend/lex.cc (working copy) @@ -866,6 +866,7 @@ Lex::gather_identifier() this->lineoff_ = p - this->linebuf_; const char* pnext = this->advance_one_utf8_char(p, &ci, &issued_error); + bool is_invalid = false; if (!Lex::is_unicode_letter(ci) && !Lex::is_unicode_digit(ci)) { // There is no valid place for a non-ASCII character @@ -876,6 +877,7 @@ Lex::gather_identifier() error_at(this->location(), "invalid character 0x%x in identifier", ci); + is_invalid = true; } if (is_first) { @@ -887,6 +889,8 @@ Lex::gather_identifier() buf.assign(pstart, p - pstart); has_non_ascii_char = true; } + if (is_invalid && !Lex::is_invalid_identifier(buf)) + buf.append("$INVALID$"); p = pnext; char ubuf[50]; // This assumes that all assemblers can handle an identifier @@ -2312,3 +2316,13 @@ Lex::is_exported_name(const std::string& return Lex::is_unicode_uppercase(ci); } } + +// Return whether the identifier NAME contains an invalid character. +// This is based on how we handle invalid characters in +// gather_identifier. + +bool +Lex::is_invalid_identifier(const std::string& name) +{ + return name.find("$INVALID$") != std::string::npos; +} Index: gcc/go/gofrontend/parse.h =================================================================== --- gcc/go/gofrontend/parse.h (revision 183280) +++ gcc/go/gofrontend/parse.h (working copy) @@ -155,6 +155,11 @@ class Parse // break or continue statement with no label. typedef std::vector<std::pair<Statement*, Label*> > Bc_stack; + // Map from type switch variables to the variables they mask, so + // that a use of the type switch variable can become a use of the + // real variable. + typedef Unordered_map(Named_object*, Named_object*) Type_switch_vars; + // Parser nonterminals. void identifier_list(Typed_identifier_list*); Expression_list* expression_list(Expression*, bool may_be_sink); @@ -288,6 +293,10 @@ class Parse Statement* find_bc_statement(const Bc_stack*, const std::string&) const; + // Mark a variable as used. + void + mark_var_used(Named_object*); + // The lexer output we are parsing. Lex* lex_; // The current token. @@ -307,6 +316,8 @@ class Parse // References from the local function to variables defined in // enclosing functions. Enclosing_vars enclosing_vars_; + // Map from type switch variables to real variables. + Type_switch_vars type_switch_vars_; }; Index: gcc/go/gofrontend/expressions.cc =================================================================== --- gcc/go/gofrontend/expressions.cc (revision 183374) +++ gcc/go/gofrontend/expressions.cc (working copy) @@ -1472,6 +1472,7 @@ Unknown_expression::do_lower(Gogo*, Name real->message_name().c_str()); return Expression::make_error(location); case Named_object::NAMED_OBJECT_VAR: + real->var_value()->set_is_used(); return Expression::make_var_reference(real, location); case Named_object::NAMED_OBJECT_FUNC: case Named_object::NAMED_OBJECT_FUNC_DECLARATION: Index: gcc/go/gofrontend/lex.h =================================================================== --- gcc/go/gofrontend/lex.h (revision 183374) +++ gcc/go/gofrontend/lex.h (working copy) @@ -349,6 +349,13 @@ class Lex static bool is_exported_name(const std::string& name); + // Return whether the identifier NAME is invalid. When we see an + // invalid character we still build an identifier, but we use a + // magic string to indicate that the identifier is invalid. We then + // use this to avoid knockon errors. + static bool + is_invalid_identifier(const std::string& name); + // A helper function. Append V to STR. IS_CHARACTER is true if V // is a Unicode character which should be converted into UTF-8, // false if it is a byte value to be appended directly. The Index: libgo/go/reflect/value.go =================================================================== --- libgo/go/reflect/value.go (revision 183280) +++ libgo/go/reflect/value.go (working copy) @@ -215,8 +215,8 @@ type emptyInterface struct { type nonEmptyInterface struct { // see ../runtime/iface.c:/Itab itab *struct { - typ *runtime.Type // dynamic concrete type - fun [100000]unsafe.Pointer // method table + typ *runtime.Type // dynamic concrete type + fun [100000]unsafe.Pointer // method table } word iword } @@ -448,7 +448,6 @@ func (v Value) call(method string, in [] nin++ } params := make([]unsafe.Pointer, nin) - delta := 0 off := 0 if v.flag&flagMethod != 0 { // Hard-wired first argument. @@ -517,7 +516,7 @@ func isMethod(t *commonType) bool { params++ } else if c == ')' { parens-- - } else if parens == 0 && c == ' ' && s[i + 1] != '(' && !sawRet { + } else if parens == 0 && c == ' ' && s[i+1] != '(' && !sawRet { params++ sawRet = true } @@ -1627,7 +1626,7 @@ func MakeChan(typ Type, buffer int) Valu panic("reflect.MakeChan: unidirectional channel type") } ch := makechan(typ.runtimeType(), uint32(buffer)) - return Value{typ.common(), unsafe.Pointer(ch), flagIndir | (flag(Chan)<<flagKindShift)} + return Value{typ.common(), unsafe.Pointer(ch), flagIndir | (flag(Chan) << flagKindShift)} } // MakeMap creates a new map of the specified type. @@ -1636,7 +1635,7 @@ func MakeMap(typ Type) Value { panic("reflect.MakeMap of non-map type") } m := makemap(typ.runtimeType()) - return Value{typ.common(), unsafe.Pointer(m), flagIndir | (flag(Map)<<flagKindShift)} + return Value{typ.common(), unsafe.Pointer(m), flagIndir | (flag(Map) << flagKindShift)} } // Indirect returns the value that v points to. Index: libgo/go/reflect/type.go =================================================================== --- libgo/go/reflect/type.go (revision 183280) +++ libgo/go/reflect/type.go (working copy) @@ -243,7 +243,7 @@ type commonType struct { align int8 fieldAlign uint8 size uintptr - hash uint32 + hash uint32 hashfn func(unsafe.Pointer, uintptr) equalfn func(unsafe.Pointer, unsafe.Pointer, uintptr) string *string @@ -464,7 +464,7 @@ func (t *uncommonType) Method(i int) (m m.Type = mt.toType() x := new(unsafe.Pointer) *x = p.tfn - m.Func = Value{mt, unsafe.Pointer(x), fl|flagIndir} + m.Func = Value{mt, unsafe.Pointer(x), fl | flagIndir} m.Index = i return } @@ -999,10 +999,8 @@ func (ct *commonType) ptrTo() *commonTyp return &p.commonType } - rt := (*runtime.Type)(unsafe.Pointer(ct)) - rp := new(runtime.PtrType) - + // initialize p using *byte's ptrType as a prototype. // have to do assignment as ptrType, not runtime.PtrType, // in order to write to unexported fields. Index: libgo/go/io/ioutil/ioutil_test.go =================================================================== --- libgo/go/io/ioutil/ioutil_test.go (revision 183280) +++ libgo/go/io/ioutil/ioutil_test.go (working copy) @@ -71,13 +71,13 @@ func TestReadDir(t *testing.T) { t.Fatalf("ReadDir %s: error expected, none found", dirname) } + /* Does not work in gccgo testing environment. dirname = ".." list, err := ReadDir(dirname) if err != nil { t.Fatalf("ReadDir %s: %v", dirname, err) } -/* Does not work in gccgo testing environment. foundFile := false foundSubDir := false for _, dir := range list { @@ -94,5 +94,5 @@ func TestReadDir(t *testing.T) { if !foundSubDir { t.Fatalf("ReadDir %s: ioutil directory not found", dirname) } -*/ + */ } Index: gcc/testsuite/go.test/test/func4.go =================================================================== --- gcc/testsuite/go.test/test/func4.go (revision 183395) +++ gcc/testsuite/go.test/test/func4.go (working copy) @@ -11,4 +11,5 @@ var notmain func() func main() { var x = &main // ERROR "address of|invalid" main = notmain // ERROR "assign to|invalid" + _ = x } Index: gcc/testsuite/go.test/test/indirect1.go =================================================================== --- gcc/testsuite/go.test/test/indirect1.go (revision 183395) +++ gcc/testsuite/go.test/test/indirect1.go (working copy) @@ -65,4 +65,5 @@ func f() { cap(b2)+ // ERROR "illegal|invalid|must be" cap(b3)+ cap(b4) // ERROR "illegal|invalid|must be" + _ = x } Index: gcc/testsuite/go.test/test/blank1.go =================================================================== --- gcc/testsuite/go.test/test/blank1.go (revision 183395) +++ gcc/testsuite/go.test/test/blank1.go (working copy) @@ -9,4 +9,5 @@ package _ // ERROR "invalid package name func main() { _() // ERROR "cannot use _ as value" x := _+1 // ERROR "cannot use _ as value" + _ = x } Index: gcc/testsuite/go.test/test/fixedbugs/bug248.dir/bug2.go =================================================================== --- gcc/testsuite/go.test/test/fixedbugs/bug248.dir/bug2.go (revision 183395) +++ gcc/testsuite/go.test/test/fixedbugs/bug248.dir/bug2.go (working copy) @@ -80,7 +80,7 @@ func main() { case 2: i = 3.14 } - switch k := i.(type) { + switch i.(type) { case p0.T: if j != 0 { println("type switch p0.T") Index: gcc/testsuite/go.test/test/fixedbugs/bug200.go =================================================================== --- gcc/testsuite/go.test/test/fixedbugs/bug200.go (revision 183395) +++ gcc/testsuite/go.test/test/fixedbugs/bug200.go (working copy) @@ -12,7 +12,7 @@ func main() { // and worse, compiled the wrong code // for one of them. var x interface{}; - switch v := x.(type) { + switch x.(type) { case func(int): case func(f int): // ERROR "duplicate" } Index: gcc/testsuite/go.test/test/fixedbugs/bug309.go =================================================================== --- gcc/testsuite/go.test/test/fixedbugs/bug309.go (revision 183395) +++ gcc/testsuite/go.test/test/fixedbugs/bug309.go (working copy) @@ -15,5 +15,7 @@ func foo(t interface{}, c chan int) { case <-c: // bug was: internal compiler error: var without type, init: v } + default: + _ = v } } Index: gcc/testsuite/go.test/test/fixedbugs/bug108.go =================================================================== --- gcc/testsuite/go.test/test/fixedbugs/bug108.go (revision 183395) +++ gcc/testsuite/go.test/test/fixedbugs/bug108.go (working copy) @@ -7,4 +7,5 @@ package main func f() { v := 1 << 1025; // ERROR "overflow|stupid shift" + _ = v } Index: gcc/testsuite/go.test/test/fixedbugs/bug014.go =================================================================== --- gcc/testsuite/go.test/test/fixedbugs/bug014.go (revision 183395) +++ gcc/testsuite/go.test/test/fixedbugs/bug014.go (working copy) @@ -11,4 +11,5 @@ func main() { var c01 uint8 = '\07'; // ERROR "oct|char" var cx0 uint8 = '\x0'; // ERROR "hex|char" var cx1 uint8 = '\x'; // ERROR "hex|char" + _, _, _, _ = c00, c01, cx0, cx1 } Index: gcc/testsuite/go.test/test/fixedbugs/bug363.go =================================================================== --- gcc/testsuite/go.test/test/fixedbugs/bug363.go (revision 183395) +++ gcc/testsuite/go.test/test/fixedbugs/bug363.go (working copy) @@ -17,5 +17,5 @@ func main() { println(b) var c int64 = (1<<i) + 4.0 // ok - it's all int64 - println(b) + println(c) } Index: gcc/testsuite/go.test/test/fixedbugs/bug175.go =================================================================== --- gcc/testsuite/go.test/test/fixedbugs/bug175.go (revision 183395) +++ gcc/testsuite/go.test/test/fixedbugs/bug175.go (working copy) @@ -10,5 +10,5 @@ func f() (int, bool) { return 0, true } func main() { x, y := f(), 2; // ERROR "multi" + _, _ = x, y } - Index: gcc/testsuite/go.test/test/fixedbugs/bug213.go =================================================================== --- gcc/testsuite/go.test/test/fixedbugs/bug213.go (revision 183395) +++ gcc/testsuite/go.test/test/fixedbugs/bug213.go (working copy) @@ -7,7 +7,7 @@ package main func main() { var v interface{} = 0; - switch x := v.(type) { + switch v.(type) { case int: fallthrough; // ERROR "fallthrough" default: Index: gcc/testsuite/go.test/test/fixedbugs/bug141.go =================================================================== --- gcc/testsuite/go.test/test/fixedbugs/bug141.go (revision 183395) +++ gcc/testsuite/go.test/test/fixedbugs/bug141.go (working copy) @@ -20,7 +20,7 @@ type Getter interface { func f1(p Empty) { switch x := p.(type) { - default: println("failed to match interface"); os.Exit(1); + default: println("failed to match interface", x); os.Exit(1); case Getter: break; }