Go patch committed: Implement goto restrictions

[Ian Lance Taylor <iant at google dot com>]

The Go language has clarified the use of goto.  It is now invalid to use
goto to jump into an inner block, and to use goto to jump across a
variable declaration.  This patch to the Go frontend implements these
restrictions, and adjust one library and a few test cases accordingly.
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 179010)
+++ gcc/go/gofrontend/gogo.cc	(working copy)
@@ -857,7 +857,7 @@ Gogo::add_label_definition(const std::st
 {
   go_assert(!this->functions_.empty());
   Function* func = this->functions_.back().function->func_value();
-  Label* label = func->add_label_definition(label_name, location);
+  Label* label = func->add_label_definition(this, label_name, location);
   this->add_statement(Statement::make_label_statement(label, location));
   return label;
 }
@@ -865,11 +865,21 @@ Gogo::add_label_definition(const std::st
 // Add a label reference.
 
 Label*
-Gogo::add_label_reference(const std::string& label_name)
+Gogo::add_label_reference(const std::string& label_name,
+			  source_location location, bool issue_goto_errors)
 {
   go_assert(!this->functions_.empty());
   Function* func = this->functions_.back().function->func_value();
-  return func->add_label_reference(label_name);
+  return func->add_label_reference(this, label_name, location,
+				   issue_goto_errors);
+}
+
+// Return the current binding state.
+
+Bindings_snapshot*
+Gogo::bindings_snapshot(source_location location)
+{
+  return new Bindings_snapshot(this->current_block(), location);
 }
 
 // Add a statement.
@@ -2843,30 +2853,24 @@ Function::is_method() const
 // Add a label definition.
 
 Label*
-Function::add_label_definition(const std::string& label_name,
+Function::add_label_definition(Gogo* gogo, const std::string& label_name,
 			       source_location location)
 {
   Label* lnull = NULL;
   std::pair<Labels::iterator, bool> ins =
     this->labels_.insert(std::make_pair(label_name, lnull));
+  Label* label;
   if (ins.second)
     {
       // This is a new label.
-      Label* label = new Label(label_name);
-      label->define(location);
+      label = new Label(label_name);
       ins.first->second = label;
-      return label;
     }
   else
     {
       // The label was already in the hash table.
-      Label* label = ins.first->second;
-      if (!label->is_defined())
-	{
-	  label->define(location);
-	  return label;
-	}
-      else
+      label = ins.first->second;
+      if (label->is_defined())
 	{
 	  error_at(location, "label %qs already defined",
 		   Gogo::message_name(label_name).c_str());
@@ -2875,31 +2879,55 @@ Function::add_label_definition(const std
 	  return new Label(label_name);
 	}
     }
+
+  label->define(location, gogo->bindings_snapshot(location));
+
+  // Issue any errors appropriate for any previous goto's to this
+  // label.
+  const std::vector<Bindings_snapshot*>& refs(label->refs());
+  for (std::vector<Bindings_snapshot*>::const_iterator p = refs.begin();
+       p != refs.end();
+       ++p)
+    (*p)->check_goto_to(gogo->current_block());
+  label->clear_refs();
+
+  return label;
 }
 
 // Add a reference to a label.
 
 Label*
-Function::add_label_reference(const std::string& label_name)
+Function::add_label_reference(Gogo* gogo, const std::string& label_name,
+			      source_location location, bool issue_goto_errors)
 {
   Label* lnull = NULL;
   std::pair<Labels::iterator, bool> ins =
     this->labels_.insert(std::make_pair(label_name, lnull));
+  Label* label;
   if (!ins.second)
     {
       // The label was already in the hash table.
-      Label* label = ins.first->second;
-      label->set_is_used();
-      return label;
+      label = ins.first->second;
     }
   else
     {
       go_assert(ins.first->second == NULL);
-      Label* label = new Label(label_name);
+      label = new Label(label_name);
       ins.first->second = label;
-      label->set_is_used();
-      return label;
     }
+
+  label->set_is_used();
+
+  if (issue_goto_errors)
+    {
+      Bindings_snapshot* snapshot = label->snapshot();
+      if (snapshot != NULL)
+	snapshot->check_goto_from(gogo->current_block(), location);
+      else
+	label->add_snapshot_ref(gogo->bindings_snapshot(location));
+    }
+
+  return label;
 }
 
 // Warn about labels that are defined but not used.
@@ -3407,6 +3435,92 @@ Block::get_backend(Translate_context* co
   return ret;
 }
 
+// Class Bindings_snapshot.
+
+Bindings_snapshot::Bindings_snapshot(const Block* b, source_location location)
+  : block_(b), counts_(), location_(location)
+{
+  while (b != NULL)
+    {
+      this->counts_.push_back(b->bindings()->size_definitions());
+      b = b->enclosing();
+    }
+}
+
+// Report errors appropriate for a goto from B to this.
+
+void
+Bindings_snapshot::check_goto_from(const Block* b, source_location loc)
+{
+  size_t dummy;
+  if (!this->check_goto_block(loc, b, this->block_, &dummy))
+    return;
+  this->check_goto_defs(loc, this->block_,
+			this->block_->bindings()->size_definitions(),
+			this->counts_[0]);
+}
+
+// Report errors appropriate for a goto from this to B.
+
+void
+Bindings_snapshot::check_goto_to(const Block* b)
+{
+  size_t index;
+  if (!this->check_goto_block(this->location_, this->block_, b, &index))
+    return;
+  this->check_goto_defs(this->location_, b, this->counts_[index],
+			b->bindings()->size_definitions());
+}
+
+// Report errors appropriate for a goto at LOC from BFROM to BTO.
+// Return true if all is well, false if we reported an error.  If this
+// returns true, it sets *PINDEX to the number of blocks BTO is above
+// BFROM.
+
+bool
+Bindings_snapshot::check_goto_block(source_location loc, const Block* bfrom,
+				    const Block* bto, size_t* pindex)
+{
+  // It is an error if BTO is not either BFROM or above BFROM.
+  size_t index = 0;
+  for (const Block* pb = bfrom; pb != bto; pb = pb->enclosing(), ++index)
+    {
+      if (pb == NULL)
+	{
+	  error_at(loc, "goto jumps into block");
+	  inform(bto->start_location(), "goto target block starts here");
+	  return false;
+	}
+    }
+  *pindex = index;
+  return true;
+}
+
+// Report errors appropriate for a goto at LOC ending at BLOCK, where
+// CFROM is the number of names defined at the point of the goto and
+// CTO is the number of names defined at the point of the label.
+
+void
+Bindings_snapshot::check_goto_defs(source_location loc, const Block* block,
+				   size_t cfrom, size_t cto)
+{
+  if (cfrom < cto)
+    {
+      Bindings::const_definitions_iterator p =
+	block->bindings()->begin_definitions();
+      for (size_t i = 0; i < cfrom; ++i)
+	{
+	  go_assert(p != block->bindings()->end_definitions());
+	  ++p;
+	}
+      go_assert(p != block->bindings()->end_definitions());
+
+      std::string n = (*p)->message_name();
+      error_at(loc, "goto jumps over declaration of %qs", n.c_str());
+      inform((*p)->location(), "%qs defined here", n.c_str());
+    }
+}
+
 // Class Variable.
 
 Variable::Variable(Type* type, Expression* init, bool is_global,
@@ -4698,6 +4812,18 @@ Bindings::traverse(Traverse* traverse, b
 
 // Class Label.
 
+// Clear any references to this label.
+
+void
+Label::clear_refs()
+{
+  for (std::vector<Bindings_snapshot*>::iterator p = this->refs_.begin();
+       p != this->refs_.end();
+       ++p)
+    delete *p;
+  this->refs_.clear();
+}
+
 // Get the backend representation for a label.
 
 Blabel*
Index: gcc/go/gofrontend/gogo.h
===================================================================
--- gcc/go/gofrontend/gogo.h	(revision 178784)
+++ gcc/go/gofrontend/gogo.h	(working copy)
@@ -22,6 +22,7 @@ class Temporary_statement;
 class Block;
 class Function;
 class Bindings;
+class Bindings_snapshot;
 class Package;
 class Variable;
 class Pointer_type;
@@ -246,6 +247,10 @@ class Gogo
   Named_object*
   current_function() const;
 
+  // Return the current block.
+  Block*
+  current_block();
+
   // Start a new block.  This is not initially associated with a
   // function.
   void
@@ -269,9 +274,16 @@ class Gogo
   Label*
   add_label_definition(const std::string&, source_location);
 
-  // Add a label reference.
+  // Add a label reference.  ISSUE_GOTO_ERRORS is true if we should
+  // report errors for a goto from the current location to the label
+  // location.
   Label*
-  add_label_reference(const std::string&);
+  add_label_reference(const std::string&, source_location,
+		      bool issue_goto_errors);
+
+  // Return a snapshot of the current binding state.
+  Bindings_snapshot*
+  bindings_snapshot(source_location);
 
   // Add a statement to the current block.
   void
@@ -551,10 +563,6 @@ class Gogo
   const Bindings*
   current_bindings() const;
 
-  // Return the current block.
-  Block*
-  current_block();
-
   // Get the name of the magic initialization function.
   const std::string&
   get_init_fn_name();
@@ -833,11 +841,14 @@ class Function
 
   // Add a label definition to the function.
   Label*
-  add_label_definition(const std::string& label_name, source_location);
+  add_label_definition(Gogo*, const std::string& label_name, source_location);
 
-  // Add a label reference to a function.
+  // Add a label reference to a function.  ISSUE_GOTO_ERRORS is true
+  // if we should report errors for a goto from the current location
+  // to the label location.
   Label*
-  add_label_reference(const std::string& label_name);
+  add_label_reference(Gogo*, const std::string& label_name,
+		      source_location, bool issue_goto_errors);
 
   // Warn about labels that are defined but not used.
   void
@@ -980,6 +991,40 @@ class Function
   bool has_recover_thunk_;
 };
 
+// A snapshot of the current binding state.
+
+class Bindings_snapshot
+{
+ public:
+  Bindings_snapshot(const Block*, source_location);
+
+  // Report any errors appropriate for a goto from the current binding
+  // state of B to this one.
+  void
+  check_goto_from(const Block* b, source_location);
+
+  // Report any errors appropriate for a goto from this binding state
+  // to the current state of B.
+  void
+  check_goto_to(const Block* b);
+
+ private:
+  bool
+  check_goto_block(source_location, const Block*, const Block*, size_t*);
+
+  void
+  check_goto_defs(source_location, const Block*, size_t, size_t);
+
+  // The current block.
+  const Block* block_;
+  // The number of names currently defined in each open block.
+  // Element 0 is this->block_, element 1 is
+  // this->block_->enclosing(), etc.
+  std::vector<size_t> counts_;
+  // The location where this snapshot was taken.
+  source_location location_;
+};
+
 // A function declaration.
 
 class Function_declaration
@@ -2108,7 +2153,8 @@ class Label
 {
  public:
   Label(const std::string& name)
-    : name_(name), location_(0), is_used_(false), blabel_(NULL)
+    : name_(name), location_(0), snapshot_(NULL), refs_(), is_used_(false),
+      blabel_(NULL)
   { }
 
   // Return the label's name.
@@ -2136,12 +2182,36 @@ class Label
   location() const
   { return this->location_; }
 
-  // Define the label at LOCATION.
+  // Return the bindings snapshot.
+  Bindings_snapshot*
+  snapshot() const
+  { return this->snapshot_; }
+
+  // Add a snapshot of a goto which refers to this label.
   void
-  define(source_location location)
+  add_snapshot_ref(Bindings_snapshot* snapshot)
   {
     go_assert(this->location_ == 0);
+    this->refs_.push_back(snapshot);
+  }
+
+  // Return the list of snapshots of goto statements which refer to
+  // this label.
+  const std::vector<Bindings_snapshot*>&
+  refs() const
+  { return this->refs_; }
+
+  // Clear the references.
+  void
+  clear_refs();
+
+  // Define the label at LOCATION with the given bindings snapshot.
+  void
+  define(source_location location, Bindings_snapshot* snapshot)
+  {
+    go_assert(this->location_ == 0 && this->snapshot_ == NULL);
     this->location_ = location;
+    this->snapshot_ = snapshot;
   }
 
   // Return the backend representation for this label.
@@ -2160,6 +2230,11 @@ class Label
   // The location of the definition.  This is 0 if the label has not
   // yet been defined.
   source_location location_;
+  // A snapshot of the set of bindings defined at this label, used to
+  // issue errors about invalid goto statements.
+  Bindings_snapshot* snapshot_;
+  // A list of snapshots of goto statements which refer to this label.
+  std::vector<Bindings_snapshot*> refs_;
   // Whether the label has been used.
   bool is_used_;
   // The backend representation.
Index: gcc/go/gofrontend/parse.cc
===================================================================
--- gcc/go/gofrontend/parse.cc	(revision 179010)
+++ gcc/go/gofrontend/parse.cc	(working copy)
@@ -3813,7 +3813,8 @@ Parse::return_stat()
   this->gogo_->add_statement(Statement::make_return_statement(vals, location));
 }
 
-// IfStmt    = "if" [ SimpleStmt ";" ] Expression Block [ "else" Statement ] .
+// IfStmt = "if" [ SimpleStmt ";" ] Expression Block
+//          [ "else" ( IfStmt | Block ) ] .
 
 void
 Parse::if_stat()
@@ -3883,10 +3884,17 @@ Parse::if_stat()
   Block* else_block = NULL;
   if (this->peek_token()->is_keyword(KEYWORD_ELSE))
     {
-      this->advance_token();
-      // We create a block to gather the statement.
       this->gogo_->start_block(this->location());
-      this->statement(NULL);
+      const Token* token = this->advance_token();
+      if (token->is_keyword(KEYWORD_IF))
+	this->if_stat();
+      else if (token->is_op(OPERATOR_LCURLY))
+	this->block();
+      else
+	{
+	  error_at(this->location(), "expected %<if%> or %<{%>");
+	  this->statement(NULL);
+	}
       else_block = this->gogo_->finish_block(this->location());
     }
 
@@ -4914,7 +4922,7 @@ Parse::break_stat()
 	{
 	  // 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());
+	  this->gogo_->add_label_reference(token->identifier(), 0, false);
 
 	  error_at(token->location(), "invalid break label %qs",
 		   Gogo::message_name(token->identifier()).c_str());
@@ -4969,7 +4977,7 @@ Parse::continue_stat()
 	{
 	  // 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());
+	  this->gogo_->add_label_reference(token->identifier(), 0, false);
 
 	  error_at(token->location(), "invalid continue label %qs",
 		   Gogo::message_name(token->identifier()).c_str());
@@ -5003,7 +5011,8 @@ Parse::goto_stat()
     error_at(this->location(), "expected label for goto");
   else
     {
-      Label* label = this->gogo_->add_label_reference(token->identifier());
+      Label* label = this->gogo_->add_label_reference(token->identifier(),
+						      location, true);
       Statement* s = Statement::make_goto_statement(label, location);
       this->gogo_->add_statement(s);
       this->advance_token();
Index: gcc/go/gofrontend/statements.cc
===================================================================
--- gcc/go/gofrontend/statements.cc	(revision 179008)
+++ gcc/go/gofrontend/statements.cc	(working copy)
@@ -2291,7 +2291,7 @@ Thunk_statement::build_thunk(Gogo* gogo,
   Label* retaddr_label = NULL;
   if (may_call_recover)
     {
-      retaddr_label = gogo->add_label_reference("retaddr");
+      retaddr_label = gogo->add_label_reference("retaddr", location, false);
       Expression* arg = Expression::make_label_addr(retaddr_label, location);
       Expression* call = Runtime::make_call(Runtime::SET_DEFER_RETADDR,
 					    location, 1, arg);
Index: libgo/syscalls/exec.go
===================================================================
--- libgo/syscalls/exec.go	(revision 178910)
+++ libgo/syscalls/exec.go	(working copy)
@@ -231,6 +231,7 @@ var zeroSysProcAttr SysProcAttr
 
 func forkExec(argv0 string, argv []string, attr *ProcAttr) (pid int, err int) {
 	var p [2]int
+	var n Ssize_t
 	var r1 int
 	var err1 uintptr
 	var wstatus WaitStatus
@@ -283,20 +284,14 @@ func forkExec(argv0 string, argv []strin
 	// Kick off child.
 	pid, err = forkAndExecInChild(argv0p, argvp, envvp, chroot, dir, attr, sys, p[1])
 	if err != 0 {
-	error:
-		if p[0] >= 0 {
-			Close(p[0])
-			Close(p[1])
-		}
-		ForkLock.Unlock()
-		return 0, err
+		goto error
 	}
 	ForkLock.Unlock()
 
 	// Read child error status from pipe.
 	Close(p[1])
-	n := libc_read(p[0], (*byte)(unsafe.Pointer(&err1)),
-		       Size_t(unsafe.Sizeof(err1)))
+	n = libc_read(p[0], (*byte)(unsafe.Pointer(&err1)),
+		      Size_t(unsafe.Sizeof(err1)))
 	err = 0
 	if n < 0 {
 		err = GetErrno()
@@ -321,6 +316,14 @@ func forkExec(argv0 string, argv []strin
 
 	// Read got EOF, so pipe closed on exec, so exec succeeded.
 	return pid, 0
+
+error:
+	if p[0] >= 0 {
+		Close(p[0])
+		Close(p[1])
+	}
+	ForkLock.Unlock()
+	return 0, err
 }
 
 // Combination of fork and exec, careful to be thread safe.
Index: gcc/testsuite/go.test/test/fixedbugs/bug178.go
===================================================================
--- gcc/testsuite/go.test/test/fixedbugs/bug178.go	(revision 178784)
+++ gcc/testsuite/go.test/test/fixedbugs/bug178.go	(working copy)
@@ -14,6 +14,9 @@ L:
 			break L
 		}
 		panic("BUG: not reached - break")
+		if false {
+			goto L1
+		}
 	}
 
 L2:
@@ -23,11 +26,8 @@ L2:
 			continue L2
 		}
 		panic("BUG: not reached - continue")
-	}
-	if false {
-		goto L1
-	}
-	if false {
-		goto L3
+		if false {
+			goto L3
+		}
 	}
 }
Index: gcc/testsuite/go.test/test/fixedbugs/bug140.go
===================================================================
--- gcc/testsuite/go.test/test/fixedbugs/bug140.go	(revision 178784)
+++ gcc/testsuite/go.test/test/fixedbugs/bug140.go	(working copy)
@@ -10,14 +10,14 @@ func main() {
 	if true {
 	} else {
 	L1:
+		goto L1
 	}
 	if true {
 	} else {
+		goto L2
 	L2:
 		main()
 	}
-	goto L1
-	goto L2
 }
 
 /*
Index: gcc/testsuite/go.test/test/if.go
===================================================================
--- gcc/testsuite/go.test/test/if.go	(revision 178784)
+++ gcc/testsuite/go.test/test/if.go	(working copy)
@@ -53,25 +53,28 @@ func main() {
 	count = 0
 	if true {
 		count = count + 1
-	} else
+	} else {
 		count = count - 1
+	}
 	assertequal(count, 1, "if else true")
 
 	count = 0
 	if false {
 		count = count + 1
-	} else
+	} else {
 		count = count - 1
+	}
 	assertequal(count, -1, "if else false")
 
 	count = 0
-	if t:=1; false {
+	if t := 1; false {
 		count = count + 1
 		_ = t
 		t := 7
 		_ = t
-	} else
+	} else {
 		count = count - t
+	}
 	assertequal(count, -1, "if else false var")
 
 	count = 0
@@ -80,8 +83,9 @@ func main() {
 		count = count + 1
 		t := 7
 		_ = t
-	} else
+	} else {
 		count = count - t
+	}
 	_ = t
 	assertequal(count, -1, "if else false var outside")
 }