]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: simplify reexport logic
authorMatthew Dempsky <mdempsky@google.com>
Sat, 31 Mar 2018 01:58:03 +0000 (18:58 -0700)
committerMatthew Dempsky <mdempsky@google.com>
Mon, 2 Apr 2018 22:41:56 +0000 (22:41 +0000)
Currently, we reexport any package-scope constant, function, type, or
variable declarations needed by an inlineable function body. However,
now that we have an early pass to walk inlineable function bodies
(golang.org/cl/74110), we can simplify the logic for finding these
declarations.

The binary export format supports writing out type declarations
in-place at their first use. Also, it always writes out constants by
value, so their declarations never need to be reexported.

Notably, we attempted this before (golang.org/cl/36170) and had to
revert it (golang.org/cl/45911). However, this was because while
writing out inline bodies, we could discover variable/function
dependencies. By collecting variable/function dependencies during
inlineable function discovery, we avoid this problem.

While here, get rid of isInlineable. We already typecheck inlineable
function bodies during inlFlood, so it's become a no-op. Just move the
comment explaining parameter numbering to its caller.

Change-Id: Ibbfaafce793733675d3a2ad98791758583055666
Reviewed-on: https://go-review.googlesource.com/103864
Reviewed-by: Robert Griesemer <gri@golang.org>
src/cmd/compile/internal/gc/bexport.go
src/cmd/compile/internal/gc/export.go
src/cmd/compile/internal/gc/inl.go

index a1bb7f84d317f94e8a93c855eb9e110a5ef70711..3c663d4bca844db1cc4f234f3328b757df0c4db2 100644 (file)
@@ -232,46 +232,50 @@ func export(out *bufio.Writer, trace bool) int {
                p.tracef("\n")
        }
 
-       // Mark all inlineable functions that the importer could call.
-       // This is done by tracking down all inlineable methods
-       // reachable from exported types.
-       p.marked = make(map[*types.Type]bool)
-       for _, n := range exportlist {
-               sym := n.Sym
-               if sym.Exported() {
-                       // Closures are added to exportlist, but with Exported
-                       // already set. The export code below skips over them, so
-                       // we have to here as well.
-                       // TODO(mdempsky): Investigate why. This seems suspicious.
-                       continue
-               }
-               p.markType(asNode(sym.Def).Type)
-       }
-       p.marked = nil
-
        // export objects
        //
-       // First, export all exported (package-level) objects; i.e., all objects
-       // in the current exportlist. These objects represent all information
-       // required to import this package and type-check against it; i.e., this
-       // is the platform-independent export data. The format is generic in the
-       // sense that different compilers can use the same representation.
+       // We've already added all exported (package-level) objects to
+       // exportlist. These objects represent all information
+       // required to import this package and type-check against it;
+       // i.e., this is the platform-independent export data. The
+       // format is generic in the sense that different compilers can
+       // use the same representation.
+       //
+       // However, due to inlineable function and their dependencies,
+       // we may need to export (or possibly reexport) additional
+       // objects. We handle these objects separately. This data is
+       // platform-specific as it depends on the inlining decisions
+       // of the compiler and the representation of the inlined
+       // function bodies.
+
+       // Remember initial exportlist length.
+       numglobals := len(exportlist)
+
+       // Phase 0: Mark all inlineable functions that an importing
+       // package could call. This is done by tracking down all
+       // inlineable methods reachable from exported declarations.
        //
