Commit graph

109 commits

Author SHA1 Message Date
David Crawshaw
2d1c033259 reflect: let StructOf define unexported fields
This was missing from the original StructOf CL because I couldn't
think of a use for it. Now I can: even with types used entirely
by reflect, unexported fields can be set using UnsafeAddr.

Change-Id: I5e7e3d81d16e8817cdd69d85796ce33930ef523b
Reviewed-on: https://go-review.googlesource.com/c/go/+/85661
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-09-28 00:19:19 +00:00
Huan Du
4dc11ae26b reflect: fix panic in DeepEqual when checking a cycle
Before this change, when DeepEqual checks values with cycle, it may
panic due to stack overflow.

Here is a sample to reproduce the issue.

    makeCycleMap := func() interface{} {
        cycleMap := map[string]interface{}{}
        cycleMap["foo"] = cycleMap
        return cycleMap
    }

    m1 := makeCycleMap()
    m2 := makeCycleMap()
    reflect.DeepEqual(m1, m2) // stack overflow

The root cause is that DeepEqual fails to cache interface values
in visited map, which is used to detect cycle. DeepEqual calls
CanAddr to check whether a value should be cached or not. However,
all values referenced by interface don't have flagAddr thus all these
values are not cached.

THe fix is to remove CanAddr calls and use underlying ptr in value
directly. As ptr is only read-only in DeepEqual for caching, it's
safe to do so. We don't use UnsafeAddr this time, because this method
panics when CanAddr returns false.

Fixes #33907

Change-Id: I2aa88cc060a2c2192b1d34c129c0aad4bd5597e7
Reviewed-on: https://go-review.googlesource.com/c/go/+/191940
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-09-11 00:56:01 +00:00
Cherry Zhang
5f45a3337e reflect: align first argument in callMethod
When calling a function obtained from reflect.Value.Method (or
MethodByName), we copy the arguments from the caller frame, which
does not include the receiver, to a new frame to call the actual
method, which does include the receiver. Here we need to align
the first (non-receiver) argument. As the receiver is pointer
sized, it is generally naturally aligned, except on amd64p32,
where the argument can have larger alignment, and this aligning
becomes necessary.

Fixes #33628.

Change-Id: I5bea0e20173f06d1602c5666d4f334e3d0de5c1e
Reviewed-on: https://go-review.googlesource.com/c/go/+/190297
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2019-08-14 19:49:15 +00:00
Cherry Zhang
2d32d3675a reflect: add a test for Calling a Method of a direct interface type
Gccgo's implementation of direct interface types has bugs that
causes reflect Call of method from Type.Method fail. CL 175837
and CL 175798 fix the bug. This CL adds a test.

Change-Id: I4e5f2cb96304c1ac7be04ca6d2851bac52b8eb24
Reviewed-on: https://go-review.googlesource.com/c/go/+/175880
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-05-08 13:56:52 +00:00
Shulhan
ed7f323c8f all: simplify code using "gofmt -s -w"
Most changes are removing redundant declaration of type when direct
instantiating value of map or slice, e.g. []T{T{}} become []T{{}}.

Small changes are removing the high order of subslice if its value
is the length of slice itself, e.g. T[:len(T)] become T[:].

The following file is excluded due to incompatibility with go1.4,

- src/cmd/compile/internal/gc/ssa.go

Change-Id: Id3abb09401795ce1e6da591a89749cba8502fb26
Reviewed-on: https://go-review.googlesource.com/c/go/+/166437
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-05-06 22:19:22 +00:00
Keith Randall
fe83731651 reflect: MakeFunc: allow assignment conversions on values returned from the wrapped function
Instead of requiring exact type match, allow assignment conversions
(those conversions allowed in the language spec without a cast) on the
returned values.

Particularly useful when the type being returned is an interface type,
but the Value actually returned is a concrete value implementing that
type (as it is tricky to return a Value which has interface type).

RELNOTE=y

Fixes #28761

