]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/asm: reject misplaced go:build comments
authorRuss Cox <rsc@golang.org>
Mon, 29 Jun 2020 21:08:49 +0000 (17:08 -0400)
committerRuss Cox <rsc@golang.org>
Tue, 13 Oct 2020 01:16:56 +0000 (01:16 +0000)
We are converting from using error-prone ad-hoc syntax // +build lines
to less error-prone, standard boolean syntax //go:build lines.
The timeline is:

Go 1.16: prepare for transition
 - Builds still use // +build for file selection.
 - Source files may not contain //go:build without // +build.
 - Builds fail when a source file contains //go:build lines without // +build lines. <<<

Go 1.17: start transition
 - Builds prefer //go:build for file selection, falling back to // +build
   for files containing only // +build.
 - Source files may contain //go:build without // +build (but they won't build with Go 1.16).
 - Gofmt moves //go:build and // +build lines to proper file locations.
 - Gofmt introduces //go:build lines into files with only // +build lines.
 - Go vet rejects files with mismatched //go:build and // +build lines.

Go 1.18: complete transition
 - Go fix removes // +build lines, leaving behind equivalent // +build lines.

This CL provides part of the <<< marked line above in the Go 1.16 step:
rejecting files containing //go:build but not // +build.

Reject any //go:build comments found after actual assembler code
(include #include etc directives), because the go command itself
doesn't read that far.

For #41184.

Change-Id: Ib460bfd380cce4239993980dd208afd07deff3f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/240602
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
src/cmd/asm/internal/asm/endtoend_test.go
src/cmd/asm/internal/asm/parse.go
src/cmd/asm/internal/asm/testdata/buildtagerror.s [new file with mode: 0644]
src/cmd/asm/internal/lex/input.go
src/cmd/asm/internal/lex/lex.go
src/cmd/asm/internal/lex/lex_test.go
src/cmd/asm/internal/lex/tokenizer.go

index 15202dc5dceb8a80932ad7c780459fcd4b623246..b21e3156aea23d21f374c53a4c9344c13702e1c5 100644 (file)
@@ -257,11 +257,11 @@ func isHexes(s string) bool {
        return true
 }
 
-// It would be nice if the error messages began with
+// It would be nice if the error messages always began with
 // the standard file:line: prefix,
 // but that's not where we are today.
 // It might be at the beginning but it might be in the middle of the printed instruction.
-var fileLineRE = regexp.MustCompile(`(?:^|\()(testdata[/\\][0-9a-z]+\.s:[0-9]+)(?:$|\))`)
+var fileLineRE = regexp.MustCompile(`(?:^|\()(testdata[/\\][0-9a-z]+\.s:[0-9]+)(?:$|\)|:)`)
 
 // Same as in test/run.go
 var (
@@ -281,6 +281,7 @@ func testErrors(t *testing.T, goarch, file string) {
        defer ctxt.Bso.Flush()
        failed := false
        var errBuf bytes.Buffer
+       parser.errorWriter = &errBuf
        ctxt.DiagFunc = func(format string, args ...interface{}) {
                failed = true
                s := fmt.Sprintf(format, args...)
@@ -292,7 +293,7 @@ func testErrors(t *testing.T, goarch, file string) {
        pList.Firstpc, ok = parser.Parse()
        obj.Flushplist(ctxt, pList, nil, "")
        if ok && !failed {
-               t.Errorf("asm: %s had no errors", goarch)
+               t.Errorf("asm: %s had no errors", file)
        }
 
        errors := map[string]string{}
@@ -368,6 +369,10 @@ func TestARMEndToEnd(t *testing.T) {
        }
 }
 
+func TestGoBuildErrors(t *testing.T) {
+       testErrors(t, "amd64", "buildtagerror")
+}
+
 func TestARMErrors(t *testing.T) {
        testErrors(t, "arm", "armerror")
 }
index 17d40ee4156dee8833b2b41d4ca3cfd232cffeab..d9dbd92cb020339f28896af1284c11cd8891914d 100644 (file)
@@ -29,6 +29,7 @@ type Parser struct {
        lineNum       int   // Line number in source file.
        errorLine     int   // Line number of last error.
        errorCount    int   // Number of errors.
+       sawCode       bool  // saw code in this file (as opposed to comments and blank lines)
        pc            int64 // virtual PC; count of Progs; doesn't advance for GLOBL or DATA.
        input         []lex.Token
        inputPos      int
@@ -132,6 +133,30 @@ func (p *Parser) ParseSymABIs(w io.Writer) bool {
        return p.errorCount == 0
 }
 
+// nextToken returns the next non-build-comment token from the lexer.
+// It reports misplaced //go:build comments but otherwise discards them.
+func (p *Parser) nextToken() lex.ScanToken {
+       for {
+               tok := p.lex.Next()
+               if tok == lex.BuildComment {
+                       if p.sawCode {
+                               p.errorf("misplaced //go:build comment")
+                       }
+                       continue
+               }
+               if tok != '\n' {
+                       p.sawCode = true
+               }
+               if tok == '#' {
+                       // A leftover wisp of a #include/#define/etc,
+                       // to let us know that p.sawCode should be true now.
+                       // Otherwise ignored.
+                       continue
+               }
+               return tok
+       }
+}
+
 // line consumes a single assembly line from p.lex of the form
 //
 //   {label:} WORD[.cond] [ arg {, arg} ] (';' | '\n')
@@ -146,7 +171,7 @@ next:
        // Skip newlines.
        var tok lex.ScanToken
        for {
-               tok = p.lex.Next()
+               tok = p.nextToken()
                // We save the line number here so error messages from this instruction
                // are labeled with this line. Otherwise we complain after we've absorbed
                // the terminating newline and the line numbers are off by one in errors.
@@ -179,11 +204,11 @@ next:
                        items = make([]lex.Token, 0, 3)
                }
                for {
-                       tok = p.lex.Next()
+                       tok = p.nextToken()
                        if len(operands) == 0 && len(items) == 0 {
                                if p.arch.InFamily(sys.ARM, sys.ARM64, sys.AMD64, sys.I386) && tok == '.' {
                                        // Suffixes: ARM conditionals or x86 modifiers.
-                                       tok = p.lex.Next()
+                                       tok = p.nextToken()
                                        str := p.lex.Text()
                                        if tok != scanner.Ident {
                                                p.errorf("instruction suffix expected identifier, found %s", str)
diff --git a/src/cmd/asm/internal/asm/testdata/buildtagerror.s b/src/cmd/asm/internal/asm/testdata/buildtagerror.s
new file mode 100644 (file)
index 0000000..5a2d65b
--- /dev/null
@@ -0,0 +1,8 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+#define X 1
+
+//go:build x // ERROR "misplaced //go:build comment"
+
index a43953b515469852e4a7b4095d32824a5751a4a1..da4ebe6d6e3447fa5a672863b68e459b720646ec 100644 (file)
@@ -109,6 +109,9 @@ func (in *Input) Next() ScanToken {
                                in.Error("'#' must be first item on line")
                        }
                        in.beginningOfLine = in.hash()
+                       in.text = "#"
+                       return '#'
+
                case scanner.Ident:
                        // Is it a macro name?
                        name := in.Stack.Text()
index f1f7da79112e350a036710ef75980c194fa49286..7cd41a55a9b33c0453962e147b9c8ee8371ff004 100644 (file)
@@ -22,11 +22,13 @@ type ScanToken rune
 const (
        // Asm defines some two-character lexemes. We make up
        // a rune/ScanToken value for them - ugly but simple.
-       LSH       ScanToken = -1000 - iota // << Left shift.
-       RSH                                // >> Logical right shift.
-       ARR                                // -> Used on ARM for shift type 3, arithmetic right shift.
-       ROT                                // @> Used on ARM for shift type 4, rotate right.
-       macroName                          // name of macro that should not be expanded
+       LSH          ScanToken = -1000 - iota // << Left shift.
+       RSH                                   // >> Logical right shift.
+       ARR                                   // -> Used on ARM for shift type 3, arithmetic right shift.
+       ROT                                   // @> Used on ARM for shift type 4, rotate right.
+       Include                               // included file started here
+       BuildComment                          // //go:build or +build comment
+       macroName                             // name of macro that should not be expanded
 )
 
 // IsRegisterShift reports whether the token is one of the ARM register shift operators.
index f606ffe07b06a0b1abe96cfa7f7b0521e0cea23b..51679d2fbc71753c742a20f8a1393534a0622015 100644 (file)
@@ -281,6 +281,9 @@ func drain(input *Input) string {
                if tok == scanner.EOF {
                        return buf.String()
                }
+               if tok == '#' {
+                       continue
+               }
                if buf.Len() > 0 {
                        buf.WriteByte('.')
                }
index aef9ea8636844abced81fc693c89de2137845d06..861a2d421d5a81b7bb97b3ed6f5ac0038bb10b15 100644 (file)
@@ -107,10 +107,13 @@ func (t *Tokenizer) Next() ScanToken {
                if t.tok != scanner.Comment {
                        break
                }
-               length := strings.Count(s.TokenText(), "\n")
-               t.line += length
-               // TODO: If we ever have //go: comments in assembly, will need to keep them here.
-               // For now, just discard all comments.
+               text := s.TokenText()
+               t.line += strings.Count(text, "\n")
+               // TODO: Use constraint.IsGoBuild once it exists.
+               if strings.HasPrefix(text, "//go:build") {
+                       t.tok = BuildComment
+                       break
+               }
        }
        switch t.tok {
        case '\n':