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: some facts are missing after an error #22467

Open
alandonovan opened this issue Oct 27, 2017 · 7 comments

Comments

Projects
None yet
5 participants
@alandonovan
Copy link
Contributor

commented Oct 27, 2017

The type checker go/types records various facts about well-typed trees. For example, Uses records the relationship between each "using" (non-defining) identifier and the object to which it refers. However, when the type checker detects certain kinds of errors, it does not attempt to record facts about the errant subtree. This issue is a request that it work harder to record facts about errant subtrees.

By way of motivation, consider a tool for generating completions in an IDE. The input to such a tool is an unsaved editor buffer that usually fails to type-check, but the tool must nonetheless extract type information from the partial tree. Consider the following fragment of a Go program in an unsaved editor buffer, with the current insertion point marked by a caret:

func _() {
	fmt.⁁
	test := 0
}

A completion tool should generate a list of qualified identifiers such as fmt.Print, fmt.Fprintf, and so on. The parser ignores the newline and generates a tree containing fmt.test := 0, for which the type checker emits an error because each operand on the left of := must be an identifier, not a qualified identifier (*ast.SelectorExpr). Having emitted the error, the type checker does not then visit the LHS subtree, so no Uses fact is recorded for fmt. As a result, the completion tool simply doesn't work on programs of this form, which are fairly common.

The fix in this specific example is for the type checker, once it has reported the error, to visit the left subtree for side effects (facts) only, and ignore the resulting type. The more general problem is to find any other places in the type checker that skip visitation of a subtree after an error.

@gopherbot

This comment has been minimized.

Copy link

commented Jul 12, 2018

Change https://golang.org/cl/123578 mentions this issue: go/types: record type information after detecting error

gopherbot pushed a commit that referenced this issue Jul 12, 2018

go/types: record type information after detecting error
The existing implementation stops recording type information once it
encounters an error. This results in missing type information that is
needed by various tools. This change handles a few commonly encountered
cases by continuing to check subtrees after errors. Also, add tests for
cases where the package fails to type-check.

Updates #22467

Change-Id: I1bb48d4cb8ae5548dca63bdd785ea2f69329e92b
Reviewed-on: https://go-review.googlesource.com/123578
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
@gopherbot

This comment has been minimized.

Copy link

commented Aug 9, 2018

Change https://golang.org/cl/128835 mentions this issue: go/types: fix errors in recording type information

gopherbot pushed a commit that referenced this issue Aug 9, 2018

go/types: fix errors in recording type information
In my previous change, I didn't use the correct functions for continuing
to record type informations after errors. Change to using the correct
functions, and add a comment to clarify in expr.go.

Updates #22467

Change-Id: I66ebb636ceb2b994db652343430f0551db0050c3
Reviewed-on: https://go-review.googlesource.com/128835
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot

This comment has been minimized.

Copy link

commented Aug 30, 2018

Change https://golang.org/cl/132235 mentions this issue: go/types: handle nil pointer when panic is written outside of a function

gopherbot pushed a commit that referenced this issue Aug 30, 2018

go/types: handle nil pointer when panic is written outside of a function
The current implementation crashes when someone writes a panic outside of
a function, which makes sense since that is broken code. This fix allows
one to type-check broken code.

Updates #22467

Change-Id: I81b90dbd918162a20c60a821340898eaf02e648d
Reviewed-on: https://go-review.googlesource.com/132235
Reviewed-by: Alan Donovan <adonovan@google.com>
@gopherbot

This comment has been minimized.

Copy link

commented Aug 30, 2018

Change https://golang.org/cl/132355 mentions this issue: go/types: handle composite literals with open array values

gopherbot pushed a commit that referenced this issue Aug 30, 2018

go/types: fix crash following misuse of [...]T in composite literal
The type-checker currently crashes when checking code such as:

	_ = map[string][...]int{"": {1, 2, 3}}

In this case, the type checker reports an error for map[string][...]int,
then proceeds to type-check the values of the map literal using a hint
type of [...]int. When type-checking the inner composite (array) literal,
the length of the open array type is computed from the elements,
then the array type is recorded, but the literal has no explicit type
syntax against which to record the type, so this code causes the
type-checker to panic. Add a nil check before calling
check.recordTypeAndValue to avoid that.

Updates #22467

Change-Id: Ic4453ba485b7b88ede2a89f209365eda9e032abc
Reviewed-on: https://go-review.googlesource.com/132355
Reviewed-by: Alan Donovan <adonovan@google.com>
@stamblerre

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2018

@gopherbot please consider this for backport to 1.11, it fixes a minor bug.

@gopherbot

This comment has been minimized.

Copy link

commented Sep 4, 2018

Backport issue(s) opened: #27497 (for 1.11).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@gopherbot

This comment has been minimized.

Copy link

commented Sep 5, 2018

Change https://golang.org/cl/133415 mentions this issue: go/types: fix internal comments and add additional test case

gopherbot pushed a commit that referenced this issue Sep 5, 2018

go/types: fix internal comments and add additional test case
https://go-review.googlesource.com/c/go/+/132355 addressed
a crash and inadvertently fixed #27346; however the comment
added to the type-checker was incorrect and misleading.

This CL fixes the comment, and adds a test case for #27346.

Fixes #27346.
Updates #22467.

Change-Id: Ib6d5caedf302fd42929c4dacc55e973c1aebfe85
Reviewed-on: https://go-review.googlesource.com/133415
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.