Change-Id: I69eef07ca51690b2086dfa1eb549db5e4724c657
Reviewed-on: https://go-review.googlesource.com/c/go/+/174531
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-05-02 22:17:31 +00:00
Daniel Martí
9b968df177 all: clean up code with token.IsExported
A handful of packages were reimplementing IsExported, so use
token.IsExported instead. This caused the deps test to fail for net/rpc.
However, net/rpc deals with Go types, and go/token is light and fairly
low-level in terms of Go tooling packages, so that's okay.

While at it, replace all uses of ast.IsExported with token.IsExported.
This is more consistent, and also means that the import graphs are
leaner. A couple of files no longer need to import go/ast, for example.

We can't get rid of cmd/compile/internal/types.IsExported, as the
compiler can only depend on go/token as of Go 1.4. However, gc used
different implementations in a couple of places, so consolidate the use
of types.IsExported there.

Finally, we can't get rid of the copied IsExported implementation in
encoding/gob, as go/token depends on it as part of a test. That test
can't be an external test either, so there's no easy way to break the
import cycle.

Overall, this removes about forty lines of unnecessary code.

Change-Id: I86a475b7614261e6a7b0b153d5ca02b9f64a7b2d
Reviewed-on: https://go-review.googlesource.com/c/go/+/172037
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-04-16 14:43:48 +00:00
Romain Baugue
c40bffd905 reflect: add Value.IsZero
Fixes #7501

Change-Id: Iac7c79cd4b30a90b14ed84bf1eba758972232a6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/171337
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-04-13 00:04:07 +00:00
Andrew Bonventre
3b66c00857 reflect: fix panic when Value.IsNil is called for UnsafePointer
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>
2018-12-24 01:23:25 +00:00
Raghavendra Nagaraj
0e4a0b93d2 reflect: fix StructOf panics from too many methods in embedded fields
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>
2018-11-06 13:52:29 +00:00
Keith Randall
cbafcc55e8 cmd/compile,runtime: implement stack objects
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>
2018-10-03 19:52:49 +00:00
Keith Randall
ef50373983 reflect: ensure correct scanning of return values
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>
2018-09-29 20:25:24 +00:00
Keith Randall
78ce3a0368 reflect: use a bigger object when we need a finalizer to run
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>
2018-08-24 22:17:54 +00:00
Alan Donovan
ede5958385 reflect: add Value.MapRange method and MapIter type
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>
2018-08-22 20:16:01 +00:00
Ian Lance Taylor
83092a40ac reflect: prevent additional StructOf embedded method cases
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 #15924
Fixes #24782

Change-Id: I01f5c76d9a07f44e1b04861bfe9f9916a04e65ca
Reviewed-on: https://go-review.googlesource.com/121316
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-06-27 22:04:08 +00:00
Ian Lance Taylor
6decd3d984 reflect: add StructOf test case that gccgo used to fail
Updates #25284

Change-Id: I8ca382dd85b428ad6899d9277cf7f3ce34e35e9a
Reviewed-on: https://go-review.googlesource.com/116416
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
2018-06-06 14:08:42 +00:00
Ian Lance Taylor
e7ee3b91c4 reflect: check that types match without calling TypeOf
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>
2018-06-05 20:00:37 +00:00
Robert Griesemer
47be3d49c7 reflect: use 'defined' rather than 'named', use 'embedded' rather than 'anonymous'
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>
2018-05-10 23:19:40 +00:00
Ian Lance Taylor
9558ba293f reflect: add embedded field test
Gccgo failed this test.

Updates #23620

