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

Jump to bottom

cmd/compile: inaccurate column number for parameters #23031

Closed
cherrymui opened this issue Dec 7, 2017 · 4 comments
Closed

cmd/compile: inaccurate column number for parameters #23031

cherrymui opened this issue Dec 7, 2017 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.11

Comments

@cherrymui
Copy link
Member

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

tip (1b9f663)

Does this issue reproduce with the latest release?

yes

What did you do?

It seems that the column numbers for parameters are not accurate. Different parameters may get same line and column numbers, as seen in #23020.

I've come up a simple example:

package p

//            15     22
//            |      |
func f(x int, x int) int {
	return 3
}

What did you expect to see?

/tmp/x5.go:5:15: duplicate argument x

What did you see instead?

$ go tool compile /tmp/x5.go 
/tmp/x5.go:5:22: duplicate argument x

It seems we don't emit column numbers for variables in DWARF -- if we do, that might be also inaccurate.
@odeke-em
Copy link
Member

odeke-em commented Dec 7, 2017

/cc @griesemer @mdempsky

@mdempsky mdempsky added this to the Go1.11 milestone Dec 7, 2017
@mdempsky
Copy link
Member

mdempsky commented Dec 7, 2017

I was looking at this the other day. The issue is we setup the ONAME Nodes to represent the parameters with the position information where the identifiers appeared, but then when we call funcargs (and in turn declare), that information gets clobbered with lineno (which happens to be at the last token in the function signature, the int return parameter type).

I think the proper fix is that gc.declare needs to stop clobbering n.Pos, and to just require that callers always setup n.Pos correctly. I think this is almost already the case, but needs a review.

@griesemer griesemer added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 27, 2018
@griesemer
Copy link
Contributor

This appears to be fixed. I cannot reproduce this.

@odeke-em
Copy link
Member

@griesemer in deed, this bug was fixed by CL 104275 aka commit 26e0e8a and precisely this line deletion in dcl.go 26e0e8a#diff-af57b81069f33da91c083d4d5b61aeeeL80
screen shot 2018-06-27 at 1 42 34 pm

@golang golang locked and limited conversation to collaborators Jun 27, 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.11
Development

No branches or pull requests
5 participants
@mdempsky @odeke-em @griesemer @gopherbot @cherrymui