With its new -linecomment flag, it is now possible to use stringer on
values whose strings aren't valid identifiers. This is the case with
tokens and operators in Go.
Operator alredy had inline comments with each operator's string
representation; only minor modifications were needed. The inline
comments were added to each of the token names, using the same strategy.
Comments that were previously inline or part of the string arrays were
moved to the line immediately before the name they correspond to.
Finally, declare tokStrFast as a function that uses the generated arrays
directly. Avoiding the branch and strconv call means that we avoid a
performance regression in the scanner, perhaps due to the lack of
mid-stack inlining.
Performance is not affected. Measured with 'go test -run StdLib -fast'
on an X1 Carbon Gen2 (i5-4300U @ 1.90GHz, 8GB RAM, SSD), the best of 5
runs before and after the changes are:
parsed 1709399 lines (3763 files) in 1.707402159s (1001169 lines/s)
allocated 449.282Mb (263.137Mb/s)
parsed 1709329 lines (3765 files) in 1.706663154s (1001562 lines/s)
allocated 449.290Mb (263.256Mb/s)
Change-Id: Idcc4f83393fcadd6579700e3602c09496ea2625b
Reviewed-on: https://go-review.googlesource.com/95357
Reviewed-by: Robert Griesemer <gri@golang.org>
The Info-ZIP Unix1 extra field is specified as such:
>>>
Value Size Description
----- ---- -----------
0x5855 Short tag for this extra block type ("UX")
TSize Short total data size for this block
AcTime Long time of last access (GMT/UTC)
ModTime Long time of last modification (GMT/UTC)
<<<
The previous handling was incorrect in that it read the AcTime field
instead of the ModTime field.
The test-osx.zip test unfortunately locked in the wrong behavior.
Manually parsing that ZIP file shows that the encoded MS-DOS
date and time are 0x4b5f and 0xa97d, which corresponds with a
date of 2017-10-31 21:11:58, which matches the correct mod time
(off by 1 second due to MS-DOS timestamp resolution).
Fixes#23901
Change-Id: I567824c66e8316b9acd103dbecde366874a4b7ef
Reviewed-on: https://go-review.googlesource.com/96895
Run-TryBot: Joe Tsai <joetsai@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
They have either already been called by preprintpanics, or they can
not be called safely because of the various conditions checked at the
start of gopanic.
Fixes#24059
Change-Id: I4a6233d12c9f7aaaee72f343257ea108bae79241
Reviewed-on: https://go-review.googlesource.com/96755
Reviewed-by: Austin Clements <austin@google.com>
Instead of calling Chmod directly on perm, stat the created file/dir to extract the
actual permission bits which can be different from perm due to umask.
Fixes#23120.
Change-Id: I3e70032451fc254bf48ce9627e98988f84af8d91
Reviewed-on: https://go-review.googlesource.com/84477
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently, we use 64MB heap arenas on 64-bit platforms. This works
well on UNIX-like OSes because they treat untouched pages as
essentially free. However, on Windows, committed memory is charged
against a process whether or not it has demand-faulted physical pages
in. Hence, on Windows, even a process with a tiny heap will commit
64MB for one heap arena, plus another 32MB for the arena map. Things
are much worse under the race detector, which increases the heap
commitment by a factor of 5.5X, leading to 384MB of committed memory
at runtime init.
Fix this by reducing the heap arena size to 4MB on Windows.
To counterbalance the effect of increasing the arena map size by a
factor of 16, and to further reduce the impact of the commitment for
the arena map, we switch from a single entry L1 arena map to a 64
entry L1 arena map.
Compared to the original arena design, this slows down the
x/benchmarks garbage benchmark by 0.49% (the slow down of this commit
alone is 1.59%, but the previous commit bought us a 1% speed-up):
name old time/op new time/op delta
Garbage/benchmem-MB=64-12 2.28ms ± 1% 2.29ms ± 1% +0.49% (p=0.000 n=17+18)
(https://perf.golang.org/search?q=upload:20180223.1)
(This was measured on linux/amd64 by modifying its arena configuration
as above.)
Fixes#23900.
Change-Id: I6b7fa5ecebee2947bf20cfeb78c248809469c6b1
Reviewed-on: https://go-review.googlesource.com/96780
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently, the heap arena map is a single, large array that covers
every possible arena frame in the entire address space. This is
practical up to about 48 bits of address space with 64 MB arenas.
However, there are two problems with this:
1. mips64, ppc64, and s390x support full 64-bit address spaces (though
on Linux only s390x has kernel support for 64-bit address spaces).
On these platforms, it would be good to support these larger
address spaces.
2. On Windows, processes are charged for untouched memory, so for
processes with small heaps, the mostly-untouched 32 MB arena map
plus a 64 MB arena are significant overhead. Hence, it would be
good to reduce both the arena map size and the arena size, but with
a single-level arena, these are inversely proportional.
This CL adds support for a two-level arena map. Arena frame numbers
are now divided into arenaL1Bits of L1 index and arenaL2Bits of L2
index.
At the moment, arenaL1Bits is always 0, so we effectively have a
single level map. We do a few things so that this has no cost beyond
the current single-level map:
1. We embed the L2 array directly in mheap, so if there's a single
entry in the L2 array, the representation is identical to the
current representation and there's no extra level of indirection.
2. Hot code that accesses the arena map is structured so that it
optimizes to nearly the same machine code as it does currently.
3. We make some small tweaks to hot code paths and to the inliner
itself to keep some important functions inlined despite their
now-larger ASTs. In particular, this is necessary for
heapBitsForAddr and heapBits.next.
Possibly as a result of some of the tweaks, this actually slightly
improves the performance of the x/benchmarks garbage benchmark:
name old time/op new time/op delta
Garbage/benchmem-MB=64-12 2.28ms ± 1% 2.26ms ± 1% -1.07% (p=0.000 n=17+19)
(https://perf.golang.org/search?q=upload:20180223.2)
For #23900.
Change-Id: If5164e0961754f97eb9eca58f837f36d759505ff
Reviewed-on: https://go-review.googlesource.com/96779
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
The front-end dead code elimination is very simple. Currently, it just
looks for if statements with constant boolean conditions. Its main
purpose is to reduce load on the compiler and shrink code before
inlining computes hairiness.
This CL teaches front-end dead code elimination about short-circuiting
boolean expressions && and ||, since they're essentially the same as
if statements.
This also teaches the inliner that the constant 'if' form left behind
by deadcode is free.
These changes will help with runtime modifications in the next CL that
would otherwise inhibit inlining in some hot code paths. Currently,
however, they have no significant impact on benchmarks.
Change-Id: I886203b3c4acdbfef08148fddd7f3a7af5afc7c1
Reviewed-on: https://go-review.googlesource.com/96778
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
There are too many places where I want to talk about "indexing into
the arena index". Make this less awkward and ambiguous by calling it
the "arena map" instead.
Change-Id: I726b0667bb2139dbc006175a0ec09a871cdf73f9
Reviewed-on: https://go-review.googlesource.com/96777
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Rick Hudson <rlh@golang.org>
On amd64, the arena is no longer in address space order, but currently
the heap dumper assumes that it is. Fix this assumption.
Change-Id: Iab1953cd36b359d0fb78ed49e5eb813116a18855
Reviewed-on: https://go-review.googlesource.com/96776
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
CL 86295 changed MkdirAll to always pass a trailing path separator to
support extended-length paths on Windows.
However, when Stat is called on an existing file followed by trailing
slash, it will return a "not a directory" error, skipping the fast
path at the beginning of MkdirAll.
This change fixes MkdirAll to only pass the trailing path separator
where required on Windows, by using an OS-specific function fixRootDirectory.
Updates #23918
Change-Id: I23f84a20e65ccce556efa743d026d352b4812c34
Reviewed-on: https://go-review.googlesource.com/95255
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David du Colombier <0intro@gmail.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Simply checking if a name is "atomic" isn't enough, as that might be a
var or another imported package. Now that vet requires type information,
we can do better. And add a simple regression test.
Change-Id: Ibd2004428374e3628cd3cd0ffb5f37cedaf448ea
Reviewed-on: https://go-review.googlesource.com/91795
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Robert Griesemer <gri@golang.org>
There are, sadly, many exceptions to EKU checking to reflect mistakes
that CAs have made in practice. However, the requirements for checking
requested EKUs against the leaf should be tighter than for checking leaf
EKUs against a CA.
Fixes#23884
Change-Id: I05ea874c4ada0696d8bb18cac4377c0b398fcb5e
Reviewed-on: https://go-review.googlesource.com/96379
Reviewed-by: Jonathan Rudenberg <jonathan@titanous.com>
Reviewed-by: Filippo Valsorda <hi@filippo.io>
Run-TryBot: Filippo Valsorda <hi@filippo.io>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If you try to find something in a slice of bytes using a Regexp object,
the byte array will not be released by GC until you use the Regexp object
on another slice of bytes. It happens because the Regexp object keep
references to the input data in its cache.
Change-Id: I873107f15c1900aa53ccae5d29dbc885b9562808
Reviewed-on: https://go-review.googlesource.com/96715
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Add "sqrt-intrisified" code generation tests for mips64 and 386, where
we weren't intrisifying math.Sqrt (see CL 96615 and CL 95916), and for
mips and amd64, which lacked sqrt intrinsics tests.
Change-Id: I0cfc08aec6eefd47f3cd7a5995a89393e8b7ed9e
Reviewed-on: https://go-review.googlesource.com/96716
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There was still the word 'Hashmap' in gc_test.go, so I renamed it to just 'Map'
Previous renaming commit: https://golang.org/cl/90336
Change-Id: I5b0e5c2229d1c30937c7216247f4533effb81ce7
Reviewed-on: https://go-review.googlesource.com/96675
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change or-ifies the last low-hanging rules in generic. Again,
this is limited at short and repetitive rules, where the use or ors
does not impact readability.
Ran rulegen, no change in the actual compiler code.
Change-Id: I972b523bc08532f173a3645b47d6936b6e1218c8
Reviewed-on: https://go-review.googlesource.com/96335
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Setting -panic will cause gotype to panic with the first reported
error, producing a stack trace for debugging.
For #23914.
Change-Id: I40c41cf10aa13d1dd9a099f727ef4201802de13a
Reviewed-on: https://go-review.googlesource.com/96375
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The syscall package is frozen and we don't want to encourage anyone to
implement these syscalls.
Change-Id: I6b6e33e32a4b097da6012226aa15300735e50e9f
Reviewed-on: https://go-review.googlesource.com/96315
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The variables on the lhs of a short variable declaration are
only in scope after the variable declaration. Specifically,
function literals on the rhs of a short variable declaration
must not see newly declared variables on the lhs.
This used to work and this bug was likely introduced with
https://go-review.googlesource.com/c/go/+/83397 for go1.11.
Luckily this is just an oversight and the fix is trivial:
Simply use the mechanism for delayed type-checkin of function
literals introduced in the before-mentioned change here as well.
Fixes#24026.
Change-Id: I74ce3a0d05c5a2a42ce4b27601645964f906e82d
Reviewed-on: https://go-review.googlesource.com/96177
Reviewed-by: Alan Donovan <adonovan@google.com>
For now, limited to the most repetitive rules that are also short and
simple, so that we can have a substantial conciseness win without
compromising rules readability.
Ran rulegen, no changes in the rewrite files.
Change-Id: I8447784895a218c5c1b4dfa1cdb355bd73dabfd1
Reviewed-on: https://go-review.googlesource.com/95955
Reviewed-by: Giovanni Bajo <rasky@develer.com>
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>
While tinkering with different block orders for the preemptible
loop experiment, crashed the register allocator with a "bad"
one (these exist). Realized that one knob was controlling
two things (register allocation and branch patterns) and
decided that life would be simpler if the two orders were
independent.
Ran some experiments and determined that we have probably,
mostly, been optimizing for register allocation effects, not
branch effects. Bad block orders for register allocation are
somewhat costly.
This will also allow separate experimentation with perhaps-
better block orders for register allocation.
Change-Id: I6ecf2f24cca178b6f8acc0d3c4caaef043c11ed9
Reviewed-on: https://go-review.googlesource.com/47314
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Enabled when the tool runs with DEBUG_MEMORY_USAGE=1 env var.
After reporting the usage, it waits until user enters input
(helpful when checking top or other memory monitor)
Also adds net/http/pprof to export debug endpoints.
From the trace included in #21870
$ DEBUG_MEMORY_USAGE=1 go tool trace trace.out
2018/02/21 16:04:49 Parsing trace...
after parsing trace
Alloc: 3385747848 Bytes
Sys: 3661654648 Bytes
HeapReleased: 0 Bytes
HeapSys: 3488907264 Bytes
HeapInUse: 3426377728 Bytes
HeapAlloc: 3385747848 Bytes
Enter to continue...
2018/02/21 16:05:09 Serializing trace...
after generating trace
Alloc: 4908929616 Bytes
Sys: 5319063640 Bytes
HeapReleased: 0 Bytes
HeapSys: 5032411136 Bytes
HeapInUse: 4982865920 Bytes
HeapAlloc: 4908929616 Bytes
Enter to continue...
2018/02/21 16:05:18 Splitting trace...
after spliting trace
Alloc: 4909026200 Bytes
Sys: 5319063640 Bytes
HeapReleased: 0 Bytes
HeapSys: 5032411136 Bytes
HeapInUse: 4983046144 Bytes
HeapAlloc: 4909026200 Bytes
Enter to continue...
2018/02/21 16:05:39 Opening browser. Trace viewer is listening on http://127.0.0.1:33661
after httpJsonTrace
Alloc: 5288336048 Bytes
Sys: 7790245896 Bytes
HeapReleased: 0 Bytes
HeapSys: 7381123072 Bytes
HeapInUse: 5324120064 Bytes
HeapAlloc: 5288336048 Bytes
Enter to continue...
Change-Id: I88bb3cb1af3cb62e4643a8cbafd5823672b2e464
Reviewed-on: https://go-review.googlesource.com/92355
Reviewed-by: Peter Weinberger <pjw@google.com>
The existing code was somewhat convoluted and made several assumptions
about the encoding of position bases:
1) The position's base for a file contained a position whose base
pointed to itself (which is true but an implementation detail
of src.Pos).
2) Updating the position base for a line directive required finding
the base of the most recent's base position.
This change simply stores the file's position base and keeps using it
directly for each line directive (instead of getting it from the most
recently updated base).
Change-Id: I4d80da513bededb636eab0ce53257fda73f0dbc0
Reviewed-on: https://go-review.googlesource.com/95736
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Now that we support the full non-contiguous virtual address space of
amd64 hardware, some of the comments and constants related to this are
out of date.
This renames memLimitBits to heapAddrBits because 1<<memLimitBits is
no longer the limit of the address space and rewrites the comment to
focus first on hardware limits (which span OSes) and then discuss
kernel limits.
Second, this eliminates the memLimit constant because there's no
longer a meaningful "highest possible heap pointer value" on amd64.
Updates #23862.
Change-Id: I44b32033d2deb6b69248fb8dda14fc0e65c47f11
Reviewed-on: https://go-review.googlesource.com/95498
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
On amd64, the virtual address space, when interpreted as signed
values, is [-2^47, 2^47). Currently, we only support heap addresses in
the "positive" half of this, [0, 2^47). This suffices for linux/amd64
and windows/amd64, but solaris/amd64 can map user addresses in the
negative part of this range. Specifically, addresses
0xFFFF8000'00000000 to 0xFFFFFD80'00000000 are part of user space.
This leads to "memory allocated by OS not in usable address space"
panic, since we don't map heap arena index space for these addresses.
Fix this by offsetting addresses when computing arena indexes so that
arena entry 0 corresponds to address -2^47 on amd64. We already map
enough arena space for 2^48 heap addresses on 64-bit (because arm64's
virtual address space is [0, 2^48)), so we don't need to grow any
structures to support this.
A different approach would be to simply mask out the top 16 bits.
However, there are two advantages to the offset approach: 1) invalid
heap addresses continue to naturally map to invalid arena indexes so
we don't need extra checks and 2) it perturbs the mapping of addresses
to arena indexes more, which helps check that we don't accidentally
compute incorrect arena indexes somewhere that happen to be right most
of the time.
Several comments and constant names are now somewhat misleading. We'll
fix that in the next CL. This CL is the core change the arena
indexing.
Fixes#23862.
Change-Id: Idb8e299fded04593a286b01a9582da6ddbac2f9a
Reviewed-on: https://go-review.googlesource.com/95497
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Accessing the arena index is about to get slightly more complicated.
Abstract this away into a set of functions for going back and forth
between addresses and arena slice indexes.
For #23862.
Change-Id: I0b20e74ef47a07b78ed0cf0a6128afe6f6e40f4b
Reviewed-on: https://go-review.googlesource.com/95496
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently, bulkBarrierPreWrite uses inheap to decide whether the
destination is in the heap or whether to check for stack or global
data. However, this isn't the best question to ask.
Instead, get the span directly and query its state. This lets us
directly determine whether this might be a global, or is stack memory,
or is heap memory.
At this point, inheap is no longer used in the hot path, so drop it
from the must-be-inlined list and substitute spanOf.
This will help in a circuitous way with #23862, since fixing that is
going to push inheap very slightly over the inline-able threshold on a
few platforms.
Change-Id: I5360fc1181183598502409f12979899e1e4d45f7
Reviewed-on: https://go-review.googlesource.com/95495
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
The task-oriented trace view presents the execution trace organized
based on goroutines. Often, which P a goroutine was running on is
useful, so this CL includes the P ids in the goroutine execution slices.
R=go1.11
Change-Id: I96539bf8215e5c1cd8cc997a90204f57347c48c8
Reviewed-on: https://go-review.googlesource.com/90221
Reviewed-by: Heschi Kreinick <heschi@google.com>
The GC time for a task is defined by the sum of GC duration
overlapping with the task's duration.
Also, grey out non-overlapping slices in the task-oriented
trace view.
R=go1.11
Change-Id: I42def0eb520f5d9bd07edd265e558706f6fab552
Reviewed-on: https://go-review.googlesource.com/90219
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reuse even more memory, and keep track of it in a long-lived debugState
object rather than piecemeal in the Cache.
Change-Id: Ib6936b4e8594dc6dda1f59ece753c00fd1c136ba
Reviewed-on: https://go-review.googlesource.com/92404
Reviewed-by: David Chase <drchase@google.com>
Change the closures to methods on debugState, mostly just for aesthetic
reasons.
Change-Id: I5242807f7300efafc7efb4eb3bd305ac3ec8e826
Reviewed-on: https://go-review.googlesource.com/92403
Reviewed-by: David Chase <drchase@google.com>
changedVars was functionally a set, but couldn't be iterated over
efficiently. In functions with many variables, the wasted iteration was
costly. Use a sparseSet instead.
(*gc.Node).String() is very expensive: it calls Sprintf, which does
reflection, etc, etc. Instead, just look at .Sym.Name, which is all we
care about.
Change-Id: Ib61cd7b5c796e1813b8859135e85da5bfe2ac686
Reviewed-on: https://go-review.googlesource.com/92402
Reviewed-by: David Chase <drchase@google.com>
Replace the OnStack boolean in VarLoc with a flag bit in StackOffset.
This doesn't get much memory savings since it's still 64-bit aligned,
but does seem to help a bit anyway.
Change liveSlot to fit into 16 bytes. Because nested structs still get
padding, this required inlining it. Fortunately there's not much logic
to copy.
Change-Id: Ie19a409daa41aa310275c4517a021eecf8886441
Reviewed-on: https://go-review.googlesource.com/92401
Reviewed-by: David Chase <drchase@google.com>
For now, limited to the most repetitive rules that are also short and
simple, so that we can have a substantial conciseness win without
compromising rules readability.
Ran rulegen, no changes in the rewrite files.
Change-Id: I8d8cc67d02faca4756cc02402b763f1645ee31de
Reviewed-on: https://go-review.googlesource.com/95935
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
For now, limited to the most repetitive rules that are also short and
simple, so that we can have a substantial conciseness win without
compromising rules readability.
Ran rulegen, no change in the actual compiler code (as expected).
Change-Id: Ia74acc389cd8310eb7fe8f927171fa3d292d2a86
Reviewed-on: https://go-review.googlesource.com/95797
Reviewed-by: Giovanni Bajo <rasky@develer.com>
For now, limited to the most repetitive rules that are also short and
simple, so that we can have a substantial conciseness win without
compromising rules readability.
Ran rulegen, no change in the actual compiler code (as expected).
Change-Id: Ib0bfbbc181fcec095fb78ac752addd1eee0c3575
Reviewed-on: https://go-review.googlesource.com/95796
Reviewed-by: Giovanni Bajo <rasky@develer.com>
No changes in the generated file, as expected.
Change-Id: I30e0404612cd150f1455378b8db1c18b1e12d34e
Reviewed-on: https://go-review.googlesource.com/95616
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We did warn on them in some cases, but not others. In particular, if one
used a slice composite literal with struct pointer elements, and omitted
the type of an element's composite literal, it would not get any warning
even if it should get one.
The issue is that typ.Underlying() can be of type *types.Pointer. Skip
those levels of indirection before checking for a *types.Struct
underlying type.
isLocalType also needed a bit of tweaking to ignore dereferences.
Perhaps that can be rewritten now that we have type info, but let's
leave it for another time.
Fixes#23539.
Change-Id: I727a497284df1325b70d47a756519f5db1add25d
Reviewed-on: https://go-review.googlesource.com/89715
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