Change-Id: I3979a6d3b87d2d014850accf9cb7f356349e6195
Reviewed-on: https://go-review.googlesource.com/91138
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
2018-02-14 15:28:25 +00:00
Russ Cox
eafa29bdce reflect: fix interface to interface conversion in Call
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>
2017-12-01 15:31:31 +00:00
Tim Cooper
245e386e4c reflect: allow Copy to a byte array or byte slice from a string
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>
2017-10-13 02:35:56 +00:00
Matthew Dempsky
6f1724ff41 reflect: fix method indexing for non-ASCII exported methods
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>
2017-09-28 18:06:53 +00:00
Matthew Dempsky
6471ace2ea reflect: fix mutability of non-exported embedded fields
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>
2017-09-27 18:03:28 +00:00
Cherry Zhang
0a48185b43 reflect: fix pointer past-the-end in Call with zero-sized return value
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>
2017-09-19 20:45:24 +00:00
Matthew Dempsky
d349fa25df cmd/compile: fix and improve struct field reflect information
The previous logic was overly complicated, generated suboptimally
encoded struct type descriptors, and mishandled embeddings of
predeclared universal types.

Fixes #21122.
Fixes #21353.
Fixes #21696.
Fixes #21702.
Updates #21357.

Change-Id: If34761fa6dbe4af2af59dee501e7f30845320376
Reviewed-on: https://go-review.googlesource.com/60410
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
2017-09-05 18:09:41 +00:00
André Carvalho
77b4beba2f reflect: handle types with unexported methods before exported ones
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>
2017-08-23 23:48:49 +00:00
Ian Lance Taylor
87d5f6b9f6 reflect: make StructOf panic for methods that don't work
When StructOf is used with an anonymous field that has methods, and
that anonymous field is not the first field, the methods we generate
are incorrect because they do not offset to the field as required.
If we encounter that case, panic rather than doing the wrong thing.

Fixes #20824
Updates #15924

Change-Id: I3b0901ddbc6d58af5f7e84660b5e3085a431035d
Reviewed-on: https://go-review.googlesource.com/47035
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-07-15 00:47:41 +00:00
Pravendra Singh
538b3a5f37 reflect: prevent structs with invalid field name
According to the language spec, a struct field name should
be an identifier.

  identifier = letter { letter | unicode_digit } .
  letter = unicode_letter | "_" .

Implements a function 'isValidFieldName(fieldName string) bool'.
To check if the field name is a valid identifier or not.
It will panic if the field name is invalid.

It uses the non-exported function implementation 'isLetter'
from the package 'scanner', used to parse an identifier.

Fixes #20600.

Change-Id: I1db7db1ad88cab5dbea6565be15cc7461cc56c44
Reviewed-on: https://go-review.googlesource.com/45590
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-06-13 21:51:17 +00:00
Daniel Martí
9bced47706 reflect: don't panic in ArrayOf if elem size is 0
We do a division by the elem type size to check if the array size would
be too large for the virtual address space. This is a silly check if the
size is 0, but the problem is that it means a division by zero and a
panic.

Since arrays of empty structs are valid in a regular program, make them
also work in reflect.

Use a separate, explicit test with struct{}{} to make sure the test for
a zero-sized type is not confused with the rest.

Fixes #20313.

Change-Id: I47b8b87e6541631280b79227bdea6a0f6035c9e0
Reviewed-on: https://go-review.googlesource.com/43131
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-05-10 13:44:53 +00:00
Daniel Martí
a4864094f0 reflect: fix String of new array types
When constructing a new type for an array type in ArrayOf, we don't
reset tflag to 0. All the other methods in the package, such as SliceOf,
do this already. This results in the new array type having weird issues
when being printed, such as having tflagExtraStar set when it shouldn't.

That flag removes the first char to get rid of '*', but when used
incorrectly in this case it eats the '[' character leading to broken
strings like "3]int".

This was fixed in 56752eb2 for issue #16722, but ArrayOf was missed.

Also make the XM test struct have a non-zero size as that leads to a
division by zero panic in ArrayOf.

Fixes #20311.

Change-Id: I18f1027fdbe9f71767201e7424269c3ceeb23eb5
Reviewed-on: https://go-review.googlesource.com/43130
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-05-10 11:42:46 +00:00
Bryan C. Mills
f5f5a00b92 reflect: parallelize benchmarks
Add a benchmark for PtrTo: it's the motivation for #17973, which is
the motivation for #18177.

