Lorenz Bauer [Wed, 10 Jan 2018 10:50:19 +0000 (10:50 +0000)]
sync: enable profiling of RWMutex
Include reader / writer interactions of RWMutex in the mutex profile.
Writer contention is already included in the profile, since a plain Mutex
is used to control exclusion.
David Crawshaw [Sun, 24 Sep 2017 14:48:18 +0000 (10:48 -0400)]
runtime: remove extraneous stackPreempt setting
The stackguard is set to stackPreempt earlier in reentersyscall, and
as it comes with throwsplit = true there's no way for the stackguard
to be set to anything else by the end of reentersyscall.
Ben Shi [Wed, 7 Feb 2018 12:24:41 +0000 (12:24 +0000)]
cmd/compile/internal/ssa: optimize arm64 with FNMULS/FNMULD
FNMULS&FNMULD are efficient arm64 instructions, which can be used
to improve FP performance. This CL use them to optimize pairs of neg-mul
operations.
Here are benchmark test results on Raspberry Pi 3 with ArchLinux.
1. A special test case gets about 15% improvement.
(https://github.com/benshi001/ugo1/blob/master/fpmul_test.go)
FPMul-4 485µs ± 0% 410µs ± 0% -15.49% (p=0.000 n=26+23)
Daniel Martí [Thu, 11 Jan 2018 20:23:22 +0000 (20:23 +0000)]
path/filepath: fix escaped chars in Glob on non-Windows
Backslashes are ignored in Match and Glob on Windows, since those
collide with the separator character. However, they should still work in
both functions on other operating systems.
hasMeta did not reflect this logic - it always treated a backslash as a
non-special character. Do that only on Windows.
Assuming this is what the TODO was referring to, remove it. There are no
other characters that scanChunk treats especially.
Fixes #23418.
Change-Id: Ie0bd795812e0ed9d8c8c1bbc3137f29d960cba84
Reviewed-on: https://go-review.googlesource.com/87455
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Daniel Martí [Tue, 16 Jan 2018 16:55:06 +0000 (16:55 +0000)]
path: remove filename mentions from pattern godocs
path.Match works purely with strings, not file paths. That's what sets
it apart from filepath.Match. For example, only filepath.Match will
change its behavior towards backslashes on Windows, to accomodate for
the file path separator on that system.
As such, path.Match should make no mention of file names. Nor should
path.ErrBadPattern mention globbing at all - the package has no notion
of globbing, and the error concerns only patterns.
For a similar reason, remove the mention of globbing from
filepath.ErrBadPattern. The error isn't reserved to just globbing, as it
can be returned from filepath.Match. And, as before, it only concerns
the patterns themselves.
Change-Id: I58a83ffa3e2549625d8e546ef916652525504bd1
Reviewed-on: https://go-review.googlesource.com/87857 Reviewed-by: Rob Pike <r@golang.org>
Martin Möhrmann [Wed, 17 Jan 2018 19:26:23 +0000 (20:26 +0100)]
cmd/compile: change type of clear argument of ordercopyexpr to bool
ordercopyexpr is only called with 0 or 1 as value for the clear
argument. The clear variable in ordercopyexpr is only used in the
call to ordertemp which has a clear argument of type bool.
Change the clear argument of ordercopyexpr from int to bool and change
calls to ordercopyexpr to use false instead of 0 and true instead of 1.
Passes toolstash -cmp.
Change-Id: Ic264aafd3b0c8b99f6ef028ffaa2e30f23f9125a
Reviewed-on: https://go-review.googlesource.com/88115 Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Agniva De Sarker [Thu, 11 Jan 2018 16:49:01 +0000 (22:19 +0530)]
doc/articles/wiki: highlight the use of _ warning
This moves the paragraph mentioning the use of _ higher up
to emphasize the warning and thereby reducing chances of getting
stuck.
Fixes #22617
Change-Id: I64352a3e966a22d86fc9d381332bade49d74714a
Reviewed-on: https://go-review.googlesource.com/87375 Reviewed-by: Ian Lance Taylor <iant@golang.org>
David du Colombier [Tue, 13 Feb 2018 21:11:20 +0000 (22:11 +0100)]
runtime/trace: fix TestTraceSymbolize when GOMAXPROCS=1
CL 92916 added the GOMAXPROCS test in TestTraceSymbolize.
This test only succeeds when the value of GOMAXPROCS changes.
Since the test calls runtime.GOMAXPROCS(1), it will fails
on machines where GOMAXPROCS=1.
This change fixes the test by calling runtime.GOMAXPROCS(oldGoMaxProcs+1).
Fixes #23816.
Change-Id: I1183dbbd7db6077cbd7fa0754032ff32793b2195
Reviewed-on: https://go-review.googlesource.com/93735
Run-TryBot: David du Colombier <0intro@gmail.com> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
HaraldNordgren [Sun, 7 Jan 2018 14:08:59 +0000 (15:08 +0100)]
database/sql: include SQL column name in Scan() error message
When 'convertAssign' gives an error, instead of giving just the index of
the failing column -- which is not always helpful, especially when there
are lots of columns in the query -- utilize 'rs.rowsi.Columns()' to
extract the underlying column name and include that in the error string:
sql: Scan error on column index 0, name "some_column": ...
Fixes #23362
Change-Id: I0fe71ff3c25f4c0dd9fc6aa2c2da2360dd93e3e0
Reviewed-on: https://go-review.googlesource.com/86537 Reviewed-by: Harald Nordgren <haraldnordgren@gmail.com> Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Austin Clements [Wed, 31 Jan 2018 17:05:24 +0000 (12:05 -0500)]
runtime: avoid bad unwinding from sigpanic in C code
Currently, if a sigpanic call is injected into C code, it's possible
for preparePanic to leave the stack in a state where traceback can't
unwind correctly past the sigpanic.
Specifically, shouldPushPanic sniffs the stack to decide where to put
the PC from the signal context. In the cgo case, it will find that
!findfunc(pc).valid() because pc is in C code, and then it will check
if the top of the stack looks like a Go PC. However, this stack slot
is just in a C frame, so it could be uninitialized and contain
anything, including what looks like a valid Go PC. For example, in
https://build.golang.org/log/c601a18e2af24794e6c0899e05dddbb08caefc17,
it sees 1c02c23a <runtime.newproc1+682>. When this condition is met,
it skips putting the signal PC on the stack at all. As a result, when
we later unwind from the sigpanic, we'll "successfully" but
incorrectly unwind to whatever PC was in this uninitialized slot and
go who knows where from there.
Fix this by making shouldPushPanic assume that the signal PC is always
usable if we're running C code, so we always make it appear like
sigpanic's caller.
This lets us be pickier again about unexpected return PCs in
gentraceback.
Updates #23640.
Change-Id: I1e8ade24b031bd905d48e92d5e60c982e8edf160
Reviewed-on: https://go-review.googlesource.com/91137
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Peter Teichman [Mon, 12 Feb 2018 19:23:31 +0000 (19:23 +0000)]
image/gif: support non-looping animated gifs (LoopCount=-1)
The Netscape looping application extension encodes how many
times the animation should restart, and if it's present
there is no way to signal that a GIF should play only once.
Use LoopCount=-1 to signal when a decoded GIF had no looping
extension, and update the encoder to omit that extension
block when LoopCount=-1.
Change-Id: I2b88fbdfc250cd548f8f08b44ce2eb172dcacf43
Reviewed-on: https://go-review.googlesource.com/84437 Reviewed-by: Giovanni Bajo <rasky@develer.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Emmanuel Odeke [Sun, 11 Feb 2018 04:10:26 +0000 (20:10 -0800)]
cmd/compile: report the struct type in invalid number of initializer values
Fixes #23732
Disambiguate "too few" or "too many" values in struct
initializer messages by reporting the name of the literal.
After:
issue23732.go:27:3: too few values in Foo literal
issue23732.go:34:12: too many values in Bar literal
issue23732.go:40:6: too few values in Foo literal
issue23732.go:40:12: too many values in Bar literal
Hana Kim [Tue, 16 Jan 2018 20:31:12 +0000 (15:31 -0500)]
runtime/gdb: use goroutine atomicstatus to determine the state
Previously find_goroutine determined whether a goroutine is
stopped by checking the sched.sp field. This heuristic doesn't
always hold but causes find_goroutine to return bogus pc/sp
info for running goroutines.
This change uses the atomicstatus bit to determine
the state which is more accurate.
Hana Kim [Thu, 8 Feb 2018 18:23:48 +0000 (13:23 -0500)]
runtime/trace: add stack tests for GOMAXPROCS
and reorganize test log messages for stack dumps
for easier debugging.
The error log will be formatted like the following:
trace_stack_test.go:282: Did not match event GoCreate with stack
runtime/trace_test.TestTraceSymbolize :39
testing.tRunner :0
Seen 30 events of the type
Offset 1890
runtime/trace_test.TestTraceSymbolize /go/src/runtime/trace/trace_stack_test.go:30
testing.tRunner /go/src/testing/testing.go:777
Offset 1899
runtime/trace_test.TestTraceSymbolize /go/src/runtime/trace/trace_stack_test.go:30
testing.tRunner /go/src/testing/testing.go:777
...
Change-Id: I0468de04507d6ae38ba84d99d13f7bf592e8d115
Reviewed-on: https://go-review.googlesource.com/92916 Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Caio Marcelo de Oliveira Filho [Tue, 2 Jan 2018 00:16:43 +0000 (16:16 -0800)]
archive/tar: automatically promote TypeRegA
Change Reader to promote TypeRegA to TypeReg in headers, unless their
name have a trailing slash which is already promoted to TypeDir. This
will allow client code to handle just TypeReg instead both TypeReg and
TypeRegA.
Change Writer to promote TypeRegA to TypeReg or TypeDir in the headers
depending on whether the name has a trailing slash. This normalization
is motivated by the specification (in pax(1)):
0 represents a regular file. For backwards-compatibility, a
typeflag value of binary zero ( '\0' ) should be recognized as
meaning a regular file when extracting files from the
archive. Archives written with this version of the archive file
format create regular files with a typeflag value of the
ISO/IEC 646:1991 standard IRV '0'.
Fixes #22768.
Change-Id: I149ec55824580d446cdde5a0d7a0457ad7b03466
Reviewed-on: https://go-review.googlesource.com/85656 Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Kevin Burke [Wed, 24 Jan 2018 06:11:51 +0000 (22:11 -0800)]
all: use HTTPS for iana.org links
iana.org, www.iana.org and data.iana.org all present a valid TLS
certificate, so let's use it when fetching data or linking to
resources to avoid errors in transit.
Austin Clements [Mon, 15 Jan 2018 17:27:17 +0000 (12:27 -0500)]
runtime: remove legacy eager write barrier
Now that the buffered write barrier is implemented for all
architectures, we can remove the old eager write barrier
implementation. This CL removes the implementation from the runtime,
support in the compiler for calling it, and updates some compiler
tests that relied on the old eager barrier support. It also makes sure
that all of the useful comments from the old write barrier
implementation still have a place to live.
Fixes #22460.
Updates #21640 since this fixes the layering concerns of the write
barrier (but not the other things in that issue).
Austin Clements [Thu, 8 Feb 2018 15:31:07 +0000 (10:31 -0500)]
cmd/compile: calls can clobber g on s390x
Because a call may ultimately invoke runtime.setg, we have to assume
that g may be clobbered by any call. All of the other architectures
that use a g register already do this, but it was missing from the
s390x caller save clobber set.
Cherry Zhang [Thu, 8 Feb 2018 16:39:50 +0000 (11:39 -0500)]
cmd/internal/obj/mips: fix use of R28 on 32-bit MIPS
R28 is used as the SB register on MIPS64, and it was printed as
"RSB" on both 32-bit and 64-bit MIPS. This is confusing on MIPS32
as there R28 is just a general purpose register. Further, this
string representation is used in the assembler's frontend to parse
register symbols, and this leads to failure in parsing R28 in
MIPS32 assembly code. Change rconv to always print the register
as R28. This fixes the parsing problem on MIPS32, and this is
a reasonable representation on both MIPS32 and MIPS64.
Tobias Klauser [Mon, 18 Dec 2017 10:59:26 +0000 (11:59 +0100)]
syscall: support syscalls without error return on Linux
Add the rawSyscallNoError wrapper function which is used for Linux
syscalls that don't return an error and convert all applicable
occurences of RawSyscall to use it instead.
Fixes #22924
Change-Id: Iff1eddb54573d459faa01471f10398b3d38528dd
Reviewed-on: https://go-review.googlesource.com/84485
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Tobias Klauser [Thu, 14 Dec 2017 10:59:33 +0000 (11:59 +0100)]
syscall: support Getwd on all BSDs
All supported BSDs provide the SYS___GETCWD syscall which can be used to
implement syscall.Getwd. With this change os.Getwd can use a single
syscall instead of falling back to the current kludge solution on the
BSDs.
This doesn't add any new exported functions to the frozen syscall
package, only ImplementsGetwd changes to true for dragonfly, freebsd,
netbsd and openbsd.
As suggested by Ian, this follows CL 83755 which did the same for
golang.org/x/sys/unix.
Also, an entry for netbsd/arm is added to mkall.sh which was used to
generate the syscall wrappers there.
Change-Id: I84da1ec61a6b8625443699a63cde556b6442ad41
Reviewed-on: https://go-review.googlesource.com/84484 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Jason A. Donenfeld [Thu, 8 Feb 2018 15:59:17 +0000 (16:59 +0100)]
runtime: use Android O friendly syscalls on 64-bit machines
Android O disallows open on 64-bit, so let's use openat with AT_FDCWD to
achieve the same behavior.
Android O disallows epoll_wait on 64-bit, so let's use epoll_pwait with
the last argument as NULL to achieve the same behavior.
See here:
https://android.googlesource.com/platform/bionic/+/master/libc/seccomp/arm64_app_policy.cpp
https://android.googlesource.com/platform/bionic/+/master/libc/seccomp/mips64_app_policy.cpp
https://android.googlesource.com/platform/bionic/+/master/libc/seccomp/x86_64_app_policy.cpp
Fixes #23750
Change-Id: If8d5a663357471e5d2c1f516151344a9d05b188a
Reviewed-on: https://go-review.googlesource.com/92895 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Rob Pike [Mon, 29 Jan 2018 00:28:17 +0000 (11:28 +1100)]
cmd/asm: fix crash on bad symbol for TEXT
Was missing a check in validSymbol.
Fixes #23580.
Can wait for go1.11. Probably safe but the crash is only for
invalid input, so not worth the risk.
Change-Id: I51f88c5be35a8880536147d1fe5c5dd6798c29de
Reviewed-on: https://go-review.googlesource.com/90398 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Daniel Martí [Sun, 4 Feb 2018 09:25:55 +0000 (09:25 +0000)]
go/types: use consistent receiver names
Inconsistent names are quite obvious on the godoc HTML rendering:
type Array
func NewArray(elem Type, len int64) *Array
func (a *Array) Elem() Type
func (a *Array) Len() int64
func (t *Array) String() string
func (t *Array) Underlying() Type
Fix all the String and Underlying methods to be consistent with their
types. This makes these two lists of methods less consistent, but that's
not visible to the user.
This also makes the inconsistent receiver names rule in golint happy.
Change-Id: I7c84d6bae1235887233a70d5f7f61a224106e952
Reviewed-on: https://go-review.googlesource.com/91736 Reviewed-by: Robert Griesemer <gri@golang.org>
Robert Griesemer [Thu, 18 Jan 2018 05:42:51 +0000 (21:42 -0800)]
cmd/compile/internal/syntax: implement comment reporting in scanner
R=go1.11
In order to collect comments in the AST and for error testing purposes,
the scanner needs to not only recognize and skip comments, but also be
able to report them if so desired. This change adds a mode flag to the
scanner's init function which controls the scanner behavior around
comments.
In the common case where comments are not needed, there must be no
significant overhead. Thus, comments are reported via a handler upcall
rather than being returned as a _Comment token (which the parser would
have to filter out with every scanner.next() call).
Because the handlers for error messages, directives, and comments all
look the same (they take a position and text), and because directives
look like comments, and errors never start with a '/', this change
simplifies the scanner's init call to only take one (error) handler
instead of 2 or 3 different handlers with identical signature. It is
trivial in the handler to determine if we have an error, directive,
or general comment.
Finally, because directives are comments, when reporting directives
the full comment text is returned now rather than just the directive
text. This simplifies the implementation and makes the scanner API
more regular. Furthermore, it provides important information about
the comment style used by a directive, which may matter eventually
when we fully implement /*line file:line:col*/ directives.
Change-Id: I2adbfcebecd615e4237ed3a832b6ceb9518bf09c
Reviewed-on: https://go-review.googlesource.com/88215 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This implements parsing of /*line file:line*/ and /*line file:line:col*/
directives and also extends the optional column format to regular //line
directives, per #22662.
For a line directive to be recognized, its comment text must start with
the prefix "line " which is followed by one of the following:
:line
:line:col
filename:line
filename:line:col
with at least one : present. The line and col values must be unsigned
decimal integers; everything before is considered part of the filename.
Valid line directives are:
//line :123
//line :123:8
//line foo.go:123
//line C:foo.go:123 (filename is "C:foo.go")
//line C:foo.go:123:8 (filename is "C:foo.go")
/*line ::123*/ (filename is ":")
No matter the comment format, at the moment all directives act as if
they were in //line comments, and column information is ignored.
To be addressed in subsequent CLs.
For #22662.
Change-Id: I1a2dc54bacc94bc6cdedc5229ee13278971f314e
Reviewed-on: https://go-review.googlesource.com/86037 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Robert Griesemer [Tue, 16 Jan 2018 23:28:57 +0000 (15:28 -0800)]
go/parser: improved error recovery after missing type
R=go1.11
This CL also introduces a new TODO in parser.go. To be
addressed in a separate CL to make this easier to review.
Also: Make parser's test harness easier to use by ignoring
auto-inserted (invisible) semicolons when computing error
positions. Adjusted testdata/commas.src accordingly.
Fixes #23434.
Change-Id: I050592d11d5f984f71185548394c000eea509205
Reviewed-on: https://go-review.googlesource.com/87898 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Robert Griesemer [Fri, 12 Jan 2018 05:52:27 +0000 (21:52 -0800)]
go/scanner: don't eat \r in comments if that shortens the comment
For consistent formatting across platforms we strip \r's from
comments. This happens in the go/scanner which already does
this for raw string literals where it is mandated by the spec.
But if a (sequence of) \r appears in a regular (/*-style) comment
between a * and a /, removing that (sequence of) \r shortens that
text segment to */ which terminates the comment prematurely.
Don't do it.
As an aside, a better approach would be to not touch comments,
(and raw string literals for that matter) in the scanner and
leave the extra processing to clients. That is the approach
taken by the cmd/compile/internal/syntax package. However, we
probably can't change the semantics here too much, so just do
the minimal change that doesn't produce invalid comments. It's
an esoteric case after all.
Fixes #11151.
Change-Id: Ib4dcb52094f13c235e840c9672e439ea65fef961
Reviewed-on: https://go-review.googlesource.com/87498 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Robert Griesemer [Sat, 23 Dec 2017 00:29:36 +0000 (16:29 -0800)]
go/parser: simplify code to read from an io.Reader (cleanup)
ioutil.ReadAll didn't exist when we wrote that parser code
originally (in 2009). Now it does, so use it. This may also
make that code path slightly more efficient.
Also, now that we are guaranteed to have a fast path for reading
from an io.Reader (and thus an io.ReadCloser), simplify setup
code for parser.ParseFile calls in srcimporter.Importer.ParseFiles.
Remove the associated TODO since we cannot reproduce any significant
performance differences when running go test -run ImportStdLib for
the case where we used to read directly from a file (even before the
change to the parser).
Fixes #19281.
Change-Id: I816459d092bb9e27fdc85089b8f21d57ec3fd79a
Reviewed-on: https://go-review.googlesource.com/85395 Reviewed-by: Alan Donovan <adonovan@google.com>
Austin Clements [Tue, 6 Feb 2018 23:00:13 +0000 (18:00 -0500)]
runtime: remove legacy comments and code from arm morestack
CL 137410043 deleted support for split stacks, which means morestack
no longer needed to save its caller's frame or argument size or its
caller's argument pointer. However, this commit failed to update the
comment or delete the line that computed the caller's argument
pointer. Clean these up now.
Austin Clements [Wed, 24 Jan 2018 22:17:38 +0000 (17:17 -0500)]
cmd/internal/obj/mips: support NOFRAME
This passes toolstash -cmp with one exception: assembly functions that
were declared with a frame size of -4 (or -8) used to record
locals=0xfffffffffffffffc in the object file and now record
locals=0x0. This doesn't affect anything.
Austin Clements [Wed, 24 Jan 2018 22:17:38 +0000 (17:17 -0500)]
cmd/internal/obj/arm64: support NOFRAME
In addition, this makes the arm64 prologue code generation much closer
to the pattern used on other platforms.
This passes toolstash -cmp with one exception: assembly functions that
were declared with a frame size of -8 used to record
locals=0xfffffffffffffff8 in the object file and now record
locals=0x0. This doesn't affect anything.
Austin Clements [Wed, 24 Jan 2018 22:17:38 +0000 (17:17 -0500)]
cmd/internal/obj/arm: support NOFRAME
This adds support on arm for the NOFRAME symbol attribute used by
ppc64 and s390x in preference to using a frame size of -4. This is
modeled on ppc64's implementation of NOFRAME.
Austin Clements [Thu, 25 Jan 2018 16:41:41 +0000 (11:41 -0500)]
cmd/compile: eliminate NoFramePointer
The NoFramePointer function flag is no longer used, so this CL
eliminates it. This cleans up some confusion between the compiler's
NoFramePointer flag and obj's NOFRAME flag. NoFramePointer was
intended to eliminate the saved base pointer on x86, but it was
translated into obj's NOFRAME flag. On x86, NOFRAME does mean to omit
the saved base pointer, but on ppc64 and s390x it has a more general
meaning of omitting *everything* from the frame, including the saved
LR and ppc64's "fixed frame". Hence, on ppc64 and s390x there are far
fewer situations where it is safe to set this flag.
Austin Clements [Thu, 25 Jan 2018 16:35:27 +0000 (11:35 -0500)]
cmd/internal/obj/x86: adjust SP correctly for tail calls
Currently, tail calls on x86 don't adjust the SP on return, so it's
important that the compiler produce a zero-sized frame and disable the
frame pointer. However, these constraints aren't necessary. For
example, on other architectures it's generally necessary to restore
the saved LR before a tail call, so obj simply makes this work.
Likewise, on x86, there's no reason we can't simply make this work.
Hence, this CL adjusts the compiler to use the same tail call
convention for x86 that we use on LR machines by producing a RET with
a target, rather than a JMP with a target. In fact, obj already
understands this convention for x86 except that it's buggy with
non-zero frame sizes. So we also fix this bug obj. As a result of
these fixes, the compiler no longer needs to mark wrappers as
NoFramePointer since it's now perfectly fine to save the frame
pointer.
In fact, this eliminates the only use of NoFramePointer in the
compiler, which will enable further cleanups.
This also fixes what is very nearly, but not quite, a code generation
bug. NoFramePointer becomes obj.NOFRAME in the object file, which on
ppc64 and s390x means to omit the saved LR. Hence, on these
architectures, NoFramePointer (and NOFRAME) is only safe to set on
leaf functions. However, on *most* architectures, wrappers aren't
necessarily leaf functions because they may call DUFFZERO. We're saved
on ppc64 and s390x only because the compiler doesn't have the rules to
produce DUFFZERO calls on these architectures. Hence, this only works
because the set of LR architectures that implement NOFRAME is disjoint
from the set where the compiler produces DUFFZERO operations. (I
discovered this whole mess when I attempted to add NOFRAME support to
arm.)
Robert Griesemer [Thu, 21 Dec 2017 22:35:21 +0000 (14:35 -0800)]
go/types: use check.lookup consistently where possible (cleanup)
This CL converts the last call to scope.LookupParent with no position
information to a check.lookup call that respects position information
provided by Eval (there's one more LookupParent call that cannot be
converted, see the respective comment in the code).
In this case, the lookup is needed to determine the variable on the
LHS of an assignment, for adjustment of its `used` information.
Outside a types.Eval call, i.e., during normal type-checking, there
is no difference between this new code and the old code.
While in a types.Eval call, it's important to use the correct position
to look up the relevant variable. If token.NoPos were used, one might
find another variable with the same name, declared later in the scope.
Caveat: Types.Eval only accepts expressions, and it's currently not
possible to evaluate assignments (except via function literals, but
then the scope is different). That is, this change is a fix for a
potential future bug, and for now a no-op.
Change-Id: I28db1fe1202c07e3f7b3fadfd185728afb9b2ae7
Reviewed-on: https://go-review.googlesource.com/85199 Reviewed-by: Alan Donovan <adonovan@google.com>
Robert Griesemer [Thu, 21 Dec 2017 22:15:20 +0000 (14:15 -0800)]
go/types: correctly determine if panic call refers to built-in
R=go1.11
The terminating statement check for functions that declare result
parameters was using the wrong scope to look up calls to `panic`
which in esoteric cases lead to a false positive.
Instead of looking up a panic call again at a time when correct
scope information would have to be recomputed, collect calls to
predeclared panic in a set when type-checking that call.
Fixes #23218.
Change-Id: I35eaf010e5cb8e43696efba7d77cefffb6f3deb2
Reviewed-on: https://go-review.googlesource.com/85198 Reviewed-by: Alan Donovan <adonovan@google.com>
Robert Griesemer [Tue, 19 Dec 2017 23:45:50 +0000 (15:45 -0800)]
go/types: perform delayed tests even for types.Eval
R=go1.11
types.Eval historically never evaluated any delayed tests, which
included verification of validity of map keys, but also function
literal bodies.
Now, embedded interfaces are also type-checked in a delayed fashion,
so it becomes imperative to do all delayed checks for eval (otherwise
obviously incorrect type expressions are silently accepted).
Enabling the delayed tests also removes the restriction that function
literals were not type-checked.
Also fixed a bug where eval wouldn't return a type-checking error
because check.handleBailout was using the wrong err variable.
Added tests that verify that method set computation is using the
right types when evaluating interfaces with embedded types.
For #18395.
For #22992.
Change-Id: I574fa84568b5158bca4b4ccd4ef5abb616fbf896
Reviewed-on: https://go-review.googlesource.com/84898 Reviewed-by: Alan Donovan <adonovan@google.com>
Robert Griesemer [Thu, 14 Dec 2017 05:37:13 +0000 (21:37 -0800)]
go/types: don't associate methods with alias type names
R=go1.11
The existing code associated methods with receiver base type
names before knowing if a type name denoted a locally defined
type. Sometimes, methods would be incorrectly associated with
alias type names and consequently were lost down the road.
This change first collects all methods with non-blank names
and in a follow-up pass resolves receiver base type names to
valid non-alias type names with which the methods are then
associated.
Fixes #23042.
Change-Id: I7699e577b70aadef6a2997e882beb0644da89fa3
Reviewed-on: https://go-review.googlesource.com/83996 Reviewed-by: Alan Donovan <adonovan@google.com>