From: Matthew Dempsky Date: Sat, 31 Mar 2018 01:58:03 +0000 (-0700) Subject: cmd/compile: simplify reexport logic X-Git-Tag: go1.11beta1~1008 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=bcc8edfd8a6b95865911e7a03a21b40bad0acc1d;p=gostls13.git cmd/compile: simplify reexport logic 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 --- diff --git a/src/cmd/compile/internal/gc/bexport.go b/src/cmd/compile/internal/gc/bexport.go index a1bb7f84d3..3c663d4bca 100644 --- a/src/cmd/compile/internal/gc/bexport.go +++ b/src/cmd/compile/internal/gc/bexport.go @@ -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 { diff --git a/src/cmd/compile/internal/gc/export.go b/src/cmd/compile/internal/gc/export.go index c5d5c52205..10ce23b16c 100644 --- a/src/cmd/compile/internal/gc/export.go +++ b/src/cmd/compile/internal/gc/export.go @@ -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 diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go index e9c36de639..8f4560f698 100644 --- a/src/cmd/compile/internal/gc/inl.go +++ b/src/cmd/compile/internal/gc/inl.go @@ -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