Patchwork go patch committed: Give error on label defined but not used

login
register
mail settings
Submitter Ian Taylor
Date March 25, 2011, 5:34 p.m.
Message ID <mcrbp0zdry5.fsf@google.com>
Download mbox | patch
Permalink /patch/88404/
State New
Headers show

Comments

Ian Taylor - March 25, 2011, 5:34 p.m.
The Go language says that it is an error to define a label but not use
it.  This patch adds that error to the gccgo frontend.  The patch
includes some changes to the testsuite to avoid new errors.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline.

Ian

Patch

diff -r aaba6044d772 go/gogo.cc
--- a/go/gogo.cc	Thu Mar 24 22:02:30 2011 -0700
+++ b/go/gogo.cc	Fri Mar 25 10:27:55 2011 -0700
@@ -1463,6 +1463,7 @@ 
   Check_types_traverse(Gogo* gogo)
     : Traverse(traverse_variables
 	       | traverse_constants
+	       | traverse_functions
 	       | traverse_statements
 	       | traverse_expressions),
       gogo_(gogo)
@@ -1475,6 +1476,9 @@ 
   constant(Named_object*, bool);
 
   int
+  function(Named_object*);
+
+  int
   statement(Block*, size_t* pindex, Statement*);
 
   int
@@ -1542,6 +1546,16 @@ 
   return TRAVERSE_CONTINUE;
 }
 
+// There are no types to check in a function, but this is where we
+// issue warnings about labels which are defined but not referenced.
+
+int
+Check_types_traverse::function(Named_object* no)
+{
+  no->func_value()->check_labels();
+  return TRAVERSE_CONTINUE;
+}
+
 // Check that types are valid in a statement.
 
 int
