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: error message dumps internal details #46558

Open
ianlancetaylor opened this issue Jun 3, 2021 · 4 comments
Open

cmd/compile: error message dumps internal details #46558

ianlancetaylor opened this issue Jun 3, 2021 · 4 comments
Assignees
Labels
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 3, 2021

Using a compiler built at 6d98301, compiling this erroneous file gives a strange error message that seems to dump an internal compiler data structure.

package p

func F(s string) {
	switch s[0] {
	case 'a':
		case s[2] {
		case 'b':
		}
	}
}
> go build foo.go
# command-line-arguments
NOT NTYPE [0xc00034e0c0]
.   INDEX # foo.go:6
.   .   NAME-p.s Class:PPARAM Offset:0 OnStack # foo.go:3
.   .   NAME-ntype
.   .   .   NONAME-p.string # foo.go:3
.   .   LITERAL-2 untyped int # foo.go:6
../foo.go:7:3: syntax error: unexpected case, expecting expression
../foo.go:8:4: syntax error: unexpected newline, expecting :

This does not happen with Go 1.16.

CC @mdempsky

@ianlancetaylor ianlancetaylor added this to the Backlog milestone Jun 3, 2021
@mdempsky
Copy link
Member

@mdempsky mdempsky commented Jun 3, 2021

I think there are two minor issues here:

  1. cmd/compile/internal/parser is now trying to parse s[2] { case 'b': } as a composite literal of instantiated type s[2]. This then fails during noding, because s[2] isn't a valid syntactic form for a type expression (prior to generics, which noder doesn't support). (I'm inferring this because of the change in error message/lines from 1.16 to master.)
  2. cmd/compile/internal/noder has an ir.Dump call that Russ added for debugging some ir.Node refactoring stuff. It will never print on valid input, and (I think) it also never printed before the change in parser semantics.

Maybe a parser change is appropriate here (/cc @griesemer). But I also think it's fine that we just remove the spurious ir.Dump. It's not serving any purpose now that things have stabilized.

@griesemer griesemer self-assigned this Jun 3, 2021
@griesemer
Copy link
Contributor

@griesemer griesemer commented Jun 3, 2021

Yeah, the parser should probably handle this case as it gets badly into the weeds otherwise. Will fix.

@griesemer griesemer modified the milestones: Backlog, Go1.18 Jun 3, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 4, 2021

Change https://golang.org/cl/324991 mentions this issue: cmd/compile: remove spurious ir.Dump

gopherbot pushed a commit that referenced this issue Jun 4, 2021
This ir.Dump call is a debugging artifact introduced in
golang.org/cl/274103, which should never be printed for valid,
non-generic code, but evidently can now sometimes appear due to how
the parser handles invalid syntax.

The parser should probably not recognize "x[2]" as a type expression
in non-generics mode, but also probably we shouldn't try noding after
reporting syntax errors. Either way, this diagnostic has outlived its
usefulness, and noder's days are numbered anyway, so we might as well
just remove it to save end users any confusion.

Updates #46558.

Change-Id: Ib68502ef834d610b883c2f2bb11d9b385bc66e37
Reviewed-on: https://go-review.googlesource.com/c/go/+/324991
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 4, 2021

Change https://golang.org/cl/325009 mentions this issue: [dev.typeparams] cmd/compile/internal/syntax: not all index expressions can be instantiated types

gopherbot pushed a commit that referenced this issue Jun 5, 2021
…ns can be instantiated types

An index expression followed by an opening "{" may indicate
a composite literal but only if the index expression can be
a type. Exclude cases where the index expression cannot be
a type (e.g. s[0], a[i+j], etc.).

This leads to a better error message in code that is erroneous.

Fixes #46558.

Change-Id: Ida9291ca30683c211812dfb95abe4969f44c474f
Reviewed-on: https://go-review.googlesource.com/c/go/+/325009
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@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
4 participants