Results remain comparable with the non-parallel version with -cpu=1:

benchmark                             old ns/op     new ns/op     delta
BenchmarkCall                         357           360           +0.84%
BenchmarkCall-6                       90.3          90.7          +0.44%
BenchmarkCallArgCopy/size=128         319           323           +1.25%
BenchmarkCallArgCopy/size=128-6       329           82.2          -75.02%
BenchmarkCallArgCopy/size=256         354           335           -5.37%
BenchmarkCallArgCopy/size=256-6       340           85.2          -74.94%
BenchmarkCallArgCopy/size=1024        374           703           +87.97%
BenchmarkCallArgCopy/size=1024-6      378           95.8          -74.66%
BenchmarkCallArgCopy/size=4096        627           631           +0.64%
BenchmarkCallArgCopy/size=4096-6      643           120           -81.34%
BenchmarkCallArgCopy/size=65536       10502         10169         -3.17%
BenchmarkCallArgCopy/size=65536-6     10298         2240          -78.25%
BenchmarkFieldByName1                 139           132           -5.04%
BenchmarkFieldByName1-6               144           24.9          -82.71%
BenchmarkFieldByName2                 2721          2778          +2.09%
BenchmarkFieldByName2-6               3953          578           -85.38%
BenchmarkFieldByName3                 19136         18357         -4.07%
BenchmarkFieldByName3-6               23072         3850          -83.31%
BenchmarkInterfaceBig                 12.7          15.5          +22.05%
BenchmarkInterfaceBig-6               14.2          2.48          -82.54%
BenchmarkInterfaceSmall               13.1          15.1          +15.27%
BenchmarkInterfaceSmall-6             13.0          2.54          -80.46%
BenchmarkNew                          43.8          43.0          -1.83%
BenchmarkNew-6                        40.5          6.67          -83.53%

benchmark                             old MB/s     new MB/s     speedup
BenchmarkCallArgCopy/size=128         400.24       395.15       0.99x
BenchmarkCallArgCopy/size=128-6       388.74       1557.76      4.01x
BenchmarkCallArgCopy/size=256         722.44       762.44       1.06x
BenchmarkCallArgCopy/size=256-6       751.98       3003.83      3.99x
BenchmarkCallArgCopy/size=1024        2733.22      1455.50      0.53x
BenchmarkCallArgCopy/size=1024-6      2706.40      10687.53     3.95x
BenchmarkCallArgCopy/size=4096        6523.32      6488.25      0.99x
BenchmarkCallArgCopy/size=4096-6      6363.85      34003.09     5.34x
BenchmarkCallArgCopy/size=65536       6239.88      6444.46      1.03x
BenchmarkCallArgCopy/size=65536-6     6363.83      29255.26     4.60x

benchmark           old allocs     new allocs     delta
BenchmarkCall       0              0              +0.00%
BenchmarkCall-6     0              0              +0.00%

benchmark           old bytes     new bytes     delta
BenchmarkCall       0             0             +0.00%
BenchmarkCall-6     0             0             +0.00%

updates #17973
updates #18177

Change-Id: If70c5c742e8d1b138347f4963ad7cff38fffc018
Reviewed-on: https://go-review.googlesource.com/36831
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-04-26 19:04:30 +00:00
Filip Gruszczyński
6c5a819a5e reflect: add MakeMapWithSize for creating maps with size hint
Providing size hint when creating a map allows avoiding re-allocating
underlying data structure if we know how many elements are going to
be inserted. This can be used for example during decoding maps in
gob.

Fixes #19599

Change-Id: I108035fec29391215d2261a73eaed1310b46bab1
Reviewed-on: https://go-review.googlesource.com/38335
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-04-04 20:01:43 +00:00
Austin Clements
627798db4e reflect: fix out-of-bounds pointers calling no-result method
reflect.callReflect heap-allocates a stack frame and then constructs
pointers to the arguments and result areas of that frame. However, if
there are no results, the results pointer will point past the end of
the frame allocation. If there are also no arguments, the arguments
pointer will also point past the end of the frame allocation. If the
GC observes either these pointers, it may panic.

