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

cmd/compile/internal/ir: add IdentExpr #42990

Open
mdempsky opened this issue Dec 4, 2020 · 3 comments
Open

cmd/compile/internal/ir: add IdentExpr #42990

mdempsky opened this issue Dec 4, 2020 · 3 comments
Assignees
Labels

Comments

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Dec 4, 2020

Currently cmd/compile's AST doesn't distinguish between definition and use of an object. This has been a long standing issue that causes bad positions for error messages.

Now that we have more flexibility in AST nodes, we can fix this. My plan is to introduce a new IdentExpr node to represent uses of a named object, while Name will continue represent the declared name itself.

@mdempsky mdempsky self-assigned this Dec 4, 2020
@mdempsky mdempsky changed the title cmd/compile: add IdentExpr cmd/compile/internal/ir: add IdentExpr Dec 4, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Dec 4, 2020

Change https://golang.org/cl/275306 mentions this issue: [dev.regabi] cmd/compile: rewrite code to use DeclaredBy

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 4, 2020

Change https://golang.org/cl/275305 mentions this issue: [dev.regabi] cmd/compile: add RefersTo and DeclaredBy helpers

gopherbot pushed a commit that referenced this issue Dec 4, 2020
Currently, because we use the same *Name to represent both declaration
and uses of an object, it's ambiguous what "n1 == n2" means when
comparing two Node values. It can mean any of: Are these the same
syntactic element? Is n1 a use of declared variable n2? Are n1 and n2
both uses of the same declared variable?

We'd like to introduce a new IdentExpr node to replace use of Name
within the AST, but that means those three cases need to be handled
differently. The first case needs to stay "n1 == n2", but the other
cases need to become "n1.Name() == n2" and "n1.Name() == n2.Name()",
respectively. ("n1.Name() == n2.Name()" also currently works for the
second case, but eventually we'll want to get rid of the Name.Name
method.)

This CL introduces helper functions SameSource and Uses to handle
these cases. It also introduces DeclaredBy, which is another somewhat
common case that the next CL introduces uses of.

Passes buildall w/ toolstash -cmp.

Updates #42990.

Change-Id: Ia816c124446e9067645d5820a8163f295968794f
Reviewed-on: https://go-review.googlesource.com/c/go/+/275305
Reviewed-by: Russ Cox <rsc@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
gopherbot pushed a commit that referenced this issue Dec 4, 2020
Passes buildall w/ toolstash -cmp.

Updates #42990.

[git-generate]
cd src/cmd/compile/internal/gc
rf '
ex {
  import "cmd/compile/internal/ir"
  var x, stmt ir.Node
  x.Name() != nil && x.Name().Defn == stmt ->  ir.DeclaredBy(x, stmt)
  x.Name() == nil || x.Name().Defn != stmt -> !ir.DeclaredBy(x, stmt)
}
'

Change-Id: I222a757296dbcb5d0889d617d221a9d7319f2d74
Reviewed-on: https://go-review.googlesource.com/c/go/+/275306
Reviewed-by: Russ Cox <rsc@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
@gopherbot
Copy link

@gopherbot gopherbot commented Dec 14, 2020

Change https://golang.org/cl/277713 mentions this issue: [dev.regabi] cmd/compile: use ir.Ident for imported identifiers

gopherbot pushed a commit that referenced this issue Dec 15, 2020
This CL substantially reworks how imported declarations are handled,
and fixes a number of issues with dot imports. In particular:

1. It eliminates the stub ir.Name declarations that are created
upfront during import-declaration processing, allowing this to be
deferred to when the declarations are actually needed. (Eventually,
this can be deferred even further so we never have to create ir.Names
w/ ONONAME, but this CL is already invasive/subtle enough.)

2. During noding, we now use ir.Idents to represent uses of imported
declarations, including of dot-imported declarations.

3. Unused dot imports are now reported after type checking, so that we
can correctly distinguish whether composite literal keys are a simple
identifier (struct literals) or expressions (array/slice/map literals)
and whether it might be a use of a dot-imported declaration.

4. It changes the "redeclared" error messages to report the previous
position information in the same style as other compiler error
messages that reference other source lines.

Passes buildall w/ toolstash -cmp.

Fixes #6428.
Fixes #43164.
Fixes #43167.
Updates #42990.

Change-Id: I40a0a780ec40daf5700fbc3cfeeb7300e1055981
Reviewed-on: https://go-review.googlesource.com/c/go/+/277713
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants