23 Commits

Author SHA1 Message Date
Robert Griesemer
a754d2993d cmd/compile/internal/syntax: don't hardwire path separator in test
Windows uses '\' not '/'.

Fixes #35175.

Change-Id: Ib3d01dcf148fc0675496d5213f5bcc9cf210a6fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/203889
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
2019-10-29 03:53:40 +00:00
Robert Griesemer
0c884d0810 cmd/compile, cmd/compile/internal/syntax: print relative column info
This change enables printing of relative column information if a
prior line directive specified a valid column. If there was no
line directive, or the line directive didn't specify a column
(or the -C flag is specified), no column information is shown in
file positions.

Implementation: Column values (and line values, for that matter)
that are zero are interpreted as "unknown". A line directive that
doesn't specify a column records that as a zero column in the
respective PosBase data structure. When computing relative columns,
a relative value is zero of the base's column value is zero.
When formatting a position, a zero column value is not printed.

To make this work without special cases, the PosBase for a file
is given a concrete (non-0:0) position 1:1 with the PosBase's
line and column also being 1:1. In other words, at the position
1:1 of a file, it's relative positions are starting with 1:1 as
one would expect.

In the package syntax, this requires self-recursive PosBases for
file bases, matching what cmd/internal/src.PosBase was already
doing. In src.PosBase, file and inlining bases also need to be
based at 1:1 to indicate "known" positions.

This change completes the cmd/compiler part of the issue below.

Fixes #22662.

Change-Id: I6c3d2dee26709581fba0d0261b1d12e93f1cba1a
Reviewed-on: https://go-review.googlesource.com/97375
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-02-28 03:51:23 +00:00
Robert Griesemer
6fa6bde924 cmd/compile/internal/syntax: implement //line :line:col handling
For line directives which have a line and a column number,
an omitted filename means that the filename has not changed
(per the issue below).

For line directives w/o a column number, an omitted filename
means the empty filename (to preserve the existing behavior).

For #22662.

Change-Id: I32cd9037550485da5445a34bb104706eccce1df1
Reviewed-on: https://go-review.googlesource.com/96476
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-02-26 18:27:44 +00:00
Robert Griesemer
5c08b9e8bd cmd/compile/internal/syntax: remove dependency on cmd/internal/src
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>
2018-02-26 18:27:15 +00:00
Robert Griesemer
ac45cb9aa0 cmd/compile/internal/syntax: permit /*line file:line:col*/ directives
R=go1.11

This implements parsing of /*line file:line*/ and /*line file:line:col*/
directives and also extends the optional column format to regular //line
directives, per #22662.

For a line directive to be recognized, its comment text must start with
the prefix "line " which is followed by one of the following:

:line
:line:col
filename:line
filename:line:col

with at least one : present. The line and col values must be unsigned
decimal integers; everything before is considered part of the filename.

Valid line directives are:

//line :123
//line :123:8
//line foo.go:123
//line C:foo.go:123	(filename is "C:foo.go")
//line C:foo.go:123:8	(filename is "C:foo.go")
/*line ::123*/		(filename is ":")

No matter the comment format, at the moment all directives act as if
they were in //line comments, and column information is ignored.
To be addressed in subsequent CLs.

For #22662.

Change-Id: I1a2dc54bacc94bc6cdedc5229ee13278971f314e
Reviewed-on: https://go-review.googlesource.com/86037
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-02-12 22:57:52 +00:00
Robert Griesemer
e87f2a1b70 cmd/compile/internal/syntax: remove ParseBytes from API - not needed
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>
2018-02-12 22:57:49 +00:00
griesemer
ca2a886cba cmd/compile: record original and absolute file names for line directives
Also, with this change, error locations don't print absolute positions
in [] brackets following positions relative to line directives. To get
the absolute positions as well, specify the -L flag.

Fixes #22660.

