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/91736Reviewed-by: Robert Griesemer <gri@golang.org>

R=go1.11.

Now that we have a syntax error test harness, we can add the
proper tests for the recent parser fixes.

For #20800.
For #20789.
For #23385.
For #23434.

A test for #20789 already exists in test/fixedbugs, but this
is the better location for that test. But leaving the existing
one where it is as well.

Change-Id: I5937b9b63bafd1efab467a00344302e717976171
Reviewed-on: https://go-review.googlesource.com/88336Reviewed-by: Matthew Dempsky <mdempsky@google.com>

R=go1.11

Fixes #20800.

Change-Id: Ifea273521d42a543a43da2f655ace7c295650e30
Reviewed-on: https://go-review.googlesource.com/88335Reviewed-by: Matthew Dempsky <mdempsky@google.com>

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/88215Reviewed-by: Matthew Dempsky <mdempsky@google.com>

R=go1.11.

This is just a copy of the fix for #23434:
https://go-review.googlesource.com/c/go/+/87898.

Test pending test harness for the syntax package.

Change-Id: I52409aebe13ec784ddd7e41190a81c7e126bdd0c
Reviewed-on: https://go-review.googlesource.com/87901Reviewed-by: Matthew Dempsky <mdempsky@google.com>

R=go1.11

A common error is to write '=' instead of '==' inside the condition
of a simple 'if' statement:

	if x = 0 { ... }

Highlight the fact that we have an assignment in the error message
to prevent further confusion.

Fixes #23385.

Change-Id: I1552050fd6da927bd12a1be0977bd2e98eca5885
Reviewed-on: https://go-review.googlesource.com/87316Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

R=go1.11

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/86037Reviewed-by: Matthew Dempsky <mdempsky@google.com>

R=go1.11

Also: Minor updates to syntax.Parse doc string.

Change-Id: I649965be9670a2f1c3de2cdb350634ed21e36ad9
Reviewed-on: https://go-review.googlesource.com/85663Reviewed-by: Matthew Dempsky <mdempsky@google.com>

R=go1.11

This is a follow up for #11377 which reported that an error like

/tmp/xx.go:9:6: expected '(', found 'IDENT' F1

shouldn't print 'IDENT', as it's just an internal detail.
The relevant change wasn't made in the original fix, so here it is.

For #11377.

Change-Id: Ib76957d86b88e3e63646fbe4abf03a3b9d045139
Reviewed-on: https://go-review.googlesource.com/87900Reviewed-by: Matthew Dempsky <mdempsky@google.com>

R=go1.11

No semantic change.

For #23434.

Change-Id: Iafdb062b0ebe6cd6e51f9a98b62b1d10f1bacc5c
Reviewed-on: https://go-review.googlesource.com/87899Reviewed-by: Matthew Dempsky <mdempsky@google.com>

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/87898Reviewed-by: Matthew Dempsky <mdempsky@google.com>

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/87498Reviewed-by: Matthew Dempsky <mdempsky@google.com>

R=go1.11

To fix this, this CL borrows code from the new syntax
package which has a better tuned parser at this point.

Fixes #11377.

Change-Id: Ib9212c945903d6f62abcc59ef5a5767d4ef36981
Reviewed-on: https://go-review.googlesource.com/87495Reviewed-by: Matthew Dempsky <mdempsky@google.com>

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/85395Reviewed-by: Alan Donovan <adonovan@google.com>

R=go1.11

Once approved, this change must be ported to golang.org/x/tools/go/gcimporter15.

Fixes #13829.

Change-Id: I26a0094d2bfd38b97f2b64bae84b9f428fc9cdf1
Reviewed-on: https://go-review.googlesource.com/85318Reviewed-by: Alan Donovan <adonovan@google.com>

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.

Change-Id: I65725d3d42c86e8adb6645d5aa80c305d473363d
Reviewed-on: https://go-review.googlesource.com/92437
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

This replaces frame size -4/-8 with the NOFRAME flag in mips and
mips64 assembly.

This was automated with:

