FAQ
Reviewers: golang-dev1,

Message:
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
cmd/gc: fix detection of initialization loop.

The compiler computes initialization order by finding
a spanning tree between a package's global variables.

But this is only required to be a tree for variables:
function names may appear multiple times in the graph,
and the algorithm used to not recurse through them,
and not detect incorrect programs.

Fixes issue 4847.

Please review this at https://codereview.appspot.com/9663047/

Affected files:
    M src/cmd/gc/sinit.c
    A test/fixedbugs/issue4847.go


Index: src/cmd/gc/sinit.c
===================================================================
--- a/src/cmd/gc/sinit.c
+++ b/src/cmd/gc/sinit.c
@@ -62,7 +62,7 @@
    if(n->initorder == InitPending) {
     if(n->class == PFUNC)
      return;
-
+
     // if there have already been errors printed,
     // those errors probably confused us and
     // there might not be a loop. let the user
@@ -82,6 +82,8 @@
     print("\t%L %S\n", n->lineno, n->sym);
     errorexit();
    }
+
+ // reached a new unvisited node.
    n->initorder = InitPending;
    l = malloc(sizeof *l);
    if(l == nil) {
@@ -94,6 +96,14 @@
    l->end = nil;
    initlist = l;

+ if(n->class == PEXTERN) {
+ // visiting a new global variable. Reset visited state of functions in
+ // order to continue graph walk.
+ for(l=xtop; l; l=l->next)
+ if(l->n->op == ODCLFUNC)
+ l->n->nname->initorder = InitNotStarted;
+ }
+
    // make sure that everything n depends on is initialized.
    // n->defn is an assignment to n
    if(n->defn != N) {
@@ -115,31 +125,16 @@
       break;
      }

- /*
- n->defn->dodata = 1;
- init1(n->defn->right, out);
+ init2(n->defn->right, out);
      if(debug['j'])
       print("%S\n", n->sym);
- *out = list(*out, n->defn);
- break;
- */
- if(1) {
- init2(n->defn->right, out);
- if(debug['j'])
- print("%S\n", n->sym);
- if(!staticinit(n, out)) {
-if(debug['%']) dump("nonstatic", n->defn);
- *out = list(*out, n->defn);
- }
- } else if(0) {
- n->defn->dodata = 1;
- init1(n->defn->right, out);
- if(debug['j'])
- print("%S\n", n->sym);
+ if(!staticinit(n, out)) {
+ if(debug['%'])
+ dump("nonstatic", n->defn);
       *out = list(*out, n->defn);
      }
      break;
-
+
     case OAS2FUNC:
     case OAS2MAPR:
     case OAS2DOTTYPE:
@@ -218,6 +213,9 @@
    }
   }

+// initfix computes initialization order for a list l of top-level
+// declarations and outputs the corresponding list of statements
+// to include in the init() function body.
   NodeList*
   initfix(NodeList *l)
   {
Index: test/fixedbugs/issue4847.go
===================================================================
new file mode 100644
--- /dev/null
+++ b/test/fixedbugs/issue4847.go
@@ -0,0 +1,22 @@
+// errorcheck
+
+// Copyright 2013 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package p
+
+type (
+ E int
+ S int
+)
+
+type matcher func(s *S) E
+
+func matchList(s *S) E { return matcher(matchAnyFn)(s) }
+
+var foo = matcher(matchList)
+
+var matchAny = matcher(matchList) // ERROR "initialization loop"
+
+func matchAnyFn(s *S) (err E) { return matchAny(s) }


--

---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Search Discussions

  • Remyoudompheng at May 27, 2013 at 6:11 am
    The error message is a bit ugly but can be improved separately:

    issue4847.go:20: initialization loop:
      issue4847.go:18 foo refers to
      issue4847.go:16 matchList refers to
      issue4847.go:22 matchAnyFn refers to
      issue4847.go:20 matchAny refers to
      issue4847.go:16 matchList refers to
      issue4847.go:22 matchAnyFn refers to
      issue4847.go:20 matchAny

    It should only say:

    issue4847.go:20: initialization loop:
      issue4847.go:20 matchAny refers to
      issue4847.go:16 matchList refers to
      issue4847.go:22 matchAnyFn refers to
      issue4847.go:20 matchAny


    https://codereview.appspot.com/9663047/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Daniel Morsing at May 27, 2013 at 7:29 am
    LGTM.

    CL description is a bit weird near the end.


    https://codereview.appspot.com/9663047/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedMay 27, '13 at 6:10a
activeMay 27, '13 at 7:29a
posts3
users2
websitegolang.org

2 users in discussion

Remyoudompheng: 2 posts Daniel Morsing: 1 post

People

Translate

site design / logo © 2022 Grokbase