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/go: go generate is fooled by multi-line raw strings #9196

Closed
dominikh opened this issue Dec 2, 2014 · 7 comments
Closed

cmd/go: go generate is fooled by multi-line raw strings #9196

dominikh opened this issue Dec 2, 2014 · 7 comments
Labels
FrozenDueToAge
Milestone
Go1.5

Comments

@dominikh
Copy link
Member

dominikh commented Dec 2, 2014 

What does 'go version' print?
go version devel +e8e6ada28fb6 Tue Dec 02 14:39:23 2014 +1100 linux/amd64

What steps reproduce the problem?
If possible, include a link to a program on play.golang.org.

1. http://play.golang.org/p/dOX7M_JPH6
2. go generate
3.

What happened?
Go executes the echo, printing "Hello World"

What should have happened instead?
Nothing.
@ianlancetaylor
Copy link
Contributor

Comment 1:

Labels changed: added repo-main, release-go1.5.

@robpike
Copy link
Contributor

robpike commented Dec 3, 2014

Comment 2:

This will be fixed with documentation. No plan to change the implementation.
The cost of fixing is not commensurate with the ease of avoiding the issue.

Status changed to Accepted.

@gopherbot
Copy link

Comment 3:

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

@dominikh
Copy link
Member Author

dominikh commented Dec 3, 2014

Comment 4:

Is the only objection to a real fix that using go/ast is more complex than just reading
lines, or would there also be performance issues?
Essentially I'm wondering if only you don't want to implement it, or if you'd actively
reject a reimplementation that actually parsed the source file to avoid the issue
(either due to complexity or performance)

@robpike
Copy link
Contributor

robpike commented Dec 3, 2014

Comment 5:

As stated, the cost of fixing is not commensurate with the ease of avoiding the issue.
It's a pretend issue, not a real one.

@robpike
Copy link
Contributor

robpike commented Dec 5, 2014

Comment 6:

This issue was closed by revision dd26fc3.

Status changed to Fixed.

@rsc
Copy link
Contributor

rsc commented Dec 5, 2014

Comment 7:

This issue was closed by revision 2c631199385b.

@bradfitz bradfitz modified the milestone: Go1.5 Dec 16, 2014
rsc added a commit that referenced this issue May 11, 2015
@rsc
[release-branch.go1.4] cmd/go: avoid use of bufio.Scanner in generate
7aead4c 
««« CL 182970043 / 573a7b5178c4
cmd/go: avoid use of bufio.Scanner in generate

Scanner can't handle stupid long lines and there are
reports of stupid long lines in production.

Note the issue isn't long "//go:generate" lines, but
any long line in any Go source file.

To be fair, if you're going to have a stupid long line
it's not a bad bet you'll want to run it through go
generate, because it's some embeddable asset that
has been machine generated. (One could ask why
that generation process didn't add a newline or two,
but we should cope anyway.)

Rewrite the file scanner in "go generate" so it can
handle arbitrarily long lines, and only stores in memory
those lines that start "//go:generate".

Also: Adjust the documentation to make clear that it
does not parse the file.

Fixes #9143.
Fixes #9196.

LGTM=rsc, dominik.honnef
R=rsc, cespare, minux, dominik.honnef
CC=golang-codereviews
https://golang.org/cl/182970043
»»»

TBR=r
CC=golang-codereviews
https://golang.org/cl/183060044
@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
@rsc @wheatman
[release-branch.go1.4] cmd/go: avoid use of bufio.Scanner in generate
e00720b 
««« CL 182970043 / 573a7b5178c4
cmd/go: avoid use of bufio.Scanner in generate

Scanner can't handle stupid long lines and there are
reports of stupid long lines in production.

Note the issue isn't long "//go:generate" lines, but
any long line in any Go source file.

To be fair, if you're going to have a stupid long line
it's not a bad bet you'll want to run it through go
generate, because it's some embeddable asset that
has been machine generated. (One could ask why
that generation process didn't add a newline or two,
but we should cope anyway.)

Rewrite the file scanner in "go generate" so it can
handle arbitrarily long lines, and only stores in memory
those lines that start "//go:generate".

Also: Adjust the documentation to make clear that it
does not parse the file.

Fixes golang#9143.
Fixes golang#9196.

LGTM=rsc, dominik.honnef
R=rsc, cespare, minux, dominik.honnef
CC=golang-codereviews
https://golang.org/cl/182970043
»»»

TBR=r
CC=golang-codereviews
https://golang.org/cl/183060044
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
@rsc @wheatman
[release-branch.go1.4] cmd/go: avoid use of bufio.Scanner in generate
f5c0485 
««« CL 182970043 / 573a7b5178c4
cmd/go: avoid use of bufio.Scanner in generate

