The new export format keeps track of all types that are exported.
If a type is seen that was exported before, only a reference to
that type is emitted. The importer maintains a list of all the
seen types and uses that list to resolve type references.
The existing compiler infrastructure's invariants assumes that
only named types are referred to before they are fully set up.
Referring to unnamed incomplete types causes problems. One of
the issues was #15548.
Added a new internal flag 'trackAllTypes' to enable/disable
this type tracking. With this change only named types are
tracked.
Verified that this fix also addresses #15548, even w/o the
prior fix for that issue (in fact that prior fix is turned
off if trackAllTypes is disabled because it's not needed).
The test for #15548 covers also this change.
For #15548.
Change-Id: Id0b3ff983629703d025a442823f99649fd728a56
Reviewed-on: https://go-review.googlesource.com/22839
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
1) Blank parameters cannot be accessed so the package doesn't matter.
Do not export it, and consistently use localpkg when importing a
blank parameter.
2) More accurately replicate fmt.go and parser.go logic when importing
a blank struct field. Blank struct fields get exported without
package qualification.
(This is actually incorrect, even with the old textual export format,
but we will fix that in a separate change. See also issue 15514.)
Fixes#15491.
Change-Id: I7978e8de163eb9965964942aee27f13bf94a7c3c
Reviewed-on: https://go-review.googlesource.com/22714
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
As a nice side-effect, this allows us to
unify several code paths.
The terminology (low, high, max, simple slice expr,
full slice expr) is taken from the spec and
the examples in the spec.
This is a trial run. The plan, probably for Go 1.8,
is to change slice expressions to use Node.List
instead of OKEY, and to do some similar
tree structure changes for other ops.
Passes toolstash -cmp. No performance change.
all.bash passes with GO_GCFLAGS=-newexport.
Updates #15350
Change-Id: Ic1efdc36e79cdb95ae1636e9817a3ac8f83ab1ac
Reviewed-on: https://go-review.googlesource.com/22425
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Exporting filenames as part of the position information can lead
to different object files which breaks tests.
Change-Id: Ia678ab64293ebf04bf83601e6ba72919d05762a4
Reviewed-on: https://go-review.googlesource.com/22385
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Instead of using TARRAY for both arrays and slices, create a new
TSLICE kind to handle slices.
Also, get rid of the "DDDArray" distinction. While kinda ugly, it
seems likely we'll need to defer evaluating the constant bounds
expressions for golang.org/issue/13890.
Passes toolstash/buildall.
Change-Id: I8e45d4900e7df3a04cce59428ec8b38035d3cc3a
Reviewed-on: https://go-review.googlesource.com/22329
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also:
- "rewrite" node Op in exporter for some nodes instead of importer
- more comments
Change-Id: I809e6754d14987b28f1da9379951ffa2e690c2a7
Reviewed-on: https://go-review.googlesource.com/22008
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Instead of indicating with each function signature if it has an inlineable
body, collect all functions in order and export function bodies with function
index in platform-specific section.
Moves this compiler specific information out of the platform-independent
export data section, and removes an int value for all functions w/o body.
Also simplifies the code a bit.
Change-Id: I8b2d7299dbe81f2706be49ecfb9d9f7da85fd854
Reviewed-on: https://go-review.googlesource.com/21939
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Many of Type's fields are etype-specific.
This CL organizes them into their own auxiliary types,
duplicating a few fields as necessary,
and adds an Extra field to hold them.
It also sorts the remaining fields for better struct packing.
It also improves documentation for most fields.
This reduces the size of Type at the cost of some extra allocations.
There's no CPU impact; memory impact below.
It also makes the natural structure of Type clearer.
Passes toolstash -cmp on all architectures.
Ideas for future work in this vein:
(1) Width and Align probably only need to be
stored for Struct and Array types.
The refactoring to accomplish this would hopefully
also eliminate TFUNCARGS and TCHANARGS entirely.
(2) Maplineno is sparsely used and could probably better be
stored in a separate map[*Type]int32, with mapqueue updated
to store both a Node and a line number.
(3) The Printed field may be removable once the old (non-binary)
importer/exported has been removed.
(4) StructType's fields field could be changed from *[]*Field to []*Field,
which would remove a common allocation.
(5) I believe that Type.Nod can be moved to ForwardType. Separate CL.
name old alloc/op new alloc/op delta
Template 57.9MB ± 0% 55.9MB ± 0% -3.43% (p=0.000 n=50+50)
Unicode 38.3MB ± 0% 37.8MB ± 0% -1.39% (p=0.000 n=50+50)
GoTypes 185MB ± 0% 180MB ± 0% -2.56% (p=0.000 n=50+50)
Compiler 824MB ± 0% 806MB ± 0% -2.19% (p=0.000 n=50+50)
name old allocs/op new allocs/op delta
Template 486k ± 0% 497k ± 0% +2.25% (p=0.000 n=50+50)
Unicode 377k ± 0% 379k ± 0% +0.55% (p=0.000 n=50+50)
GoTypes 1.39M ± 0% 1.42M ± 0% +1.63% (p=0.000 n=50+50)
Compiler 5.52M ± 0% 5.57M ± 0% +0.84% (p=0.000 n=47+50)
Change-Id: I828488eeb74902b013d5ae4cf844de0b6c0dfc87
Reviewed-on: https://go-review.googlesource.com/21611
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Briefly document what the importfoo functions do.
Get rid of importsym's unused result parameter.
Get rid of the redundant calls to importsym(s, OTYPE)
after we've already called pkgtype(s).
Passes toolstash -cmp.
Change-Id: I4c057358144044f5356e4dec68907ec85f1fe806
Reviewed-on: https://go-review.googlesource.com/21498
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Completed implementation for exporting inlined functions
using the new binary export format. This change passes
(export GO_GCFLAGS=-newexport; make all.bash) but for
gc's builtin_test.go which we need to adjust before enabling
this code by default.
For a high-level description of the export format see the
comment at the top of bexport.go.
Major changes:
1) The export format for the platform independent export data
changed: When we export inlined function bodies, additional
objects (other functions, types, etc.) that are referred to
by the function bodies will need to be exported. While this
doesn't affect the platform-independent portion directly, it
adds more objects to the exportlist while we are exporting.
Instead of trying to sort the objects into groups, just export
objects as they appear in the export list. This is slightly
less compact (one extra byte per object), but it is simpler
and much more flexible.
2) The export format contains now three sections: 1) The plat-
form independent objects, 2) the objects pulled in for export
via inlined function bodies, and 3) the inlined function bodies.
3) Completed the exporting and importing code for inlined function
bodies. The format is completely compiler-specific and easily
changeable w/o affecting other tools. There is still quite a
bit of room for denser encoding. This can happen at any time
in the future.
This change contains also the adjustments for go/internal/gcimporter,
necessary because of the export format change 1) mentioned above.
For #13241.
Change-Id: I86bca0bd984b12ccf13d0d30892e6e25f6d04ed5
Reviewed-on: https://go-review.googlesource.com/21172
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Replace isideal(t) with t.IsUntyped().
Replace Istype(t, k) with t.IsKind(k).
Replace isnilinter(t) with t.IsEmptyInterface().
Also replace a lot of t.IsKind(TFOO) with t.IsFoo().
Replacements prepared mechanically with gofmt -w -r.
Passes toolstash -cmp.
Change-Id: Iba48058f3cc863e15af14277b5ff5e729e67e043
Reviewed-on: https://go-review.googlesource.com/21424
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
This removes all access to Type.Bound
from outside type.go.
Update sinit to make a new type rather than
copy and mutate.
Update bimport to create a new slice type
instead of mutating TDDDFIELD.
These are rare, so the extra allocs are nominal.
I’m not happy about having a setter,
but it appears the most practical route
forward at the moment, and it only has a few uses.
Passes toolstash -cmp.
Change-Id: I174f07c8f336afc656904bde4bdbde4f3ef0db96
Reviewed-on: https://go-review.googlesource.com/21423
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Add a constant for the magic -1 for slice bounds.
Use it.
Enforce more aggressively that bounds must be
slice, ddd, or non-negative.
Remove ad hoc check in plive.go.
Check bounds before constructing an array type
when typechecking.
All changes are manual.
Passes toolstash -cmp.
Change-Id: I9fd9cc789d7d4b4eea3b30b24037a254d3788add
Reviewed-on: https://go-review.googlesource.com/21348
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Generated by eg, manually fixed up.
I’m not thrilled about having a setter,
but given the variety of contexts in which this
gets fiddled with, it is the cleanest
available alternative.
Change-Id: Ibdf23e638fe0bdabded014c9e59d557fab8c955f
Reviewed-on: https://go-review.googlesource.com/21341
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This allows us to get rid of Isptr and Issigned. Still some code to
clean up for Isint, Isfloat, and Iscomplex.
CL produced mechanically using gofmt -w -r.
Passes toolstash -cmp.
Change-Id: If4f807bb7f2b357288d2547be2380eb511875786
Reviewed-on: https://go-review.googlesource.com/21339
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Also give them more idiomatic Go names. Adding godocs is outside the
scope of this CL. (Besides, the method names almost all directly
parallel an underlying math/big.Int or math/big.Float method.)
CL prepared mechanically with sed (for rewriting mpint.go/mpfloat.go)
and gofmt (for rewriting call sites).
Passes toolstash -cmp.
Change-Id: Id76f4aee476ba740f48db33162463e7978c2083d
Reviewed-on: https://go-review.googlesource.com/20909
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
The Node type ODOT and its variants all represent a selector, with a
simple name to the right of the dot. Before this change this was
represented by using an ONAME Node in the Right field. This ONAME node
served no useful purpose. This CL changes these Node types to store the
symbol in the Sym field instead, thus not requiring allocating a Node
for each selector.
When compiling x/tools/go/types this CL eliminates nearly 5000 calls to
newname and reduces the total number of Nodes allocated by about 6.6%.
It seems to cut compilation time by 1 to 2 percent.
Getting this right was somewhat subtle, and I added two dubious changes
to produce the exact same output as before. One is to ishairy in
inl.go: the ONAME node increased the cost of ODOT and friends by 1, and
I retained that, although really ODOT is not more expensive than any
other node. The other is to varexpr in walk.go: because the ONAME in
the Right field of an ODOT has no class, varexpr would always return
false for an ODOT, although in fact for some ODOT's it seemingly ought
to return true; I added an && false for now. I will send separate CLs,
that will break toolstash -cmp, to clean these up.
This CL passes toolstash -cmp.
Change-Id: I4af8a10cc59078c436130ce472f25abc3a9b2f80
Reviewed-on: https://go-review.googlesource.com/20890
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Consider functions with an ODCLCONST for inlining and modify exprfmt to
ignore those nodes when exporting. Don't add symbols to the export list
if there is no definition. This occurs when OLITERAL symbols are looked
up via Pkglookup for non-exported symbols.
Fixes#7655
Change-Id: I1de827850f4c69e58107447314fe7433e378e069
Reviewed-on: https://go-review.googlesource.com/20773
Run-TryBot: Todd Neal <todd@tneal.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Another step towards hooking up exported inlined function bodies.
Change-Id: Ib8094b03ac7970fee0e51b5826b5f8aa232e23fb
Reviewed-on: https://go-review.googlesource.com/20605
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Griesemer <gri@golang.org>
The structpkg global variable was only used to verify internal
consistency when declaring methods during import. Track the
value in the parser and binary importer directly and pass it
to the relevant function as an argument.
Change-Id: I7e5e006f9046d84f9a3959616f073798fda36c97
Reviewed-on: https://go-review.googlesource.com/20606
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Removes an intermediate layer of functions that was clogging up a
corner of the compiler's profile graph.
I can't measure a performance improvement running a large build
like jujud, but the profile reports less total time spent in
gc.(*lexer).getr.
Change-Id: I3000585cfcb0f9729d3a3859e9023690a6528591
Reviewed-on: https://go-review.googlesource.com/20565
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This should is preparatory cleanup to make it easier to use separate
types to represent each kind of Go type, rather than a single omnibus
Type struct with heavily overloaded fields.
Also, add TODO comments marking assignments that change an existing
Type's kind, as they need to be removed before we can factor Type.
Change-Id: If4b551fdea4ae045b10b1a3de2ee98f5cf32a517
Reviewed-on: https://go-review.googlesource.com/20494
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
so the code is more readable.
Also use n[i] = val instead of n = append(n, val),
because this avoids a function call to append.
NOTE: compiles, but I had trouble running toolstash -cmp and need sleep
now.
@Ian this might save you some grunt work :-)
Change-Id: I2a4c70396c58905f7d5aabf83f3020f11dea0e89
Reviewed-on: https://go-review.googlesource.com/20430
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL was automatically generated using a special-purpose AST
rewriting tool, followed by manual editing to put some comments back in
the right places and fix some bad line breaks.
The result is not perfect but it's a big step toward getting back to
sanity, and because it was automatically generated there is a decent
chance that it is correct.
Passes toolstash -cmp.
Update #14473.
Change-Id: I01c09078a6d78e2b008bc304d744b79469a38d3d
Reviewed-on: https://go-review.googlesource.com/20440
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Found by temporarily flipping fields from *NodeList to Nodes and fixing
all the compilation errors. This CL does not actually change any
fields.
Passes toolstash -cmp.
Update #14473.
Change-Id: Ib98fa37e8752f96358224c973a743618a6a0e736
Reviewed-on: https://go-review.googlesource.com/20320
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Also rewrite bexport.go to use nodeSeqIterate.
The new setNodeSeq is a transitional generic function to set either a
NodeList or a slice to either a NodeList or a slice. This should permit
us to flip fields from *NodeList to []*Node, or Nodes, without changing
other code.
Passes toolstash -cmp.
Change-Id: I872cbfe45bc5f432595737c1f6da641c502b1ab6
Reviewed-on: https://go-review.googlesource.com/20194
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Function bodies are not yet hooked up because the node structure is not
100% correct. This commit establishes that we can correctly write bodies
out and read them in again.
- export and import all exported inlined function bodies:
(export GO_GCFLAGS="-newexport"; sh all.bash) working
- inlined functions are not yet hooked up (just dropped on the floor)
- improved tracing output and error messages
- make mkbuiltin.go work for both textual and binary export data
so we can run tests with the new format
Change-Id: I70dc4de419df1b604389c3747041d6dba8730b0b
Reviewed-on: https://go-review.googlesource.com/16284
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The tree's pretty inconsistent about single space vs double space
after a period in documentation. Make it consistently a single space,
per earlier decisions. This means contributors won't be confused by
misleading precedence.
This CL doesn't use go/doc to parse. It only addresses // comments.
It was generated with:
$ perl -i -npe 's,^(\s*// .+[a-z]\.) +([A-Z]),$1 $2,' $(git grep -l -E '^\s*//(.+\.) +([A-Z])')
$ go test go/doc -update
Change-Id: Iccdb99c37c797ef1f804a94b22ba5ee4b500c4f7
Reviewed-on: https://go-review.googlesource.com/20022
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Dave Day <djd@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
By using a Pragma bit set (8 bits) rather than 8 booleans, also
reduce Func type size by 8 bytes (208B -> 200B on 64bit platforms,
116B -> 108B on 32bit platforms).
Change-Id: Ibb7e1f8c418a0b5bc6ff813cbdde7bc6f0013b5a
Reviewed-on: https://go-review.googlesource.com/19966
Reviewed-by: Dave Cheney <dave@cheney.net>
Type Op is enfored now.
Type EType will need further CLs.
Added TODOs where Node.EType is used as a union type.
The TODOs have the format `TODO(marvin): Fix Node.EType union type.`.
Furthermore:
-The flag of Econv function in fmt.go is removed, since unused.
-Some cleaning along the way, e.g. declare vars first when getting initialized.
Passes go build -toolexec 'toolstash -cmp' -a std.
Fixes#11846
Change-Id: I908b955d5a78a195604970983fb9194bd9e9260b
Reviewed-on: https://go-review.googlesource.com/14956
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Marvin Stenger <marvin.stenger94@gmail.com>
Necessary to ensure that subsequent tools can continue to find
then end of the export data section simply by searching for "$$".
Adjusted gcimporter used by go/types accordingly.
Also, fixed a bug in gcimporter related to reading export data
in debug format.
Change-Id: Iaea4ed05edd8a5bab28ebe5b19a4740f5e537d35
Reviewed-on: https://go-review.googlesource.com/16283
Reviewed-by: Chris Manghane <cmang@golang.org>