-       // During this first phase, more objects may be added to the exportlist
-       // (due to inlined function bodies and their dependencies). Export those
-       // objects in a second phase. That data is platform-specific as it depends
-       // on the inlining decisions of the compiler and the representation of the
-       // inlined function bodies.
-
-       // remember initial exportlist length
-       var numglobals = len(exportlist)
-
-       // Phase 1: Export objects in _current_ exportlist; exported objects at
-       //          package level.
-       // Use range since we want to ignore objects added to exportlist during
-       // this phase.
+       // Along the way, we add to exportlist any function and
+       // variable declarations needed by the inline bodies.
+       if exportInlined {
+               p.marked = make(map[*types.Type]bool)
+               for _, n := range exportlist {
+                       sym := n.Sym
+                       if sym.Exported() {
+                               // Closures are added to exportlist, but with Exported
+                               // already set. The export code below skips over them, so
+                               // we have to here as well.
+                               // TODO(mdempsky): Investigate why. This seems suspicious.
+                               continue
+                       }
+                       p.markType(asNode(sym.Def).Type)
+               }
+               p.marked = nil
+       }
+
+       // Phase 1: Export package-level objects.
        objcount := 0
-       for _, n := range exportlist {
+       for _, n := range exportlist[:numglobals] {
                sym := n.Sym
 
                if sym.Exported() {
@@ -323,12 +327,9 @@ func export(out *bufio.Writer, trace bool) int {
                p.tracef("\n")
        }
 
-       // Phase 2: Export objects added to exportlist during phase 1.
-       // Don't use range since exportlist may grow during this phase
-       // and we want to export all remaining objects.
+       // Phase 2: Export objects added to exportlist during phase 0.
        objcount = 0
-       for i := numglobals; exportInlined && i < len(exportlist); i++ {
-               n := exportlist[i]
+       for _, n := range exportlist[numglobals:] {
                sym := n.Sym
 
                // TODO(gri) The rest of this loop body is identical with
@@ -389,7 +390,7 @@ func export(out *bufio.Writer, trace bool) int {
        // Don't use range since funcList may grow.
        objcount = 0
        for i := 0; i < len(p.funcList); i++ {
-               if f := p.funcList[i]; f != nil {
+               if f := p.funcList[i]; f.ExportInline() {
                        // function has inlineable body:
                        // write index and body
                        if p.trace {
@@ -584,24 +585,22 @@ func (p *exporter) obj(sym *types.Sym) {
                        p.qualifiedName(sym)
 
                        sig := asNode(sym.Def).Type
-                       inlineable := isInlineable(asNode(sym.Def))
-
-                       p.paramList(sig.Params(), inlineable)
-                       p.paramList(sig.Results(), inlineable)
-
-                       var f *Func
-                       if inlineable && asNode(sym.Def).Func.ExportInline() {
-                               f = asNode(sym.Def).Func
-                               // TODO(gri) re-examine reexportdeplist:
-                               // Because we can trivially export types
-                               // in-place, we don't need to collect types
-                               // inside function bodies in the exportlist.
-                               // With an adjusted reexportdeplist used only
-                               // by the binary exporter, we can also avoid
-                               // the global exportlist.
-                               reexportdeplist(f.Inl)
-                       }
-                       p.funcList = append(p.funcList, f)
+
+                       // Theoretically, we only need numbered
+                       // parameters if we're supplying an inline
+                       // function body. However, it's possible to
+                       // import a function from a package that
+                       // didn't supply the inline body, and then
+                       // another that did. In this case, we would
+                       // need to rename the parameters during
+                       // import, which is a little sketchy.
+                       //
+                       // For simplicity, just always number
+                       // parameters.
+                       p.paramList(sig.Params(), true)
+                       p.paramList(sig.Results(), true)
+
+                       p.funcList = append(p.funcList, asNode(sym.Def).Func)
                } else {
                        // variable
                        p.tag(varTag)
@@ -675,36 +674,6 @@ func fileLine(n *Node) (file string, line int) {
        return
 }
 
-func isInlineable(n *Node) bool {
-       if exportInlined && n != nil {
-               // When lazily typechecking inlined bodies, some
-               // re-exported ones may not have been typechecked yet.
-               // Currently that can leave unresolved ONONAMEs in
-               // import-dot-ed packages in the wrong package.
-               //
-               // TODO(mdempsky): Having the ExportInline check here
-               // instead of the outer if statement means we end up
-               // exporting parameter names even for functions whose
-               // inline body won't be exported by this package. This
-               // is currently necessary because we might first
-               // import a function/method from a package where it
-               // doesn't need to be re-exported, and then from a
-               // package where it does. If this happens, we'll need
-               // the parameter names.
-               //
-               // We could initially do without the parameter names,
-               // and then fill them in when importing the inline
-               // body. But parameter names are attached to the
-               // function type, and modifying types after the fact
-               // is a little sketchy.
-               if Debug_typecheckinl == 0 && n.Func.ExportInline() {
-                       typecheckinl(n)
-               }
-               return true
-       }
-       return false
-}
-
 func (p *exporter) typ(t *types.Type) {
        if t == nil {
                Fatalf("exporter: nil type")
@@ -788,19 +757,15 @@ func (p *exporter) typ(t *types.Type) {
 
                        sig := m.Type
                        mfn := asNode(sig.FuncType().Nname)
-                       inlineable := isInlineable(mfn)
 
-                       p.paramList(sig.Recvs(), inlineable)
-                       p.paramList(sig.Params(), inlineable)
-                       p.paramList(sig.Results(), inlineable)
+                       // See comment in (*exporter).obj about
+                       // numbered parameters.
+                       p.paramList(sig.Recvs(), true)
+                       p.paramList(sig.Params(), true)
+                       p.paramList(sig.Results(), true)
                        p.bool(m.Nointerface()) // record go:nointerface pragma value (see also #16243)
 
-                       var f *Func
-                       if inlineable && mfn.Func.ExportInline() {
-                               f = mfn.Func
-                               reexportdeplist(mfn.Func.Inl)
-                       }
-                       p.funcList = append(p.funcList, f)
+                       p.funcList = append(p.funcList, mfn.Func)
                }
 
                if p.trace && len(methods) > 0 {
index c5d5c52205da7c0ba26c8b517d463ecf5bf5ff54..10ce23b16c021d19949a256ac41774ec4bd09c41 100644 (file)
@@ -53,6 +53,18 @@ func exportsym(n *Node) {
        exportlist = append(exportlist, n)
 }
 
+// reexportsym marks n for reexport.
+func reexportsym(n *Node) {
+       if exportedsym(n.Sym) {
+               return
+       }
+
+       if Debug['E'] != 0 {
+               fmt.Printf("reexport name %v\n", n.Sym)
+       }
+       exportlist = append(exportlist, n)
+}
+
 func exportname(s string) bool {
        if r := s[0]; r < utf8.RuneSelf {
                return 'A' <= r && r <= 'Z'
@@ -96,125 +108,6 @@ func autoexport(n *Node, ctxt Class) {
        }
 }
 
-// Look for anything we need for the inline body
-func reexportdeplist(ll Nodes) {
-       for _, n := range ll.Slice() {
-               reexportdep(n)
-       }
-}
-
-func reexportdep(n *Node) {
-       if n == nil {
-               return
-       }
-
-       //print("reexportdep %+hN\n", n);
-       switch n.Op {
-       case ONAME:
-               switch n.Class() {
-               case PFUNC:
-                       // methods will be printed along with their type
-                       // nodes for T.Method expressions
-                       if n.isMethodExpression() {
-                               break
-                       }
-
-                       // nodes for method calls.
-                       if n.Type == nil || n.IsMethod() {
-                               break
-                       }
-                       fallthrough
-
-               case PEXTERN:
-                       if n.Sym != nil && !exportedsym(n.Sym) {
-                               if Debug['E'] != 0 {
-                                       fmt.Printf("reexport name %v\n", n.Sym)
-                               }
-                               exportlist = append(exportlist, n)
-                       }
-               }
-
-       // Local variables in the bodies need their type.
-       case ODCL:
-               t := n.Left.Type
-
-               if t != types.Types[t.Etype] && t != types.Idealbool && t != types.Idealstring {
-                       if t.IsPtr() {
-                               t = t.Elem()
-                       }
-                       if t != nil && t.Sym != nil && t.Sym.Def != nil && !exportedsym(t.Sym) {
-                               if Debug['E'] != 0 {
-                                       fmt.Printf("reexport type %v from declaration\n", t.Sym)
-                               }
-                               exportlist = append(exportlist, asNode(t.Sym.Def))
-                       }
-               }
-
-       case OLITERAL:
-               t := n.Type
-               if t != types.Types[n.Type.Etype] && t != types.Idealbool && t != types.Idealstring {
-                       if t.IsPtr() {
-                               t = t.Elem()
-                       }
-                       if t != nil && t.Sym != nil && t.Sym.Def != nil && !exportedsym(t.Sym) {
-                               if Debug['E'] != 0 {
-                                       fmt.Printf("reexport literal type %v\n", t.Sym)
-                               }
-                               exportlist = append(exportlist, asNode(t.Sym.Def))
-                       }
-               }
-               fallthrough
-
-       case OTYPE:
-               if n.Sym != nil && n.Sym.Def != nil && !exportedsym(n.Sym) {
-                       if Debug['E'] != 0 {
-                               fmt.Printf("reexport literal/type %v\n", n.Sym)
-                       }
-                       exportlist = append(exportlist, n)
-               }
-
-       // for operations that need a type when rendered, put the type on the export list.
-       case OCONV,
-               OCONVIFACE,
-               OCONVNOP,
-               ORUNESTR,
-               OARRAYBYTESTR,
-               OARRAYRUNESTR,
-               OSTRARRAYBYTE,
-               OSTRARRAYRUNE,
-               ODOTTYPE,
-               ODOTTYPE2,
-               OSTRUCTLIT,
-               OARRAYLIT,
-               OSLICELIT,
-               OPTRLIT,
-               OMAKEMAP,
-               OMAKESLICE,
-               OMAKECHAN:
-               t := n.Type
-
-               switch t.Etype {
-               case TARRAY, TCHAN, TPTR32, TPTR64, TSLICE:
-                       if t.Sym == nil {
-                               t = t.Elem()
-                       }
-               }
-               if t != nil && t.Sym != nil && t.Sym.Def != nil && !exportedsym(t.Sym) {
-                       if Debug['E'] != 0 {
-                               fmt.Printf("reexport type for expression %v\n", t.Sym)
-                       }
-                       exportlist = append(exportlist, asNode(t.Sym.Def))
-               }
-       }
-
-       reexportdep(n.Left)
-       reexportdep(n.Right)
-       reexportdeplist(n.List)
-       reexportdeplist(n.Rlist)
-       reexportdeplist(n.Ninit)
-       reexportdeplist(n.Nbody)
-}
-
 // methodbyname sorts types by symbol name.
 type methodbyname []*types.Field
 
index e9c36de639acf805c4e4685aab42e7e166af6f01..8f4560f698bf905d93a1c0bca31c697150cbbcae 100644 (file)
@@ -200,10 +200,20 @@ func inlFlood(n *Node) {
 
        typecheckinl(n)
 
-       // Recursively flood any functions called by this one.
        inspectList(n.Func.Inl, func(n *Node) bool {
                switch n.Op {
+               case ONAME:
+                       // Mark any referenced global variables or
+                       // functions for reexport. Skip methods,
+                       // because they're reexported alongside their
+                       // receiver type.
+                       if n.Class() == PEXTERN || n.Class() == PFUNC && !n.isMethodExpression() {
+                               reexportsym(n)
+                       }
+
                case OCALLFUNC, OCALLMETH:
+                       // Recursively flood any functions called by
+                       // this one.
                        inlFlood(asNode(n.Left.Type.Nname()))
                }
                return true