cmd/compile: detect and diagnose invalid //go: directive placement

Thie CL changes cmd/compile/internal/syntax to give the gc half of
the compiler more control over pragma handling, so that it can prepare
better errors, diagnose misuse, and so on. Before, the API between
the two was hard-coded as a uint16. Now it is an interface{}.
This should set us up better for future directives.

In addition to the split, this CL emits a "misplaced compiler directive"
error for any directive that is in a place where it has no effect.
I've certainly been confused in the past by adding comments
that were doing nothing and not realizing it. This should help
avoid that kind of confusion.

The rule, now applied consistently, is that a //go: directive
must appear on a line by itself immediately before the declaration
specifier it means to apply to. See cmd/compile/doc.go for
precise text and test/directive.go for examples.

This may cause some code to stop compiling, but that code
was broken. For example, this code formerly applied the
//go:noinline to f (not c) but now will fail to compile:

	//go:noinline
	const c = 1

	func f() {}

Change-Id: Ieba9b8d90a27cfab25de79d2790a895cefe5296f
Reviewed-on: https://go-review.googlesource.com/c/go/+/228578
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

4 files changed, 91 insertions, 44 deletions

diff --git a/src/cmd/compile/internal/syntax/nodes.go b/src/cmd/compile/internal/syntax/nodes.go
index 9a74c0250b..815630fcd4 100644
--- a/src/cmd/compile/internal/syntax/nodes.go
+++ b/src/cmd/compile/internal/syntax/nodes.go
@@ -34,6 +34,7 @@ func (*node) aNode()     {}
 
 // package PkgName; DeclList[0], DeclList[1], ...
 type File struct {
+	Pragma   Pragma
 	PkgName  *Name
 	DeclList []Decl
 	Lines    uint
@@ -52,9 +53,10 @@ type (
 	//              Path
 	// LocalPkgName Path
 	ImportDecl struct {
+		Group        *Group // nil means not part of a group
+		Pragma       Pragma
 		LocalPkgName *Name // including "."; nil means no rename present
 		Path         *BasicLit
-		Group        *Group // nil means not part of a group
 		decl
 	}
 
@@ -62,20 +64,21 @@ type (
 	// NameList      = Values
 	// NameList Type = Values
 	ConstDecl struct {
-		NameList []*Name
-		Type     Expr   // nil means no type
-		Values   Expr   // nil means no values
 		Group    *Group // nil means not part of a group
+		Pragma   Pragma
+		NameList []*Name
+		Type     Expr // nil means no type
+		Values   Expr // nil means no values
 		decl
 	}
 
 	// Name Type
 	TypeDecl struct {
+		Group  *Group // nil means not part of a group
+		Pragma Pragma
 		Name   *Name
 		Alias  bool
 		Type   Expr
-		Group  *Group // nil means not part of a group
-		Pragma Pragma
 		decl
 	}
 
@@ -83,10 +86,11 @@ type (
 	// NameList Type = Values
 	// NameList      = Values
 	VarDecl struct {
-		NameList []*Name
-		Type     Expr   // nil means no type
-		Values   Expr   // nil means no values
 		Group    *Group // nil means not part of a group
+		Pragma   Pragma
+		NameList []*Name
+		Type     Expr // nil means no type
+		Values   Expr // nil means no values
 		decl
 	}
 
@@ -95,12 +99,11 @@ type (
 	// func Receiver Name Type { Body }
 	// func Receiver Name Type
 	FuncDecl struct {
-		Attr   map[string]bool // go:attr map
-		Recv   *Field          // nil means regular function
+		Pragma Pragma
+		Recv   *Field // nil means regular function
 		Name   *Name
 		Type   *FuncType
 		Body   *BlockStmt // nil means no body (forward declaration)
-		Pragma Pragma     // TODO(mdempsky): Cleaner solution.
 		decl
 	}
 )
diff --git a/src/cmd/compile/internal/syntax/parser.go b/src/cmd/compile/internal/syntax/parser.go
index 5e52800b39..9601fab9e0 100644
--- a/src/cmd/compile/internal/syntax/parser.go
+++ b/src/cmd/compile/internal/syntax/parser.go
@@ -15,15 +15,16 @@ const debug = false
 const trace = false
 
 type parser struct {
-	file *PosBase
-	errh ErrorHandler
-	mode Mode
+	file  *PosBase
+	errh  ErrorHandler
+	mode  Mode
+	pragh PragmaHandler
 	scanner
 
 	base   *PosBase // current position base
 	first  error    // first error encountered
 	errcnt int      // number of errors encountered
-	pragma Pragma   // pragma flags
+	pragma Pragma   // pragmas
 
 	fnest  int    // function nesting level (for error handling)
 	xnest  int    // expression nesting level (for complit ambiguity resolution)
@@ -34,6 +35,7 @@ func (p *parser) init(file *PosBase, r io.Reader, errh ErrorHandler, pragh Pragm
 	p.file = file
 	p.errh = errh
 	p.mode = mode
+	p.pragh = pragh
 	p.scanner.init(
 		r,
 		// Error and directive handler for scanner.
@@ -47,9 +49,11 @@ func (p *parser) init(file *PosBase, r io.Reader, errh ErrorHandler, pragh Pragm
 				return
 			}
 
-			// otherwise it must be a comment containing a line or go: directive
+			// otherwise it must be a comment containing a line or go: directive.
+			// //line directives must be at the start of the line (column colbase).
+			// /*line*/ directives can be anywhere in the line.
 			text := commentText(msg)
-			if strings.HasPrefix(text, "line ") {
+			if (col == colbase || msg[1] == '*') && strings.HasPrefix(text, "line ") {
 				var pos Pos // position immediately following the comment
 				if msg[1] == '/' {
 					// line comment (newline is part of the comment)
@@ -67,7 +71,7 @@ func (p *parser) init(file *PosBase, r io.Reader, errh ErrorHandler, pragh Pragm
 
 			// go: directive (but be conservative and test)
 			if pragh != nil && strings.HasPrefix(text, "go:") {
-				p.pragma |= pragh(p.posAt(line, col+2), text) // +2 to skip over // or /*
+				p.pragma = pragh(p.posAt(line, col+2), p.scanner.blank, text, p.pragma) // +2 to skip over // or /*
 			}
 		},
 		directives,
@@ -76,13 +80,32 @@ func (p *parser) init(file *PosBase, r io.Reader, errh ErrorHandler, pragh Pragm
 	p.base = file
 	p.first = nil
 	p.errcnt = 0
-	p.pragma = 0
+	p.pragma = nil
 
 	p.fnest = 0
 	p.xnest = 0
 	p.indent = nil
 }
 
+// takePragma returns the current parsed pragmas
+// and clears them from the parser state.
+func (p *parser) takePragma() Pragma {
+	prag := p.pragma
+	p.pragma = nil
+	return prag
+}
+
+// clearPragma is called at the end of a statement or
+// other Go form that does NOT accept a pragma.
+// It sends the pragma back to the pragma handler
+// to be reported as unused.
+func (p *parser) clearPragma() {
+	if p.pragma != nil {
+		p.pragh(p.pos(), p.scanner.blank, "", p.pragma)
+		p.pragma = nil
+	}
+}
+
 // updateBase sets the current position base to a new line base at pos.
 // The base's filename, line, and column values are extracted from text
 // which is positioned at (tline, tcol) (only needed for error messages).
@@ -362,6 +385,7 @@ func (p *parser) fileOrNil() *File {
 		p.syntaxError("package statement must be first")
 		return nil
 	}
+	f.Pragma = p.takePragma()
 	f.PkgName = p.name()
 	p.want(_Semi)
 
@@ -410,7 +434,7 @@ func (p *parser) fileOrNil() *File {
 
 		// Reset p.pragma BEFORE advancing to the next token (consuming ';')
 		// since comments before may set pragmas for the next function decl.
-		p.pragma = 0
+		p.clearPragma()
 
 		if p.tok != _EOF && !p.got(_Semi) {
 			p.syntaxError("after top level declaration")
@@ -419,6 +443,7 @@ func (p *parser) fileOrNil() *File {
 	}
 	// p.tok == _EOF
 
+	p.clearPragma()
 	f.Lines = p.line
 
 	return f
@@ -469,6 +494,7 @@ func (p *parser) list(open, sep, close token, f func() bool) Pos {
 func (p *parser) appendGroup(list []Decl, f func(*Group) Decl) []Decl {
 	if p.tok == _Lparen {
 		g := new(Group)
+		p.clearPragma()
 		p.list(_Lparen, _Semi, _Rparen, func() bool {
 			list = append(list, f(g))
 			return false
@@ -497,6 +523,8 @@ func (p *parser) importDecl(group *Group) Decl {
 
 	d := new(ImportDecl)
 	d.pos = p.pos()
+	d.Group = group
+	d.Pragma = p.takePragma()
 
 	switch p.tok {
 	case _Name:
@@ -511,7 +539,6 @@ func (p *parser) importDecl(group *Group) Decl {
 		p.advance(_Semi, _Rparen)
 		return nil
 	}
-	d.Group = group
 
 	return d
 }
@@ -524,6 +551,8 @@ func (p *parser) constDecl(group *Group) Decl {
 
 	d := new(ConstDecl)
 	d.pos = p.pos()
+	d.Group = group
+	d.Pragma = p.takePragma()
 
 	d.NameList = p.nameList(p.name())
 	if p.tok != _EOF && p.tok != _Semi && p.tok != _Rparen {
@@ -532,7 +561,6 @@ func (p *parser) constDecl(group *Group) Decl {
 			d.Values = p.exprList()
 		}
 	}
-	d.Group = group
 
 	return d
 }
@@ -545,6 +573,8 @@ func (p *parser) typeDecl(group *Group) Decl {
 
 	d := new(TypeDecl)
 	d.pos = p.pos()
+	d.Group = group
+	d.Pragma = p.takePragma()
 
 	d.Name = p.name()
 	d.Alias = p.gotAssign()
@@ -554,8 +584,6 @@ func (p *parser) typeDecl(group *Group) Decl {
 		p.syntaxError("in type declaration")
 		p.advance(_Semi, _Rparen)
 	}
-	d.Group = group
-	d.Pragma = p.pragma
 
 	return d
 }
@@ -568,6 +596,8 @@ func (p *parser) varDecl(group *Group) Decl {
 
 	d := new(VarDecl)
 	d.pos = p.pos()
+	d.Group = group
+	d.Pragma = p.takePragma()
 
 	d.NameList = p.nameList(p.name())
 	if p.gotAssign() {
@@ -578,7 +608,6 @@ func (p *parser) varDecl(group *Group) Decl {
 			d.Values = p.exprList()
 		}
 	}
-	d.Group = group
 
 	return d
 }
@@ -595,6 +624,7 @@ func (p *parser) funcDeclOrNil() *FuncDecl {
 
 	f := new(FuncDecl)
 	f.pos = p.pos()
+	f.Pragma = p.takePragma()
 
 	if p.tok == _Lparen {
 		rcvr := p.paramList()
@@ -620,7 +650,6 @@ func (p *parser) funcDeclOrNil() *FuncDecl {
 	if p.tok == _Lbrace {
 		f.Body = p.funcBody()
 	}
-	f.Pragma = p.pragma
 
 	return f
 }
@@ -2054,6 +2083,7 @@ func (p *parser) stmtOrNil() Stmt {
 	// Most statements (assignments) start with an identifier;
 	// look for it first before doing anything more expensive.
 	if p.tok == _Name {
+		p.clearPragma()
 		lhs := p.exprList()
 		if label, ok := lhs.(*Name); ok && p.tok == _Colon {
 			return p.labeledStmtOrNil(label)
@@ -2062,9 +2092,6 @@ func (p *parser) stmtOrNil() Stmt {
 	}
 
 	switch p.tok {
-	case _Lbrace:
-		return p.blockStmt("")
-
 	case _Var:
 		return p.declStmt(p.varDecl)
 
@@ -2073,6 +2100,13 @@ func (p *parser) stmtOrNil() Stmt {
 
 	case _Type:
 		return p.declStmt(p.typeDecl)
+	}
+
+	p.clearPragma()
+
+	switch p.tok {
+	case _Lbrace:
+		return p.blockStmt("")
 
 	case _Operator, _Star:
 		switch p.op {
@@ -2151,6 +2185,7 @@ func (p *parser) stmtList() (l []Stmt) {
 
 	for p.tok != _EOF && p.tok != _Rbrace && p.tok != _Case && p.tok != _Default {
 		s := p.stmtOrNil()
+		p.clearPragma()
 		if s == nil {
 			break
 		}
diff --git a/src/cmd/compile/internal/syntax/scanner.go b/src/cmd/compile/internal/syntax/scanner.go
index 6cb7ff83a0..9fe4965984 100644
--- a/src/cmd/compile/internal/syntax/scanner.go
+++ b/src/cmd/compile/internal/syntax/scanner.go
@@ -34,6 +34,7 @@ type scanner struct {
 
 	// current token, valid after calling next()
 	line, col uint
+	blank     bool // line is blank up to col
 	tok       token
 	lit       string   // valid if tok is _Name, _Literal, or _Semi ("semicolon", "newline", or "EOF"); may be malformed if bad is true
 	bad       bool     // valid if tok is _Literal, true if a syntax error occurred, lit may be malformed
@@ -83,10 +84,7 @@ func (s *scanner) setLit(kind LitKind, ok bool) {
 //
 // If the scanner mode includes the directives (but not the comments)
 // flag, only comments containing a //line, /*line, or //go: directive
-// are reported, in the same way as regular comments. Directives in
-// //-style comments are only recognized if they are at the beginning
-// of a line.
-//
+// are reported, in the same way as regular comments.
 func (s *scanner) next() {
 	nlsemi := s.nlsemi
 	s.nlsemi = false
@@ -94,12 +92,14 @@ func (s *scanner) next() {
 redo:
 	// skip white space
 	s.stop()
+	startLine, startCol := s.pos()
 	for s.ch == ' ' || s.ch == '\t' || s.ch == '\n' && !nlsemi || s.ch == '\r' {
 		s.nextch()
 	}
 
 	// token start
 	s.line, s.col = s.pos()
+	s.blank = s.line > startLine || startCol == colbase
 	s.start()
 	if isLetter(s.ch) || s.ch >= utf8.RuneSelf && s.atIdentChar(true) {
 		s.nextch()
@@ -741,8 +741,8 @@ func (s *scanner) lineComment() {
 		return
 	}
 
-	// directives must start at the beginning of the line (s.col == colbase)
-	if s.mode&directives == 0 || s.col != colbase || (s.ch != 'g' && s.ch != 'l') {
+	// are we saving directives? or is this definitely not a directive?
+	if s.mode&directives == 0 || (s.ch != 'g' && s.ch != 'l') {
 		s.stop()
 		s.skipLine()
 		return
diff --git a/src/cmd/compile/internal/syntax/syntax.go b/src/cmd/compile/internal/syntax/syntax.go
index b8c387419f..e51b5538b3 100644
--- a/src/cmd/compile/internal/syntax/syntax.go
+++ b/src/cmd/compile/internal/syntax/syntax.go
@@ -33,15 +33,24 @@ var _ error = Error{} // verify that Error implements error
 // An ErrorHandler is called for each error encountered reading a .go file.
 type ErrorHandler func(err error)
 
-// A Pragma value is a set of flags that augment a function or
-// type declaration. Callers may assign meaning to the flags as
-// appropriate.
-type Pragma uint16
+// A Pragma value augments a package, import, const, func, type, or var declaration.
+// Its meaning is entirely up to the PragmaHandler,
+// except that nil is used to mean “no pragma seen.”
+type Pragma interface{}
 
-// A PragmaHandler is used to process //go: directives as
-// they're scanned. The returned Pragma value will be unioned into the
-// next FuncDecl node.
-type PragmaHandler func(pos Pos, text string) Pragma
+// A PragmaHandler is used to process //go: directives while scanning.
+// It is passed the current pragma value, which starts out being nil,
+// and it returns an updated pragma value.
+// The text is the directive, with the "//" prefix stripped.
+// The current pragma is saved at each package, import, const, func, type, or var
+// declaration, into the File, ImportDecl, ConstDecl, FuncDecl, TypeDecl, or VarDecl node.
+//
+// If text is the empty string, the pragma is being returned
+// to the handler unused, meaning it appeared before a non-declaration.
+// The handler may wish to report an error. In this case, pos is the
+// current parser position, not the position of the pragma itself.
+// Blank specifies whether the line is blank before the pragma.
+type PragmaHandler func(pos Pos, blank bool, text string, current Pragma) Pragma
 
 // Parse parses a single Go source file from src and returns the corresponding
 // syntax tree. If there are errors, Parse will return the first error found,