mvdan / goreduce

Reduce Go programs
BSD 3-Clause "New" or "Revised" License
216 stars 7 forks source link

Clarify what functions are touched, and what "producing a compiler error" means #15

Closed ALTree closed 7 years ago

ALTree commented 7 years ago

There's another thing I don't understand (but maybe this is just a documentation issue).

Take this (it crashes the compiler on tip, it's already reported):

package p

func f1() {
    f2()
}

func f2() {
    if false {
        _ = func() {}
    }
}

I ran goreduce -v -match="cannot inline" . f2 and -v says: crash-goreduce.go:4: ExprStmt removed, which is true: it removed the call at line 4:

package p

func f1() {
}

func f2() {
    if false {
        _ = func() {}
    }
}

I see two problems:

mvdan commented 7 years ago

why did it touch f1 at all?

What it means by "reduce a function" is reduce a program as long as it crashes given an entry function. I agree that the terminology is confusing. This is because I haven't decided how to limit the scope of the reductions yet.

For now it reduces the entire ast.File. I don't know whether it should reduce the rest of the Go files in the package yet. As to limiting it to just the entry func and the code it touches/calls, see #8.

the new snippet does not crash the compiler

Will investigate, thanks for the testing.

mvdan commented 7 years ago

Actually, this makes me realise that "entry functions" are not necessary at all if we're after compiler errors. In that case, we should just walk the package/file and never actually run any binary.

mvdan commented 7 years ago

Also, just found the reason why goreducer thought f2() could be removed. This is because to be able to use f2 as an entry point, it swaps the package name to main and adds this:

func main() {
    f2()
}

The solution here is #16 - to not do any of this main stuff and only build when we're after a compiler/linker bug, and not a run-time one. I'm going to close this issue as both points raised are being tracked in two separate issues.