Scanner can't handle stupid long lines and there are
reports of stupid long lines in production.

Note the issue isn't long "//go:generate" lines, but
any long line in any Go source file.

To be fair, if you're going to have a stupid long line
it's not a bad bet you'll want to run it through go
generate, because it's some embeddable asset that
has been machine generated. (One could ask why
that generation process didn't add a newline or two,
but we should cope anyway.)

Rewrite the file scanner in "go generate" so it can
handle arbitrarily long lines, and only stores in memory
those lines that start "//go:generate".

Also: Adjust the documentation to make clear that it
does not parse the file.

Fixes golang#9143.
Fixes golang#9196.

LGTM=rsc, dominik.honnef
R=rsc, cespare, minux, dominik.honnef
CC=golang-codereviews
https://golang.org/cl/182970043
»»»

TBR=r
CC=golang-codereviews
https://golang.org/cl/183060044
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
@rsc @wheatman
[release-branch.go1.4] cmd/go: avoid use of bufio.Scanner in generate
69a7031 
««« CL 182970043 / 573a7b5178c4
cmd/go: avoid use of bufio.Scanner in generate

Scanner can't handle stupid long lines and there are
reports of stupid long lines in production.

Note the issue isn't long "//go:generate" lines, but
any long line in any Go source file.

To be fair, if you're going to have a stupid long line
it's not a bad bet you'll want to run it through go
generate, because it's some embeddable asset that
has been machine generated. (One could ask why
that generation process didn't add a newline or two,
but we should cope anyway.)

Rewrite the file scanner in "go generate" so it can
handle arbitrarily long lines, and only stores in memory
those lines that start "//go:generate".

Also: Adjust the documentation to make clear that it
does not parse the file.

Fixes golang#9143.
Fixes golang#9196.

LGTM=rsc, dominik.honnef
R=rsc, cespare, minux, dominik.honnef
CC=golang-codereviews
https://golang.org/cl/182970043
»»»

TBR=r
CC=golang-codereviews
https://golang.org/cl/183060044
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 20, 2018
@rsc @wheatman
[release-branch.go1.4] cmd/go: avoid use of bufio.Scanner in generate
6fae07d 
««« CL 182970043 / 573a7b5178c4
cmd/go: avoid use of bufio.Scanner in generate

Scanner can't handle stupid long lines and there are
reports of stupid long lines in production.

Note the issue isn't long "//go:generate" lines, but
any long line in any Go source file.

To be fair, if you're going to have a stupid long line
it's not a bad bet you'll want to run it through go
generate, because it's some embeddable asset that
has been machine generated. (One could ask why
that generation process didn't add a newline or two,
but we should cope anyway.)

Rewrite the file scanner in "go generate" so it can
handle arbitrarily long lines, and only stores in memory
those lines that start "//go:generate".

Also: Adjust the documentation to make clear that it
does not parse the file.

Fixes golang#9143.
Fixes golang#9196.

LGTM=rsc, dominik.honnef
R=rsc, cespare, minux, dominik.honnef
CC=golang-codereviews
https://golang.org/cl/182970043
»»»

TBR=r
CC=golang-codereviews
https://golang.org/cl/183060044
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
@rsc @wheatman
[release-branch.go1.4] cmd/go: avoid use of bufio.Scanner in generate
f7d67fd 
««« CL 182970043 / 573a7b5178c4
cmd/go: avoid use of bufio.Scanner in generate

Scanner can't handle stupid long lines and there are
reports of stupid long lines in production.

Note the issue isn't long "//go:generate" lines, but
any long line in any Go source file.

To be fair, if you're going to have a stupid long line
it's not a bad bet you'll want to run it through go
generate, because it's some embeddable asset that
has been machine generated. (One could ask why
that generation process didn't add a newline or two,
but we should cope anyway.)

Rewrite the file scanner in "go generate" so it can
handle arbitrarily long lines, and only stores in memory
those lines that start "//go:generate".

Also: Adjust the documentation to make clear that it
does not parse the file.

Fixes golang#9143.
Fixes golang#9196.

LGTM=rsc, dominik.honnef
R=rsc, cespare, minux, dominik.honnef
CC=golang-codereviews
https://golang.org/cl/182970043
»»»

TBR=r
CC=golang-codereviews
https://golang.org/cl/183060044
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.5
Development

No branches or pull requests
6 participants
@bradfitz @dominikh @rsc @ianlancetaylor @robpike @gopherbot