@@ -2744,7 +2758,7 @@ 
 	}
       else
 	{
-	  error_at(location, "redefinition of label %qs",
+	  error_at(location, "label %qs already defined",
 		   Gogo::message_name(label_name).c_str());
 	  inform(label->location(), "previous definition of %qs was here",
 		 Gogo::message_name(label_name).c_str());
@@ -2764,17 +2778,36 @@ 
   if (!ins.second)
     {
       // The label was already in the hash table.
-      return ins.first->second;
+      Label* label = ins.first->second;
+      label->set_is_used();
+      return label;
     }
   else
     {
       gcc_assert(ins.first->second == NULL);
       Label* label = new Label(label_name);
       ins.first->second = label;
+      label->set_is_used();
       return label;
     }
 }
 
+// Warn about labels that are defined but not used.
+
+void
+Function::check_labels() const
+{
+  for (Labels::const_iterator p = this->labels_.begin();
+       p != this->labels_.end();
+       p++)
+    {
+      Label* label = p->second;
+      if (!label->is_used())
+	error_at(label->location(), "label %qs defined and not used",
+		 Gogo::message_name(label->name()).c_str());
+    }
+}
+
 // Swap one function with another.  This is used when building the
 // thunk we use to call a function which calls recover.  It may not
 // work for any other case.
diff -r aaba6044d772 go/gogo.h
--- a/go/gogo.h	Thu Mar 24 22:02:30 2011 -0700
+++ b/go/gogo.h	Fri Mar 25 10:27:55 2011 -0700
@@ -882,6 +882,10 @@ 
   Label*
   add_label_reference(const std::string& label_name);
 
+  // Warn about labels that are defined but not used.
+  void
+  check_labels() const;
+
   // Whether this function calls the predeclared recover function.
   bool
   calls_recover() const
@@ -2090,7 +2094,7 @@ 
 {
  public:
   Label(const std::string& name)
-    : name_(name), location_(0), decl_(NULL)
+    : name_(name), location_(0), is_used_(false), decl_(NULL)
   { }
 
   // Return the label's name.
@@ -2103,6 +2107,16 @@ 
   is_defined() const
   { return this->location_ != 0; }
 
+  // Return whether the label has been used.
+  bool
+  is_used() const
+  { return this->is_used_; }
+
+  // Record that the label is used.
+  void
+  set_is_used()
+  { this->is_used_ = true; }
+
   // Return the location of the definition.
   source_location
   location() const
@@ -2130,6 +2144,8 @@ 
   // The location of the definition.  This is 0 if the label has not
   // yet been defined.
   source_location location_;
+  // Whether the label has been used.
+  bool is_used_;
   // The LABEL_DECL.
   tree decl_;
 };
diff -r aaba6044d772 go/parse.cc
--- a/go/parse.cc	Thu Mar 24 22:02:30 2011 -0700
+++ b/go/parse.cc	Fri Mar 25 10:27:55 2011 -0700
@@ -3112,7 +3112,7 @@ 
 // LABEL is the label of this statement if it has one.
 
 void
-Parse::statement(const Label* label)
+Parse::statement(Label* label)
 {
   const Token* token = this->peek_token();
   switch (token->classification())
@@ -3288,6 +3288,10 @@ 
 
   if (!this->statement_may_start_here())
     {
+      // Mark the label as used to avoid a useless error about an
+      // unused label.
+      label->set_is_used();
+
       error_at(location, "missing statement after label");
       this->unget_token(Token::make_operator_token(OPERATOR_SEMICOLON,
 						   location));
@@ -3774,7 +3778,7 @@ 
 // TypeSwitchGuard = [ identifier ":=" ] Expression "." "(" "type" ")" .
 
 void
-Parse::switch_stat(const Label* label)
+Parse::switch_stat(Label* label)
 {
   gcc_assert(this->peek_token()->is_keyword(KEYWORD_SWITCH));
   source_location location = this->location();
@@ -3873,7 +3877,7 @@ 
 //   "{" { ExprCaseClause } "}"
 
 Statement*
-Parse::expr_switch_body(const Label* label, Expression* switch_val,
+Parse::expr_switch_body(Label* label, Expression* switch_val,
 			source_location location)
 {
   Switch_statement* statement = Statement::make_switch_statement(switch_val,
@@ -3983,7 +3987,7 @@ 
 //   "{" { TypeCaseClause } "}" .
 
 Statement*
-Parse::type_switch_body(const Label* label, const Type_switch& type_switch,
+Parse::type_switch_body(Label* label, const Type_switch& type_switch,
 			source_location location)
 {
   Named_object* switch_no = NULL;
@@ -4127,7 +4131,7 @@ 
 // SelectStat = "select" "{" { CommClause } "}" .
 
 void
-Parse::select_stat(const Label* label)
+Parse::select_stat(Label* label)
 {
   gcc_assert(this->peek_token()->is_keyword(KEYWORD_SELECT));
   source_location location = this->location();
@@ -4435,7 +4439,7 @@ 
 // Condition = Expression .
 
 void
-Parse::for_stat(const Label* label)
+Parse::for_stat(Label* label)
 {
   gcc_assert(this->peek_token()->is_keyword(KEYWORD_FOR));
   source_location location = this->location();
@@ -4654,7 +4658,7 @@ 
 // Push a statement on the break stack.
 
 void
-Parse::push_break_statement(Statement* enclosing, const Label* label)
+Parse::push_break_statement(Statement* enclosing, Label* label)
 {
   if (this->break_stack_ == NULL)
     this->break_stack_ = new Bc_stack();
@@ -4664,7 +4668,7 @@ 
 // Push a statement on the continue stack.
 
 void
-Parse::push_continue_statement(Statement* enclosing, const Label* label)
+Parse::push_continue_statement(Statement* enclosing, Label* label)
 {
   if (this->continue_stack_ == NULL)
     this->continue_stack_ = new Bc_stack();
@@ -4697,8 +4701,13 @@ 
   for (Bc_stack::const_reverse_iterator p = bc_stack->rbegin();
        p != bc_stack->rend();
        ++p)
-    if (p->second != NULL && p->second->name() == label)
-      return p->first;
+    {
+      if (p->second != NULL && p->second->name() == label)
+	{
+	  p->second->set_is_used();
+	  return p->first;
+	}
+    }
   return NULL;
 }
 
@@ -4728,9 +4737,11 @@ 
 					  token->identifier());
       if (enclosing == NULL)
 	{
-	  error_at(token->location(),
-		   ("break label %qs not associated with "
-		    "for or switch or select"),
+	  // If there is a label with this name, mark it as used to
+	  // avoid a useless error about an unused label.
+	  this->gogo_->add_label_reference(token->identifier());
+
+	  error_at(token->location(), "invalid break label %qs",
 		   Gogo::message_name(token->identifier()).c_str());
 	  this->advance_token();
 	  return;
@@ -4781,8 +4792,11 @@ 
 					  token->identifier());
       if (enclosing == NULL)
 	{
-	  error_at(token->location(),
-		   "continue label %qs not associated with for",
+	  // If there is a label with this name, mark it as used to
+	  // avoid a useless error about an unused label.
+	  this->gogo_->add_label_reference(token->identifier());
+
+	  error_at(token->location(), "invalid continue label %qs",
 		   Gogo::message_name(token->identifier()).c_str());
 	  this->advance_token();
 	  return;
diff -r aaba6044d772 go/parse.h
--- a/go/parse.h	Thu Mar 24 22:02:30 2011 -0700
+++ b/go/parse.h	Fri Mar 25 10:27:55 2011 -0700
@@ -150,7 +150,7 @@ 
   // For break and continue we keep a stack of statements with
   // associated labels (if any).  The top of the stack is used for a
   // break or continue statement with no label.
-  typedef std::vector<std::pair<Statement*, const Label*> > Bc_stack;
+  typedef std::vector<std::pair<Statement*, Label*> > Bc_stack;
 
   // Parser nonterminals.
   void identifier_list(Typed_identifier_list*);
@@ -220,7 +220,7 @@ 
 			 bool* is_type_switch);
   Expression* qualified_expr(Expression*, source_location);
   Expression* id_to_expression(const std::string&, source_location);
-  void statement(const Label*);
+  void statement(Label*);
   bool statement_may_start_here();
   void labeled_stmt(const std::string&, source_location);
   Expression* simple_stat(bool, bool, Range_clause*, Type_switch*);
@@ -236,26 +236,25 @@ 
   void go_or_defer_stat();
   void return_stat();
   void if_stat();
-  void switch_stat(const Label*);
-  Statement* expr_switch_body(const Label*, Expression*, source_location);
+  void switch_stat(Label*);
+  Statement* expr_switch_body(Label*, Expression*, source_location);
   void expr_case_clause(Case_clauses*, bool* saw_default);
   Expression_list* expr_switch_case(bool*);
-  Statement* type_switch_body(const Label*, const Type_switch&,
-			      source_location);
+  Statement* type_switch_body(Label*, const Type_switch&, source_location);
   void type_case_clause(Named_object*, Type_case_clauses*, bool* saw_default);
   void type_switch_case(std::vector<Type*>*, bool*);
-  void select_stat(const Label*);
+  void select_stat(Label*);
   void comm_clause(Select_clauses*, bool* saw_default);
   bool comm_case(bool*, Expression**, Expression**, Expression**,
 		 std::string*, std::string*, bool*);
   bool send_or_recv_stmt(bool*, Expression**, Expression**, Expression**,
 			 std::string*, std::string*);
-  void for_stat(const Label*);
+  void for_stat(Label*);
   void for_clause(Expression**, Block**);
   void range_clause_decl(const Typed_identifier_list*, Range_clause*);
   void range_clause_expr(const Expression_list*, Range_clause*);
-  void push_break_statement(Statement*, const Label*);
-  void push_continue_statement(Statement*, const Label*);
+  void push_break_statement(Statement*, Label*);
+  void push_continue_statement(Statement*, Label*);
   void pop_break_statement();
   void pop_continue_statement();
   Statement* find_bc_statement(const Bc_stack*, const std::string&);
Index: gcc/testsuite/go.test/test/fixedbugs/bug178.go
===================================================================
--- gcc/testsuite/go.test/test/fixedbugs/bug178.go	(revision 171359)
+++ gcc/testsuite/go.test/test/fixedbugs/bug178.go	(working copy)
@@ -9,19 +9,25 @@  package main
 func main() {
 L:
 	for i := 0; i < 1; i++ {
-L1:
+	L1:
 		for {
-			break L;
+			break L
 		}
-		panic("BUG: not reached - break");
+		panic("BUG: not reached - break")
 	}
 
 L2:
 	for i := 0; i < 1; i++ {
-L3:
+	L3:
 		for {
-			continue L2;
+			continue L2
 		}
-		panic("BUG: not reached - continue");
+		panic("BUG: not reached - continue")
+	}
+	if false {
+		goto L1
+	}
+	if false {
+		goto L3
 	}
 }
Index: gcc/testsuite/go.test/test/fixedbugs/bug179.go
===================================================================
--- gcc/testsuite/go.test/test/fixedbugs/bug179.go	(revision 171359)
+++ gcc/testsuite/go.test/test/fixedbugs/bug179.go	(working copy)
@@ -10,16 +10,18 @@  func main() {
 L:
 	for {
 		for {
-			break L2;	// ERROR "L2"
-			continue L2;	// ERROR "L2"
+			break L2    // ERROR "L2"
+			continue L2 // ERROR "L2"
 		}
 	}
 
 L1:
-	x := 1;
-	_ = x;
+	x := 1
+	_ = x
 	for {
-		break L1;	// ERROR "L1"
-		continue L1;	// ERROR "L1"
+		break L1    // ERROR "L1"
+		continue L1 // ERROR "L1"
 	}
+
+	goto L
 }
Index: gcc/testsuite/go.test/test/fixedbugs/bug076.go
===================================================================
--- gcc/testsuite/go.test/test/fixedbugs/bug076.go	(revision 171359)
+++ gcc/testsuite/go.test/test/fixedbugs/bug076.go	(working copy)
@@ -1,4 +1,4 @@ 
-// $G $D/$F.go && $L $F.$A && ./$A.out
+// $G $D/$F.go && $L $F.$A
 
 // Copyright 2009 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
@@ -7,12 +7,16 @@ 
 package main
 
 func f() {
-exit: ;
+exit:
+	;
+	goto exit
 }
 
 
 func main() {
-exit: ; // this should be legal (labels not properly scoped?)
+exit:
+	; // this should be legal (labels not properly scoped?)
+	goto exit
 }
 
 /*
Index: gcc/testsuite/go.test/test/fixedbugs/bug077.go
===================================================================
--- gcc/testsuite/go.test/test/fixedbugs/bug077.go	(revision 171359)
+++ gcc/testsuite/go.test/test/fixedbugs/bug077.go	(working copy)
@@ -7,7 +7,8 @@ 
 package main
 
 func main() {
-	var exit int;
+	var exit int
 exit:
-	_ = exit;
+	_ = exit
+	goto exit
 }
Index: gcc/testsuite/go.test/test/fixedbugs/bug274.go
===================================================================
--- gcc/testsuite/go.test/test/fixedbugs/bug274.go	(revision 171359)
+++ gcc/testsuite/go.test/test/fixedbugs/bug274.go	(working copy)
@@ -24,6 +24,7 @@  func main() {
 	case 1:
 		L1:  // ERROR "statement"
 	default:
-		L2:  // correct since no semicolon is required before a '}'
+		     // correct since no semicolon is required before a '}'
+		L2:  // GCCGO_ERROR "not used"
 	}
 }
Index: gcc/testsuite/go.test/test/fixedbugs/bug055.go
===================================================================
--- gcc/testsuite/go.test/test/fixedbugs/bug055.go	(revision 171359)
+++ gcc/testsuite/go.test/test/fixedbugs/bug055.go	(working copy)
@@ -7,16 +7,21 @@ 
 package main
 
 func main() {
-	var i int;
-	var j int;
-	if true {}
-	{ return }
-	i = 0;
-	if true {} else i++;
-	type s struct {};
-	i = 0;
-	type s2 int;
-	var k = func (a int) int { return a+1 }(3);
-	_, _ = j, k;
-ro: ;
+	var i int
+	var j int
+	if true {
+	}
+	{
+		return
+	}
+	i = 0
+	if true {
+	} else {
+		i++
+	}
+	type s struct{}
+	i = 0
+	type s2 int
+	var k = func(a int) int { return a + 1 }(3)
+	_, _ = j, k
 }
Index: gcc/testsuite/go.test/test/fixedbugs/bug140.go
===================================================================
--- gcc/testsuite/go.test/test/fixedbugs/bug140.go	(revision 171377)
+++ gcc/testsuite/go.test/test/fixedbugs/bug140.go	(working copy)
@@ -7,8 +7,17 @@ 
 package main
 
 func main() {
-	if true {} else L1: ;
-	if true {} else L2: main() ;
+	if true {
+	} else {
+	L1:
+	}
+	if true {
+	} else {
+	L2:
+		main()
+	}
+	goto L1
+	goto L2
 }
 
 /*
Index: gcc/testsuite/go.test/test/fixedbugs/bug137.go
===================================================================
--- gcc/testsuite/go.test/test/fixedbugs/bug137.go	(revision 171359)
+++ gcc/testsuite/go.test/test/fixedbugs/bug137.go	(working copy)
@@ -8,16 +8,21 @@  package main
 
 func main() {
 L1:
-L2:	for i := 0; i < 10; i++ {
-		print(i);
-		break L2;
+L2:
+	for i := 0; i < 10; i++ {
+		print(i)
+		break L2
 	}
 
-L3: ;
-L4:	for i := 0; i < 10; i++ {
-		print(i);
-		break L4;
+L3:
+	;
+L4:
+	for i := 0; i < 10; i++ {
+		print(i)
+		break L4
 	}
+	goto L1
+	goto L3
 }
 
 /*
Index: gcc/testsuite/go.test/test/fixedbugs/bug091.go
===================================================================
--- gcc/testsuite/go.test/test/fixedbugs/bug091.go	(revision 171359)
+++ gcc/testsuite/go.test/test/fixedbugs/bug091.go	(working copy)
@@ -7,18 +7,19 @@ 
 package main
 
 func f1() {
-	exit:
-		print("hi\n");
+exit:
+	print("hi\n")
+	goto exit
 }
 
 func f2() {
-	const c = 1234;
+	const c = 1234
 }
 
 func f3() {
-	i := c;	// ERROR "undef"
+	i := c // ERROR "undef"
 }
 
 func main() {
-	f3();
+	f3()
 }