mustBe was barely over budget, so manually inlining the first flag.kind
call is enough. Add a TODO to reverse that in the future, once the
compiler gets better.
mustBeExported and mustBeAssignable were over budget by a larger amount,
so add slow path functions instead. This is the same strategy used in
the sync package for common methods like Once.Do, for example.
Lots of exported reflect.Value methods call these assert-like unexported
methods, so avoiding the function call overhead in the common case does
shave off a percent from most exported APIs.
Finally, add the methods to TestIntendedInlining.
While at it, replace a couple of uses of the 0 Kind with its descriptive
name, Invalid.
name old time/op new time/op delta
Call-8 68.0ns ± 1% 66.8ns ± 1% -1.81% (p=0.000 n=10+9)
PtrTo-8 8.00ns ± 2% 7.83ns ± 0% -2.19% (p=0.000 n=10+9)
Updates #7818.
Change-Id: Ic1603b640519393f6b50dd91ec3767753eb9e761
Reviewed-on: https://go-review.googlesource.com/c/go/+/166462
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@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>
First the insidious bug:
var n uintptr
for n := elemPtrs; n > 120; n -= 120 {
prog = append(prog, 120)
prog = append(prog, mask[:15]...)
mask = mask[15:]
}
prog = append(prog, byte(n))
prog = append(prog, mask[:(n+7)/8]...)
The := breaks this code, because the n after the loop is always 0!
We also do need to handle field padding correctly. In particular
the old padding code doesn't correctly handle fields that are not
a multiple of a pointer in size.
Fixes#30606.
Change-Id: Ifcab9494dc25c20116753c5d7e0145d6c2053ed8
Reviewed-on: https://go-review.googlesource.com/c/go/+/165860
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
They are missing a stop byte at the end.
Normally this doesn't matter, but when including a GC program
in another GC program, we strip the last byte. If that last byte
wasn't a stop byte, then we've thrown away part of the program
we actually need.
Fixes#30606
Change-Id: Ie9604beeb84f7f9442e77d31fe64c374ca132cce
Reviewed-on: https://go-review.googlesource.com/c/go/+/165857
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The names of some instructions have been updated in the WebAssembly
specification to be more consistent, see
994591e51c.
This change to the spec is possible because it is still in a draft
state.
Go's support for WebAssembly is still experimental and thus excempt from
the compatibility promise. Being consistent with the spec should
warrant this breaking change to the assembly instruction names.
Change-Id: Iafb8b18ee7f55dd0e23c6c7824aa1fad43117ef1
Reviewed-on: https://go-review.googlesource.com/c/153797
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
We are copying the results to uninitialized stack space. Write
barrier is not needed.
Fixes#30041.
Change-Id: Ia91d74dbafd96dc2bd92de0cb479808991dda03e
Reviewed-on: https://go-review.googlesource.com/c/160737
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reorg map flags a bit so we don't need any extra space for the extra flag.
Fixes#23734
Change-Id: I436812156240ae90de53d0943fe1aabf3ea37417
Reviewed-on: https://go-review.googlesource.com/c/155918
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
UnsafePointer is a valid type kind to call IsNil on.
Fixes#29381
Change-Id: Iaf65d582c67f4be52cd1885badf40f174920500b
Reviewed-on: https://go-review.googlesource.com/c/155797
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
On reflect documentation page only this function
doesn't have description, this commit add simple description.
Change-Id: Idcda89ddd1f6fdd1938c4030e89ebdc186255ce6
GitHub-Last-Rev: 1553b834bb
GitHub-Pull-Request: golang/go#28818
Reviewed-on: https://go-review.googlesource.com/c/149721
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, package runtime contains the definition of reflect.call,
even though it's just a jump to runtime.reflectcall. This "push"
symbol is confusing, since it's not clear where the definition of
reflect.call comes from when you're in the reflect package.
Replace this with a "pull" symbol: the runtime now defines only
runtime.reflectcall and package reflect uses a go:linkname to access
this symbol directly. This makes it clear where reflect.call is coming
from without any spooky action at a distance and eliminates all of the
definitions of reflect.call in the runtime.
Change-Id: I3ec73cd394efe9df8d3061a57c73aece2e7048dd
Reviewed-on: https://go-review.googlesource.com/c/148657
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Previously we panicked if the number of methods present for an embedded
field was >= 32. This change removes that limit and now StructOf
dynamically calls itself to create space for the number of methods.
Fixes#25402
Change-Id: I3b1deb119796d25f7e6eee1cdb126327b49a0b5e
GitHub-Last-Rev: 16da71ad6b
GitHub-Pull-Request: golang/go#26865
Reviewed-on: https://go-review.googlesource.com/c/128479
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Go documentation style for boolean funcs is to say:
// Foo reports whether ...
func Foo() bool
(rather than "returns true if")
This CL also replaces 4 uses of "iff" with the same "reports whether"
wording, which doesn't lose any meaning, and will prevent people from
sending typo fixes when they don't realize it's "if and only if". In
the past I think we've had the typo CLs updated to just say "reports
whether". So do them all at once.
(Inspired by the addition of another "returns true if" in CL 146938
in fd_plan9.go)
Created with:
$ perl -i -npe 's/returns true if/reports whether/' $(git grep -l "returns true iff" | grep -v vendor)
$ perl -i -npe 's/returns true if/reports whether/' $(git grep -l "returns true if" | grep -v vendor)
Change-Id: Ided502237f5ab0d25cb625dbab12529c361a8b9f
Reviewed-on: https://go-review.googlesource.com/c/147037
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Rework how the compiler+runtime handles stack-allocated variables
whose address is taken.
Direct references to such variables work as before. References through
pointers, however, use a new mechanism. The new mechanism is more
precise than the old "ambiguously live" mechanism. It computes liveness
at runtime based on the actual references among objects on the stack.
Each function records all of its address-taken objects in a FUNCDATA.
These are called "stack objects". The runtime then uses that
information while scanning a stack to find all of the stack objects on
a stack. It then does a mark phase on the stack objects, using all the
pointers found on the stack (and ancillary structures, like defer
records) as the root set. Only stack objects which are found to be
live during this mark phase will be scanned and thus retain any heap
objects they point to.
A subsequent CL will remove all the "ambiguously live" logic from
the compiler, so that the stack object tracing will be required.
For this CL, the stack tracing is all redundant with the current
ambiguously live logic.
Update #22350
Change-Id: Ide19f1f71a5b6ec8c4d54f8f66f0e9a98344772f
Reviewed-on: https://go-review.googlesource.com/c/134155
Reviewed-by: Austin Clements <austin@google.com>
R0 isn't the zero register any more. Oops.
Update #27695.
Change-Id: I46a975ed37d5e570afe2e228d3edf74949e08ad7
Reviewed-on: https://go-review.googlesource.com/138580
Reviewed-by: Michael Munday <mike.munday@ibm.com>
During a call to a reflect-generated function or method (via
makeFuncStub or methodValueCall), when should we scan the return
values?
When we're starting a reflect call, the space on the stack for the
return values is not initialized yet, as it contains whatever junk was
on the stack of the caller at the time. The return space must not be
scanned during a GC.
When we're finishing a reflect call, the return values are
initialized, and must be scanned during a GC to make sure that any
pointers in the return values are found and their referents retained.
When the GC stack walk comes across a reflect call in progress on the
stack, it needs to know whether to scan the results or not. It doesn't
know the progress of the reflect call, so it can't decide by
itself. The reflect package needs to tell it.
This CL adds another slot in the frame of makeFuncStub and
methodValueCall so we can put a boolean in there which tells the
runtime whether to scan the results or not.
This CL also adds the args length to reflectMethodValue so the
runtime can restrict its scanning to only the args section (not the
results) if the reflect package says the results aren't ready yet.
Do a delicate dance in the reflect package to set the "results are
valid" bit. We need to make sure we set the bit only after we've
copied the results back to the stack. But we must set the bit before
we drop reflect's copy of the results. Otherwise, we might have a
state where (temporarily) no one has a live copy of the results.
That's the state we were observing in issue #27695 before this CL.
The bitmap used by the runtime currently contains only the args.
(Actually, it contains all the bits, but the size is set so we use
only the args portion.) This is safe for early in a reflect call, but
unsafe late in a reflect call. The test issue27695.go demonstrates
this unsafety. We change the bitmap to always include both args
and results, and decide at runtime which portion to use.
issue27695.go only has a test for method calls. Function calls were ok
because there wasn't a safepoint between when reflect dropped its copy
of the return values and when the caller is resumed. This may change
when we introduce safepoints everywhere.
This truncate-to-only-the-args was part of CL 9888 (in 2015). That
part of the CL fixed the problem demonstrated in issue27695b.go but
introduced the problem demonstrated in issue27695.go.
TODO, in another CL: simplify FuncLayout and its test. stack return
value is now identical to frametype.ptrdata + frametype.gcdata.
Fixes#27695
Change-Id: I2d49b34e34a82c6328b34f02610587a291b25c5f
Reviewed-on: https://go-review.googlesource.com/137440
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Fix the code to use write barriers on heap memory, and no
write barriers on stack memory.
These errors were discoverd as part of fixing #27695. They may
have something to do with that issue, but hard to be sure.
The core cause is different, so this fix is a separate CL.
Update #27695
Change-Id: Ib005f6b3308de340be83c3d07d049d5e316b1e3c
Reviewed-on: https://go-review.googlesource.com/137438
Reviewed-by: Austin Clements <austin@google.com>
If an object is allocated as part of a tinyalloc, then other live
objects in the same tinyalloc chunk keep the finalizer from being run,
even if the object that has the finalizer is dead.
Make sure the object we're setting the finalizer on is big enough
to not trigger tinyalloc allocation.
Fixes#26857
Update #21717
Change-Id: I56ad8679426283237ebff20a0da6c9cf64eb1c27
Reviewed-on: https://go-review.googlesource.com/128475
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Example of use:
iter := reflect.ValueOf(m).MapRange()
for iter.Next() {
k := iter.Key()
v := iter.Value()
...
}
See issue golang/go#11104
Q. Are there any benchmarks that would exercise the new calls to
copyval in existing code?
Change-Id: Ic469fcab5f1d9d853e76225f89bde01ee1d36e7a
Reviewed-on: https://go-review.googlesource.com/33572
Reviewed-by: Keith Randall <khr@golang.org>
Before CL 4281055 in 2011, the reflect package was quite different.
rtype, then called commonType, was embedded in exported structs with
names like StructType. In order to avoid accidental conversions
between pointers to these public structs, which sometimes had
identical fields, the embedded commonType fields were tagged.
In CL 4281055 the formerly public structs were unexported, and all
access was done through the Type interface. At that point the field
tags in the reflect structs were no longer useful.
In Go 1.8 the language was changed to ignore struct field tags when
converting between types. This made the field tags in the reflect
structs doubly useless.
This CL simply removes them.
Fixes#20914
Change-Id: I9af4d6d0709276a91a6b6ee5323cad9dcd0cd0a0
Reviewed-on: https://go-review.googlesource.com/121475
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The current implementation does not generate wrappers for methods of
embedded non-interface types. We can only skip the wrapper if
kindDirectIface of the generated struct type matches kindDirectIface
of the embedded type. Panic if that is not the case.
It would be better to actually generate wrappers, but that can be done
later.
Updates #15924Fixes#24782
Change-Id: I01f5c76d9a07f44e1b04861bfe9f9916a04e65ca
Reviewed-on: https://go-review.googlesource.com/121316
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gccgo fails this test before CL 115577.
Updates #25284
Change-Id: Id4550b7b3e268f3c294420ed31c57ad3f1002b5e
Reviewed-on: https://go-review.googlesource.com/115635
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
On the API level this is just an update of the documentation to match
the current spec more closely.
On the implementation side, this is a rename of various unexported names.
For #22005.
Change-Id: Ie5ae32f3b10f003805240efcceab3d0fd373cd51
Reviewed-on: https://go-review.googlesource.com/112717
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The hmap field in the maptype is only used by the runtime to check the sizes of
the hmap structure created by the compiler and runtime agree.
Comments are already present about the hmap structure definitions in the
compiler and runtime needing to be in sync.
Add a test that checks the runtimes hmap size is as expected to detect
when the compilers and runtimes hmap sizes diverge instead of checking
this at runtime when a map is created.
Change-Id: I974945ebfdb66883a896386a17bbcae62a18cf2a
Reviewed-on: https://go-review.googlesource.com/91796
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
There are no strange messages anymore, so define type MyBuffer in the
block where it is used.
Change-Id: Ic65b15dc76a40f6f734d9ac2116338502fbb66fd
Reviewed-on: https://go-review.googlesource.com/107735
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
By moving exported methods to the front of method lists, filtering
down to only the exported methods just needs a count of how many
exported methods exist, which the compiler can statically
provide. This allows getting rid of the exported method cache.
For #22075.
Change-Id: I8eeb274563a2940e1347c34d673f843ae2569064
Reviewed-on: https://go-review.googlesource.com/100846
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If the type of Type is known to be *rtype than the common
function is a no-op and does not need to be called.
name old time/op new time/op delta
New 31.0ns ± 5% 30.2ns ± 4% -2.74% (p=0.008 n=20+20)
Change-Id: I5d00346dbc782e34c530166d1ee0499b24068b51
Reviewed-on: https://go-review.googlesource.com/96115
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Rename all map implementation and test files to use "map"
as a file name prefix instead of "hashmap" for the implementation
and "map" for the test file names.
Change-Id: I7b317c1f7a660b95c6d1f1a185866f2839e69446
Reviewed-on: https://go-review.googlesource.com/90336
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Type values being comparable implies that Type is a valid map key type.
As previously written, they sound unrelated.
Change-Id: I8e2235275d62898bfb47de850e8257b51ab5cbd6
Reviewed-on: https://go-review.googlesource.com/87021
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It's not safe to do p+x with unsafe if that would point past the
end of the object. (Valid in C, not safe in Go.)
Pass a "whySafe" reason (compiled away) to explain at each
call site why it's safe.
Fixes#21733.
Change-Id: I5da8c25bde66f5c9beac232f2135dcab8e8bf3b1
Reviewed-on: https://go-review.googlesource.com/80738
Reviewed-by: Austin Clements <austin@google.com>
Call is meant to mirror the language semantics, which allow:
var r io.ReadWriter
f := func(io.Reader){}
f(r)
even though the conversion from io.ReadWriter to io.Reader is
being applied to a nil interface. This is different from an explicit
conversion:
_ = r.(io.Reader)
f(r.(io.Reader))
Both of those lines panic, but the implicit conversion does not.
By using E2I, which is the implementation of the explicit conversion,
the reflect.Call equivalent of f(r) was inadvertently panicking.
Avoid the panic.
Fixes#22143.
Change-Id: I6b2f5b808e0cd3b89ae8bc75881e307bf1c25558
Reviewed-on: https://go-review.googlesource.com/80736
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@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>
This somewhat mirrors the special case behavior of the copy built-in.
Fixes#22215
Change-Id: Ic353003ad3de659d3a6b4e9d97295b42510f3bf7
Reviewed-on: https://go-review.googlesource.com/70431
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, methods are sorted by name. This happens to guarantee that
exported ASCII methods appear before non-exported ASCII methods, but
this breaks down when Unicode method names are considered.
Type.Method already accounts for this by always indexing into the
slice returned by exportedMethods. This CL makes Value.Method do the
same.
Fixes#22073.
Change-Id: I9bfc6bbfb7353e0bd3c439a15d1c3da60d16d209
Reviewed-on: https://go-review.googlesource.com/66770
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Adding the "https://" scheme allows godoc to properly detect
the URL and provide a hyperlink for it.
Change-Id: I76dc309368c86975de01bc6e6e9196037b2114d7
Reviewed-on: https://go-review.googlesource.com/66570
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The reflect API normally grants only read-only access to non-exported
fields, but it specially handles non-exported embedded fields so that
users can still fully access promoted fields and methods. For example,
if v.Field(i) refers to a non-exported embedded field, it would be
limited to RO access. But if v.Field(i).Field(j) is an exported field,
then the resulting Value will have full access.
However, the way this was implemented allowed other operations to be
interspersed between the Field calls, which could grant inappropriate
access.
Relatedly, Elem() is safe to use on pointer-embeddings, but it was
also being allowed on embeddings of interface types. This is
inappropriate because it could allow accessing methods of the dynamic
value's complete method set, not just those that were promoted via the
interface embedding.
Fixes#22031.
Fixes#22053.
Change-Id: I9db9be88583f1c1d80c1b4705a76f23a4379182f
Reviewed-on: https://go-review.googlesource.com/66331
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If a function with nonzero frame but zero-sized return value is
Call'd, we may write a past-the-end pointer in preparing the
return Values. Fix by return the zero value for zero-sized
return value.
Fixes#21717.
Change-Id: I5351cd86d898467170a888b4c3fc9392f0e7aa3b
Reviewed-on: https://go-review.googlesource.com/60811
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The method Method expects index to be an index of exported fields,
but, before this change, the index used by MethodByName could
take into account unexported fields if those happened sort
before the exported one.
Fixes#21177
Change-Id: I90bb64a47b23e2e43fdd2b8a1e0a2c9a8a63ded2
Reviewed-on: https://go-review.googlesource.com/51810
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
pkgPath always received the empty string. Worse yet, it panicked if it
received anything else. This has been the case ever since newName was
introduced in early 2016.
Change-Id: I5f164305bd30c34455ef35e776c7616f303b37e4
Reviewed-on: https://go-review.googlesource.com/54331
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
We don't use it any more, remove it.
Change-Id: I76ce1a4c2e7048fdd13a37d3718b5abf39ed9d26
Reviewed-on: https://go-review.googlesource.com/44474
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