Fix this by not constructing these pointers if these areas of the
frame are empty.

This adds a test of calling no-argument/no-result methods via reflect,
since nothing in std did this before. However, it's quite difficult to
demonstrate the actual failure because it depends on both exact
allocation patterns and on GC scanning the goroutine's stack while
inside one of the typedmemmovepartial calls.

I also audited other uses of typedmemmovepartial and
memclrNoHeapPointers in reflect, since these are the most susceptible
to this. These appear to be the only two cases that can construct
out-of-bounds arguments to these functions.

Fixes #19724.

Change-Id: I4b83c596b5625dc4ad0567b1e281bad4faef972b
Reviewed-on: https://go-review.googlesource.com/38736
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-03-29 15:28:49 +00:00
Michael Hudson-Doyle
066ac428cd reflect: clear ptrToThis in Ptr when allocating result on heap
Otherwise, calling PtrTo on the result will fail.

Fixes #19003

Change-Id: I8d7d1981a5d0417d5aee52740469d71e90734963
Reviewed-on: https://go-review.googlesource.com/36731
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2017-02-10 17:22:07 +00:00
Russ Cox
c47df7ae17 all: merge dev.typealias into master
For #18130.

f8b4123613 [dev.typealias] spec: use term 'embedded field' rather than 'anonymous field'
9ecc3ee252 [dev.typealias] cmd/compile: avoid false positive cycles from type aliases
49b7af8a30 [dev.typealias] reflect: add test for type aliases
9bbb07ddec [dev.typealias] cmd/compile, reflect: fix struct field names for embedded byte, rune
43c7094386 [dev.typealias] reflect: fix StructOf use of StructField to match StructField docs
9657e0b077 [dev.typealias] cmd/doc: update for type alias
de2e5459ae [dev.typealias] cmd/compile: declare methods after resolving receiver type
9259f3073a [dev.typealias] test: match gccgo error messages on alias2.go
5d92916770 [dev.typealias] cmd/compile: change Func.Shortname to *Sym
a7c884efc1 [dev.typealias] go/internal/gccgoimporter: support for type aliases
5802cfd900 [dev.typealias] cmd/compile: export/import test cases for type aliases
d7cabd40dd [dev.typealias] go/types: clarified doc string
cc2dcce3d7 [dev.typealias] cmd/compile: a few better comments related to alias types
5c160b28ba [dev.typealias] cmd/compile: improved error message for cyles involving type aliases
b2386dffa1 [dev.typealias] cmd/compile: type-check type alias declarations
ac8421f9a5 [dev.typealias] cmd/compile: various minor cleanups
f011e0c6c3 [dev.typealias] cmd/compile, go/types, go/importer: various alias related fixes
49de5f0351 [dev.typealias] cmd/compile, go/importer: define export format and implement importing of type aliases
5ceec42dc0 [dev.typealias] go/types: export TypeName.IsAlias so clients can use it
aa1f0681bc [dev.typealias] go/types: improved Object printing
c80748e389 [dev.typealias] go/types: remove some more vestiges of prior alias implementation
80d8b69e95 [dev.typealias] go/types: implement type aliases
a917097b5e [dev.typealias] go/build: add go1.9 build tag
3e11940437 [dev.typealias] cmd/compile: recognize type aliases but complain for now (not yet supported)
e0a05c274a [dev.typealias] cmd/gofmt: added test cases for alias type declarations
2e5116bd99 [dev.typealias] go/ast, go/parser, go/printer, go/types: initial type alias support

Change-Id: Ia65f2e011fd7195f18e1dce67d4d49b80a261203
2017-01-31 13:01:31 -05:00
Russ Cox
49b7af8a30 [dev.typealias] reflect: add test for type aliases
For #18130.

