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/gofmt: moves comment in for #9460

Open
sethwklein opened this issue Dec 27, 2014 · 3 comments
Open

cmd/gofmt: moves comment in for #9460

sethwklein opened this issue Dec 27, 2014 · 3 comments
Assignees
Milestone

Comments

@sethwklein
Copy link

@sethwklein sethwklein commented Dec 27, 2014

On playground (and elsewhere), this code

package main

func main() {
        for i := 0; i < 1 /* 9000 */ ; i++ {
                _ = i
        }
}

is transformed by gofmt into this code:

package main

func main() {
    for i := 0; i < 1; /* 9000 */ i++ {
        _ = i
    }
}

It has swapped the comment and the nearby semi-colon.

This code arises when temporarily changing the limit of a loop.

I think this swap changes the context of the comment, possibly changing its meaning.

It also means that I'll have to swap 9000 (in this example) and the semi-colon back when restoring the original loop limit.

@mikioh mikioh changed the title gofmt moves comment in for cmd/gofmt: moves comment in for Dec 27, 2014
@griesemer
Copy link
Contributor

@griesemer griesemer commented Dec 29, 2014

The issue here is that the AST (go/ast) doesn't record the position of semicolons for the 'for' statement and thus comments get placed afterwards. The true fix would be to record all semicolon positions, but we don't do this elsewhere because it just costs extra space and such comments are exceedingly rare. This was a pre-2009 design decision; we're not going to change it now.

The other option is to heuristically assume that in a for-loop such semicolons are assumed to be immediate following (and adjacent) to the pre-, cond-, or post-expression in a for loop. This would fix this case but then misplace a comment in the situation

for i := 0; i < n; /* foo */ i++ {...

where the /* foo */ comment is supposed to be before the post-statement. This might be an option if the assumption is that comments are likely to be after an expression rather than before (which is not unreasonable).

This is not urgent, and there's a trivial work-around: simply put the comment before the 1 in your example and the meaning doesn't get distorted. This program

package main

func main() {
for i := 0; i < /* 9000 */ 1; i++ {
_ = i
}
}

gets formatted as is.

Leaving open for now as a reminder for future AST changes.

@griesemer griesemer self-assigned this Dec 29, 2014
@griesemer griesemer added priority-low and removed priority-low labels Dec 29, 2014
@griesemer
Copy link
Contributor

@griesemer griesemer commented Dec 29, 2014

This issue is very specific to 'for'-statements and not related to other comment formatting issues. Removing adg's comment above.

@speter
Copy link

@speter speter commented Dec 30, 2014

For reference, a similar change occurs in composite literals: http://play.golang.org/p/1MPnS7RUuF

var ints = []int{ /* one */ 1, /* two */ 2}

becomes

var ints = []int{ /* one */ 1 /* two */, 2}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.