Patchwork Go patch committed: Error for vars that are set but not used

login
register
mail settings
Submitter Ian Taylor
Date July 20, 2014, 8:26 p.m.
Message ID <mcregxfkc9c.fsf@iant-glaptop.roam.corp.google.com>
Download mbox | patch
Permalink /patch/371940/
State New
Headers show

Comments

Ian Taylor - July 20, 2014, 8:26 p.m.
This patch improves the Go frontend to give an error for any variables
that are set but not used.  This matches the behaviour of the gc Go
compiler.  This Go language spec does not require this, but it is
permitted as an implementation restriction, and it seems like a good
idea to make the compilers compatible.  This required changing one test
in the testsuite; I've sent a patch to the master testsuite with this
change (https://codereview.appspot.com/116050043).  Bootstrapped and ran
updated Go testsuite on x86_64-unknown-linux-gnu.  Committed to
mainline.

Ian

Patch

Index: gcc/go/gofrontend/parse.cc
===================================================================
--- gcc/go/gofrontend/parse.cc	(revision 212872)
+++ gcc/go/gofrontend/parse.cc	(working copy)
@@ -2115,8 +2115,8 @@  Parse::simple_var_decl_or_assignment(con
 	  for (Typed_identifier_list::const_iterator p = til.begin();
 	       p != til.end();
 	       ++p)
-	    exprs->push_back(this->id_to_expression(p->name(),
-						    p->location()));
+	    exprs->push_back(this->id_to_expression(p->name(), p->location(),
+						    true));
 
 	  Expression_list* more_exprs =
 	    this->expression_list(NULL, true, may_be_composite_lit);
@@ -2509,7 +2509,10 @@  Parse::operand(bool may_be_sink, bool* i
 	    }
 	  case Named_object::NAMED_OBJECT_VAR:
 	  case Named_object::NAMED_OBJECT_RESULT_VAR:
-	    this->mark_var_used(named_object);
+	    // Any left-hand-side can be a sink, so if this can not be
+	    // a sink, then it must be a use of the variable.
+	    if (!may_be_sink)
+	      this->mark_var_used(named_object);
 	    return Expression::make_var_reference(named_object, location);
 	  case Named_object::NAMED_OBJECT_SINK:
 	    if (may_be_sink)
@@ -2724,7 +2727,7 @@  Parse::composite_lit(Type* type, int dep
 	      Gogo* gogo = this->gogo_;
 	      val = this->id_to_expression(gogo->pack_hidden_name(identifier,
 								  is_exported),
-					   location);
+					   location, false);
 	      is_name = true;
 	    }
 	  else
@@ -3241,7 +3244,8 @@  Parse::call(Expression* func)
 // Return an expression for a single unqualified identifier.
 
 Expression*
-Parse::id_to_expression(const std::string& name, Location location)
+Parse::id_to_expression(const std::string& name, Location location,
+			bool is_lhs)
 {
   Named_object* in_function;
   Named_object* named_object = this->gogo_->lookup(name, &in_function);
@@ -3260,7 +3264,8 @@  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);
+      if (!is_lhs)
+	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);
@@ -5025,7 +5030,7 @@  Parse::send_or_recv_stmt(bool* is_send,
 
 	  *val = this->id_to_expression(gogo->pack_hidden_name(recv_var,
 							       is_rv_exported),
-					recv_var_loc);
+					recv_var_loc, true);
 	  saw_comma = true;
 	}
       else
@@ -5727,6 +5732,13 @@  Parse::verify_not_sink(Expression* expr)
       error_at(expr->location(), "cannot use _ as value");
       expr = Expression::make_error(expr->location());
     }
+
+  // If this can not be a sink, and it is a variable, then we are
+  // using the variable, not just assigning to it.
+  Var_expression* ve = expr->var_expression();
+  if (ve != NULL)
+    this->mark_var_used(ve->named_object());
+
   return expr;
 }
 
Index: gcc/go/gofrontend/parse.h
===================================================================
--- gcc/go/gofrontend/parse.h	(revision 212872)
+++ gcc/go/gofrontend/parse.h	(working copy)
@@ -236,7 +236,7 @@  class Parse
 			 bool* is_type_switch, bool* is_parenthesized);
   Type* reassociate_chan_direction(Channel_type*, Location);
   Expression* qualified_expr(Expression*, Location);
-  Expression* id_to_expression(const std::string&, Location);
+  Expression* id_to_expression(const std::string&, Location, bool);
   void statement(Label*);
   bool statement_may_start_here();
   void labeled_stmt(const std::string&, Location);
Index: gcc/testsuite/go.test/test/shift1.go
===================================================================
--- gcc/testsuite/go.test/test/shift1.go	(revision 212872)
+++ gcc/testsuite/go.test/test/shift1.go	(working copy)
@@ -238,4 +238,6 @@  func _() {
 	z = (1. << s) << (1 << s)    // ERROR "non-integer|type complex128"
 	z = (1. << s) << (1. << s)   // ERROR "non-integer|type complex128"
 	z = (1.1 << s) << (1.1 << s) // ERROR "invalid|truncated|complex128"
+
+	_, _, _ = x, y, z
 }