The existing code was somewhat convoluted and made several assumptions
about the encoding of position bases:
1) The position's base for a file contained a position whose base
pointed to itself (which is true but an implementation detail
of src.Pos).
2) Updating the position base for a line directive required finding
the base of the most recent's base position.
This change simply stores the file's position base and keeps using it
directly for each line directive (instead of getting it from the most
recently updated base).
Change-Id: I4d80da513bededb636eab0ce53257fda73f0dbc0
Reviewed-on: https://go-review.googlesource.com/95736
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Assume that an expression that is not a function call in a defer/go
statement is indeed a function that is just missing its invocation.
Report the error but continue with a sane syntax tree.
Fixes#23586.
Change-Id: Ib45ebac57c83b3e39ae4a1b137ffa291dec5b50d
Reviewed-on: https://go-review.googlesource.com/94156
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Also, remove parser.error method (in favor of parser.errorAt) as it's only
used twice.
This is a purely cosmetic change.
Change-Id: Idb3b8b50f1c2e4d10de2ffb1c1184ceba8f7de8a
Reviewed-on: https://go-review.googlesource.com/94030
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
R=go1.11
In order to collect comments in the AST and for error testing purposes,
the scanner needs to not only recognize and skip comments, but also be
able to report them if so desired. This change adds a mode flag to the
scanner's init function which controls the scanner behavior around
comments.
In the common case where comments are not needed, there must be no
significant overhead. Thus, comments are reported via a handler upcall
rather than being returned as a _Comment token (which the parser would
have to filter out with every scanner.next() call).
Because the handlers for error messages, directives, and comments all
look the same (they take a position and text), and because directives
look like comments, and errors never start with a '/', this change
simplifies the scanner's init call to only take one (error) handler
instead of 2 or 3 different handlers with identical signature. It is
trivial in the handler to determine if we have an error, directive,
or general comment.
Finally, because directives are comments, when reporting directives
the full comment text is returned now rather than just the directive
text. This simplifies the implementation and makes the scanner API
more regular. Furthermore, it provides important information about
the comment style used by a directive, which may matter eventually
when we fully implement /*line file:line:col*/ directives.
Change-Id: I2adbfcebecd615e4237ed3a832b6ceb9518bf09c
Reviewed-on: https://go-review.googlesource.com/88215
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
R=go1.11
A common error is to write '=' instead of '==' inside the condition
of a simple 'if' statement:
if x = 0 { ... }
Highlight the fact that we have an assignment in the error message
to prevent further confusion.
Fixes#23385.
Change-Id: I1552050fd6da927bd12a1be0977bd2e98eca5885
Reviewed-on: https://go-review.googlesource.com/87316
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
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>
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>
Be more pessimistic when parsing if/switch/for headers for better error
messages when things go wrong.
Fixes#22581.
Change-Id: Ibb99925291ff53f35021bc0a59a4c9a7f695a194
Reviewed-on: https://go-review.googlesource.com/76290
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
When an opening "{" of a block is missing and after advancing we
find a closing "}", it's likely better to assume the end of the
block. Fixed and removed TODO.
Change-Id: I20c9b4ecca798933a7cd4cbf21185bd4ca04f5f7
Reviewed-on: https://go-review.googlesource.com/71291
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
- checking for the correct closing token leads to slightly better
behavior for some randomly bogus programs
- removed `switch` in favor of an `if` statement
Follow-up on https://go-review.googlesource.com/c/go/+/71250.
Change-Id: I47f6c47b43baf790907f55ed97a947661687a9db
Reviewed-on: https://go-review.googlesource.com/71252
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Fine-tune skipping of tokens after missing closing parentheses in lists.
Fixes#22164.
Change-Id: I575d86e21048cd40340a2c08399e8b0deec337cf
Reviewed-on: https://go-review.googlesource.com/71250
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Instead of repeating the same list parsing pattern for parenthesized
of braced comma or semicolon-separated lists, introduce a single list
parsing function that can be parametrized and which takes a closure
to parse list elements.
This ensures the same error handling and recovery logic is used across
all lists and simplifies the code.
No semantic change.
Change-Id: Ia738d354d6c2e0c3d84a5f1c7269a6eb95685edc
Reviewed-on: https://go-review.googlesource.com/70492
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
No semantic change. Move functionality not related to argument
out of the argument parsing function, and thus match parameter
parsing. Also, use a better function name.
Change-Id: Ic550875251d64e6fe1ebf91c11d33a9e4aec9fdd
Reviewed-on: https://go-review.googlesource.com/70491
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This re-enables functionality that inadvertently was disabled in the
(long) past.
Also, don't perform branch checks if we had errors in a function
to avoid spurious errors or (worst-case) crashes.
Slightly modified test/fixedbugs/issue14006.go to make sure the
test still reports invalid label errors (the surrounding function
must be syntactically correct).
Change-Id: Id5642930877d7cf3400649094ec75c753b5084b7
Reviewed-on: https://go-review.googlesource.com/69770
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@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#21373Fixes#21720Fixes#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>
Fixes crash when printing a related error message later on.
Fixes#20789.
Change-Id: I6d2c35aafcaeda26a211fc6c8b7dfe4a095a3efe
Reviewed-on: https://go-review.googlesource.com/46713
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Instead of a separate check control flow pass (checkcfg.go)
operating on nodes, perform this check at parse time on the
new syntax tree. Permits this check to be done concurrently,
and doesn't depend on the specifics of the symbol's dclstack
implementation anymore. The remaining dclstack uses will be
removed in a follow-up change.
- added CheckBranches Mode flag (so we can turn off the check
if we only care about syntactic correctness, e.g. for tests)
- adjusted test/goto.go error messages: the new branches
checker only reports if a goto jumps into a block, but not
which block (we may want to improve this again, eventually)
- also, the new branches checker reports one variable that
is being jumped over by a goto, but it may not be the first
one declared (this is fine either way)
- the new branches checker reports additional errors for
fixedbugs/issue14006.go (not crucial to avoid those errors)
- the new branches checker now correctly reports only
variable declarations being jumped over, rather than
all declarations (issue 8042). Added respective tests.
Fixes#8042.
Change-Id: I53b6e1bda189748e1e1fb5b765a8a64337c27d40
Reviewed-on: https://go-review.googlesource.com/39998
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Now that we have consistent use of xOrNil parse methods, we don't
need a special missing_statement singleton to distinguish between
missing actually statements and other errors (which have returned
nil before).
For #19663.
Change-Id: I8364f1441bdf8dd966bcd6d8219b2a42d6b88abd
Reviewed-on: https://go-review.googlesource.com/38656
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
- parser creates sensible nodes in case of syntax errors instead of nil
- a new BadExpr node is used in places where we can't do better
- fixed error message for incorrect type switch guard
- minor cleanups
Fixes#19663.
Change-Id: I028394c6db9cba7371f0e417ebf93f594659786a
Reviewed-on: https://go-review.googlesource.com/38653
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Avoid construction of incorrect syntax trees in presence of errors.
For #19663.
Change-Id: I43025a3cf0fe02cae9a57e8bb9489b5f628c3fd7
Reviewed-on: https://go-review.googlesource.com/38604
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
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>
Report syntax error that was missed when moving to new parser.
Fixes#19610.
Change-Id: Ie5625f907a84089dc56fcccfd4f24df546042783
Reviewed-on: https://go-review.googlesource.com/38375
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The new syntax tree introduced with 1.8 represents send statements
(ch <- x) as statements; the old syntax tree represented them as
expressions (and parsed them as such) but complained if they were
used in expression context. As a consequence, some of the errors
that in the past were of the form "ch <- x used as value" now look
like "unexpected <- ..." because a "<-" is not valid according to
Go syntax in those situations. Accept the new error message.
Also: Fine-tune handling of misformed for loop headers.
Also: Minor cleanups/better comments.
Fixes#17590.
Change-Id: Ia541dea1f2f015c1b21f5b3ae44aacdec60a8aba
Reviewed-on: https://go-review.googlesource.com/37386
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>
Use distinction between explicit and automatically inserted semicolons
to provide a better error message if the condition in an 'if' statement
is missing.
For #18747.
Change-Id: Iac167ae4e5ad53d2dc73f746b4dee9912434bb59
Reviewed-on: https://go-review.googlesource.com/36930
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Added missing nil-check. We will get rid of the gcCompat corrections
shortly but it's still worthwhile having the new test case added.
Fixes#19056.
Change-Id: I35bd938a4d789058da15724e34c05e5e631ecad0
Reviewed-on: https://go-review.googlesource.com/36908
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The gcCompat mode was introduced to match the new parser's node position
setup exactly with the positions used by the original parser. Some of the
gcCompat adjustments were required to satisfy syntax error test cases,
and the rest were required to make toolstash cmp pass.
This change removes the former gcCompat adjustments and instead adjusts
the respective test cases as necessary. In some cases this makes the error
lines consistent with the ones reported by gccgo.
Where it has changed, the position associated with a given syntactic construct
is the position (line/col number) of the left-most token belonging to the
construct.
Change-Id: I5b60c00c5999a895c4d6d6e9b383c6405ccf725c
Reviewed-on: https://go-review.googlesource.com/36695
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Starting the error message with "expecting" rather than "missing"
causes the syntax error mechanism to add additional helpful info
(it recognizes "expecting" but not "missing").
Fixes#17328.
Change-Id: I8482ca5e5a6a6b22e0ed0d831b7328e264156334
Reviewed-on: https://go-review.googlesource.com/36637
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Towards better syntax error messages: With this change, the parser knows whether
a semicolon was an actual ';' in the source, or whether it was an automatically
inserted semicolon as result of a '\n' or EOF. Using this information in error
messages makes them more understandable.
For #17328.
Change-Id: I8cd9accee8681b62569d0ecef922d38682b401eb
Reviewed-on: https://go-review.googlesource.com/36636
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This is a follow-up on https://go-review.googlesource.com/36470
and leads to a more stable fix. The above CL relied on filtering
of multiple errors on the same line to avoid more than one error
for an `if` statement of the form `if a := 10 {}`. This CL avoids
the secondary error ("missing condition in if statement") in the
first place.
For #18915.
Change-Id: I8517f485cc2305965276c17d8f8797d61ef9e999
Reviewed-on: https://go-review.googlesource.com/36479
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
For code such as
if a := 10 { ...
the 1.7 compiler reported
a := 10 used as value
while the 1.8 compiler reported
invalid condition, tag, or type switch guard
Changed the error message to match the 1.7 compiler.
Fixes#18915.
Change-Id: I01308862e461922e717f9f8295a9db53d5a914eb
Reviewed-on: https://go-review.googlesource.com/36470
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
- make the scanner unconditionally gc compatible
- consistently use "invalid" instead "illegal" in errors
Reviewed in and cherry-picked from https://go-review.googlesource.com/#/c/33896/.
Change-Id: I4c4253e7392f3311b0d838bbe503576c9469b203
Reviewed-on: https://go-review.googlesource.com/34237
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
- use syntax.Pos in syntax.Error (rather than line, col)
- use syntax.Pos in syntax.PragmaHandler (rather than just line)
- update uses
- better documentation in various places
Also:
- make Pos methods use Pos receiver (rather than *Pos)
Reviewed in and cherry-picked from https://go-review.googlesource.com/#/c/33891/.
With minor adjustments to noder.go to make merge compile.
Change-Id: I5507cea6c2be46a7677087c1aeb69382d31033eb
Reviewed-on: https://go-review.googlesource.com/34236
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed in and cherry-picked from https://go-review.googlesource.com/#/c/33873/.
- simplify error handling in source.go
(move handling of first error into parser, where it belongs)
- clean up error handling in scanner.go
- move pragma and position base handling from scanner
to parser where it belongs
- have separate error methods in parser to avoid confusion
with handlers from scanner.go and source.go
- (source.go) and (scanner.go, source.go, tokens.go)
may be stand-alone packages if so desired, which means
these files are now less entangled and easier to maintain
Change-Id: I81510fc7ef943b78eaa49092c0eab2075a05878c
Reviewed-on: https://go-review.googlesource.com/34235
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
