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: suboptimal error for &T{} literal mismatch #26855

Closed
dsymonds opened this issue Aug 8, 2018 · 7 comments

Comments

Projects
None yet
6 participants
@dsymonds
Copy link
Member

commented Aug 8, 2018

Does this issue reproduce with the latest release?

In the playground, yes.

What did you do?

https://play.golang.org/p/uQjX9qTwBz1

package main

type S struct {
	Name T
}

type T struct {
	First string
}

func main() {
	_ = &S{
		Name: &T{First: "Rob"},
	}
}

What did you expect to see?

An error message pointing out the T vs. *T type problem.

What did you see instead?

prog.go:13:7: cannot use T literal (type *T) as type T in field value

The error message is there, but it is phrased oddly. cannot use T literal is confusing. I'm using a &T literal. If you put aside the parenthetical for a moment, the oddness is apparent: cannot use T literal as type T in field value. I think this error message could be improved.

@smasher164

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2018

It is also odd because without the parentheses, the phrase T literal as type T compares a value with a type.
Would the following be a more clear message?

prog.go:13:7: cannot use literal of type *T for field value of type T.
@bcmills

This comment has been minimized.

Copy link
Member

commented Aug 9, 2018

@bcmills bcmills added this to the Go1.12 milestone Aug 9, 2018

@griesemer griesemer self-assigned this Aug 9, 2018

@rsc

This comment has been minimized.

Copy link
Contributor

commented Aug 10, 2018

It should say &T literal.

@griesemer

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2018

typecheckcomplit makes a copy (typecheck.go:2915) of the complit node it type-checks and then modifies its Op in place. Because n.Orig points to n, this implicitly modifies also n.Orig. During printing, the original node doesn't have the correct Op anymore leading to this error.

@gopherbot

This comment has been minimized.

Copy link

commented Sep 19, 2018

Change https://golang.org/cl/136395 mentions this issue: cmd/compile: fix error message for &T{} literal mismatch

@gopherbot

This comment has been minimized.

Copy link

commented Sep 19, 2018

Change https://golang.org/cl/136396 mentions this issue: cmd/compile/internal/gc: minor code reorg (cleanup)

@gopherbot gopherbot closed this in ae37f5a Sep 20, 2018

gopherbot pushed a commit that referenced this issue Sep 20, 2018

cmd/compile/internal/gc: minor code reorg (cleanup)
Found while tracking down #26855.

Change-Id: Ice137fe390820ba351e1c7439b6a9a1b3bdc966b
Reviewed-on: https://go-review.googlesource.com/136396
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
@gopherbot

This comment has been minimized.

Copy link

commented Sep 20, 2018

Change https://golang.org/cl/136540 mentions this issue: cmd/compile/internal/gc: fix Node.copy and introduce (raw|sep)copy

gopherbot pushed a commit that referenced this issue Sep 20, 2018

cmd/compile/internal/gc: fix Node.copy and introduce (raw|sep)copy
Node.copy used to make a shallow copy of a node. Often, this is not
correct: If a node n's Orig field pointed to itself, the copy's Orig
field has to be adjusted to point to the copy. Otherwise, if n is
modified later, the copy's Orig appears modified as well (because it
points to n).

This was fixed for one specific case with
https://go-review.googlesource.com/c/go/+/136395 (issue #26855).

This change instead addresses copy in general:

In two cases we don't want the Orig adjustment as it causes escape
analysis output to fail (not match the existing error messages).
rawcopy is used in those cases.

In several cases Orig is set to the copy immediately after making
a copy; a new function sepcopy is used there.

Updates #26855.
Fixes #27765.

Change-Id: Idaadeb5c4b9a027daabd46a2361348f7a93f2b00
Reviewed-on: https://go-review.googlesource.com/136540
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.