sed -i -e 's/\(^TEXT.*[A-Z]\),\( *\)\$-[84]/\1|NOFRAME,\2$0/' $(find -name '*_mips*.s')

Plus a manual fix to mkduff.go.

The go binary is identical on both architectures before and after this
change.

Change-Id: I0310384d1a584118c41d1cd3a042bb8ea7227efb
Reviewed-on: https://go-review.googlesource.com/92044
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

This replaces frame size -8 with the NOFRAME flag in arm64 assembly.

This was automated with:

sed -i -e 's/\(^TEXT.*[A-Z]\),\( *\)\$-8/\1|NOFRAME,\2$0/' $(find -name '*_arm64.s')

Plus a manual fix to mkduff.go.

The go binary is identical before and after this change.

Change-Id: I0310384d1a584118c41d1cd3a042bb8ea7227efa
Reviewed-on: https://go-review.googlesource.com/92043
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

This replaces frame size -4 with the NOFRAME flag in arm assembly.

This was automated with:

sed -i -e 's/\(^TEXT.*[A-Z]\),\( *\)\$-4/\1|NOFRAME,\2$0/' $(find -name '*_arm.s')

Plus three manual comment changes found by:

grep '\$-4' $(find -name '*_arm.s')

The go binary is identical before and after this change.

Change-Id: I0310384d1a584118c41d1cd3a042bb8ea7227ef9
Reviewed-on: https://go-review.googlesource.com/92042
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

Change-Id: I3f71228e391f122f9cc5656ca6835fdf51a424b7
Reviewed-on: https://go-review.googlesource.com/92435
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

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.

Change-Id: I0d15e81770e54222ae329ce4496da06016736771
Reviewed-on: https://go-review.googlesource.com/92041
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

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.

Change-Id: I0d15e81770e54222ae329ce4496da06016736770
Reviewed-on: https://go-review.googlesource.com/92040
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

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.

This passes toolstash -cmp.

Change-Id: I0d15e81770e54222ae329ce4496da0601673677f
Reviewed-on: https://go-review.googlesource.com/92039
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

"-8" is not a sensible frame size on arm and we're about to start
rejecting it. Replace it with -4.

Likewise, "-4" is not a sensible frame size on arm64 and we're about
to start rejecting it. Replace it with -8.

Finally, clean up some places we're weirdly inconsistent about using 0
versus -8.

Change-Id: If85e229993d5f7f1f0cfa9852b4e294d053bd784
Reviewed-on: https://go-review.googlesource.com/92038
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

For leaf functions with zero-sized frames, there's no point in doing a
stack check, so omit it.

This aligns arm64 with other architectures.

Change-Id: I1fb483d62f1736af10c5110815d3f5a875a46d7f
Reviewed-on: https://go-review.googlesource.com/92037
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

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.

Change-Id: If68991310b4d00638128c296bdd57f4ed731b46d
Reviewed-on: https://go-review.googlesource.com/92036
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

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.)

Change-Id: Icc589aeb86beacb850d0a6a80bd3024974a33947
Reviewed-on: https://go-review.googlesource.com/92035
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

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/85199Reviewed-by: Alan Donovan <adonovan@google.com>

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/85198Reviewed-by: Alan Donovan <adonovan@google.com>

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/84898Reviewed-by: Alan Donovan <adonovan@google.com>

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/83996Reviewed-by: Alan Donovan <adonovan@google.com>

R=go1.11

Fixes #22345.

Change-Id: I7cf22d17bdd0143efb6ee48981e649ffe797aed9
Reviewed-on: https://go-review.googlesource.com/83579Reviewed-by: Alan Donovan <adonovan@google.com>

R=go1.11

Functions (at the package level) were collected and their bodies
type-checked after all other package-level objects were checked.
But function literals where type-checked right away when they were
encountered so that they could see the correct, partially populated
surrounding scope, and also to mark variables of the surrounding
function as used.

This approach, while simple, breaks down in esoteric cases where
a function literal appears inside the declaration of an object
that its body depends on: If the body is type-checked before the
object is completely set up, the literal may use incomplete data
structures, possibly leading to spurious errors.

