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/compile: bad export data causes "unexpected string literal" on import #13777

Closed
dr2chase opened this issue Dec 29, 2015 · 3 comments
Closed
Milestone

Comments

@dr2chase
Copy link
Contributor

The root cause appears to be function-summary escape data appearing within a function type where it is not expected.

Compilation of this file

package burnin

type sendCmdFunc func(string)

func sendCommand(c string) {}

func NewSomething() {
    // This works...
    // var sendCmd sendCmdFunc
    // sendCmd = sendCommand

    // This fails...
    sendCmd := sendCommand

    _ = sendCmd
}

creates export data that won't parse. The syntax error "unexpected string literal" is attributed to the importing line. For example, building this file will yield such an error:

package main

import (
    x "./burnin" // Syntax error is attributed to this line.
)

func main() {
    x.NewSomething()
}

Comparing the export data from the "This works" variant to the one that fails, it appears that escape data attached to an in-line (as opposed to named) function type causes the problem:

diff -c {bad,good}/__.PKGDEF
*** bad/__.PKGDEF   2015-12-29 13:42:38.000000000 -0500
--- good/__.PKGDEF  2015-12-29 13:42:32.000000000 -0500
***************
*** 3,9 ****

  $$
  package burnin
!   func @"".NewSomething () {  var @"".sendCmd·1 func(@"".c·1 string "esc:0x1"); @"".sendCmd·1 = @"".sendCommand; _ = @"".sendCmd·1 }
    func @"".sendCommand (@"".c·1 string "esc:0x1") {  }

  $$
--- 3,10 ----

  $$
  package burnin
!   func @"".NewSomething () { var @"".sendCmd·1 @"".sendCmdFunc; ; @"".sendCmd·1 = @"".sendCommand; _ = @"".sendCmd·1 }
!   type @"".sendCmdFunc func(? string)
    func @"".sendCommand (@"".c·1 string "esc:0x1") {  }

  $$
@dr2chase dr2chase self-assigned this Dec 29, 2015
@dr2chase
Copy link
Contributor Author

@Sajmani

@ianlancetaylor ianlancetaylor changed the title Bad export data causes "unexpected string literal" on import. cmd/compile: bad export data causes "unexpected string literal" on import Dec 30, 2015
@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Dec 30, 2015
@gopherbot
Copy link

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

@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Feb 16, 2016
This bug was introduced in golang.org/cl/18217,
while trying to fix #13777.

Originally I wanted to just disable inlining for the case
being handled incorrectly, but it's fairly difficult to detect
and much easier just to fix. Since the case being handled
incorrectly was inlined correctly in Go 1.5, not inlining it
would also be somewhat of a regression.
So just fix it.

Test case copied from Ian's CL 19520.

The mistake to worry about in this CL would be relaxing
the condition too much (we now print the note more often
than we did yesterday). To confirm that we'd catch this mistake,
I checked that changing (!fmtbody || !t.Funarg) to (true) does
cause fixedbugs/issue13777.go to fail. And putting it back
to what is written in this CL makes that test pass again
as well as the new fixedbugs/issue14331.go.
So I believe that the new condition is correct for both constraints.

Fixes #14331.

Change-Id: I91f75a4d5d07c53af5caea1855c780d9874b8df6
Reviewed-on: https://go-review.googlesource.com/19514
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Feb 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants