cmd/compile: refactor export data parsing

Merge push_parser and pop_parser into a single parse_import function
and inline unimportfile. Shake out function boundaries a little bit so
that the symmetry is readily visible.

Move the import_package call into parse_import (and inline
import_there into import_package).  This means importfile no longer
needs to provide fake import data to be needlessly lexed/parsed every
time it's called.

Also, instead of indicating import success/failure by whether the next
token is "package", import_spec can just check whether importpkg is
non-nil.

Tangentially, this somehow alters the diagnostics produced for
test/fixedbugs/issue11610.go.  However, the new diagnostics are more
consistent with those produced when the empty import statement is
absent, which seems more desirable than maintaining the previous
errors.

Change-Id: I5cd1c22aa14da8a743ef569ff084711d137279d5
Reviewed-on: https://go-review.googlesource.com/19650Reviewed-by: Robert Griesemer <gri@golang.org>

src/cmd/compile/internal/gc/lex.go

@@ -671,38 +671,30 @@ func loadsys() {
 
 	importpkg = Runtimepkg
 	cannedimports("runtime.Builtin", runtimeimport)
-	thenewparser.import_package()
-	thenewparser.import_there()
 
 	importpkg = unsafepkg
 	cannedimports("unsafe.o", unsafeimport)
-	thenewparser.import_package()
-	thenewparser.import_there()
 
 	importpkg = nil
 }
 
-func fakeimport() {
-	importpkg = mkpkg("fake")
-	cannedimports("fake.o", "$$\n")
-}
-
 func importfile(f *Val) {
+	if importpkg != nil {
+		Fatalf("importpkg not nil")
+	}
+
 	path_, ok := f.U.(string)
 	if !ok {
 		Yyerror("import statement not a string")
-		fakeimport()
 		return
 	}
 
 	if len(path_) == 0 {
 		Yyerror("import path is empty")
-		fakeimport()
 		return
 	}
 
 	if isbadimport(path_) {
-		fakeimport()
 		return
 	}
 
@@ -731,7 +723,6 @@ func importfile(f *Val) {
 		}
 
 		importpkg = unsafepkg
-		cannedimports("unsafe.o", "package unsafe\n\n$$\n\n")
 		imported_unsafe = true
 		return
 	}