This change postpones type-checking of function literals to later;
after the current expression or statement, but before any changes
to the enclosing scope (so that the delayed type-checking sees the
correct scope contents).

The new mechanism is general and now is also used for other
(non-function) delayed checks.

Fixes #22992.

Change-Id: Ic95f709560858b4bdf8c645be70abe4449f6184d
Reviewed-on: https://go-review.googlesource.com/83397Reviewed-by: Alan Donovan <adonovan@google.com>

R=go1.11

Also: Moved Checker.pos field into context where it belongs.

This is a cleanup/code factoring.

For #22992.

Change-Id: If9d4f0af537cb181f73735e709ebc8258b2a1378
Reviewed-on: https://go-review.googlesource.com/83017Reviewed-by: Alan Donovan <adonovan@google.com>

R=go1.11.

The existing algorithm for type-checking interfaces breaks down in
complex cases of recursive types, e.g.:

	package issue21804

	type (
		_ interface{ m(B) }
		A interface{ a(D) }
		B interface{ A }
		C interface{ B }
		D interface{ C }
	)

	var _ A = C(nil)

The underlying problem is that the method set of C is computed by
following a chain of embedded interfaces at a point when the method
set for one of those interfaces is not yet complete. A more general
problem is largely avoided by topological sorting of interfaces
depending on their dependencies on embedded interfaces (but not
method parameters).

This change fixes this problem by fundamentally changing how
interface method sets are computed: Instead of determining them
based on recursively type-checking embedded interfaces, the new
approach computes the method sets of interfaces separately,
based on syntactic structure and name resolution; and using type-
checked types only when readily available (e.g., for local types
which can at best refer to themselves, and imported interfaces).

This approach avoids cyclic dependencies arising in the method
sets by separating the collection of embedded methods (which
fundamentally cannot have cycles in correct programs) and type-
checking of the method's signatures (which may have arbitrary
cycles).

As a consequence, type-checking interfaces can rely on the
pre-computed method sets which makes the code simpler: Type-
checking of embedded interface types doesn't have to happen
anymore during interface construction since we already have
all methods and now is delayed to the end of type-checking.
Also, the topological sort of global interfaces is not needed
anymore.

Fixes #18395.

Change-Id: I0f849ac9568e87a32c9c27bbf8fab0e2bac9ebb1
Reviewed-on: https://go-review.googlesource.com/79575Reviewed-by: Alan Donovan <adonovan@google.com>

This reverts commit 3b6abd8a.

Change-Id: I8ca100501c1844af78a547989786d14dac6b494a
Reviewed-on: https://go-review.googlesource.com/93456Reviewed-by: Andrew Bonventre <andybons@golang.org>

This reverts commit b83d7f72.

Reason for revert: wrong branch

Change-Id: I28ebb121f7b3e331729e8a6a00961b876f3dd0f2
Reviewed-on: https://go-review.googlesource.com/93455Reviewed-by: Andrew Bonventre <andybons@golang.org>

* 74b56022 doc: note that x509 cert parsing rejects some more certs now
* c52e27e6 CONTRIBUTING: remove Pull Request bit
* 829b64c1 cmd/fix: fix cast check
* ee59f6df doc: minor wording improvement to the diagnostics guide
* c6e7330e all: remove PULL_REQUEST_TEMPLATE from .github
* d814c2be doc: remove Pull Request note in README.md
* 104445e3 doc: document Go 1.9.4 and Go 1.8.7

Change-Id: I58bfc6800964504258690d774a9b0aeaba509086

Fixes #23711

Also remove the "Also, please do not post patches on the issue
tracker" part, since that didn't seem to reduce the number of patches
inlined into bug reports. And now that we accept PRs, people will
probably try that first. We'll see.

Fixes #23779
Updates #18517

Change-Id: I449e0afd7292718e57d9d428494799c78296a0d2
Reviewed-on: https://go-review.googlesource.com/93335Reviewed-by: Andrew Bonventre <andybons@golang.org>