Patchwork Go patch committed: Implement Go 1.1 terminating statements

login
register
mail settings
Submitter Ian Taylor
Date June 12, 2013, 11:50 p.m.
Message ID <mcr38smopn6.fsf@iant-glaptop.roam.corp.google.com>
Download mbox | patch
Permalink /patch/250931/
State New
Headers show

Comments

Ian Taylor - June 12, 2013, 11:50 p.m.
The Go 1.1 release defines what it means for a function to have a
terminating statement, and makes it an error for a function with return
values to not have one.  This patch, from Rémy Oudompheng, implements
that for gccgo.  Bootstrapped and ran testsuite on
x86_64-unknown-linux-gnu.  Committed to mainline and 4.8 branch.

Ian

Patch

diff -r 8c0d464139d1 go/go.cc
--- a/go/go.cc	Fri Mar 01 11:26:17 2013 -0800
+++ b/go/go.cc	Wed Jun 12 15:27:02 2013 -0700
@@ -44,7 +44,7 @@ 
 GO_EXTERN_C
 void
 go_parse_input_files(const char** filenames, unsigned int filename_count,
-		     bool only_check_syntax, bool require_return_statement)
+		     bool only_check_syntax, bool)
 {
   go_assert(filename_count > 0);
 
@@ -84,6 +84,9 @@ 
   // Finalize method lists and build stub methods for named types.
   ::gogo->finalize_methods();
 
+  // Check that functions have a terminating statement.
+  ::gogo->check_return_statements();
+
   // Now that we have seen all the names, lower the parse tree into a
   // form which is easier to use.
   ::gogo->lower_parse_tree();
@@ -104,10 +107,6 @@ 
   if (only_check_syntax)
     return;
 
-  // Check that functions have return statements.
-  if (require_return_statement)
-    ::gogo->check_return_statements();
-
   // Export global identifiers as appropriate.
   ::gogo->do_exports();
 
diff -r 8c0d464139d1 go/statements.cc
--- a/go/statements.cc	Fri Mar 01 11:26:17 2013 -0800
+++ b/go/statements.cc	Wed Jun 12 15:27:02 2013 -0700
@@ -1707,8 +1707,8 @@ 
     this->expr_->discarding_value();
 }
 
-// An expression statement may fall through unless it is a call to a
-// function which does not return.
+// An expression statement is only a terminating statement if it is
+// a call to panic.
 
 bool
 Expression_statement::do_may_fall_through() const
@@ -1717,22 +1717,28 @@ 
   if (call != NULL)
     {
       const Expression* fn = call->fn();
-      const Func_expression* fe = fn->func_expression();
-      if (fe != NULL)
+      // panic is still an unknown named object.
+      const Unknown_expression* ue = fn->unknown_expression();
+      if (ue != NULL)
 	{
-	  const Named_object* no = fe->named_object();
-
-	  Function_type* fntype;
-	  if (no->is_function())
-	    fntype = no->func_value()->type();
-	  else if (no->is_function_declaration())
-	    fntype = no->func_declaration_value()->type();
-	  else
-	    fntype = NULL;
-
-	  // The builtin function panic does not return.
-	  if (fntype != NULL && fntype->is_builtin() && no->name() == "panic")
-	    return false;
+	  Named_object* no = ue->named_object();
+
+          if (no->is_unknown())
+            no = no->unknown_value()->real_named_object();
+          if (no != NULL)
+            {
+              Function_type* fntype;
+              if (no->is_function())
+                fntype = no->func_value()->type();
+              else if (no->is_function_declaration())
+                fntype = no->func_declaration_value()->type();
+              else
+                fntype = NULL;
+
+              // The builtin function panic does not return.
+              if (fntype != NULL && fntype->is_builtin() && no->name() == "panic")
+                return false;
+            }
 	}
     }
   return true;
@@ -3700,9 +3706,6 @@ 
   void
   do_check_types(Gogo*);
 
-  bool
-  do_may_fall_through() const;
-
   Bstatement*
   do_get_backend(Translate_context*);
 
@@ -3746,22 +3749,6 @@ 
     this->set_is_error();
 }
 
-// Return whether this switch may fall through.
-
-bool
-Constant_switch_statement::do_may_fall_through() const
-{
-  if (this->clauses_ == NULL)
-    return true;
-
-  // If we have a break label, then some case needed it.  That implies
-  // that the switch statement as a whole can fall through.
-  if (this->break_label_ != NULL)
-    return true;
-
-  return this->clauses_->may_fall_through();
-}
-
 // Convert to GENERIC.
 
 Bstatement*
@@ -3911,6 +3898,22 @@ 
   ast_dump_context->ostream() << std::endl;
 }
 
+// Return whether this switch may fall through.
+
+bool
+Switch_statement::do_may_fall_through() const
+{
+  if (this->clauses_ == NULL)
+    return true;
+
+  // If we have a break label, then some case needed it.  That implies
+  // that the switch statement as a whole can fall through.
+  if (this->break_label_ != NULL)
+    return true;
+
+  return this->clauses_->may_fall_through();
+}
+
 // Make a switch statement.
 
 Switch_statement*