@@ -739,7 +730,6 @@ func importfile(f *Val) {
 	if islocalname(path_) {
 		if path_[0] == '/' {
 			Yyerror("import path cannot be absolute path")
-			fakeimport()
 			return
 		}
 
@@ -754,7 +744,6 @@ func importfile(f *Val) {
 		path_ = cleanbuf
 
 		if isbadimport(path_) {
-			fakeimport()
 			return
 		}
 	}
@@ -767,28 +756,18 @@ func importfile(f *Val) {
 
 	importpkg = mkpkg(path_)
 
-	// If we already saw that package, feed a dummy statement
-	// to the lexer to avoid parsing export data twice.
 	if importpkg.Imported {
-		tag := ""
-		if importpkg.Safe {
-			tag = "safe"
-		}
-
-		p := fmt.Sprintf("package %s %s\n$$\n", importpkg.Name, tag)
-		cannedimports(file, p)
 		return
 	}
 
 	importpkg.Imported = true
 
-	var err error
-	var imp *obj.Biobuf
-	imp, err = obj.Bopenr(file)
+	imp, err := obj.Bopenr(file)
 	if err != nil {
 		Yyerror("can't open import: %q: %v", path_, err)
 		errorexit()
 	}
+	defer obj.Bterm(imp)
 
 	if strings.HasSuffix(file, ".a") {
 		if !skiptopkgdef(imp) {
@@ -845,74 +824,44 @@ func importfile(f *Val) {
 	case '\n':
 		// old export format
 		pushedio = curio
-
-		curio.bin = imp
-		curio.peekc = 0
-		curio.peekc1 = 0
-		curio.infile = file
-		curio.nlsemi = false
+		curio = Io{bin: imp, infile: file}
 		typecheckok = true
 
-		push_parser()
+		parse_import()
+
+		typecheckok = false
+		curio = pushedio
+		pushedio.bin = nil
 
 	case 'B':
 		// new export format
 		obj.Bgetc(imp) // skip \n after $$B
 		Import(imp)
 
-		// continue as if the package was imported before (see above)
-		tag := ""
-		if importpkg.Safe {
-			tag = "safe"
-		}
-		p := fmt.Sprintf("package %s %s\n$$\n", importpkg.Name, tag)
-		cannedimports(file, p)
-		// Reset incannedimport flag (we are not truly in a
-		// canned import) - this will cause importpkg.Direct to
-		// be set via parser.import_package (was issue #13977).
-		//
-		// TODO(gri) Remove this global variable and convoluted
-		// code in the process of streamlining the import code.
-		incannedimport = 0
-
 	default:
 		Yyerror("no import in %q", path_)
+		errorexit()
 	}
-}
-
-func unimportfile() {
-	pop_parser()
 
-	if curio.bin != nil {
-		obj.Bterm(curio.bin)
-		curio.bin = nil
-	} else {
-		lexlineno-- // re correct sys.6 line number
+	if safemode != 0 && !importpkg.Safe {
+		Yyerror("cannot import unsafe package %q", importpkg.Path)
 	}
-
-	curio = pushedio
-
-	pushedio.bin = nil
-	incannedimport = 0
-	typecheckok = false
 }
 
 func cannedimports(file string, cp string) {
 	lexlineno++ // if sys.6 is included on line 1,
-
 	pushedio = curio
-
-	curio.bin = nil
-	curio.peekc = 0
-	curio.peekc1 = 0
-	curio.infile = file
-	curio.cp = cp
-	curio.nlsemi = false
-
+	curio = Io{infile: file, cp: cp}
 	typecheckok = true
 	incannedimport = 1
 
-	push_parser()
+	parse_import()
+
+	typecheckok = false
+	incannedimport = 0
+	curio = pushedio
+	pushedio.bin = nil
+	lexlineno-- // re correct sys.6 line number
 }
 
 func isSpace(c int) bool {

src/cmd/compile/internal/gc/parser.go

@@ -20,13 +20,11 @@ import (
 
 const trace = false // if set, parse tracing can be enabled with -x
 
-// TODO(gri) Once we handle imports w/o redirecting the underlying
-// source of the lexer we can get rid of these. They are here for
-// compatibility with the existing yacc-based parser setup (issue 13242).
-var thenewparser parser // the parser in use
-var savedstate []parser // saved parser state, used during import
+// TODO(gri) Once we stop supporting the legacy export data format
+// we can get rid of this (issue 13242).
+var fileparser parser // the Go source file parser in use
 
-func push_parser() {
+func parse_import() {
 	// Indentation (for tracing) must be preserved across parsers
 	// since we are changing the lexer source (and parser state)
 	// under foot, in the middle of productions. This won't be
@@ -34,25 +32,16 @@ func push_parser() {
 	// be the push/pop_parser functionality.
 	// (Instead we could just use a global variable indent, but
 	// but eventually indent should be parser-specific anyway.)
-	indent := thenewparser.indent
-	savedstate = append(savedstate, thenewparser)
-	thenewparser = parser{indent: indent} // preserve indentation
-	thenewparser.next()
-}
-
-func pop_parser() {
-	indent := thenewparser.indent
-	n := len(savedstate) - 1
-	thenewparser = savedstate[n]
-	thenewparser.indent = indent // preserve indentation
-	savedstate = savedstate[:n]
+	importparser := parser{indent: fileparser.indent} // preserve indentation
+	importparser.next()
+	importparser.import_package()
 }
 
 // parse_file sets up a new parser and parses a single Go source file.
 func parse_file() {
-	thenewparser = parser{}
-	thenewparser.next()
-	thenewparser.file()
+	fileparser = parser{}
+	fileparser.next()
+	fileparser.file()
 }
 
 type parser struct {
@@ -364,23 +353,18 @@ func (p *parser) importdcl() {
 	p.next()
 
 	importfile(&path)
-	if p.tok != LPACKAGE {
-		// When an invalid import path is passed to importfile,
-		// it calls Yyerror and then sets up a fake import with
-		// no package statement. This allows us to test more
-		// than one invalid import statement in a single file.
-		p.import_there()
+	if importpkg == nil {
 		if nerrors == 0 {
 			Fatalf("phase error in import")
 		}
 		return
 	}
-	p.import_package()
-	p.import_there()
 
 	ipkg := importpkg
 	importpkg = nil
 
+	ipkg.Direct = true
+
 	if my == nil {
 		my = Lookup(ipkg.Name)
 	}
@@ -442,23 +426,8 @@ func (p *parser) import_package() {
 	} else if importpkg.Name != name {
 		Yyerror("conflicting names %s and %s for package %q", importpkg.Name, name, importpkg.Path)
 	}
-	if incannedimport == 0 {
-		importpkg.Direct = true
-	}
 	importpkg.Safe = importsafe
 
-	if safemode != 0 && !importsafe {
-		Yyerror("cannot import unsafe package %q", importpkg.Path)
-	}
-}
-
-// import_there parses the imported package definitions and then switches
-// the underlying lexed source back to the importing package.
-func (p *parser) import_there() {
-	if trace && Debug['x'] != 0 {
-		defer p.trace("import_there")()
-	}
-
 	defercheckwidth()
 
 	p.hidden_import_list()
@@ -469,7 +438,6 @@ func (p *parser) import_there() {
 	}
 
 	resumecheckwidth()
-	unimportfile()
 }
 
 // Declaration = ConstDecl | TypeDecl | VarDecl .

test/fixedbugs/issue11610.go

@@ -9,9 +9,9 @@
 
 package a
 import""  // ERROR "import path is empty"
-var?      // ERROR "invalid declaration"
+var?      // ERROR "unexpected \?"
 
-var x int // ERROR "unexpected var"
+var x int // ERROR "unexpected var" "cannot declare name"
 
 func main() {
 }