This patch to the Go frontend fixes the dependency analysis for code
like
func foo() (int, int)
var c = b
var a, b = foo()
This code used to crash the compiler because the initialization of c
would refer to b, which would build the call, and then the
initialization of a would not be able to see the temporary variables
used to hold the function result. This patch fixes the problem by
explicitly making b depend on a. Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu. Committed to mainline and 4.7 branch.
Ian
diff -r 26b7e0c38353 go/gogo-tree.cc
--- a/go/gogo-tree.cc Mon Apr 30 08:55:42 2012 -0700
+++ b/go/gogo-tree.cc Thu May 03 09:04:06 2012 -0700
@@ -590,10 +590,11 @@
return TRAVERSE_CONTINUE;
}
-// Return true if EXPR refers to VAR.
+// Return true if EXPR, PREINIT, or DEP refers to VAR.
static bool
-expression_requires(Expression* expr, Block* preinit, Named_object* var)
+expression_requires(Expression* expr, Block* preinit, Named_object* dep,
+ Named_object* var)
{
Find_var::Seen_objects seen_objects;
Find_var find_var(var, &seen_objects);
@@ -601,7 +602,15 @@
Expression::traverse(&expr, &find_var);
if (preinit != NULL)
preinit->traverse(&find_var);
-
+ if (dep != NULL)
+ {
+ Expression* init = dep->var_value()->init();
+ if (init != NULL)
+ Expression::traverse(&init, &find_var);
+ if (dep->var_value()->has_pre_init())
+ dep->var_value()->preinit()->traverse(&find_var);
+ }
+
return find_var.found();
}
@@ -658,7 +667,7 @@
// variable V2 then we initialize V1 after V2.
static void
-sort_var_inits(Var_inits* var_inits)
+sort_var_inits(Gogo* gogo, Var_inits* var_inits)
{
Var_inits ready;
while (!var_inits->empty())
@@ -667,6 +676,7 @@
Named_object* var = p1->var();
Expression* init = var->var_value()->init();
Block* preinit = var->var_value()->preinit();
+ Named_object* dep = gogo->var_depends_on(var->var_value());
// Start walking through the list to see which variables VAR
// needs to wait for. We can skip P1->WAITING variables--that
@@ -678,20 +688,22 @@
for (; p2 != var_inits->end(); ++p2)
{
- if (expression_requires(init, preinit, p2->var()))
+ Named_object* p2var = p2->var();
+ if (expression_requires(init, preinit, dep, p2var))
{
// Check for cycles.
- if (expression_requires(p2->var()->var_value()->init(),
- p2->var()->var_value()->preinit(),
+ if (expression_requires(p2var->var_value()->init(),
+ p2var->var_value()->preinit(),
+ gogo->var_depends_on(p2var->var_value()),
var))
{
error_at(var->location(),
("initialization expressions for %qs and "
"%qs depend upon each other"),
var->message_name().c_str(),
- p2->var()->message_name().c_str());
+ p2var->message_name().c_str());
inform(p2->var()->location(), "%qs defined here",
- p2->var()->message_name().c_str());
+ p2var->message_name().c_str());
p2 = var_inits->end();
}
else
@@ -714,9 +726,11 @@
// VAR does not depends upon any other initialization expressions.
// Check for a loop of VAR on itself. We only do this if
- // INIT is not NULL; when INIT is NULL, it means that
- // PREINIT sets VAR, which we will interpret as a loop.
- if (init != NULL && expression_requires(init, preinit, var))
+ // INIT is not NULL and there is no dependency; when INIT is
+ // NULL, it means that PREINIT sets VAR, which we will
+ // interpret as a loop.
+ if (init != NULL && dep == NULL
+ && expression_requires(init, preinit, NULL, var))
error_at(var->location(),
"initialization expression for %qs depends upon itself",
var->message_name().c_str());
@@ -783,7 +797,7 @@
}
// There is nothing useful we can output for constants which
- // have ideal or non-integeral type.
+ // have ideal or non-integral type.
if (no->is_const())
{
Type* type = no->const_value()->type();
@@ -834,7 +848,9 @@
;
else if (TREE_CONSTANT(init))
{
- if (expression_requires(no->var_value()->init(), NULL, no))
+ if (expression_requires(no->var_value()->init(), NULL,
+ this->var_depends_on(no->var_value()),
+ no))
error_at(no->location(),
"initialization expression for %qs depends "
"upon itself",
@@ -879,6 +895,14 @@
else
var_inits.push_back(Var_init(no, var_init_tree));
}
+ else if (this->var_depends_on(no->var_value()) != NULL)
+ {
+ // This variable is initialized from something that is
+ // not in its init or preinit. This variable needs to
+ // participate in dependency analysis sorting, in case
+ // some other variable depends on this one.
+ var_inits.push_back(Var_init(no, integer_zero_node));
+ }
if (!is_sink && no->var_value()->type()->has_pointer())
var_gc.push_back(no);
@@ -896,7 +920,7 @@
// workable order.
if (!var_inits.empty())
{
- sort_var_inits(&var_inits);
+ sort_var_inits(this, &var_inits);
for (Var_inits::const_iterator p = var_inits.begin();
p != var_inits.end();
++p)
diff -r 26b7e0c38353 go/gogo.cc
--- a/go/gogo.cc Mon Apr 30 08:55:42 2012 -0700
+++ b/go/gogo.cc Thu May 03 09:04:06 2012 -0700
@@ -32,6 +32,7 @@
imported_unsafe_(false),
packages_(),
init_functions_(),
+ var_deps_(),
need_init_fn_(false),
init_fn_name_(),
imported_init_fns_(),
@@ -3820,6 +3821,10 @@
Variable::lower_init_expression(Gogo* gogo, Named_object* function,
Statement_inserter* inserter)
{
+ Named_object* dep = gogo->var_depends_on(this);
+ if (dep != NULL && dep->is_variable())
+ dep->var_value()->lower_init_expression(gogo, function, inserter);
+
if (this->init_ != NULL && !this->init_is_lowered_)
{
if (this->seen_)
diff -r 26b7e0c38353 go/gogo.h
--- a/go/gogo.h Mon Apr 30 08:55:42 2012 -0700
+++ b/go/gogo.h Thu May 03 09:04:06 2012 -0700
@@ -384,6 +384,23 @@
void
clear_file_scope();
+ // Record that VAR1 must be initialized after VAR2. This is used
+ // when VAR2 does not appear in VAR1's INIT or PREINIT.
+ void
+ record_var_depends_on(Variable* var1, Named_object* var2)
+ {
+ go_assert(this->var_deps_.find(var1) == this->var_deps_.end());
+ this->var_deps_[var1] = var2;
+ }
+
+ // Return the variable that VAR depends on, or NULL if none.
+ Named_object*
+ var_depends_on(Variable* var) const
+ {
+ Var_deps::const_iterator p = this->var_deps_.find(var);
+ return p != this->var_deps_.end() ? p->second : NULL;
+ }
+
// Queue up a type-specific function to be written out. This is
// used when a type-specific function is needed when not at the top
// level.
@@ -639,8 +656,9 @@
// Type used to map package names to packages.
typedef std::map<std::string, Package*> Packages;
- // Type used to map special names in the sys package.
- typedef std::map<std::string, std::string> Sys_names;
+ // Type used to map variables to the function calls that set them.
+ // This is used for initialization dependency analysis.
+ typedef std::map<Variable*, Named_object*> Var_deps;
// Type used to queue writing a type specific function.
struct Specific_type_function
@@ -683,6 +701,10 @@
Packages packages_;
// The functions named "init", if there are any.
std::vector<Named_object*> init_functions_;
+ // A mapping from variables to the function calls that initialize
+ // them, if it is not stored in the variable's init or preinit.
+ // This is used for dependency analysis.
+ Var_deps var_deps_;
// Whether we need a magic initialization function.
bool need_init_fn_;
// The name of the magic initialization function.
diff -r 26b7e0c38353 go/parse.cc
--- a/go/parse.cc Mon Apr 30 08:55:42 2012 -0700
+++ b/go/parse.cc Thu May 03 09:04:06 2012 -0700
@@ -1667,6 +1667,7 @@
// the right number of values, but it might. Declare the variables,
// and then assign the results of the call to them.
+ Named_object* first_var = NULL;
unsigned int index = 0;
bool any_new = false;
for (Typed_identifier_list::const_iterator pv = vars->begin();
@@ -1674,7 +1675,22 @@
++pv, ++index)
{
Expression* init = Expression::make_call_result(call, index);
- this->init_var(*pv, type, init, is_coloneq, false, &any_new);
+ Named_object* no = this->init_var(*pv, type, init, is_coloneq, false,
+ &any_new);
+
+ if (this->gogo_->in_global_scope() && no->is_variable())
+ {
+ if (first_var == NULL)
+ first_var = no;
+ else
+ {
+ // The subsequent vars have an implicit dependency on
+ // the first one, so that everything gets initialized in
+ // the right order and so that we detect cycles
+ // correctly.
+ this->gogo_->record_var_depends_on(no->var_value(), first_var);
+ }
+ }
}
if (is_coloneq && !any_new)
