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.

Sign up for GitHub

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

Jump to bottom

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
Labels
FrozenDueToAge
Milestone
Go1.8

Comments

@nlacasse
Copy link

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
@nlacasse nlacasse mentioned this issue May 6, 2015
Closed
@ianlancetaylor ianlancetaylor added this to the Go1.5Maybe milestone Jun 3, 2015
@griesemer
Copy link
Contributor

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.

@griesemer griesemer modified the milestones: Go1.6, Go1.5Maybe Jul 20, 2015
@rsc rsc modified the milestones: Unplanned, Go1.6 Nov 30, 2015
@griesemer griesemer modified the milestones: Go1.7, Unplanned Feb 25, 2016
@cierniak
Copy link

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

@griesemer
Copy link
Contributor

This is still high on the list for 1.7.

@rsc rsc modified the milestones: Go1.8, Go1.7 May 18, 2016
@gopherbot
Copy link

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

mk0x9 pushed a commit to mk0x9/go that referenced this issue May 27, 2016
@griesemer
go/types: better debugging output for init order computation
795809b 
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

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

@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.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.8
Development

No branches or pull requests
6 participants
@rsc @nlacasse @cierniak @ianlancetaylor @griesemer @gopherbot