Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go/types: recursive functions cause broken InitOrder #10709

Closed
nlacasse opened this issue May 5, 2015 · 5 comments
Closed

go/types: recursive functions cause broken InitOrder #10709

nlacasse opened this issue May 5, 2015 · 5 comments
Assignees
Milestone

Comments

@nlacasse
Copy link

@nlacasse nlacasse commented May 5, 2015

If you have a recursive function called in an init function (or toplevel var block), the parsed InitOrder can be broken. It will report that some variables should be initialized before their dependencies.

For example, in program.go below, we have two variables: m and fooInst. The variable m depends on fooInst, and fooInst is the result of a recursive function call.

package program

var (
    m       = fooInst.Method()
    fooInst = newFoo()
)

type Foo struct{}

func (t *Foo) Method() bool {
    return true
}

func newFoo() *Foo {
    return newFoo()
}

The initialization order should be fooInst then m.

However, after parsing program.go with parser.ParseFile and then calling config.Check, the resulting InitOrder has m initialized before fooInst.

Here is the parsing program I used:

package main

import (
    "fmt"
    "go/ast"
    "go/parser"
    "go/token"
    "golang.org/x/tools/go/types"
)

func main() {
    fset := token.NewFileSet()

    f, err := parser.ParseFile(fset, "program.go", nil, 0)
    if err != nil {
        panic(err)
    }

    info := &types.Info{}
    config := types.Config{}

    _, err = config.Check("", fset, []*ast.File{f}, info)
    if err != nil {
        panic(err)
    }

    for k, v := range info.InitOrder {
        fmt.Printf("Initializer #%v: %v\n", k, v)
    }
}

And the output:

Initializer #0: m = fooInst.Method()
Initializer #1: fooInst = newFoo()

This order is clearly broken, since m cannot be initialized before fooInst.

(All tests done with go version go1.4 linux/amd64)

@minux minux changed the title x/tools/go/types: Recursive functions cause broken InitOrder go/types: recursive functions cause broken InitOrder May 5, 2015
@ianlancetaylor ianlancetaylor added this to the Go1.5Maybe milestone Jun 3, 2015
@griesemer
Copy link
Contributor

@griesemer griesemer commented Jun 4, 2015

Here's a more "realistic" example in the sense that the code would actually run (rather than lead to endless recursion and thus stack overflow):

package p

var (
    m = x.m()
    x = makeT(10)
)

type T struct{}

func (T) m() int {
    return 0
}

func makeT(n int) T {
    if n > 0 {
        return makeT(n-1)
    }
    return T{}
}

go/types also reports the wrong initialization order in this case.

The issue is that initialization cycles through functions only are silently ignored and then lead to a wrong initialization order (initorder.go:52). Instead, cycles involving only functions should probably be removed beforehand.

Loading

@griesemer griesemer added this to the Go1.6 milestone Jul 20, 2015
@griesemer griesemer removed this from the Go1.5Maybe milestone Jul 20, 2015
@rsc rsc added this to the Unplanned milestone Nov 30, 2015
@rsc rsc removed this from the Go1.6 milestone Nov 30, 2015
@griesemer griesemer added this to the Go1.7 milestone Feb 25, 2016
@griesemer griesemer removed this from the Unplanned milestone Feb 25, 2016
@cierniak
Copy link

@cierniak cierniak commented Feb 25, 2016

This bug makes it impossible to use go/types as part of a frontend for a compiler. Here's an example of a Go package that is compiled incorrectly and fails at runtime because of incorrect variable initialization order: https://github.com/vanadium/go.v23/tree/master/vdl

Loading

@griesemer
Copy link
Contributor

@griesemer griesemer commented May 6, 2016

This is still high on the list for 1.7.

Loading

@rsc rsc added this to the Go1.8 milestone May 18, 2016
@rsc rsc removed this from the Go1.7 milestone May 18, 2016
@gopherbot
Copy link

@gopherbot gopherbot commented May 27, 2016

CL https://golang.org/cl/23442 mentions this issue.

Loading

mk0x9 pushed a commit to mk0x9/go that referenced this issue May 27, 2016
Also: Added some test cases for issue golang#10709.
No impact when debugging output is disabled (default).

For golang#10709.

Change-Id: I0751befb222c86d46225377a674f6bad2990349e
Reviewed-on: https://go-review.googlesource.com/23442
Reviewed-by: Alan Donovan <adonovan@google.com>
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 15, 2016

CL https://golang.org/cl/24131 mentions this issue.

Loading

@gopherbot gopherbot closed this in 5c84441 Aug 16, 2016
@golang golang locked and limited conversation to collaborators Aug 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants