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/go2go: improve error messages for missing or superfluous type constraints #43527

Closed
candlerb opened this issue Jan 5, 2021 · 8 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@candlerb
Copy link

candlerb commented Jan 5, 2021

What version of Go are you using (go version)?

go2go playground

Does this issue reproduce with the latest release?

N/A

What operating system and processor architecture are you using (go env)?

N/A

What did you do?

  1. https://go2goplay.golang.org/p/m9YUDBTWwpA

    func (m *MyMap[KT,VT comparable]) set(k KT, v VT) { ...
    
  2. https://go2goplay.golang.org/p/GDxiXiIIti7

    func blah[T1, T2](x T1) T2 { ...
    

What did you expect to see?

  1. error to the effect that a constraint was used in a place where it's not allowed
  2. error to the effect that a type constraint is missing

What did you see instead?

  1. "missing ',' in type argument list"
  2. "all type parameters must be named" (even though they were named!)
@ianlancetaylor ianlancetaylor changed the title go2: improve error messages for missing or superfluous type constraints cmd/go2go: improve error messages for missing or superfluous type constraints Jan 5, 2021
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 5, 2021
@ianlancetaylor ianlancetaylor added this to the Unreleased milestone Jan 5, 2021
@ianlancetaylor
Copy link
Contributor

CC @griesemer

@griesemer griesemer self-assigned this Jan 6, 2021
@griesemer griesemer modified the milestones: Unreleased, Go1.17 Jan 6, 2021
@griesemer
Copy link
Contributor

The first error is fixed in the latest (internal, not deployed version). The 2nd error is correct but misleading: Since the type parameter list is [T1, T2] and since type parameter lists are syntactically like regular parameter lists (but for the []'s), it looks like a parameter list with two constraints T1 and T2 and without explicitly named type parameters. For comparison, note that func f(int, int){} is a valid function declaration with unnamed value parameters.

Anyway, agreed that the error message should be better.

@candlerb
Copy link
Author

candlerb commented Feb 8, 2021

A similar case occurs when defining generic types if you omit the constraint on the type parameter, e.g.
https://go2goplay.golang.org/p/GqCc1gE36NL

That is, if you write

type slot[T] ...

instead of

type slot[T any] ...

In the go2go playground, the error currently says "undeclared name: T", rather than "type parameter T is missing type constraint"

For comparison, note that func f(int, int){} is a valid function declaration with unnamed value parameters.

Ugh, I didn't realise that was allowed. If you want to discard arguments, you can also write func f(_, _ int){}, and to me that makes the intention clearer.

@ianlancetaylor
Copy link
Contributor

"type slot[T] some_type" defines slot as an array of length T of element type some_type. If T is not defined anywhere, then undeclared name: T is a correct error message.

That said, we might want to tweak the compiler error message here to make clear that the type is being interpreted as an array type.

1 similar comment
@ianlancetaylor
Copy link
Contributor

"type slot[T] some_type" defines slot as an array of length T of element type some_type. If T is not defined anywhere, then undeclared name: T is a correct error message.

That said, we might want to tweak the compiler error message here to make clear that the type is being interpreted as an array type.

@griesemer griesemer modified the milestones: Go1.17, Go1.18 Apr 14, 2021
@gopherbot
Copy link

Change https://golang.org/cl/348730 mentions this issue: cmd/compile/internal/syntax: better error message for missing type constraint

@gopherbot
Copy link

Change https://golang.org/cl/348731 mentions this issue: cmd/compile/internal/types2: better error message for invalid array decls

gopherbot pushed a commit that referenced this issue Sep 9, 2021
…nstraint

For #43527.

Change-Id: I8c706e68572286d5675383eb2dfd75b5618b646b
Reviewed-on: https://go-review.googlesource.com/c/go/+/348730
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/348742 mentions this issue: cmd/compile/internal/syntax: correct follow token for type parameter lists

gopherbot pushed a commit that referenced this issue Sep 10, 2021
…lists

When parsing a type parameter declaration, parts of the code still
expected a ) as closing token. Use the correct follow token ) or ]
depending on parameter list kind.

Also, consistently use tokstring (not tok.String()) for user-facing
(error) messages.

Follow-up on comment in CL 348730.

For #43527.

Change-Id: Ib1d4feb526771a1668a54c3bb7a671f6c8a65940
Reviewed-on: https://go-review.googlesource.com/c/go/+/348742
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants