Brief background on "why heap allocate". Things can be
forced to the heap for the following reasons:
1) address published, hence lifetime unknown.
2) size unknown/too large, cannot be stack allocated
3) multiplicity unknown/too large, cannot be stack allocated
4) reachable from heap (not necessarily published)
The bug here is a case of failing to enforce 4) when an
object Y was reachable from a heap allocation X forced
because of 3). It was found in the case of a closure
allocated within a loop (X) and assigned to a variable
outside the loop (multiplicity unknown) where the closure
also captured a map (Y) declared outside the loop (reachable
from heap). Note the variable declared outside the loop (Y)
is not published, has known size, and known multiplicity
(one). The only reason for heap allocation is that it was
reached from a heap allocated item (X), but because that was
not forced by publication, it has to be tracked by loop
level, but escape-loop level was not tracked and thus a bug
results.
The fix is that when a heap allocation is newly discovered,
use its looplevel as the minimum loop level for downstream
escape flooding.
Every attempt to generalize this bug to X-in-loop-
references-Y-outside loop succeeded, so the fix was aimed
to be general. Anywhere that loop level forces heap
allocation, the loop level is tracked. This is not yet
tested for all possible X and Y, but it is correctness-
conservative and because it caused only one trivial
regression in the escape tests, it is probably also
performance-conservative.
The new test checks the following:
1) in the map case, that if fn escapes, so does the map.
2) in the map case, if fn does not escape, neither does the map.
3) in the &x case, that if fn escapes, so does &x.
4) in the &x case, if fn does not escape, neither does &x.
Fixes#13799.
Change-Id: Ie280bef2bb86ec869c7c206789d0b68f080c3fdb
Reviewed-on: https://go-review.googlesource.com/18234
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Adding the evconst(n) call for OANDAND and OOROR in
golang.org/cl/18262 was originally just to parallel the above iscmp
branch, but upon further inspection it seemed odd that removing it
caused test/fixedbugs/issue6671.go's
var b mybool
// ...
b = bool(true) && true // ERROR "cannot use"
to start failing (i.e., by not emitting the expected "cannot use"
error).
The problem is that evconst(n)'s settrue and setfalse paths always
reset n.Type to idealbool, even for logical operators where n.Type
should preserve the operand type. Adding the evconst(n) call for
OANDAND/OOROR inadvertantly worked around this by turning the later
evconst(n) call at line 2167 into a noop, so the "n.Type = t"
assignment at line 739 would preserve the operand type.
However, that means evconst(n) was still clobbering n.Type for ONOT,
so declarations like:
const _ bool = !mybool(true)
were erroneously accepted.
Update #13821.
Change-Id: I18e37287f05398fdaeecc0f0d23984e244f025da
Reviewed-on: https://go-review.googlesource.com/18362
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Added a format option to inhibit output of .Note field in
printing, and enabled that option during export.
Added test.
Fixes#13777.
Change-Id: I739f9785eb040f2fecbeb96d5a9ceb8c1ca0f772
Reviewed-on: https://go-review.googlesource.com/18217
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: David Chase <drchase@google.com>
Fixes#12411.
Change-Id: I2202a754c7750e3b2119e3744362c98ca0d2433e
Reviewed-on: https://go-review.googlesource.com/17818
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Another (historic) artifact due to partially resolving symbols too early.
Fixes#13539.
Change-Id: Ie720c491cfa399599454f384b3a9735e75d4e8f1
Reviewed-on: https://go-review.googlesource.com/17600
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Following an empty import, a declaration involving a ? symbol
generates an internal compiler error when the name of the
symbol (in newname function).
package a
import""
var?
go.go:2: import path is empty
go.go:3: internal compiler error: newname nil
Make sure dclname is not called when the symbol is nil.
The error message is now:
go.go:2: import path is empty
go.go:3: invalid declaration
go.go:4: syntax error: unexpected EOF
This CL was initially meant to be applied to the old parser,
and has been updated to apply to the new parser.
Fixes#11610
Change-Id: I75e07622fb3af1d104e3a38c89d9e128e3b94522
Reviewed-on: https://go-review.googlesource.com/15268
Reviewed-by: Russ Cox <rsc@golang.org>
The following code:
func n() {(interface{int})}
generates:
3: interface contains embedded non-interface int
3: type %!v(PANIC=runtime error: invalid memory address or nil pointer dereference) is not an expression
It is because the corresponding symbol (Sym field in Type object)
is nil, resulting in a panic in typefmt.
Just skip the symbol if it is nil, so that the error message becomes:
3: interface contains embedded non-interface int
3: type interface { int } is not an expression
Fixes#11614
Change-Id: I219ae7eb01edca264fad1d4a1bd261d026294b00
Reviewed-on: https://go-review.googlesource.com/14015
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
- use same local variable name (lno) for line number for LCOLAS everywhere
- remove now unneeded assignment of line number to yylval.i in lexer
Fix per suggestion of mdempsky.
Fixes#13415.
Change-Id: Ie3c7f5681615042a12b81b26724b3a5d8a979c25
Reviewed-on: https://go-review.googlesource.com/17248
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Use a combination of follow- and stop-token lists and nesting levels
to better synchronize parser after a syntax error.
Fixes#13319.
Change-Id: I9592e0b5b3ba782fb9f9315fea16163328e204f7
Reviewed-on: https://go-review.googlesource.com/17080
Reviewed-by: Chris Manghane <cmang@golang.org>
Handling of &(T{}) assumed that the parser would not introduce ()'s.
Also: Better comments around handling of OPAREN syntax tree optimization.
Fixes#13261.
Change-Id: Ifc5047a0448f5e7d74cd42f6608b87dcc9c2f2fb
Reviewed-on: https://go-review.googlesource.com/17040
Reviewed-by: Chris Manghane <cmang@golang.org>
Also:
- better error messages in some cases
- factored out function to produce syntax error at given line number
Fixes#13273.
Change-Id: I0192a94731cc23444680a26bd0656ef663e6da0b
Reviewed-on: https://go-review.googlesource.com/16992
Reviewed-by: Chris Manghane <cmang@golang.org>
This is a translation of the yacc-based parser with adjustements
to make the grammar work for a recursive-descent parser followed
by cleanups and simplifications.
The yacc actions were mostly literally copied for correctness
with better temporary names.
A few of the syntax tests were adjusted for slightly different
error messages (it is very difficult to match the yacc-based
error messages in all cases, and sometimes the new parser could
produce better errors).
The new parser is enabled by default.
To switch back to the yacc-based parser, set -oldparser.
To hardwire the switch back, uncomment "oldparser = 1" in lex.go.
- passes all.bash
- ~18% reduced parse time per file on average for make.bash
- ~3% reduced compile time for building cmd/compile
Change-Id: Icb5651bb9d8b9f66261762d2c94a03793050d4ce
Reviewed-on: https://go-review.googlesource.com/16665
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Skip fixedbugs/issue10607.go because external linking is not supported
yet.
Skip nilptr3.go because of issue #9058 (same as ppc64).
Change-Id: Ib3dfbd9a03ee4052871cf57c74b3cc5e745e1f80
Reviewed-on: https://go-review.googlesource.com/14461
Reviewed-by: Minux Ma <minux@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change is the same as CL #9345 which was reverted,
except for a small bug fix.
The only change is to the body of sendDirect and its callsite.
Also added a test.
The problem was during a channel send operation. The target
of the send was a sleeping goroutine waiting to receive. We
basically do:
1) Read the destination pointer out of the sudog structure
2) Copy the value we're sending to that destination pointer
Unfortunately, the previous change had a goroutine suspend
point between 1 & 2 (the call to sendDirect). At that point
the destination goroutine's stack could be copied (shrunk).
The pointer we read in step 1 is no longer valid for step 2.
Fixed by not allowing any suspension points between 1 & 2.
I suspect the old code worked correctly basically by accident.
Fixes#13169
The original 9345:
This change removes the retry mechanism we use for buffered channels.
Instead, any sender waking up a receiver or vice versa completes the
full protocol with its counterpart. This means the counterpart does
not need to relock the channel when it wakes up. (Currently
buffered channels need to relock on wakeup.)
For sends on a channel with waiting receivers, this change replaces
two copies (sender->queue, queue->receiver) with one (sender->receiver).
For receives on channels with a waiting sender, two copies are still required.
This change unifies to a large degree the algorithm for buffered
and unbuffered channels, simplifying the overall implementation.
Fixes#11506
Change-Id: I57dfa3fc219cffa4d48301ee15fe5479299efa09
Reviewed-on: https://go-review.googlesource.com/16740
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Make sure that we're moving or zeroing pointers atomically.
Anything that is a multiple of pointer size and at least
pointer aligned might have pointers in it. All the code looks
ok except for the 1-pointer-sized moves.
Fixes#13160
Update #12552
Change-Id: Ib97d9b918fa9f4cc5c56c67ed90255b7fdfb7b45
Reviewed-on: https://go-review.googlesource.com/16668
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Duffcopy now uses X0, as of 5cf281a. Teach the peephole
optimizer that duffcopy clobbers X0 so that it does not
rename registers use X0 across the duffcopy instruction.
Fixes#13171
Change-Id: I389cbf1982cb6eb2f51e6152ac96736a8589f085
Reviewed-on: https://go-review.googlesource.com/16715
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
sradi and sradi. hide the top bit of their immediate argument apart from the
rest of it, but the code only handled the sradi case.
I'm pretty sure this is the only instruction missing (a couple of the rotate
instructions encode their immediate the same way but their handling looks OK).
This fixes the failure of "GOARCH=amd64 ~/go/bin/go install -v runtime" as
reported in the bug.
Fixes#11987
Change-Id: I0cdefcd7a04e0e8fce45827e7054ffde9a83f589
Reviewed-on: https://go-review.googlesource.com/16710
Reviewed-by: Minux Ma <minux@golang.org>
Small fix: looks like a short variable declaration with a type switch
checks to make sure the variable used had valid shape (ONAME, OTYPE, or
ONONAME) and rejects everything else. Then a new variable is declared.
If the symbol contained in the declaration was a named OLITERAL (still a
valid identifier obviously) it would be rejected, even though a new
variable would have been declared.
Fix adds this case to the check.
Added a test case from issue12413.
Fixes#12413
Change-Id: I150dadafa8ee5612c867d58031027f2dca8c6ebc
Reviewed-on: https://go-review.googlesource.com/15760
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Apply static bounds checking logic during type checking even to
zero-element arrays, but skip synthesized OINDEX nodes that the
compiler has asserted are within bounds (such as the ones generated
while desugaring ORANGE nodes). This matches the logic in walkexpr
that also skips static bounds checking when Bounded is true.
Passes toolstash/buildall.
Fixes#12944.
Change-Id: I14ba03d71c002bf969d69783bec8d1a8e10e7d75
Reviewed-on: https://go-review.googlesource.com/15902
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Following the C to Go translation, some useless variables
were left in the code. In fmt.go, this was harmless.
In lex.go, it broke the error message related to
non-canonical import paths.
Fix it, and remove the useless variables.
The added test case is ignored in the go/types tests, since
the behavior of the non-canonical import path check seems
to be different.
Fixes#11362
Change-Id: Ic9129139ede90357dc79ebf167af638cf44536fa
Reviewed-on: https://go-review.googlesource.com/15580
Reviewed-by: Marvin Stenger <marvin.stenger94@gmail.com>
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This lets us re-enable duffzero.
Fixes#12108
Change-Id: Iefd24d26eaa56067caa2c29ff99cd20a42d8714a
Reviewed-on: https://go-review.googlesource.com/14937
Reviewed-by: Keith Randall <khr@golang.org>