Change-Id: Idd77cb391178c185227cfd779c70fec16351f825
Reviewed-on: https://go-review.googlesource.com/35733
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-01-25 18:57:29 +00:00
Russ Cox
9bbb07ddec [dev.typealias] cmd/compile, reflect: fix struct field names for embedded byte, rune
Will also fix type aliases.

Fixes #17766.
For #18130.

Change-Id: I9e1584d47128782152e06abd0a30ef423d5c30d2
Reviewed-on: https://go-review.googlesource.com/35732
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-01-25 18:57:20 +00:00
Russ Cox
43c7094386 [dev.typealias] reflect: fix StructOf use of StructField to match StructField docs
The runtime internal structField interprets name=="" as meaning anonymous,
but the exported reflect.StructField has always set Name, even for anonymous
fields, and also set Anonymous=true.

The initial implementation of StructOf confused the internal and public
meanings of the StructField, expecting the runtime representation of
anonymous fields instead of the exported reflect API representation.
It also did not document this fact, so that users had no way to know how
to create an anonymous field.

This CL changes StructOf to use the previously documented interpretation
of reflect.StructField instead of an undocumented one.

The implementation of StructOf also, in some cases, allowed creating
structs with unexported fields (if you knew how to ask) but set the
PkgPath incorrectly on those fields. Rather than try to fix that, this CL
changes StructOf to reject attempts to create unexported fields.
(I think that may be the right design choice, not just a temporary limitation.
In any event, it's not the topic for today's work.)

For #17766.
Fixes #18780.

Change-Id: I585a4e324dc5a90551f49d21ae04d2de9ea04b6c
Reviewed-on: https://go-review.googlesource.com/35731
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-01-25 18:56:51 +00:00
Austin Clements
22689c4450 reflect: keep makeFuncImpl live across makeFuncStub
When traceback sees reflect.makeFuncStub (or reflect.methodValueCall)
on the stack, it expects to be able to get the *reflect.makeFuncImpl
(or *reflect.methodValue) for that call from the first outgoing
argument slot of makeFuncStub/methodValueCall.

However, currently this object isn't necessarily kept live across
makeFuncStub. This means it may get garbage collected while in a
reflect call and reused for something else. If we then try to
traceback, the runtime will see a corrupted makeFuncImpl object and
panic. This was not a problem in previous releases because we always
kept arguments live across the whole function. This became a problem
when we stopped doing this.

Fix this by using reflect.KeepAlive to keep the
makeFuncImpl/methodValue live across all of callReflect/callMethod,
which in turn keeps it live as long as makeFuncStub/methodValueCall
are on the stack.

Fixes #18635.

Change-Id: I91853efcf17912390fddedfb0230648391c33936
Reviewed-on: https://go-review.googlesource.com/35151
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-01-13 03:45:28 +00:00
Ian Lance Taylor
50c4dbced9 reflect: fix size of StructOf ending in zero-sized field
Update #9401.
Fixes #18016.

Change-Id: Icc24dd10dab1ad8e5cf295e0727d437afa5025c0
Reviewed-on: https://go-review.googlesource.com/33475
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2016-11-23 00:44:01 +00:00
Ian Lance Taylor
011cb64231 cmd/compile, reflect: use field pkgPath if needed
It's possible for the pkgPath of a field to be different than that of
the struct type as a whole. In that case, store the field's pkgPath in
the name field. Use the field's pkgPath when setting PkgPath and when
checking for type identity.

Fixes #17952.

Change-Id: Iebaf92f0054b11427c8f6e4158c3bebcfff06f45
Reviewed-on: https://go-review.googlesource.com/33333
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
2016-11-17 01:19:46 +00:00
David Crawshaw
66477ec830 reflect: rename, document TestUnaddressableField
Change-Id: I94e0f3e4bccd44a67934ddb4d5fc7da57bb8ac9f
Reviewed-on: https://go-review.googlesource.com/33112
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
2016-11-11 14:38:40 +00:00
David Crawshaw
8d0c105407 reflect: unexported fields are tied to a package
An unexported field of a struct is not visible outside of the package
that defines it, so the package path is implicitly part of the
definition of any struct with an unexported field.

