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: accepts weird invalid program #20770

Closed
josharian opened this issue Jun 23, 2017 · 7 comments
Closed

go/types: accepts weird invalid program #20770

josharian opened this issue Jun 23, 2017 · 7 comments
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@josharian
Copy link
Contributor

package p

var r = newReader()

func newReader() r
$ gotype x.go
$ go tool compile x.go 
x.go:3:9: typechecking loop involving newReader
	x.go:3:18 newReader()
	x.go:3:5 r = newReader()
	x.go:3:5 r
	x.go:5:15 <T>
	x.go:5:18 newReader
	x.go:5:6 <node DCLFUNC>
x.go:5:18: r is not a type

The cmd/compile error message is pretty ugly, but I'll file a separate issue for that.

Marking as unplanned, because of the oddity and unlikeliness of the program.

Found with go-fuzz.

@josharian
Copy link
Contributor Author

@ianlancetaylor I'd be curious to know what gccgo does with this snowflake.

@ianlancetaylor
Copy link
Contributor

gccgo output:

foo.go:5:18: error: expected type
 func newReader() r
                  ^
foo.go:3:9: error: reference to undefined name ‘newReader’
 var r = newReader()
         ^

@josharian
Copy link
Contributor Author

gccgo definitely wins the compiler shootout on this one. :)

@griesemer griesemer self-assigned this Jun 23, 2017
@griesemer griesemer modified the milestones: Go1.10, Unplanned Jun 23, 2017
@griesemer
Copy link
Contributor

go/types cycle detection has several issues. It's time to rethink/overhaul the mechanism.

@griesemer griesemer added the early-in-cycle A change that should be done early in the 3 month dev cycle. label Jun 29, 2017
@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017
@griesemer griesemer added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Dec 16, 2017
@griesemer
Copy link
Contributor

diff --git a/src/go/types/decl.go b/src/go/types/decl.go
index b1ae5ef3f2..b1e98d91fb 100644
--- a/src/go/types/decl.go
+++ b/src/go/types/decl.go
@@ -96,6 +96,7 @@ func (check *Checker) objDecl(obj Object, def *Named, path []*TypeName) {
                check.typeDecl(obj, d.typ, def, path, d.alias)
        case *Func:
                // functions may be recursive - no need to track dependencies
+               check.decl = d
                check.funcDecl(obj, d)
        default:
                unreachable()

leads to usable error message. Need to investigate further to see if that is the right approach.

(cycle detection in go/types could use a fundamental overhaul).

@griesemer
Copy link
Contributor

griesemer commented May 31, 2018

Possibly related to Duplicate of #8699.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/116815 mentions this issue: go/types: extend cycle detection past simple type cycles

gopherbot pushed a commit that referenced this issue Jun 7, 2018
This change improves upon cycle detection by taking into account
cycles involving constants, variables, _and_ types. All new code
(except for the additional tests) is guarded by the useCycleMarking
(internal) flag and thus can be disabled on short notice if it
introduced new problems. (The intent is to remove this flag shortly
after 1.11 is released.)

The test suite has been extended with various additional (and mostly
esoteric) test cases which now correctly report cycles. A handful of
existing test cases now report additional errors, but those are mostly
esoteric cases. Overall, this is an improvement over the status quo.

Fixes #8699.
For #20770.

Change-Id: I6086719acd0d5200edca4a3dbe703d053496af31
Reviewed-on: https://go-review.googlesource.com/116815
Run-TryBot: Robert Griesemer <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
@golang golang locked and limited conversation to collaborators Jun 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

5 participants