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/parser: missing object for composite lit types identifiers beginning statements #45136

Closed
findleyr opened this issue Mar 19, 2021 · 4 comments
Closed

go/parser: missing object for composite lit types identifiers beginning statements #45136

findleyr opened this issue Mar 19, 2021 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@findleyr
Copy link
Contributor

In the following snippet, the obj identifiers in statements beginning with the composite literal obj{} are not resolved to the obj declaration.

type obj struct {}

func _() {
  obj{}
  obj{}.run()
}

The reason for this is that, while in the middle of parsing, LHS logic is pretty tricky to get right. Specifically, it's non-trivial to differentiate this case of a composite literal from, for example, the ok { in if _, ok := m[k]; ok {....

See also #45104: this is easy to fix when post-processing the AST, but it's nearly impossible to match the broken behavior (because it relates to source layout rather than syntax).

Filing as a separate issue as I'm going to fix it prior to #45104, so that I can match behavior exactly with the new resolution logic.

CC @griesemer
@griesemer
Copy link
Contributor

It might be easier to reproduce this bug in the new code than trying to fix the old code. Than you can still do exact behavior matching, and afterwards "fix the bug".

@cherrymui cherrymui added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 19, 2021
@cherrymui cherrymui added this to the Backlog milestone Mar 19, 2021
@findleyr findleyr self-assigned this Mar 21, 2021
@findleyr
Copy link
Contributor Author

It might be easier to reproduce this bug in the new code than trying to fix the old code.

I have a fix for the old code. Reproducing in the new code would actually be quite tricky, because once parsed into a tree it's difficult to know if the CompositeLit you're looking at is at the start of an ExprStmt (this is what I meant above by "it relates to source layout rather than syntax").

@gopherbot
Copy link

Change https://golang.org/cl/304451 mentions this issue: go/parser: resolve the type name when parsing a composite lit value

@gopherbot
Copy link

Change https://golang.org/cl/304450 mentions this issue: go/parser: add data-driven tests for object resolution

gopherbot pushed a commit that referenced this issue Mar 30, 2021
@findleyr
go/parser: add data-driven tests for object resolution
0fdd371 
Add new tests for object resolution driven by source files with
declarations and uses marked via special comments. This made it easier
to add test coverage while refactoring object resolution for #45104.

Tests are added to a new resolver_test.go file. In a subsequent CL the
resolver.go file will be added, making this choice of file name more
sensible.

For #45104
For #45136
For #45160

Change-Id: I240fccc0de95aa8f2d03e39c77146d4c61f1ef9e
Reviewed-on: https://go-review.googlesource.com/c/go/+/304450
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Mar 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@griesemer @gopherbot @cherrymui @findleyr