Change-Id: I17c6aac822bd0c24188ab8ba1cc406d6b5d82771
Reviewed-on: https://go-review.googlesource.com/32820
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2016-11-10 14:06:23 +00:00
Josh Bleecher Snyder
accf5cc386 all: minor vet fixes
Change-Id: I22f0f3e792052762499f632571155768b4052bc9
Reviewed-on: https://go-review.googlesource.com/31759
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-10-24 17:27:37 +00:00
Russ Cox
19adf8aeaa reflect: fix DeepEqual for some cyclic corner cases
Fixes #15610.

Change-Id: Idbc8a9b328b92034d53b8009471678a166d5cf3f
Reviewed-on: https://go-review.googlesource.com/31588
TryBot-Result: Gobot Gobot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Quentin Smith <quentin@golang.org>
2016-10-24 15:20:23 +00:00
Robert Griesemer
ddb77100a6 reflect: ignore struct tags when converting structs
Implementation of spec change https://golang.org/cl/24190/.

For #16085.

Change-Id: Ib7cb513354269282dfad663c7d2c6e624149f3cd
Reviewed-on: https://go-review.googlesource.com/30191
Reviewed-by: David Crawshaw <crawshaw@golang.org>
2016-10-04 17:15:59 +00:00
Brad Fitzpatrick
ca04091f5b reflect: add Swapper func
Swapper returns a func that swaps two elements in a slice.

Updates #16721

Change-Id: I7f2287a675c10a05019e02b7d62fb870af31216f
Reviewed-on: https://go-review.googlesource.com/30088
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2016-09-30 20:26:54 +00:00
Michael Munday
3436f0776f reflect, runtime: optimize Value.Call on s390x and add benchmark
Use an MVC loop to copy arguments in runtime.call* rather than copying
bytes individually.

I've added the benchmark CallArgCopy to test the speed of Value.Call
for various argument sizes.

name                    old speed      new speed       delta
CallArgCopy/size=128     439MB/s ± 1%    582MB/s ± 1%   +32.41%  (p=0.000 n=10+10)
CallArgCopy/size=256     695MB/s ± 1%   1172MB/s ± 1%   +68.67%  (p=0.000 n=10+10)
CallArgCopy/size=1024    573MB/s ± 8%   4175MB/s ± 2%  +628.11%  (p=0.000 n=10+10)
CallArgCopy/size=4096   1.46GB/s ± 2%  10.19GB/s ± 1%  +600.52%  (p=0.000 n=10+10)
CallArgCopy/size=65536  1.51GB/s ± 0%  12.30GB/s ± 1%  +716.30%   (p=0.000 n=9+10)

Change-Id: I87dae4809330e7964f6cb4a9e40e5b3254dd519d
Reviewed-on: https://go-review.googlesource.com/28096
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bill O'Farrell <billotosyr@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-09-27 16:00:19 +00:00
David Crawshaw
56752eb2b8 reflect: clear tflag on new types
Fixes #16722

Change-Id: I50a0e69d3e79d13bc1860cd983267c3db087a4b8
Reviewed-on: https://go-review.googlesource.com/27119
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-08-16 16:58:15 +00:00
Ian Lance Taylor
1f4e68d92b reflect: an unnamed type has no PkgPath
The reflect package was returning a non-empty PkgPath for an unnamed
type with methods, such as a type whose methods have a pointer
receiver.

Fixes #16328.

Change-Id: I733e93981ebb5c5c108ef9b03bf5494930b93cf3
Reviewed-on: https://go-review.googlesource.com/24862
Reviewed-by: David Crawshaw <crawshaw@golang.org>
2016-07-12 12:43:48 +00:00