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

go/printer: Source position broken #5945

Closed
gopherbot opened this issue Jul 24, 2013 · 14 comments
Closed

go/printer: Source position broken #5945

gopherbot opened this issue Jul 24, 2013 · 14 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.9

Comments

@gopherbot
Copy link

by tobias.schwerdtfeger:

What steps will reproduce the problem?
First issue:
The reported line numbers after function declarations or package XXX etc. with a
following blank newline are reported wrong (and probably shouldn't even displayed, or is
this intentional?).

Second issue:
Replacing function calls with a ast.NewIdent("") will result in an incorrect
and actually useless //line comment (line number shifted by -1).
Indirectly this issue affects cgo while reporting errors. (See
https://golang.org/issue/5674)

For both issues see:
http://play.golang.org/p/tlli7esWMO
"Output 1" describes the first issue and "Output 2" the second.

Which version are you using?  (run 'go version')
go version devel +84cafba689b1 Wed Jul 24 17:48:13 2013 +0900 linux/amd64
@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 1:

Labels changed: added priority-later, go1.2maybe, removed priority-triage.

Status changed to Accepted.

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 2:

Labels changed: added feature.

@robpike
Copy link
Contributor

robpike commented Aug 30, 2013

Comment 3:

Not for 1.2.

Labels changed: removed go1.2maybe.

@gopherbot
Copy link
Author

Comment 4 by camilo.aguilar:

I'm running into a similar issue. But, I'm not sure if it is really related to this
particular issue. I have an undeclared identifier in line 72 of my go source file and
cgo error notice is giving me line 38.

@gopherbot
Copy link
Author

Comment 5 by camilo.aguilar:

I'm running into a similar issue. But, I'm not sure if it is really related to this
particular issue. I have an undeclared identifier in line 72 of my go source file and
cgo error notice is giving me line 38. 
Darwin grinch-2.local 13.0.0 Darwin Kernel Version 13.0.0: Thu Sep 19 22:22:27 PDT 2013;
root:xnu-2422.1.72~6/RELEASE_X86_64 x86_64
go version go1.2rc3 darwin/amd64

@rsc
Copy link
Contributor

rsc commented Nov 27, 2013

Comment 6:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Nov 27, 2013

Comment 7:

Labels changed: removed feature.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 8:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 9:

Labels changed: added repo-main.

@griesemer
Copy link
Contributor

Comment 10:

Owner changed to @griesemer.

@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@griesemer
Copy link
Contributor

Look into this and decide if this is fixed/needs a fix. At the very least the 2nd issue in the initial bug report looks like a programmer error: Inserting a new identifier w/o setting its position is not going to give a correct position.

@griesemer griesemer added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Feb 25, 2017
@griesemer griesemer modified the milestones: Go1.9, Unplanned Feb 25, 2017
@griesemer
Copy link
Contributor

Follow-up regarding the 2nd issue: This is clearly a programmer error. It's not clear what you're trying to do with:

if x.Name == "Call" {
    *x = *ast.NewIdent("XXX")
}

If you just want to change the name, simply do:

if x.Name == "Call" {
    x.Name = "XXX"
}

Then the //line for the call is correct.
Overwriting the entire node is going to destroy the position information. If you want to put in another node for whatever reason, you need to save the position first and then restore it.

Looking into the first issue now (spurious //line comments before empty lines).

@griesemer
Copy link
Contributor

The first issue is clearly a bug in go/printer. Fix forthcoming.

@griesemer griesemer added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Jun 2, 2017
@gopherbot
Copy link
Author

CL https://golang.org/cl/44651 mentions this issue.

@griesemer griesemer mentioned this issue Jun 5, 2017
Closed
@griesemer griesemer mentioned this issue Jun 20, 2017
Closed
@golang golang locked and limited conversation to collaborators Jun 2, 2018
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.9
Development

No branches or pull requests
4 participants
@rsc @robpike @griesemer @gopherbot