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.

Sign up for GitHub

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

cmd/compile: suboptimal error for &T{} literal mismatch #26855

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

cmd/compile: suboptimal error for &T{} literal mismatch #26855

dsymonds opened this issue Aug 8, 2018 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.12

Comments

@dsymonds
Copy link
Contributor

dsymonds 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
Copy link
Member

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
Copy link
Contributor

bcmills commented Aug 9, 2018

CC: @randall77 @griesemer

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 9, 2018
@bcmills bcmills added this to the Go1.12 milestone Aug 9, 2018
@griesemer griesemer self-assigned this Aug 9, 2018
@rsc
Copy link
Contributor

rsc commented Aug 10, 2018

It should say &T literal.

@griesemer
Copy link
Contributor

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.

@griesemer griesemer added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Sep 19, 2018
@gopherbot
Copy link
Contributor

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

@gopherbot
Copy link
Contributor

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

@griesemer griesemer mentioned this issue Sep 19, 2018
Closed
gopherbot pushed a commit that referenced this issue Sep 20, 2018
@griesemer
cmd/compile/internal/gc: minor code reorg (cleanup)
048c766 
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
Copy link
Contributor

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
@griesemer
cmd/compile/internal/gc: fix Node.copy and introduce (raw|sep)copy
ce58a39 
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>
@golang golang locked and limited conversation to collaborators Sep 20, 2019
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
Go1.12
Development

No branches or pull requests
6 participants
@dsymonds @rsc @bcmills @griesemer @gopherbot @smasher164