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: comments were unexpected inserted in import decl #40546

Closed
oraluben opened this issue Aug 3, 2020 · 4 comments
Closed

go/printer: comments were unexpected inserted in import decl #40546

oraluben opened this issue Aug 3, 2020 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@oraluben
Copy link

oraluben commented Aug 3, 2020 •
edited

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

$ go version
go version go1.15beta1 darwin/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/yyc/Library/Caches/go-build"
GOENV="/Users/yyc/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/yyc/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/yyc/go"
GOPRIVATE=""
GOPROXY="https://goproxy.cn"
GOROOT="/Users/yyc/go/go1.15beta1"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/yyc/go/go1.15beta1/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/yyc/GolandProjects/fuzz-test/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/7y/t_wypjhs6hq78tzsj85w2xgw0000gn/T/go-build457538297=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

https://play.golang.org/p/65fHR5rtBXR

What I'm trying to do here is inserting an import decl in the front of a simple go source file, and as shown, an unexpected comment appeared in the middle of the inserted import, which breaks the source file.

What did you expect to see?

What did you see instead?

Buggy version and expected version (fixed with some ad-hoc logic) are both in the above link.

<del>
I believe what happened here is after remembering the offset of one comment at

go/src/go/printer/printer.go

Line 137 in 6f26480

p.commentOffset = p.posFor(list[0].Pos()).Offset
, the offset will not be updated even if new content printed, so commentBefore

go/src/go/printer/printer.go

Line 153 in 6f26480

return p.commentOffset < next.Offset && (!p.impliedSemi || !p.commentNewline)
will unexpected returns true for the next but not reached comment.
</del>

I can draft a patch if I'm not misunderstanding about the root cause.

update: the guess above is incorrect, still investigating.
@ianlancetaylor ianlancetaylor changed the title Comments were unexpected inserted in import decl go/printer: comments were unexpected inserted in import decl Aug 3, 2020
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 3, 2020
@ianlancetaylor ianlancetaylor added this to the Backlog milestone Aug 3, 2020
@ianlancetaylor
Copy link
Contributor

CC @griesemer

@oraluben oraluben mentioned this issue Aug 5, 2020
Closed
@dgryski
Copy link
Contributor

dgryski commented Aug 11, 2020

Given that this breaks go-fuzz, does that increase the priority for 1.15 ?

@griesemer
Copy link
Contributor

griesemer commented Aug 11, 2020 •
edited

Just seeing this (I was on vacation last week). It's too late for 1.15.

@dgryski dgryski mentioned this issue Aug 18, 2020
Closed
@oraluben
Copy link
Author

oraluben commented Aug 19, 2020 •
edited

I think I can hardly say this is a bug in go/printer. For instance we have one comment at position n, and we inserted some statements without any position information just before n, it's hard to tell where should the comment be, in the middle of those inserted statements or after.

One workaround useful to go-fuzz would be giving all inserted statements a position not accurate but less than the position of the subsequent comment. How this would work could be found at https://play.golang.org/p/KNEE2J4FrWt.

@mohammed90 mohammed90 mentioned this issue Aug 20, 2020
Merged
@ksurent ksurent mentioned this issue Oct 1, 2020
Merged
@golang golang locked and limited conversation to collaborators Nov 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@dgryski @ianlancetaylor @oraluben @griesemer @gopherbot