Change-Id: I9ecfa254f053defba9c802222874155fa12fee2c
Reviewed-on: https://go-review.googlesource.com/77090
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2017-11-13 16:47:41 +00:00
David Crawshaw
27e80f7c4d cmd/compile: replace GOROOT in //line directives
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>
2017-09-15 17:18:43 +00:00
Robert Griesemer
b5f81eae17 cmd/compile/internal/syntax: replace inlined statement lists with 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>
2017-03-22 22:37:08 +00:00
Robert Griesemer
e0329248d5 cmd/compile/internal/syntax: add position info for { and } braces
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>
2017-03-22 22:36:37 +00:00
Robert Griesemer
1f9f0ea32b cmd/compile/internal/syntax: start line offset (column) numbers at 1
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>
2017-03-09 23:28:48 +00:00
Robert Griesemer
d390283ff4 cmd/compile/internal/syntax: compiler directives must start at beginning of line
- ignore them, if they don't.
- added tests

Fixes #18393.

Change-Id: I13f87b81ac6b9138ab5031bb3dd6bebc4c548156
Reviewed-on: https://go-review.googlesource.com/37020
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-02-15 06:49:21 +00:00
Robert Griesemer
4808fc4443 [dev.inline] cmd/internal/src: replace src.Pos with syntax.Pos
This replaces the src.Pos LineHist-based position tracking with
the syntax.Pos implementation and updates all uses.

The LineHist table is not used anymore - the respective code is still
there but should be removed eventually. CL forthcoming.

