Lines with single cells prompt a flush. Unfortunately, a call to
Writer.Flush also means two defers, which is an expensive operation to
do if many lines consist of single cells.
This is common when formatting code with aligned comments. Most lines
aren't going to have any comments at all, so the performance hit is
going to be noticeable.
The Write method already has a "defer handlePanic" of its own, so we
don't need to worry about panics leaking out. The error will now mention
"Write" instead of "Flush" if a panic is encountered during that nested
flush, but arguably that's a good thing; the user called Write, not
Flush.
For the reset call, add a non-deferred call as part of flushNoDefers, as
that's still necessary. Otherwise, the exported Flush method still does
a "defer b.reset".
The current tabwriter benchmarks are unaffected, since they don't
contain many single-cell lines, and because lines are written one at a
time. For that reason, we add a benchmark which has both of these
characteristics.
name old time/op new time/op delta
Code-8 2.72µs ± 0% 1.77µs ± 0% -34.88% (p=0.000 n=6+5)
name old alloc/op new alloc/op delta
Code-8 648B ± 0% 648B ± 0% ~ (all equal)
name old allocs/op new allocs/op delta
Code-8 13.0 ± 0% 13.0 ± 0% ~ (all equal)
Perhaps unsurprisingly, go/printer also gets a bit faster, as it too
buffers its output before writing it to tabwriter.
name old time/op new time/op delta
Print-8 6.53ms ± 0% 6.39ms ± 0% -2.22% (p=0.008 n=5+5)
Change-Id: Ie01fea5ced43886a9eb796cb1e6c810f7a810853
Reviewed-on: https://go-review.googlesource.com/c/go/+/166797
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
If a for loop has a simple condition and begins with a simple
"if x { break; }"; we can simply add "!x" to the loop's condition.
While at it, simplify a few assignments to use the common pattern
"x := staticDefault; if cond { x = otherValue(); }".
Finally, simplify a couple of var declarations.
Change-Id: I413982c6abd32905adc85a9a666cb3819139c19f
Reviewed-on: https://go-review.googlesource.com/c/go/+/165342
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trying to call a method on a nil interface is a panic in Go. For
example:
var stringer fmt.Stringer
println(stringer.String()) // nil pointer dereference
In https://golang.org/cl/143097 we started recovering panics encountered
during function and method calls. However, we didn't handle this case,
as text/template panics before evalCall is ever run.
In particular, reflect's MethodByName will panic if the receiver is of
interface kind and nil:
panic: reflect: Method on nil interface value
Simply add a check for that edge case, and have Template.Execute return
a helpful error. Note that Execute shouldn't just error if the interface
contains a typed nil, since we're able to find a method to call in that
case.
Finally, add regression tests for both the nil and typed nil interface
cases.
Fixes#30143.
Change-Id: Iffb21b40e14ba5fea0fcdd179cd80d1f23cabbab
Reviewed-on: https://go-review.googlesource.com/c/161761
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
If we're accessing a field on a nil struct pointer, and that field is
present in the type, we should print a "nil pointer evaluating X.Y" error
instead of the broader "can't evaluate field Y in X". The latter error
should still be used for the cases where the field is simply missing.
While at it, remove the isNil checks in the struct and map cases. The
indirect func will only return a true isNil when returning a pointer or
interface reflect.Value, so it's impossible for either of these checks
to be useful.
Finally, extend the test suite to test a handful of these edge cases,
including the one shown in the original issue.
Fixes#29137.
Change-Id: I53408ced8a7b53807a0a8461b6baef1cd01d25ae
Reviewed-on: https://go-review.googlesource.com/c/153341
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
This CL updates text/template's scanner to accept the
new number syntaxes:
- Hexadecimal floating-point values.
- Digit-separating underscores.
- Leading 0b and 0o prefixes.
See golang.org/design/19308-number-literals for background.
For #12711.
For #19308.
For #28493.
For #29008.
Change-Id: I68c16ea35c3f506701063781388de72bafee6b8d
Reviewed-on: https://go-review.googlesource.com/c/160248
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This is a follow-up on https://golang.org/cl/161199 which introduced
the new Go 2 number literals to text/scanner.
That change introduced a bug by allowing decimal and hexadecimal floats
to be consumed even if the scanner was not configured to accept floats.
This CL changes the code to not consume a radix dot '.' or exponent
unless the scanner is configured to accept floats.
This CL also introduces a new mode "AllowNumberbars" which controls
whether underbars '_' are permitted as digit separators in numbers
or not.
There is a possibility that we may need to refine text/scanner
further (e.g., the Float mode now includes hexadecimal floats
which it didn't recognize before). We're very early in the cycle,
so let's see how it goes.
RELNOTE=yes
Updates #12711.
Updates #19308.
Updates #28493.
Updates #29008.
Fixes#30320.
Change-Id: I6481d314f0384e09ef6803ffad38dc529b1e89a3
Reviewed-on: https://go-review.googlesource.com/c/163079
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This CL introduces text/scanner support for the new binary and octal integer
literals, hexadecimal floats, and digit separators for all number literals.
The new code is closely mirroring the respective code for number literals in
cmd/compile/internal/syntax/scanner.go.
Uniformly use the term "invalid" rather than "illegal" in error messages
to match the respective error messages in the other scanners directly.
R=Go1.13
Updates #12711.
Updates #19308.
Updates #28493.
Updates #29008.
Change-Id: I2f291de13ba5afc0e530cd8326e6bf4c3858ebac
Reviewed-on: https://go-review.googlesource.com/c/161199
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Make sure Scanner.tokEnd is set before we call Scanner.Error
and update documentation accordingly.
(Until now tokEnd was only set before returning from Scan,
so a call to TokenText during error handling may have crashed.)
While at it, tighten a check in Scanner.TokenText to ensure
Scanner.tokEnd >= Scanner.tokPos if we have a token.
Also, silence error messages to Stderr in unrelated TestIllegalExponent.
Fixes#29723.
Change-Id: Ia97beeae91eaf9e0ed3dada0a806f1f7122461cc
Reviewed-on: https://go-review.googlesource.com/c/157819
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
The pipeline parsing code was unnecessarily complex. It used a for loop
with a trailing break, a complex switch, and up to seven levels of
indentation.
Instead, drop the loop in favor of a single named goto with a comment,
and flatten out the complex switch to be easier to follow. Two lines of
code are now duplicated, but they're simple and only three lines apart.
While at it, move the pipe initialization further up to remove the need
for three variables.
Change-Id: I07b29de195f4000336219aadeadeacaaa4285c58
Reviewed-on: https://go-review.googlesource.com/c/145285
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The package used to accept invalid range pipelines, such as:
{{range $k, .}}
{{range $k, 123 := .}}
This is because the logic that allowed a range pipeline to declare
multiple variables was broken. When encountering a single comma inside a
range pipeline, it would happily continue parsing a second variable,
even if we didn't have a variable token at all.
Then, the loop would immediately break, and we'd parse the pipeline we'd
be ranging over. That is, we'd parse {{range $k, .}} as if it were
{{range $k = .}}.
To fix this, only allow the loop to continue if we know we're going to
parse another variable or a token that would end the pipeline. Also add
a few test cases for these error edge cases.
While at it, make use of T.Run, which was useful in debugging
Tree.pipeline via print statements.
Fixes#28437.
Change-Id: Idc9966bf643f0f3bc1b052620357e5b0aa2022ea
Reviewed-on: https://go-review.googlesource.com/c/145282
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Reviewed-by: Rob Pike <r@golang.org>
There's precedent in handling panics that happen in functions called
from the standard library. For example, if a fmt.Formatter
implementation fails, fmt will absorb the panic into the output text.
Recovering panics is useful, because otherwise one would have to wrap
some Template.Execute calls with a recover. For example, if there's a
chance that the callbacks may panic, or if part of the input data is nil
when it shouldn't be.
In particular, it's a common confusion amongst new Go developers that
one can call a method on a nil receiver. Expecting text/template to
error on such a call, they encounter a long and confusing panic if the
method expects the receiver to be non-nil.
To achieve this, introduce safeCall, which takes care of handling error
returns as well as recovering panics. Handling panics in the "call"
function isn't strictly necessary, as that func itself is run via
evalCall. However, this makes the code more consistent, and can allow
for better context in panics via the "call" function.
Finally, add some test cases with a mix of funcs, methods, and func
fields that panic.
Fixes#28242.
Change-Id: Id67be22cc9ebaedeb4b17fa84e677b4b6e09ec67
Reviewed-on: https://go-review.googlesource.com/c/143097
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Recent change golang.org/cl/142737 drops the only call site for the
sortKeys function. If it's not in use, it should probably not be there in
the code, lurking and preparing to bite us when someone calls that instead
of the new key sorter in fmtsort, resulting in strange inconsistencies.
Since the function isn't called, this should have no impact.
Related to, but does not fix, #21095.
Change-Id: I4695503ef4d5ce90d989ec952f01ea00cc15c79d
Reviewed-on: https://go-review.googlesource.com/c/143178
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
For easier testing, change the way maps are printed so they
appear in a consistent order between runs. Do this by printing
them in key-sorted order.
To do this, we add a package at the root, internal/fmtsort,
that implements a general mechanism for sorting map keys
regardless of their type. This is a little messy and probably
slow, but formatted printing of maps has never been fast and
is already always reflection-driven.
The rules are:
The ordering rules are more general than with Go's < operator:
- when applicable, nil compares low
- ints, floats, and strings order by <
- NaN compares less than non-NaN floats
- bool compares false before true
- complex compares real, then imag
- pointers compare by machine address
- channel values compare by machine address
- structs compare each field in turn
- arrays compare each element in turn.
- interface values compare first by reflect.Type describing the concrete type
and then by concrete value as described in the previous rules.
The new package is internal because we really do not want
everyone using this to sort things. It is slow, not general, and
only suitable for the subset of types that can be map keys.
Also use the package in text/template, which already had a
weaker version of this mechanism.
This change requires adding a dependency on sort to the fmt
package, but that isn't disruptive to the dependency tree.
Fixes#21095
Change-Id: Ia602115c7de5d95993dbd609611d8bd96e054157
Reviewed-on: https://go-review.googlesource.com/c/142737
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
This behavior is the same as in Go: constants can be coerced to int
and whether overflow occurs depends on how big an int is, but
this surprises people sometimes, so document it again here.
Fixes#25833.
Change-Id: I557995f1a1e8e871b21004953923d16f36cb9037
Reviewed-on: https://go-review.googlesource.com/c/141378
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
I omitted vendor directories and anything necessary for bootstrapping.
(Tested by bootstrapping with Go 1.4)
Updates #27864
Change-Id: I7d9b68d0372d3a34dee22966cca323513ece7e8a
Reviewed-on: https://go-review.googlesource.com/137856
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
First, move the strings.Count logic out of emit, since only itemText
requires that. Use it in those call sites. itemLeftDelim and
itemRightDelim cannot contain newlines, as they're the "{{" and "}}"
tokens.
Secondly, introduce a startLine lexer field so that we don't have to
keep track of it elsewhere. That's also a requirement to move the
strings.Count out of emit, as emit modifies the start position field.
Change-Id: I69175f403487607a8e5b561b3f1916ee9dc3c0c6
Reviewed-on: https://go-review.googlesource.com/132275
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
lexRawQuote already uses the next method, which keeps track of newlines
on a character by character basis. Adding up newlines in emit again
results in the newlines being counted twice, which can mean bad position
information in error messages.
Fix that, and add a test.
Fixes#27319.
Change-Id: Id803be065c541412dc808d388bc6d8a86a0de41e
Reviewed-on: https://go-review.googlesource.com/131996
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This turns
panic: function name is not a valid identifier
into
panic: function name "" is not a valid identifier
and also makes it consistent with the func signature check.
This CL also makes the testBadFuncName func a test helper.
Change-Id: Id967cb61ac28228de81e1cd76a39f5195a5ebd11
Reviewed-on: https://go-review.googlesource.com/130998
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Previously Scanner would allow float literals like "1.5e" and "1e+"
that weren't actually valid Go float literals, and also not valid
when passed to ParseFloat. This commit fixes that behaviour to match
the documentation ("recognizes all literals as defined by the Go
language specification"), and Scanner emits an error in these cases.
Fixes#26374
Change-Id: I6855402ea43febb448c6dff105b9578e31803c01
Reviewed-on: https://go-review.googlesource.com/129095
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 95215 changed text/template so that untyped nil arguments were no
longer ignored, but were instead passed to functions as expected.
This had an unexpected effect on html/template, where all data is
implicitly passed to functions: originally untyped nil arguments were
not passed and were thus effectively ignored, but after CL 95215 they
were passed and were printed, typically as an escaped version of "<nil>".
This CL restores some of the behavior of html/template by ignoring
untyped nil arguments passed implicitly to escaper functions.
While eliminating one change to html/template relative to earlier
releases, this unfortunately introduces a different one: originally
values of interface type with the value nil were printed as an escaped
version of "<nil>". With this CL they are ignored as though they were
untyped nil values. My judgement is that this is a less common case.
We'll see.
This CL adds some tests of typed and untyped nil values to
html/template and text/template to capture the current behavior.
Updates #18716Fixes#25875
Change-Id: I5912983ca32b31ece29e929e72d503b54d7b0cac
Reviewed-on: https://go-review.googlesource.com/121815
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
golang.org/cl/84480 altered the API for the parse package for
clarity and consistency. However, the changes also broke the
API for consumers of the package. This CL reverts the API
to the previous spelling, adding only a single new exported
symbol.
Fixes#25968
Change-Id: Ieb81054b61eeac7df3bc3864ef446df43c26b80f
Reviewed-on: https://go-review.googlesource.com/120355
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
These were added in CL 106979. I got them wrong.
They were fixed in CL 111643. They were still wrong.
Hopefully this change will be the last fix.
With this fix, CL 106979 is allocation-neutral for BenchmarkRagged.
The performance results for BenchmarkPyramid reported in CL 111643 stand.
Change-Id: Id6a522e6602e5df31f504adf5a3bec9969c18649
Reviewed-on: https://go-review.googlesource.com/116015
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
\f triggers a flush.
This is used (by gofmt, among others) to indicate that
the current aligned segment has ended.
When flushed, it is unlikely that the previous line is
in fact a good predictor of the upcoming line,
so stop treating it as such.
No performance impact on the existing benchmarks,
which do not perform any flushes.
Change-Id: Ifdf3e6d4600713c90db7b51a10e429d9260dc08c
Reviewed-on: https://go-review.googlesource.com/111644
Reviewed-by: Robert Griesemer <gri@golang.org>
The general policy for the current state of js/wasm is that it only
has to support tests that are also supported by nacl.
The test nilptr3.go makes assumptions about which nil checks can be
removed. Since WebAssembly does not signal on reading a null pointer,
all nil checks have to be explicit.
Updates #18892
Change-Id: I06a687860b8d22ae26b1c391499c0f5183e4c485
Reviewed-on: https://go-review.googlesource.com/110096
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Make comment example consistent with its description.
Fixes#24767
Change-Id: Icff54b489040b1ce66c644a21343c6576304cf75
Reviewed-on: https://go-review.googlesource.com/107663
Reviewed-by: Rob Pike <r@golang.org>
Noticed that we can simply use a []byte slice while investigating
a separate issue. Did the obvious simplification.
Change-Id: I921ebbb42135b5f1a10109236ceb9ae6e94ae7e2
Reviewed-on: https://go-review.googlesource.com/104757
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Variables can be declared and shadowing is supported, but modifying
existing variables via assignments was not available.
This meant that modifying a variable from a nested block was not
possible:
{{ $v := "init" }}
{{ if true }}
{{ $v := "changed" }}
{{ end }}
v: {{ $v }} {{/* "init" */}}
Introduce the "=" assignment token, such that one can now do:
{{ $v := "init" }}
{{ if true }}
{{ $v = "changed" }}
{{ end }}
v: {{ $v }} {{/* "changed" */}}
To avoid confusion, rename PipeNode.Decl to PipeNode.Vars, as the
variables may not always be declared after this change. Also change a
few other names to better reflect the added ambiguity of variables in
pipelines.
Modifying the text/template/parse package in a backwards incompatible
manner is acceptable, given that the package godoc clearly states that
it isn't intended for general use. It's the equivalent of an internal
package, back when internal packages didn't exist yet.
To make the changes to the parse package sit well with the cmd/api test,
update except.txt with the changes that we aren't worried about.
Fixes#10608.
Change-Id: I1f83a4297ee093fd45f9993cebb78fc9a9e81295
Reviewed-on: https://go-review.googlesource.com/84480
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Added examples for use of Mode, Whitespace, and IsIdentRune properties.
Fixes#23768
Change-Id: I2528e14fde63a4476f3c25510bf0c5b73f38ba5d
Reviewed-on: https://go-review.googlesource.com/93199
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
It's always nil, so simplify its signature. Found with unparam.
Change-Id: I45dd0f868ec2f5de98a970776be686417c8d73b6
Reviewed-on: https://go-review.googlesource.com/95235
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
reflect.Value is a struct and does not have a kind nor any flag for
untyped nils. As a result, it is tricky to differentiate when we're
missing a value, from when we have one but it is untyped nil.
We could start using *reflect.Value instead, to add one level of
indirection, using nil for missing values and new(reflect.Value) for
untyped nils. However, that is a fairly invasive change, and would also
mean unnecessary allocations.
Instead, use a special reflect.Value that depicts when a value is
missing. This is the case for the "final" reflect.Value in multiple
scenarios, such as the start of a pipeline. Give it a specific,
unexported type too, to make sure it cannot be mistaken for any other
valid value.
Finally, replace "final.IsValid()" with "final != missingVal", since
final.IsValid() will be false when final is an untyped nil.
Also add a few test cases, all different variants of the untyped nil
versus missing value scenario.
Fixes#18716.
Change-Id: Ia9257a84660ead5a7007fd1cced7782760b62d9d
Reviewed-on: https://go-review.googlesource.com/95215
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
It makes no sense to try to get the zero value of a nil type, hence the
panic. When we have a nil type, use reflect.ValueOf(nil) instead.
This was showing itself if one used a missing field on the data between
parentheses, when the data was a nil interface:
t := template.Must(template.New("test").Parse(`{{ (.).foo }}`))
var v interface{}
t.Execute(os.Stdout, v)
Resulting in:
panic: reflect: Zero(nil) [recovered]
panic: reflect: Zero(nil)
Fixes#21171.
Change-Id: Ifcc4a0c67e6df425b65bc9f82fde6fcf03828579
Reviewed-on: https://go-review.googlesource.com/84482
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
The new break and continue actions do not work in html/template, and
fixing them requires thinking about security issues that seem too
tricky at this stage of the release. We will try again for 1.11.
Original CL description:
text/template: add break, continue actions in ranges
Adds the two range control actions "break" and "continue". They act the
same as the Go keywords break and continue, but are simplified in that
only the innermost range statement can be broken out of or continued.
Fixes#20531
Updates #20531
Updates #23683
Change-Id: Ia7fd3c409163e3bcb5dc42947ae90b15bdf89853
Reviewed-on: https://go-review.googlesource.com/92155
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
this change removes the state field from the lexer,
because it's only used by the run method and can be
replaced with a local variable
Change-Id: Ib7a90ab6e9a894716cba2c7d9ed71bf2ad1240c0
Reviewed-on: https://go-review.googlesource.com/47338
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The lastPos field used in the past to track the line number of a token.
it's irrelevant anymore, and we can remove it.
Change-Id: I42c0bf55e884b79574a7da4926489f2d77618cd0
Reviewed-on: https://go-review.googlesource.com/49591
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
All of these had a return or break in the else body, so flipping the
condition means we can unindent and simplify.
Change-Id: If93e97504480d18a0dac3f2c8ffe57ab8bcb929c
Reviewed-on: https://go-review.googlesource.com/74190
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Adds the two range control actions "break" and "continue". They act the
same as the Go keywords break and continue, but are simplified in that
only the innermost range statement can be broken out of or continued.
Fixes#20531
Change-Id: I4412b3bbfd4dadb0ab74ae718e308c1ac7a0a1e9
Reviewed-on: https://go-review.googlesource.com/66410
Reviewed-by: Rob Pike <r@golang.org>
Do the low-hanging fruit - tiny Less functions that are used exactly
once. This reduces the amount of code and puts the logic in a single
place.
Change-Id: I9d4544cd68de5a95e55019bdad1fca0a1dbfae9c
Reviewed-on: https://go-review.googlesource.com/63171
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Found with staticcheck. Not terribly important since the test would
likely fail anyway, but at least it will fail with a better explanation
now.
Change-Id: Ic3f9a94a2152404b7873cc8cd47b6db79d78c2e6
Reviewed-on: https://go-review.googlesource.com/62990
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It doesn't change the outcome. It might have been useful at some point
to avoid Replace from doing work or allocating. However, nowadays the
func returns early without doing any work if Count returns 0.
Change-Id: Id69dc74042a6e39672b405016484db8b50f43d58
Reviewed-on: https://go-review.googlesource.com/62991
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dave Cheney <dave@cheney.net>
The lexer keeps the byte offset and the line for the rune it's currently
on. This was simple enough up until whitespace trimming was introduced.
With whitespace trimming, we might skip over newlines. In that case, the
lexer wasn't properly updating the line counter. Fix it.
Also, TestPos now checks that the line is correct too, which it was
ignoring before. This was necessary to test this scenario in the lexer.
Fixes#21778.
Change-Id: I3880f3adf02662eac8f818d5caa6935cca9cb33b
Reviewed-on: https://go-review.googlesource.com/61870
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Found with mvdan.cc/unindent. It skipped the cases where parentheses
would need to be added, where comments would have to be moved elsewhere,
or where actions and simple logic would mix.
One of them was of the form "err != nil && err == io.EOF", so the first
part was removed.
Change-Id: Ie504c2b03a2c87d10ecbca1b9270069be1171b91
Reviewed-on: https://go-review.googlesource.com/57690
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Found with mvdan.cc/unindent. Prioritized the ones with the biggest wins
for now.
Change-Id: I2b032e45cdd559fc9ed5b1ee4c4de42c4c92e07b
Reviewed-on: https://go-review.googlesource.com/56470
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Daniel Martí
Russ Cox
Robert Griesemer
Alexandre Maari
Seebs
Rob Pike
Brad Fitzpatrick
Michal Bohuslávek
Ben Hoyt
Ian Lance Taylor
Andrew Braunstein
Daniel Theophanes
Josh Bleecher Snyder
Shengyu Zhang
Richard Musiol
Zheng Dayu
Adam Woodbeck
Yury Smolsky
Kunpei Sakai
Ariel Mashraki
Tim Cooper