@@ -4050,6 +4053,17 @@ 
     }
 }
 
+// Return true if this type clause may fall through to the statements
+// following the switch.
+
+bool
+Type_case_clauses::Type_case_clause::may_fall_through() const
+{
+  if (this->statements_ == NULL)
+    return true;
+  return this->statements_->may_fall_through();
+}
+
 // Dump the AST representation for a type case clause
 
 void
@@ -4148,6 +4162,25 @@ 
 			NULL);
 }
 
+// Return true if these clauses may fall through to the statements
+// following the switch statement.
+
+bool
+Type_case_clauses::may_fall_through() const
+{
+  bool found_default = false;
+  for (Type_clauses::const_iterator p = this->clauses_.begin();
+       p != this->clauses_.end();
+       ++p)
+    {
+      if (p->may_fall_through())
+	return true;
+      if (p->is_default())
+	found_default = true;
+    }
+  return !found_default;
+}
+
 // Dump the AST representation for case clauses (from a switch statement)
 
 void
@@ -4237,6 +4270,22 @@ 
   return Statement::make_block_statement(b, loc);
 }
 
+// Return whether this switch may fall through.
+
+bool
+Type_switch_statement::do_may_fall_through() const
+{
+  if (this->clauses_ == NULL)
+    return true;
+
+  // If we have a break label, then some case needed it.  That implies
+  // that the switch statement as a whole can fall through.
+  if (this->break_label_ != NULL)
+    return true;
+
+  return this->clauses_->may_fall_through();
+}
+
 // Return the break label for this type switch statement, creating it
 // if necessary.
 
@@ -4954,6 +5003,19 @@ 
   return Statement::make_block_statement(b, loc);
 }
 
+// Whether the select statement itself may fall through to the following
+// statement.
+
+bool
+Select_statement::do_may_fall_through() const
+{
+  // A select statement is terminating if no break statement
+  // refers to it and all of its clauses are terminating.
+  if (this->break_label_ != NULL)
+    return true;
+  return this->clauses_->may_fall_through();
+}
+
 // Return the backend representation for a select statement.
 
 Bstatement*
@@ -5114,6 +5176,20 @@ 
   this->continue_label_ = continue_label;
 }
 
+// Whether the overall statement may fall through.
+
+bool
+For_statement::do_may_fall_through() const
+{
+  // A for loop is terminating if it has no condition and
+  // no break statement.
+  if(this->cond_ != NULL)
+    return true;
+  if(this->break_label_ != NULL)
+    return true;
+  return false;
+}
+
 // Dump the AST representation for a for statement.
 
 void
diff -r 8c0d464139d1 go/statements.h
--- a/go/statements.h	Fri Mar 01 11:26:17 2013 -0800
+++ b/go/statements.h	Wed Jun 12 15:27:02 2013 -0700
@@ -894,8 +894,7 @@ 
   { this->clauses_->check_types(); }
 
   bool
-  do_may_fall_through() const
-  { return this->clauses_->may_fall_through(); }
+  do_may_fall_through() const;
 
   Bstatement*
   do_get_backend(Translate_context*);
@@ -1086,6 +1085,9 @@ 
   Statement*
   do_lower(Gogo*, Named_object*, Block*, Statement_inserter*);
 
+  bool
+  do_may_fall_through() const;
+
   Bstatement*
   do_get_backend(Translate_context*)
   { go_unreachable(); }
@@ -1399,6 +1401,9 @@ 
   void
   do_dump_statement(Ast_dump_context*) const;
 
+  bool
+  do_may_fall_through() const;
+
  private:
   // The value to switch on.  This may be NULL.
   Expression* val_;
@@ -1449,6 +1454,11 @@ 
   lower(Type*, Block*, Temporary_statement* descriptor_temp,
 	Unnamed_label* break_label) const;
 
+  // Return true if these clauses may fall through to the statements
+  // following the switch statement.
+  bool
+  may_fall_through() const;
+
   // Dump the AST representation to a dump context.
   void
   dump_clauses(Ast_dump_context*) const;
@@ -1493,6 +1503,12 @@ 
     lower(Type*, Block*, Temporary_statement* descriptor_temp,
 	  Unnamed_label* break_label, Unnamed_label** stmts_label) const;
 
+    // Return true if this clause may fall through to execute the
+    // statements following the switch statement.  This is not the
+    // same as whether this clause falls through to the next clause.
+    bool
+    may_fall_through() const;
+
     // Dump the AST representation to a dump context.
     void
     dump_clause(Ast_dump_context*) const;
@@ -1556,6 +1572,9 @@ 
   void
   do_dump_statement(Ast_dump_context*) const;
 
+  bool
+  do_may_fall_through() const;
+
  private:
   // The variable holding the value we are switching on.
   Named_object* var_;