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

Jump to bottom

go/types: some facts are missing after an error #22467

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

go/types: some facts are missing after an error #22467

alandonovan opened this issue Oct 27, 2017 · 7 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@alandonovan
Copy link
Contributor

alandonovan commented Oct 27, 2017 •
edited

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.
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 29, 2018
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Mar 29, 2018
@gopherbot
Copy link

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
@stamblerre @adonovan
go/types: record type information after detecting error
6f8e8e1 
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
Copy link

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
@stamblerre @adonovan
go/types: fix errors in recording type information
79bf795 
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
Copy link

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
@stamblerre
go/types: handle nil pointer when panic is written outside of a function
c99687f 
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
Copy link

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
@stamblerre
go/types: fix crash following misuse of [...]T in composite literal
6c0b8b5 
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
Copy link
Contributor

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

@gopherbot gopherbot mentioned this issue Sep 4, 2018
Closed
@gopherbot
Copy link

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
Copy link

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
@griesemer
go/types: fix internal comments and add additional test case
3c1b7bc 
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
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@ianlancetaylor @stamblerre @alandonovan @griesemer @gopherbot