| Age | Commit message (Collapse) | Author |
|---|
|
For dependency reasons, the data structure implementing source
positions in the compiler is in cmd/internal/src. It contains
highly compiler specific details (e.g. inlining index).
This change introduces a parallel but simpler position
representation, defined in the syntax package, which removes
that package's dependency on cmd/internal/src, and also removes
the need to deal with certain filename-specific operations
(defined by the needs of the compiler) in the syntax package.
As a result, the syntax package becomes again a compiler-
independent, stand-alone package that at some point might
replace (or augment) the existing top-level go/* syntax-related
packages.
Additionally, line directives that update column numbers
are now correctly tracked through the syntax package, with
additional tests added. (The respective changes also need to
be made in cmd/internal/src; i.e., the compiler accepts but
still ignores column numbers in line directives.)
This change comes at the cost of a new position translation
step, but that step is cheap because it only needs to do real
work if the position base changed (i.e., if there is a new file,
or new line directive).
There is no noticeable impact on overall compiler performance
measured with `compilebench -count 5 -alloc`:
name old time/op new time/op delta
Template 220ms ± 8% 228ms ±18% ~ (p=0.548 n=5+5)
Unicode 119ms ±11% 113ms ± 5% ~ (p=0.056 n=5+5)
GoTypes 684ms ± 6% 677ms ± 3% ~ (p=0.841 n=5+5)
Compiler 3.19s ± 7% 3.01s ± 1% ~ (p=0.095 n=5+5)
SSA 7.92s ± 8% 7.79s ± 1% ~ (p=0.690 n=5+5)
Flate 141ms ± 7% 139ms ± 4% ~ (p=0.548 n=5+5)
GoParser 173ms ±12% 171ms ± 4% ~ (p=1.000 n=5+5)
Reflect 417ms ± 5% 411ms ± 3% ~ (p=0.548 n=5+5)
Tar 205ms ± 5% 198ms ± 2% ~ (p=0.690 n=5+5)
XML 232ms ± 4% 229ms ± 4% ~ (p=0.690 n=5+5)
StdCmd 28.7s ± 5% 28.2s ± 2% ~ (p=0.421 n=5+5)
name old user-time/op new user-time/op delta
Template 269ms ± 4% 265ms ± 5% ~ (p=0.421 n=5+5)
Unicode 153ms ± 7% 149ms ± 3% ~ (p=0.841 n=5+5)
GoTypes 850ms ± 7% 862ms ± 4% ~ (p=0.841 n=5+5)
Compiler 4.01s ± 5% 3.86s ± 0% ~ (p=0.190 n=5+4)
SSA 10.9s ± 4% 10.8s ± 2% ~ (p=0.548 n=5+5)
Flate 166ms ± 7% 167ms ± 6% ~ (p=1.000 n=5+5)
GoParser 204ms ± 8% 206ms ± 7% ~ (p=0.841 n=5+5)
Reflect 514ms ± 5% 508ms ± 4% ~ (p=0.548 n=5+5)
Tar 245ms ± 6% 244ms ± 3% ~ (p=0.690 n=5+5)
XML 280ms ± 4% 278ms ± 4% ~ (p=0.841 n=5+5)
name old alloc/op new alloc/op delta
Template 37.9MB ± 0% 37.9MB ± 0% ~ (p=0.841 n=5+5)
Unicode 28.8MB ± 0% 28.8MB ± 0% ~ (p=0.841 n=5+5)
GoTypes 113MB ± 0% 113MB ± 0% ~ (p=0.151 n=5+5)
Compiler 468MB ± 0% 468MB ± 0% -0.01% (p=0.032 n=5+5)
SSA 1.50GB ± 0% 1.50GB ± 0% ~ (p=0.548 n=5+5)
Flate 24.4MB ± 0% 24.4MB ± 0% ~ (p=1.000 n=5+5)
GoParser 30.7MB ± 0% 30.7MB ± 0% ~ (p=1.000 n=5+5)
Reflect 76.5MB ± 0% 76.5MB ± 0% ~ (p=0.548 n=5+5)
Tar 38.9MB ± 0% 38.9MB ± 0% ~ (p=0.222 n=5+5)
XML 41.6MB ± 0% 41.6MB ± 0% ~ (p=0.548 n=5+5)
name old allocs/op new allocs/op delta
Template 382k ± 0% 382k ± 0% +0.01% (p=0.008 n=5+5)
Unicode 343k ± 0% 343k ± 0% ~ (p=0.841 n=5+5)
GoTypes 1.19M ± 0% 1.19M ± 0% +0.01% (p=0.008 n=5+5)
Compiler 4.53M ± 0% 4.53M ± 0% +0.03% (p=0.008 n=5+5)
SSA 12.4M ± 0% 12.4M ± 0% +0.00% (p=0.008 n=5+5)
Flate 235k ± 0% 235k ± 0% ~ (p=0.079 n=5+5)
GoParser 318k ± 0% 318k ± 0% ~ (p=0.730 n=5+5)
Reflect 978k ± 0% 978k ± 0% ~ (p=1.000 n=5+5)
Tar 393k ± 0% 393k ± 0% ~ (p=0.056 n=5+5)
XML 405k ± 0% 405k ± 0% ~ (p=0.548 n=5+5)
name old text-bytes new text-bytes delta
HelloSize 672kB ± 0% 672kB ± 0% ~ (all equal)
CmdGoSize 7.12MB ± 0% 7.12MB ± 0% ~ (all equal)
name old data-bytes new data-bytes delta
HelloSize 133kB ± 0% 133kB ± 0% ~ (all equal)
CmdGoSize 390kB ± 0% 390kB ± 0% ~ (all equal)
name old exe-bytes new exe-bytes delta
HelloSize 1.07MB ± 0% 1.07MB ± 0% ~ (all equal)
CmdGoSize 11.2MB ± 0% 11.2MB ± 0% ~ (all equal)
Passes toolstash compare.
For #22662.
Change-Id: I19edb53dd9675af57f7122cb7dba2a6d8bdcc3da
Reviewed-on: https://go-review.googlesource.com/94515
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
|
R=go1.11
Also: Minor updates to syntax.Parse doc string.
Change-Id: I649965be9670a2f1c3de2cdb350634ed21e36ad9
Reviewed-on: https://go-review.googlesource.com/85663
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
|
This reverts https://golang.org/cl/65930.
Fixes #22148
Change-Id: Ie0712621ed89c43bef94417fc32de9af77607760
Reviewed-on: https://go-review.googlesource.com/68430
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
strings.IndexByte was introduced in go1.2 and it can be used
effectively wherever the second argument to strings.Index is
exactly one byte long.
This avoids generating unnecessary string symbols and saves
a few calls to strings.Index.
Change-Id: I1ab5edb7c4ee9058084cfa57cbcc267c2597e793
Reviewed-on: https://go-review.googlesource.com/65930
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
The compiler replaces any path of the form /path/to/goroot/src/net/port.go
with GOROOT/src/net/port.go so that the same object file is
produced if the GOROOT is moved. It was skipping this transformation
for any absolute path into the GOROOT that came from //line directives,
such as those generated by cmd/cgo.
Fixes #21373
Fixes #21720
Fixes #21825
Change-Id: I2784c701b4391cfb92e23efbcb091a84957d61dd
Reviewed-on: https://go-review.googlesource.com/63693
Run-TryBot: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
|
Change-Id: I1f1cfb161640eb8756fb1a283892d06b30b7a8fa
Reviewed-on: https://go-review.googlesource.com/39356
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
syntax.BlockStmt
This simplifies the code and removes a premature optimization.
It increases the amount of allocated syntax.Node space by ~0.4%
for parsing all of std lib, which is negligible.
Before the change (best of 5 runs):
$ go test -run StdLib -fast
parsed 1517022 lines (3394 files) in 793.487886ms (1911840 lines/s)
allocated 387.086Mb (267B/line, 487.828Mb/s)
After the change (best of 5 runs):
$ go test -run StdLib -fast
parsed 1516911 lines (3392 files) in 805.028655ms (1884294 lines/s)
allocated 388.466Mb (268B/line, 482.549Mb/s)
Change-Id: Id19d6210fdc62393862ba3b04913352d95c599be
Reviewed-on: https://go-review.googlesource.com/38439
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
|
This change adds position information for { and } braces in the
source. There's a 1.9% increase in memory use for syntax.Nodes,
which is negligible relative to overall compiler memory consumption.
Parsing the std library (using syntax package only) and memory
consumption before this change (fastest of 5 runs):
$ go test -run StdLib -fast
parsed 1516827 lines (3392 files) in 780.612335ms (1943124 lines/s)
allocated 379.903Mb (486.673Mb/s)
After this change (fastest of 5 runs):
$ go test -run StdLib -fast
parsed 1517022 lines (3394 files) in 793.487886ms (1911840 lines/s)
allocated 387.086Mb (267B/line, 487.828Mb/s)
While not an exact apples-to-apples comparison (the syntax package
has changed and is also parsed), the overall impact is small.
Also: Small improvements to nodes_test.go.
Change-Id: Ib8a7f90bbe79de33d83684e33b1bf8dbc32e644a
Reviewed-on: https://go-review.googlesource.com/38435
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
|
We could leave it alone and fix line offset (column) numbers when
reporting errors, but that is likely to cause confusion (internal
numbers don't match reported numbers). Instead, switch to default
numbering starting at 1.
For package syntax-internal use only, introduced constants defining
the line and column bases, and use them throughout the code and its
tests. It is possible to change these constants and package syntax
will continue to work. But changing them is going to break any client
that makes explicit assumptions about line and column numbers (which
is "all of them").
Change-Id: Ia3d136a8ec8d9372ed9c05ca47d3dff222cf030e
Reviewed-on: https://go-review.googlesource.com/37996
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
|
Until now, the parser set the position for each Node to the position of
the first token belonging to that node. For compatibility with the now
defunct gc parser, in many places that position information was modified
when the gcCompat flag was set (which it was, by default). Furthermore,
in some places, position information was not set at all.
This change removes the gcCompat flag and all associated code, and sets
position information for all nodes in a more principled way, as proposed
by mdempsky (see #16943 for details). Specifically, the position of a
node may not be at the very beginning of the respective production. For
instance for an Operation `a + b`, the position associated with the node
is the position of the `+`. Thus, for `a + b + c` we now get different
positions for the two additions.
This change does not pass toolstash -cmp because position information
recorded in export data and pcline tables is different. There are no
other functional changes.
Added test suite testing the position of all nodes.
Fixes #16943.
Change-Id: I3fc02bf096bc3b3d7d2fa655dfd4714a1a0eb90c
Reviewed-on: https://go-review.googlesource.com/37017
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