Passes toolstash -cmp when comparing to the master repo (with the
exception of a couple of swapped assembly instructions, likely due
to different instruction scheduling because the line-based sorting
has changed; though this is won't affect correctness).

The sizes of various important compiler data structures have increased
significantly (see the various sizes_test.go files); this is probably
the reason for an increase of compilation times (to be addressed). Here
are the results of compilebench -count 5, run on a "quiet" machine (no
apps running besides a terminal):

name       old time/op     new time/op     delta
Template       256ms ± 1%      280ms ±15%  +9.54%          (p=0.008 n=5+5)
Unicode        132ms ± 1%      132ms ± 1%    ~             (p=0.690 n=5+5)
GoTypes        891ms ± 1%      917ms ± 2%  +2.88%          (p=0.008 n=5+5)
Compiler       3.84s ± 2%      3.99s ± 2%  +3.95%          (p=0.016 n=5+5)
MakeBash       47.1s ± 1%      47.2s ± 2%    ~             (p=0.841 n=5+5)

name       old user-ns/op  new user-ns/op  delta
Template        309M ± 1%       326M ± 2%  +5.18%          (p=0.008 n=5+5)
Unicode         165M ± 1%       168M ± 4%    ~             (p=0.421 n=5+5)
GoTypes        1.14G ± 2%      1.18G ± 1%  +3.47%          (p=0.008 n=5+5)
Compiler       5.00G ± 1%      5.16G ± 1%  +3.12%          (p=0.008 n=5+5)

Change-Id: I241c4246cdff627d7ecb95cac23060b38f9775ec
Reviewed-on: https://go-review.googlesource.com/34273
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-01-09 22:33:23 +00:00
Robert Griesemer
a0c5405c18 [dev.inline] cmd/compile/internal/syntax: add tests for //line directives
Change-Id: I77dc73bfe79e43bbadf85d7eb3c5f8990ec72023
Reviewed-on: https://go-review.googlesource.com/34248
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2016-12-09 23:34:30 +00:00
Robert Griesemer
32bf2829a1 [dev.inline] cmd/compile/internal/syntax: process //line pragmas in scanner
Reviewed in and cherry-picked from https://go-review.googlesource.com/#/c/33764/.

Minor adjustment in noder.go to make merge compile again.

Change-Id: Ib5029b52b59944f207b0f2438c8a5aa576eb25b8
Reviewed-on: https://go-review.googlesource.com/34233
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-12-09 00:43:04 +00:00
Robert Griesemer
8d20b25779 [dev.inline] cmd/compile/internal/syntax: introduce general position info for nodes
Reviewed in and cherry-picked from https://go-review.googlesource.com/#/c/33758/.
Minor adjustments in noder.go to fix merge.

Change-Id: Ibe429e327c7f8554f8ac205c61ce3738013aed98
Reviewed-on: https://go-review.googlesource.com/34231
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2016-12-09 00:42:42 +00:00
Josh Bleecher Snyder
eb8f2a8320 all: fix vet nits
Fixes these complaints from vet:

cmd/compile/internal/gc/noder.go:32: cmd/compile/internal/syntax.Error composite literal uses unkeyed fields
cmd/compile/internal/gc/noder.go:1035: cmd/compile/internal/syntax.Error composite literal uses unkeyed fields
cmd/compile/internal/gc/noder.go:1051: cmd/compile/internal/syntax.Error composite literal uses unkeyed fields
cmd/compile/internal/syntax/parser_test.go:182: possible formatting directive in Error call
net/http/client_test.go:1334: possible formatting directive in Fatal call

Change-Id: I5f90ec30f3c106c7e66c92e2b6f8d3b4874fec66
Reviewed-on: https://go-review.googlesource.com/33133
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-11-11 16:01:59 +00:00
Robert Griesemer
60a9bf9f95 cmd/compile/internal/syntax: fix error handling for Read/Parse calls
- define syntax.Error for cleaner error reporting
- abort parsing after first error if no error handler is installed
- make sure to always report the first error, if any
- document behavior of API calls
- while at it: rename ReadXXX -> ParseXXX (clearer)
- adjust cmd/compile noder.go accordingly

Fixes #17774.

Change-Id: I7893eedea454a64acd753e32f7a8bf811ddbb03c
Reviewed-on: https://go-review.googlesource.com/32950
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2016-11-09 17:48:40 +00:00
Robert Griesemer
012fec3821 cmd/compile/internal/syntax: don't depend on hardwired $GOROOT name
Fixes #17697.

Change-Id: I3c47e139b09bde81566e29a1ac0ec8c58d55a34a
Reviewed-on: https://go-review.googlesource.com/32539
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-11-01 22:26:11 +00:00
Josh Bleecher Snyder
3f1ed24551 cmd/compile/internal/syntax: avoid deadlock
When the err from ReadFile is non-nil, we call t.Fatal(err).
Switch t.Fatal to t.Error + return.
ensure that close(results) happens on that code path as well.

Updates #17697.

Change-Id: Ifaacf27a76c175446d642086ff32f4386428080d
Reviewed-on: https://go-review.googlesource.com/32486
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-11-01 20:19:01 +00:00
Robert Griesemer
7a26d9fced cmd/compile/internal/syntax: don't panic if no error handler is provided
If no error handler is provided, terminate parsing with first error
and report that error.

Fixes #17697.

Change-Id: I9070faf7239bd53725de141507912b92ded3474b
Reviewed-on: https://go-review.googlesource.com/32456
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2016-11-01 05:18:47 +00:00
Matthew Dempsky
ee161e8591 cmd/compile: handle pragmas immediately with -newparser=1
Instead of saving all pragmas and processing them after parsing is
finished, process them immediately during scanning like the current
lexer does.

This is a bit unfortunate because it means we can't use
syntax.ParseFile to concurrently parse files yet, but it fixes how we
report syntax errors in the presence of //line pragmas.

While here, add a bunch more gcCompat entries to syntax/parser.go to
get "go build -toolexec='toolstash -cmp' std cmd" passing. There are
still a few remaining cases only triggered building unit tests, but
this seems like a nice checkpoint.

Change-Id: Iaf3bbcf2849857a460496f31eea228e0c585ce13
Reviewed-on: https://go-review.googlesource.com/28226
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2016-08-31 19:49:53 +00:00
Robert Griesemer
c8683ff797 cmd/compile/internal/syntax: fast Go syntax trees, initial commit.
Syntax tree nodes, scanner, parser, basic printers.

Builds syntax trees for entire Go std lib at a rate of ~1.8M lines/s
in warmed up state (MacMini, 2.3 GHz Intel Core i7, 8GB RAM):

$ go test -run StdLib -fast
parsed 1074617 lines (2832 files) in 579.66364ms (1853863 lines/s)
allocated 282.212Mb (486.854Mb/s)
PASS

Change-Id: Ie26d9a7bf4e5ff07457aedfcc9b89f0eba72ae3f
Reviewed-on: https://go-review.googlesource.com/27195
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
2016-08-18 21:33:38 +00:00