]> Cypherpunks repositories - gostls13.git/log
gostls13.git
3 years ago[dev.typeparams] cmd/compile: add a field (method) name for function in TestABIUtilsI...
Dan Scales [Mon, 28 Jun 2021 17:57:26 +0000 (10:57 -0700)]
[dev.typeparams] cmd/compile: add a field (method) name for function in TestABIUtilsInterfaces

Not having a field name for the method is not really correct, and makes
it look like an embedded field. In fact, currently types.CalcSize() in
abitest() is creating an error that is not actually reported.

Change-Id: I98c3a4abf5b6d610d9c3c56ce1042078374b5417
Reviewed-on: https://go-review.googlesource.com/c/go/+/331469
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
3 years ago[dev.typeparams] cmd/compile: port fix for issue46725 to transform.go
Dan Scales [Mon, 28 Jun 2021 18:50:26 +0000 (11:50 -0700)]
[dev.typeparams] cmd/compile: port fix for issue46725 to transform.go

Allow fix for issue46725 to work for -G=3 mode.

Change-Id: Id522fbc2278cf878cb3f95b3205a2122c164ae29
Reviewed-on: https://go-review.googlesource.com/c/go/+/331470
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: move MethodValueWrapper to walk
Cuong Manh Le [Sun, 27 Jun 2021 15:25:21 +0000 (22:25 +0700)]
[dev.typeparams] cmd/compile: move MethodValueWrapper to walk

walk is the only pass that use the function, so un-export it, too.

Change-Id: I32ec64d1c15fda7bb54bd8efa528ed32c102876d
Reviewed-on: https://go-review.googlesource.com/c/go/+/330841
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: port CL 330838 for -G=3
Cuong Manh Le [Sun, 27 Jun 2021 15:10:03 +0000 (22:10 +0700)]
[dev.typeparams] cmd/compile: port CL 330838 for -G=3

So next CL can move MethodValueWrapper to walk, since when walk is now
the only place which uses this function.

Change-Id: Id3be359bfc38efb022451cb7f9e53c2868fe7e12
Reviewed-on: https://go-review.googlesource.com/c/go/+/330840
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: delay method value wrapper generation until walk
Cuong Manh Le [Sat, 26 Jun 2021 18:31:03 +0000 (01:31 +0700)]
[dev.typeparams] cmd/compile: delay method value wrapper generation until walk

As walk already create the wrapper if necessary.

With this change, test/inline.go need to be changed to use
errorcheckwithauto, for matching "inlining call to ..." in autogenerated
position for method value wrapper, since when we don't generate the
wrapper immediately during typecheck.

Change-Id: I9ffbec9ad3c2b7295546976e2fa517336c13c89b
Reviewed-on: https://go-review.googlesource.com/c/go/+/330838
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: rename OCALLPART to OMETHVALUE
Cuong Manh Le [Sat, 26 Jun 2021 18:28:38 +0000 (01:28 +0700)]
[dev.typeparams] cmd/compile: rename OCALLPART to OMETHVALUE

Go spec call them "method values", not "partial calls". Note that
we use "OMETHVALUE" (as opposed to "OMETHODVALUE") to be consistent
with "OMETHEXPR".

Change-Id: I1efd985d4b567a1b4b20aeb603eb82db579edbd5
Reviewed-on: https://go-review.googlesource.com/c/go/+/330837
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: add method value wrappers to unified IR
Matthew Dempsky [Sat, 26 Jun 2021 19:44:53 +0000 (12:44 -0700)]
[dev.typeparams] cmd/compile: add method value wrappers to unified IR

Method value wrappers will need dictionary support too, so bring them
under the unified IR umbrella as well.

Change-Id: Iec36bb04efdf59843d1b00f55d2c44bc841fa2ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/331190
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Matthew Dempsky <mdempsky@google.com>

3 years ago[dev.typeparams] cmd/compile: do not skip TestUnifiedCompare in short mode
Cuong Manh Le [Sat, 26 Jun 2021 17:51:16 +0000 (00:51 +0700)]
[dev.typeparams] cmd/compile: do not skip TestUnifiedCompare in short mode

Instead, just testing the runtime package in short mode instead of std.
So trybot can help catching any mistake earlier.

Change-Id: I5bc859a59afa13a10d436a798258ac154c7d27b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/330836
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: remove OCALLMETH Fatals in SSA generation
Cuong Manh Le [Sat, 26 Jun 2021 08:28:16 +0000 (15:28 +0700)]
[dev.typeparams] cmd/compile: remove OCALLMETH Fatals in SSA generation

CL 330831 moved rewrite method calls to typecheck pass, then add Fatalf
check for mis-used of OCALLMETH in all frontend passes. The check in SSA
generation pass is now redundant.

Change-Id: If959b774ab3c3aeefabf32333f77b9a93c156ce3
Reviewed-on: https://go-review.googlesource.com/c/go/+/330834
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: rewrite method calls during typecheck
Cuong Manh Le [Fri, 25 Jun 2021 15:24:29 +0000 (22:24 +0700)]
[dev.typeparams] cmd/compile: rewrite method calls during typecheck

CL 330671 move rewriting method call to method expression to escape
analysis. This CL move the rewriting up further, into typecheck. It
helps simplify the code for dowstream passes, as they now only have to
deal with OCALLFUNC.

There're two notes:

 - For -G=3, we can't rewrite d.M() where d is an instantiated receiver
   in transformCall, but let irgen.stencil to rewrite it.

 - Escape analysis still have to check for rewriting method calls, as
   the devirtualization pass can still generate OCALLMETH.

Does not pass toolstash, since when the export data now contains method
expression calls instead of method calls.

Change-Id: I77465ef04d50dc4efedddca7eb55b3fc9483db0e
Reviewed-on: https://go-review.googlesource.com/c/go/+/330831
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: restore check for OCALLMETH in walkCall
Cuong Manh Le [Sat, 26 Jun 2021 07:34:56 +0000 (14:34 +0700)]
[dev.typeparams] cmd/compile: restore check for OCALLMETH in walkCall

Since when walkCall1 still need to handle OCALLMETH.

Change-Id: Iadb12a0965169c63f97978c96412012ef4fa02df
Reviewed-on: https://go-review.googlesource.com/c/go/+/330833
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: fix wrong AST generation in devirtualization
Cuong Manh Le [Sat, 26 Jun 2021 06:57:51 +0000 (13:57 +0700)]
[dev.typeparams] cmd/compile: fix wrong AST generation in devirtualization

CL 330671 moved rewrite method calls to escape analysis. It accidently
made the AST invalid, by removing the OCALLMETH set operation during
devirtualization pass.

Change-Id: I862ffd7f880de55969d7784d9e7b3c38894f6b68
Reviewed-on: https://go-review.googlesource.com/c/go/+/330832
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: clarify comment about checking reflect.Method in usemethod
Cuong Manh Le [Fri, 25 Jun 2021 07:01:54 +0000 (14:01 +0700)]
[dev.typeparams] cmd/compile: clarify comment about checking reflect.Method in usemethod

Followup discussion in CL 330670.

Change-Id: Icec2d5d5715666a6967e18ac9ab3d802b47569da
Reviewed-on: https://go-review.googlesource.com/c/go/+/330830
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: use Type.LinkString for map keys
Matthew Dempsky [Fri, 25 Jun 2021 17:15:40 +0000 (10:15 -0700)]
[dev.typeparams] cmd/compile: use Type.LinkString for map keys

This CL changes typecheck and order to use Type.LinkString for
computing map keys instead of Type.NameString. As mentioned in the
LinkString docs (added by the previous CL), LinkString reliably maps
type identity to string equality as long as the LinkString calls all
happen within the same compilation unit (which they do here).

This eliminates the need for subsequent types.Identical checks.

Change-Id: I32ff591e69d6f23f2dc6ebd5af343618ebe89013
Reviewed-on: https://go-review.googlesource.com/c/go/+/330911
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
3 years ago[dev.typeparams] cmd/compile: rename types.Type.{Short,Long}String to {Link,Name...
Matthew Dempsky [Fri, 25 Jun 2021 17:04:51 +0000 (10:04 -0700)]
[dev.typeparams] cmd/compile: rename types.Type.{Short,Long}String to {Link,Name}String

The original names "ShortString" and "LongString" refer back to the
fmt verbs used to request their formatting styles. However, I always
get confused working with them, in particular because (1) the
"ShortString" description, which uses package-path qualification, is
actually generally longer than the "LongString" description, which
uses package-name qualification; and (2) the documentation mentions
how they're often used, but doesn't actually describe why they're safe
for those purposes.

This CL renames them to "LinkString" and "NameString", respectively,
based on their primary use cases. It also attempts to more completely
describe the strings they return and how they can be used correctly.

Change-Id: I9158ae3eafa8ac53da31a78c7a6d929dc0199afe
Reviewed-on: https://go-review.googlesource.com/c/go/+/330910
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
3 years agoMerge "[dev.typeparams] all: merge master (37f9a8f) into dev.typeparams" into dev...
Gerrit Code Review [Fri, 25 Jun 2021 15:51:12 +0000 (15:51 +0000)]
Merge "[dev.typeparams] all: merge master (37f9a8f) into dev.typeparams" into dev.typeparams

3 years ago[dev.typeparams] cmd/compile: simplify variable capturing in unified IR
Matthew Dempsky [Fri, 25 Jun 2021 08:54:50 +0000 (01:54 -0700)]
[dev.typeparams] cmd/compile: simplify variable capturing in unified IR

While initially building out unified IR, I didn't have any indexing
scheme. Everything was written out in order. Consequently, if I wanted
to write A before B, I had to compute A before B.

One particular example of this is handling closure variables: the
reader needs the list of closure variables before it can start reading
the function body, so I had to write them out first, and so I had to
compute them first in a separate, dedicated pass.

However, that constraint went away a while ago. For example, it's now
possible to replace the two-pass closure variable capture with a
single pass. We just write out the function body earlier, but then
wait to write out its index.

I anticipate this approach will make it easier to implement
dictionaries: rather than needing a separate pass to correctly
recognize and handle all of the generics cases, we can just hook into
the existing logic.

Change-Id: Iab1e07f9202cd5d2b6864eef10116960456214df
Reviewed-on: https://go-review.googlesource.com/c/go/+/330851
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
3 years ago[dev.typeparams] cmd/compile: fix TestUnifiedCompare
Matthew Dempsky [Fri, 25 Jun 2021 08:49:26 +0000 (01:49 -0700)]
[dev.typeparams] cmd/compile: fix TestUnifiedCompare

We need to start the capture walk from expr.Body, not expr, otherwise
in quirks mode we'll set all of the captured variables' positions to
expr.Body.Rbrace.

Change-Id: Ic93f2773ae3756c2ec88dac17b4e9fb5a0771734
Reviewed-on: https://go-review.googlesource.com/c/go/+/330889
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
3 years ago[dev.typeparams] cmd/compile: generate wrappers within unified IR
Matthew Dempsky [Thu, 24 Jun 2021 04:33:24 +0000 (21:33 -0700)]
[dev.typeparams] cmd/compile: generate wrappers within unified IR

This CL extends unified IR to handle creating wrapper methods. There's
relatively little about this code that's actually specific to unified
IR, but rewriting this logic allows a few benefits:

1. It decouples unified IR from reflectdata.methodWrapper, so the
latter code can evolve freely for -G=3's needs. This will also allow
the new code to evolve to unified IR's wrapper needs, which I
anticipate will operate slightly differently.

2. It provided an opportunity to revisit a lot of the code and
simplify/update it to current style. E.g., in the process, I
discovered #46903, which unified IR now gets correctly. (I have not
yet attempted to fix reflectdata.methodWrapper.)

3. It gives a convenient way for unified IR to ensure all of the
wrapper methods it needs are generated correctly.

For now, the wrapper generation is specific to non-quirks mode.

Change-Id: I5798de6b141f29e8eb6a5c563e7049627ff2868a
Reviewed-on: https://go-review.googlesource.com/c/go/+/330569
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
3 years ago[dev.typeparams] cmd/compile: refactor "need to emit" logic for types
Matthew Dempsky [Thu, 24 Jun 2021 18:31:56 +0000 (11:31 -0700)]
[dev.typeparams] cmd/compile: refactor "need to emit" logic for types

This CL refactors out a single reflectdata.NeedEmit function that
reports whether the current compilation unit needs to emit the runtime
type descriptor and method wrappers for a given type.

As a minor side bonus, this CL also skips compiling the "error.Error"
wrapper in non-runtime packages. Package runtime already
unconditionally emitted the runtime type descriptor for error, so we
just need to make sure it emits the wrapper and other packages don't.

Passes toolstash -cmp.

Change-Id: Ic9ea219dfba8a0a57f2f42f817bdff7618732bff
Reviewed-on: https://go-review.googlesource.com/c/go/+/330754
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
3 years ago[dev.typeparams] cmd/compile: switch CaptureVars to use syntax.Walk
Matthew Dempsky [Wed, 23 Jun 2021 21:45:34 +0000 (14:45 -0700)]
[dev.typeparams] cmd/compile: switch CaptureVars to use syntax.Walk

This CL refactors CaptureVars to use a visitor type so it's easier to
break out helper functions to review.

It also simplifies the quirks-mode handling of function literals:
instead of trying to maintain information about whether we're inside a
function literal or not, it now just rewrites the recorded position
information for any newly added free variables after walking the
function literal.

(Quirks mode is only for "toolstash -cmp"-style binary output testing
of normal code and will eventually be removed, so I don't think it's
important that this is an O(N^2) algorithm for deeply nested function
literals with lots of free variables.)

Change-Id: I0689984f6d88cf9937d4706d2d8de96415eaeee3
Reviewed-on: https://go-review.googlesource.com/c/go/+/330789
Trust: Matthew Dempsky <mdempsky@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
3 years ago[dev.typeparams] all: merge master (37f9a8f) into dev.typeparams
Cuong Manh Le [Fri, 25 Jun 2021 04:17:04 +0000 (11:17 +0700)]
[dev.typeparams] all: merge master (37f9a8f) into dev.typeparams

Conflicts:

- src/go/types/check_test.go

  CL 330629 fixed a bug in package qualification logic

- src/internal/buildcfg/exp.go

  CL 329930 make parseExperiments get go arch string as input param

Merge List:

+ 2021-06-25 37f9a8f69d go/types: fix a bug in package qualification logic
+ 2021-06-24 c309c89db5 reflect: document that InterfaceData is a low-entropy RNG
+ 2021-06-24 cce621431a cmd/compile: fix wrong type in SSA generation for OSLICE2ARRPTR
+ 2021-06-24 600a2a4ffb cmd/go: don't try to add replaced versions that won't be selected
+ 2021-06-24 a9bb38222a net: remove hard-coded timeout in dialClosedPort test helper
+ 2021-06-24 86d72fa2cb time: handle invalid UTF-8 byte sequences in quote to prevent panic
+ 2021-06-24 44a12e5f33 cmd/go: search breadth-first instead of depth-first for test dependency cycles
+ 2021-06-24 73496e0df0 net: use absDomainName in the Windows lookupPTR test helper
+ 2021-06-24 222ed1b38a os: enable TestFifoEOF on openbsd
+ 2021-06-22 0ebd5a8de0 cmd/go: update ToolTags based on GOARCH value
+ 2021-06-22 5bd09e5efc spec: unsafe.Add/Slice are not permitted in statement context
+ 2021-06-22 666315b4d3 runtime/internal/atomic: remove incorrect pointer indirection in comment
+ 2021-06-22 63daa774b5 go/types: guard against checking instantiation when generics is disabled
+ 2021-06-22 197a5ee2ab cmd/gofmt: remove stale documentation for the -G flag
+ 2021-06-22 9afd158eb2 go/parser: parse an ast.IndexExpr for a[]
+ 2021-06-21 1bd5a20e3c cmd/go: add a -go flag to 'go mod graph'
+ 2021-06-21 761edf71f6 cmd/internal/moddeps: use a temporary directory for GOMODCACHE if needed
+ 2021-06-21 a0400420ad cmd/internal/moddeps: use -mod=readonly instead of -mod=mod
+ 2021-06-21 3f9ec83b10 cmd/go: document GOPPC64 environment variable
+ 2021-06-21 20bdfba325 go/scanner: fall back to next() when encountering 0 bytes in parseIdentifier
+ 2021-06-21 44f9a3566c database/sql: fix deadlock test in prepare statement

Change-Id: I16490e8ea70ee65081f467223857033842da513a

3 years ago[dev.typeparams] cmd/compile: rewrite method call into method expression during escap...
Cuong Manh Le [Thu, 24 Jun 2021 16:05:09 +0000 (23:05 +0700)]
[dev.typeparams] cmd/compile: rewrite method call into method expression during escape analysis

CL 330331 extended escape analysis to analyze method expression calls
the same as normal method calls. We can now simply desugar method calls
into function calls in escape analysis.

To do this, two things must be changed:

 - Folding the rewrite method call to method expression call into an
   export function in typecheck package, so others can re-use it.

 - walkCall now have to call usemethod for method expression calls.
   (It seems to me this is a bug in current tip, because if one write
   (*rtype).Method(typ, i) in package "reflect", then the function won't
   be marked with AttrReflectMethod)

Passes toolstash -cmp.

Change-Id: I4745ab6110b417c7fd32949cc799811a882cd2ec
Reviewed-on: https://go-review.googlesource.com/c/go/+/330671
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: simplify usemethod
Cuong Manh Le [Thu, 24 Jun 2021 15:39:16 +0000 (22:39 +0700)]
[dev.typeparams] cmd/compile: simplify usemethod

By checking for method name first.

Passes toolstash -cmp

Change-Id: I1f4125157a8bc247e4766a882467b805a205a3c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/330670
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: catch another mis-used OCALLMETH in backend
Cuong Manh Le [Thu, 24 Jun 2021 07:13:39 +0000 (14:13 +0700)]
[dev.typeparams] cmd/compile: catch another mis-used OCALLMETH in backend

OCALLMETH is rewritten by walkCall to OCALLFUNC, and other places in
backend have already caught it. So do the same thing in state.expr for
consistency and prevent mis-use in frontend side.

While at it, also remove un-used function getParam.

Change-Id: I03e1ea907e0bcb05fa35fa81804c33b5c9a4d77e
Reviewed-on: https://go-review.googlesource.com/c/go/+/330669
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years agogo/types: fix a bug in package qualification logic
Rob Findley [Thu, 24 Jun 2021 15:01:49 +0000 (11:01 -0400)]
go/types: fix a bug in package qualification logic

CL 313035 had a bug, initializing pkgPathMap by walking the imported
package being considered rather than check.pkg.

Fix this, and enhance our tests to exercise this bug as well as other
edge cases.

Also fix error assertions in issues.src to not use quotation marks
inside the error regexp. The check tests only matched the error regexp
up to the first quotation mark.

Fixes #46905

Change-Id: I6aa8eae4bec6495006a5c03fc063db0d66b44cd6
Reviewed-on: https://go-review.googlesource.com/c/go/+/330629
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
3 years ago[dev.typeparams] test: skip -G=3 testing under GOEXPERIMENT=unified
Matthew Dempsky [Thu, 24 Jun 2021 21:32:21 +0000 (14:32 -0700)]
[dev.typeparams] test: skip -G=3 testing under GOEXPERIMENT=unified

In normal build configurations, we test both -G=0 and -G=3 so that we
can test both typecheck and types2. However, GOEXPERIMENT=unified
always uses types2, so testing both is redundant.

Change-Id: I697d2ad916d8b17cfaf4f0b6b32eec380d4e7906
Reviewed-on: https://go-review.googlesource.com/c/go/+/330755
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
3 years ago[dev.typeparams] cmd/compile: suppress liveness diagnostics of wrappers
Matthew Dempsky [Thu, 24 Jun 2021 19:57:20 +0000 (12:57 -0700)]
[dev.typeparams] cmd/compile: suppress liveness diagnostics of wrappers

Similar to the previous CL to suppress escape analysis diagnostics for
method wrappers, suppress liveness analysis diagnostics too. It's
hardly useful to know that all of a wrapper method's arguments are
live at entry.

Change-Id: I0d1e44552c6334ee3b454adc107430232abcb56a
Reviewed-on: https://go-review.googlesource.com/c/go/+/330749
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
3 years ago[dev.typeparams] cmd/compile: add derived types and subdictionaries to dictionaries
Dan Scales [Tue, 8 Jun 2021 01:13:15 +0000 (18:13 -0700)]
[dev.typeparams] cmd/compile:  add derived types and subdictionaries to dictionaries

This is code in progress to generate the two main other types of entries
in dictionaries:
 - all types in the instantiated function derived from the type
   arguments (which are currently concrete, but will eventually be
   gcshapes)
 - pointers (i.e. mainly the unique name) to all needed sub-dictionaries

In order to generate these entries, we now generate cached information
gfInfo about generic functions/methods that can be used for creating the
instantiated dictionaries. We use the type substituter to compute the
right type args for instantiated sub-dictionaries.

If infoPrintMode is changed to true, the code prints out all the
information gathered about generic functions, and also the entries in
all the dictionaries that are instantiated. The debug mode also prints
out the locations where we need main dictionaries in non-instantiated
functions.

Other changes:
 - Moved the dictionary generation back to stencil.go from reflect.go,
   since we need to do extra analysis for the new dictionary entries. In
   the process, made getInstantiation generate both the function
   instantiation and the associated dictionary.

 - Put in small change for now in reflect.go, so that we don't try
   generate separate dictionaries for Value[T].get and the
   auto-generated (*Value[T]).get.  The auto-generated wrapper shouldn't really
   need a dictionary.

 - Detected, but not handling yet, a new case which needs
   dictionaries - closures that have function params or captured
   variables whose types are derived from type arguments.

 - Added new tests in dictionaryCapture for use of method
   value/expressions in generic functions and for mutually recursive
   generic functions.

Change-Id: If0cbde8805a9f673a23f5ec798769c85c9c5359b
Reviewed-on: https://go-review.googlesource.com/c/go/+/327311
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
3 years agoreflect: document that InterfaceData is a low-entropy RNG
Matthew Dempsky [Thu, 24 Jun 2021 17:44:39 +0000 (10:44 -0700)]
reflect: document that InterfaceData is a low-entropy RNG

Change-Id: Ie26b9060630e2e774ac23d8492eaaf785bfca6b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/330709
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>

3 years agocmd/compile: fix wrong type in SSA generation for OSLICE2ARRPTR
Cuong Manh Le [Thu, 24 Jun 2021 18:45:32 +0000 (01:45 +0700)]
cmd/compile: fix wrong type in SSA generation for OSLICE2ARRPTR

Fixes #46907

Change-Id: I6a2728d2f2159df583b32f40f6100d3e90c34dd7
Reviewed-on: https://go-review.googlesource.com/c/go/+/330672
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>

3 years ago[dev.typeparams] cmd/compile: skip escape analysis diagnostics for wrappers
Matthew Dempsky [Thu, 24 Jun 2021 16:07:52 +0000 (09:07 -0700)]
[dev.typeparams] cmd/compile: skip escape analysis diagnostics for wrappers

This CL changes escape analysis to skip reporting diagnostics (at
least for parameter tagging) for generated wrappers.

We're inconsistent about when/where wrappers are generated, which made
errorcheck tests of escape analysis unnecessarily brittle to changes
in wrapper generation. This CL addresses this making errorcheck tests
only care about tagging of the actual functions themselves, not the
wrappers too.

Change-Id: Ia1a0b9dabee4d4162b05647f871db03b032c945a
Reviewed-on: https://go-review.googlesource.com/c/go/+/330689
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
3 years agocmd/go: don't try to add replaced versions that won't be selected
Bryan C. Mills [Thu, 24 Jun 2021 03:29:10 +0000 (23:29 -0400)]
cmd/go: don't try to add replaced versions that won't be selected

In Go 1.12, we added a heuristic to 'go mod tidy' to resolve packages
by adding replaced-but-not-required modules before falling back to
searching for modules from the network. Unfortunately, that heuristic
fails when the replaced version is already lower than the selected
version: adding such a module to the build list doesn't change the
selected version of that module, and so it doesn't make progress
toward resolving the missing package.

Fixes #46659

Change-Id: I75e2387d5290e769f6b0fa1231dcc4605db68597
Reviewed-on: https://go-review.googlesource.com/c/go/+/330432
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
3 years ago[dev.typeparams] cmd/compile: use r.hasTypeParams in typIdx
Cuong Manh Le [Wed, 23 Jun 2021 16:56:01 +0000 (23:56 +0700)]
[dev.typeparams] cmd/compile: use r.hasTypeParams in typIdx

CL 329571 fold the checking has type params logic, but did not realize
that the instance in typIdx can be folded, too.

Change-Id: I4682af3779535af6a6e843972cada12ba1bae6ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/330389
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: simplify writer.collectDecls
Matthew Dempsky [Wed, 23 Jun 2021 21:04:11 +0000 (14:04 -0700)]
[dev.typeparams] cmd/compile: simplify writer.collectDecls

The previous code for walking the syntax AST to find declarations
needed to know whether a declaration appeared within block scope, but
syntax.Crawl (née syntax.Walk) made that somewhat awkward.

This CL simplifies it a little, taking advantage of syntax.Walk's
support for keeping per-subtree state.

Change-Id: I03c7da8c44bec40f88e983852dc6bbab7e6ac13c
Reviewed-on: https://go-review.googlesource.com/c/go/+/330549
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
3 years agonet: remove hard-coded timeout in dialClosedPort test helper
Bryan C. Mills [Wed, 23 Jun 2021 01:48:11 +0000 (21:48 -0400)]
net: remove hard-coded timeout in dialClosedPort test helper

The helper function claims that dialing a closed port should be
"nearly instantaneous", but that is empirically not the case on
OpenBSD or Windows. The tests do not appear to be particularly
sensitive to the exact upper bound otherwise, so let's just
remove the arbitrary latency assumption.

Fixes #46884

Change-Id: If00c9fdc3063da6aaf60d365d4a2ee2c94dc6df1
Reviewed-on: https://go-review.googlesource.com/c/go/+/330250
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
3 years agotime: handle invalid UTF-8 byte sequences in quote to prevent panic
Andy Pan [Wed, 23 Jun 2021 04:59:48 +0000 (12:59 +0800)]
time: handle invalid UTF-8 byte sequences in quote to prevent panic

Fixes #46883
Updates CL 267017

Change-Id: I15c307bfb0aaa2877a148d32527681f79df1a650
Reviewed-on: https://go-review.googlesource.com/c/go/+/330289
Reviewed-by: Kevin Burke <kev@inburke.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>

3 years agocmd/go: search breadth-first instead of depth-first for test dependency cycles
Bryan C. Mills [Wed, 23 Jun 2021 19:28:37 +0000 (15:28 -0400)]
cmd/go: search breadth-first instead of depth-first for test dependency cycles

When we are looking for a dependency cycle involving a specific
package, we need to keep track of visited packages in order to avoid
repeatedly traversing a cycle that does not involve that package.

If we're keeping track of all visited packages anyway, we're already
spending O(N) memory on the traversal, so we may as well use
breadth-first search. That not only keeps the bookkeeping simple, but
also guarantees that we will find a shortest path (rather than a
completely arbitrary one).

Fixes #45863

Change-Id: I810c7337857e42dcb83630abbdea75021554be45
Reviewed-on: https://go-review.googlesource.com/c/go/+/330430
Trust: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
3 years agonet: use absDomainName in the Windows lookupPTR test helper
Bryan C. Mills [Wed, 23 Jun 2021 01:24:57 +0000 (21:24 -0400)]
net: use absDomainName in the Windows lookupPTR test helper

The real net code uses subtle heuristics to transform a domain name
to its absolute form. Since lookupPTR isn't checking that
transformation specifically, it should use the real code instead of
using a different heuristic.

Fixes #46882

Change-Id: I503357e0f62059c37c359cd54b44d343c7d5ab2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/330249
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
3 years agoos: enable TestFifoEOF on openbsd
siddharth [Tue, 22 Jun 2021 01:50:09 +0000 (21:50 -0400)]
os: enable TestFifoEOF on openbsd

The test successfully runs on currently supported versions (6.8 and
6.9) of openbsd.

Fixes #25877

Change-Id: I2694f08c5596b486453c2ac829f17b8bc455f828
Reviewed-on: https://go-review.googlesource.com/c/go/+/329732
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Tobias Klauser <tobias.klauser@gmail.com>

3 years ago[dev.typeparams] Fix issues related to dictionaries and method calls with embedded...
Dan Scales [Fri, 18 Jun 2021 21:09:21 +0000 (14:09 -0700)]
[dev.typeparams] Fix issues related to dictionaries and method calls with embedded fields

 - Fix handling of method expressions with embedded fields. Fix an
   incorrect lookup for method expressions, which have only the
   top-level type (and don't have DOT operations for the embedded
   fields). Add the embedded field dot operations into the closure.

 - Don't need a dictionary and so don't build a closure if the last
   embedded field reached in a method expression is an interface value.

 - Fix methodWrapper() to use the computed 'dot' node in the
   generic-only part of the code.

 - For a method expression, don't create a generic wrapper if the last
   embedded field reached before the method lookup is an interface.

Copied cmd/compile/internal/types2/testdata/fixedbugs/issue44688.go2 to
test/typeparam/issue44688.go, made it fully runnable (rather than just
for compilation), and added a bunch more tests.

Change-Id: I90c1aa569e1c7272e986c9d2ae683e553c3a38a1
Reviewed-on: https://go-review.googlesource.com/c/go/+/329550
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
3 years ago[dev.typeparams] cmd/compile/internal/syntax: go/ast-style walk API
Matthew Dempsky [Wed, 23 Jun 2021 19:08:42 +0000 (12:08 -0700)]
[dev.typeparams] cmd/compile/internal/syntax: go/ast-style walk API

This CL adds go/ast's Visitor, Walk, and Inspect functions to package
syntax. Having functions with the same API and semantics as their
go/ast counterparts reduces the mental load of context switching
between go/ast and syntax.

It also renames the existing Walk function into Crawl, and marks it as
a deprecated wrapper around Inspect. (I named it "Crawl" because it's
less functional than "Walk"... get it??)

There aren't that many callers to Crawl, so we can probably remove it
in the future. But it doesn't seem pressing, and I'm more concerned
about the risk of forgetting to invert a bool condition somewhere.

Change-Id: Ib2fb275873a1d1a730249c9cb584864cb6ec370e
Reviewed-on: https://go-review.googlesource.com/c/go/+/330429
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Trust: Matthew Dempsky <mdempsky@google.com>
Trust: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
3 years ago[dev.typeparams] cmd/compile: optimize wrapping of constant arguments
Matthew Dempsky [Wed, 23 Jun 2021 07:49:03 +0000 (00:49 -0700)]
[dev.typeparams] cmd/compile: optimize wrapping of constant arguments

When wrapping a go/defer statement like:

go f(g(), "x", 42)

we were wrapping it like:

_0, _1, _2, _3 := f, g(), "x", 42
go func() { _0(_1, _2, _3) }()

This is simple and general (and often necessary), but suboptimal in
some cases, such as this. Instead of evaluating the constant arguments
at the go/defer statement, and storing them into the closure context,
we can just keep them in the wrapped call expression.

This CL changes the code to instead generate (assuming f is a declared
function, not a function-typed variable):

_0 := g()
go func() { f(_0, "x", 42) }()

Change-Id: I2bdd4951e7ee93363e1656ecf9b5bd69a121c38a
Reviewed-on: https://go-review.googlesource.com/c/go/+/330332
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
3 years ago[dev.typeparams] cmd/compile: escape analysis of method expression calls
Matthew Dempsky [Wed, 23 Jun 2021 07:31:16 +0000 (00:31 -0700)]
[dev.typeparams] cmd/compile: escape analysis of method expression calls

This CL extends escape analysis to analyze function calls using method
expressions the same as it would a normal method call. That is, it now
analyzes "T.M(recv, args...)" the same as "recv.M(args...)".

This is useful because it means the frontend can eventually stop
supporting both function calls and method calls. We can simply desugar
method calls into function calls, like we already do in the backend to
simplify SSA construction.

Change-Id: I9cd5ec0d534cbcd9860f0014c86e4ae416920c26
Reviewed-on: https://go-review.googlesource.com/c/go/+/330331
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
3 years ago[dev.typeparams] cmd/compile: move call logic from order.go to escape
Matthew Dempsky [Tue, 22 Jun 2021 05:35:01 +0000 (22:35 -0700)]
[dev.typeparams] cmd/compile: move call logic from order.go to escape

This CL moves two bits of related code from order.go to escape
analysis:

1. The recognition of "unsafe uintptr" arguments passed to
syscall-like functions.

2. The wrapping of go/defer function calls in parameter-free function
literals.

As with previous CLs, it would be nice to push this logic even further
forward, but for now escape analysis seems most pragmatic.

A couple side benefits:

1. It allows getting rid of the uintptrEscapesHack kludge.

2. When inserting wrappers, we can move some expressions into the
wrapper and escape analyze them better. For example, the test
expectation changes are all due to slice literals in go/defer calls
where the slice is now constructed at the call site, and can now be
stack allocated.

Change-Id: I73679bcad7fa8d61d2fc52d4cea0dc5ff0de8c0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/330330
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
3 years ago[dev.typeparams] cmd/compile: desugar ORECOVER into ORECOVERFP
Matthew Dempsky [Tue, 22 Jun 2021 20:44:18 +0000 (13:44 -0700)]
[dev.typeparams] cmd/compile: desugar ORECOVER into ORECOVERFP

Currently ORECOVER is a single operation that both (1) calculates
the (logical) caller frame pointer and (2) calls runtime.gorecover.
This is normally fine, but it's inconvenient for regabi, which wants
to wrap "defer recover()" into "defer func() { recover() }" and
needs (1) and (2) to happen at different times.

The current solution is to apply walkRecover early to split it into
the two steps, but calling it during order is a minor layering
violation. It works well today because the order and walk phases are
closely related anyway and walkRecover is relatively simple, but it
won't work for go/defer wrapping earlier into the frontend.

This CL adds a new, lower-level ORECOVERFP primitive, which represents
just part (2); and OGETCALLER{PC,SP} primitives, which provide a way
to compute (1) in the frontend too.

OGETCALLERPC isn't needed/used today, but it seems worth including for
completeness. Maybe it will be useful at some point for intrinsifying
runtime.getcaller{pc,sp}, like we already do for runtime.getg.

Change-Id: Iaa8ae51e09306c45c147b6759a5b7c24dcc317ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/330192
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
3 years ago[dev.typeparams] cmd/compile: add ORECOVERFP, OGETCALLER{PC,SP} ops
Matthew Dempsky [Tue, 22 Jun 2021 20:45:51 +0000 (13:45 -0700)]
[dev.typeparams] cmd/compile: add ORECOVERFP, OGETCALLER{PC,SP} ops

This CLs adds new frontend ops that will be used in the next CL. Split
out separately so generated code is less distracting in the main CL.

Change-Id: I66125e0ec2217bfa05f7b0ea0bc99ada13f563f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/330191
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
3 years ago[dev.typeparams] cmd/compile: remove SetClosureCalled(false) hacks
Matthew Dempsky [Wed, 23 Jun 2021 05:21:09 +0000 (22:21 -0700)]
[dev.typeparams] cmd/compile: remove SetClosureCalled(false) hacks

The current go/defer wrapping code goes to some length to clear
ClosureCalled when a function call will end up not being called
directly, and so it will need to use the context register.

But we already have a flag to indicate we need to use the context
register: Needctxt. The real issue here is just that buildssa was
using fn.ClosureCalled instead of fn.Needctxt.

Change-Id: Ic9f5f23b66eb467fc61fa84eacb45d46c54133d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/330329
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
3 years ago[dev.typeparams] cmd/compile: explain why expandInline needed
Cuong Manh Le [Wed, 23 Jun 2021 07:39:44 +0000 (14:39 +0700)]
[dev.typeparams] cmd/compile: explain why expandInline needed

Change-Id: Ica9817675b4eb929a000640f9ae873b75fc5a2e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/330290
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: refactor escape analysis of calls
Matthew Dempsky [Wed, 23 Jun 2021 03:53:14 +0000 (20:53 -0700)]
[dev.typeparams] cmd/compile: refactor escape analysis of calls

This CL is a prep refactoring for an upcoming CL to move go/defer
wrapping into escape analysis. That CL is unfortunately unavoidably
complex and subtle, so this CL takes care of some more mundane
refactoring details.

Change-Id: Ifbefe1d522a8d57066646be09536437f42e7082c
Reviewed-on: https://go-review.googlesource.com/c/go/+/330251
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
3 years ago[dev.typeparams] cmd/compile: remove CallExpr.PreserveClosure
Matthew Dempsky [Wed, 23 Jun 2021 03:37:42 +0000 (20:37 -0700)]
[dev.typeparams] cmd/compile: remove CallExpr.PreserveClosure

This flag is only needed to prevent the directClosureCall optimization
in walkCall, when called for walkGoDefer. But walkGoDefer don't need
to call walkCall: at this point in the compile, the call expression
isn't a real call anymore.

Instead, we just need to walkExpr on the function expression.

Change-Id: I8a5176cfe1bff53700cbd21ed1b479ebd9a839ad
Reviewed-on: https://go-review.googlesource.com/c/go/+/330271
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Matthew Dempsky <mdempsky@google.com>

3 years ago[dev.typeparams] cmd/compile: simplify walkGoDefer
Matthew Dempsky [Wed, 23 Jun 2021 03:33:00 +0000 (20:33 -0700)]
[dev.typeparams] cmd/compile: simplify walkGoDefer

order already takes care of wrapping all go/defer function calls, so
there's no need for walk to duplicate that logic: it's never going to
be used.

Change-Id: I54e545404e52ab8f9d60151d1bd2aff4b9bd8b72
Reviewed-on: https://go-review.googlesource.com/c/go/+/330270
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>

3 years ago[dev.typeparams] cmd/compile: allow typecheck of OCHECKNIL
Matthew Dempsky [Wed, 23 Jun 2021 01:15:21 +0000 (18:15 -0700)]
[dev.typeparams] cmd/compile: allow typecheck of OCHECKNIL

This CL makes OCHECKNIL typecheckable. Simplifies IR construction code
slightly, and gives one convenient place to check for misuse.

Change-Id: I280b8e47eddcac12947a41d6f911b25bc12a66bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/330194
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
3 years ago[dev.typeparams] cmd/compile: refactor CaptureName
Matthew Dempsky [Wed, 23 Jun 2021 01:10:59 +0000 (18:10 -0700)]
[dev.typeparams] cmd/compile: refactor CaptureName

CaptureName currently does a few things: checks if a variable needs to
be captured at all; checks if the variable has already been captured;
and creates and saves a new variable. This full suite of functionality
is useful for noder and irgen, but unified IR and other backend code
only has a need for the last feature.

This CL refactors CaptureName a little bit and extracts out
NewClosureVar as a function usable for callers that don't need the
extra features of CaptureName.

Change-Id: I8a67c6375e44babe53344bf78e335535c57f9607
Reviewed-on: https://go-review.googlesource.com/c/go/+/330193
Trust: Matthew Dempsky <mdempsky@google.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
3 years ago[dev.typeparams] go/types: adjust logic for method expression arg naming
Rob Findley [Tue, 22 Jun 2021 14:13:54 +0000 (10:13 -0400)]
[dev.typeparams] go/types: adjust logic for method expression arg naming

CL 325369 improved this logic in types2. Port this improvement back to
go/types.

Change-Id: I5f859cbffd88bb3db09a81c2389269f7bd0869f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/330069
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
3 years ago[dev.typeparams] cmd/gofmt: remove typeparams guards
Rob Findley [Tue, 22 Jun 2021 00:12:20 +0000 (20:12 -0400)]
[dev.typeparams] cmd/gofmt: remove typeparams guards

Remove logic related to guarding against allowing type parameters from
cmd/gofmt. At this point, it was only restricting tests.

Change-Id: Idd198389aaa422636d61af547a37be49f3be6c97
Reviewed-on: https://go-review.googlesource.com/c/go/+/329931
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
3 years agocmd/go: update ToolTags based on GOARCH value
Ian Lance Taylor [Mon, 21 Jun 2021 21:48:54 +0000 (14:48 -0700)]
cmd/go: update ToolTags based on GOARCH value

The build.Context ToolTags value is set based on the set of enabled
experiments, which in turn depends on GOARCH. Before this CL the set
of experiments was being set based on GOARCH in the environment.
That is normally fine, but fails with cmd/go when somebody has run
"go env -w GOARCH=val"; in that case cmd/go changes its GOARCH value
after initialization. The new GOARCH value was affect the set of
enabled experiments, which can affect the ToolTags value. With this
CL, we update ToolTags in cmd/go based on the GOARCH value it is using.

This is a pretty ugly fix. We should do something cleaner for 1.18.

Fixes #46815

Change-Id: Ie9416781a168248813c3da8afdc257acdd3fef7e
Reviewed-on: https://go-review.googlesource.com/c/go/+/329930
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years agospec: unsafe.Add/Slice are not permitted in statement context
Robert Griesemer [Tue, 22 Jun 2021 05:20:11 +0000 (22:20 -0700)]
spec: unsafe.Add/Slice are not permitted in statement context

Add unsafe.Add and unsafe.Slice to the list of built-in functions
which are not permitted in statement context. The compiler and
type checker already enforce this restriction, this just fixes
a documentation oversight.

For #19367.
For #40481.

Change-Id: Iabc63a8db048eaf40a5f5b5573fdf00b79d54119
Reviewed-on: https://go-review.googlesource.com/c/go/+/329925
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Rob Pike <r@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>

3 years agoruntime/internal/atomic: remove incorrect pointer indirection in comment
Xing Gao [Tue, 22 Jun 2021 02:12:29 +0000 (02:12 +0000)]
runtime/internal/atomic: remove incorrect pointer indirection in comment

Change-Id: I9d743b7f6b001158299bea4af4aede678654bc8e
GitHub-Last-Rev: 7e07834abc861e21030fe4a8eb323bac01e18f7a
GitHub-Pull-Request: golang/go#46851
Reviewed-on: https://go-review.googlesource.com/c/go/+/329730
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ben Shi <powerman1st@163.com>
3 years agogo/types: guard against checking instantiation when generics is disabled
Rob Findley [Tue, 22 Jun 2021 00:16:37 +0000 (20:16 -0400)]
go/types: guard against checking instantiation when generics is disabled

When type checking t[_], where t is a type name, it was possible to leak
an error message related to generics. Fix this by guarding on
typeparams.Enabled.

In order to test this fix, we need to be able to run the new go/types
test only if type parameters are disabled. Introduce the .go1 test data
suffix (similar to .go2) to control this behavior.

Originally found via fuzzing, though the test case was manually
simplified.

Updates #46404

Change-Id: Ib1e2c27cf974c2a5ca5b9d6d01b84a30ba4d583b
Reviewed-on: https://go-review.googlesource.com/c/go/+/329793
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
3 years ago[dev.typeparams] cmd/compile: split package escape into multiple files
Matthew Dempsky [Tue, 22 Jun 2021 08:26:34 +0000 (01:26 -0700)]
[dev.typeparams] cmd/compile: split package escape into multiple files

This CL reorganizes the code from package escape into multiple files,
so the relationships between bits of code are hopefully easier to
follow. Besides moving code around and adding necessary
copyright/import declarations, no code is touched at all.

Change-Id: Iddd396c3a140f4eb1a7a6266d92a4098118b575b
Reviewed-on: https://go-review.googlesource.com/c/go/+/329989
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
3 years ago[dev.typeparams] cmd/compile: remove special escape analysis tags
Matthew Dempsky [Tue, 22 Jun 2021 05:03:02 +0000 (22:03 -0700)]
[dev.typeparams] cmd/compile: remove special escape analysis tags

This CL removes the special escape analysis tags added to support
//go:uintptrescapes and calls to external functions. Instead, these
are kept as function pragmas.

This CL by itself isn't very interesting, but I expect will help with
subsequent cleanups I have planned here.

Change-Id: Ifb960289a27e0a6295ce2d2f5ec233cac590522b
Reviewed-on: https://go-review.googlesource.com/c/go/+/329969
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Matthew Dempsky <mdempsky@google.com>

3 years ago[dev.typeparams] cmd/compile: add -d=unifiedquirks for quirks mode
Matthew Dempsky [Tue, 22 Jun 2021 03:03:58 +0000 (20:03 -0700)]
[dev.typeparams] cmd/compile: add -d=unifiedquirks for quirks mode

Originally, overloading -d=inlfuncswithclosures=0 to enable quirks
mode was convenient because toolstash -cmp doesn't provide a way to
pass different gcflags to the installed vs stashed toolchains. Prior
to unified IR being merged, the stashed toolchain wouldn't know about
or accept any unified-specific flags.

However, this concern is no longer applicable since unified IR has
been merged, and the TestUnifiedCompare test can easily specify
different flag sets for the baseline and experiment build configs.

This CL adds a new -d=unifiedquirks flag to enable quirks mode, so
that it's possible to test unified IR with -d=inlfuncswithclosures=0
without also affecting a bunch of other compilation details.

Change-Id: Id1932f332822622aa8617278e82ec6d1a53b1b46
Reviewed-on: https://go-review.googlesource.com/c/go/+/329733
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>

3 years agocmd/gofmt: remove stale documentation for the -G flag
Rob Findley [Tue, 22 Jun 2021 00:14:21 +0000 (20:14 -0400)]
cmd/gofmt: remove stale documentation for the -G flag

This documentation remained from the original dev.typeparams merge. This
flag no longer exists.

Change-Id: Ic9a82071c512614dc1382780d69ef13253fca21d
Reviewed-on: https://go-review.googlesource.com/c/go/+/329792
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
3 years agogo/parser: parse an ast.IndexExpr for a[]
Rob Findley [Mon, 21 Jun 2021 20:10:14 +0000 (16:10 -0400)]
go/parser: parse an ast.IndexExpr for a[]

To be consistent with Go 1.16, and to preserve as much information in
the AST as possible, parse an ast.IndexExpr with BadExpr Index for the
invalid expression a[].

A go/types test had to be adjusted to account for an additional error
resulting from this change.

We don't have a lot of test coverage for parser error recovery, so
rather than write an ad-hoc test for this issue, add a new go/types test
that checks that the indexed operand is used.

Updates #46403

Change-Id: I21e6ff4179746aaa50e530d4091fded450e69824
Reviewed-on: https://go-review.googlesource.com/c/go/+/329791
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
3 years agocmd/go: add a -go flag to 'go mod graph'
Bryan C. Mills [Fri, 18 Jun 2021 21:28:29 +0000 (17:28 -0400)]
cmd/go: add a -go flag to 'go mod graph'

For #46366

Change-Id: I8417e6e4dbb8cb56ff7afc16893a01b7bb938217
Reviewed-on: https://go-review.googlesource.com/c/go/+/329529
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
3 years agocmd/internal/moddeps: use a temporary directory for GOMODCACHE if needed
Bryan C. Mills [Wed, 16 Jun 2021 20:33:29 +0000 (16:33 -0400)]
cmd/internal/moddeps: use a temporary directory for GOMODCACHE if needed

CL 328770 should be sufficient to fix the specific failure in the
report, but when attempting to reproduce it I noticed a related
failure mode, triggered by the environment variables set in
src/run.bash.

The failure mode is currently masked on the Go project builders due to
the lack of any 'longtest' builder running as a non-root user
(#10719).

It is also masked from Go contributors running 'run.bash' locally
because 'run.bash' does not actually run all of the tests unless
GO_TEST_SHORT=0 is set in the environment (#29266, #46054).

Fixes #46695

Change-Id: I272c09dae462734590dce59b3d3c5b6d3f733c92
Reviewed-on: https://go-review.googlesource.com/c/go/+/328771
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
3 years agocmd/internal/moddeps: use -mod=readonly instead of -mod=mod
Bryan C. Mills [Wed, 16 Jun 2021 18:34:54 +0000 (14:34 -0400)]
cmd/internal/moddeps: use -mod=readonly instead of -mod=mod

TestAllDependencies is attempting to check that the modules in GOROOT
satisfy certain properties; it should not modify those modules itself.

The “quick” part of the test checks that vendored packages are present
and complete, without constructing a parallel GOROOT. It shouldn't
resolve new dependencies or change formatting in any way.

The longer version of the test already constructs a parallel GOROOT
and tidies the modules within it. That part of the test will flag any
modifications needed to the go.mod and go.sum files, without modifying
the original GOROOT.

From what I can tell, the failure mode in #46695 is caused by running
the test on a module rooted in $GOROOT proper. There is no such module
in the mainline Go repo, but it may have been introduced in the fork
and could also be introduced by stray edits in contributor CLs. It
should be diagnosed clearly.

For #46695

Change-Id: I62b90ccbd54cb3e3b413017021c952a7b1d455e7
Reviewed-on: https://go-review.googlesource.com/c/go/+/328770
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
3 years agocmd/go: document GOPPC64 environment variable
Ian Lance Taylor [Sun, 20 Jun 2021 19:22:16 +0000 (12:22 -0700)]
cmd/go: document GOPPC64 environment variable

Change-Id: I2d2c02eec4ac6eca218fa5334d32650c1620692c
Reviewed-on: https://go-review.googlesource.com/c/go/+/329689
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Carlos Eduardo Seo <carlos.seo@linaro.org>
3 years agogo/scanner: fall back to next() when encountering 0 bytes in parseIdentifier
Rob Findley [Mon, 21 Jun 2021 16:52:17 +0000 (12:52 -0400)]
go/scanner: fall back to next() when encountering 0 bytes in parseIdentifier

CL 308611 optimized parseIdentifier for ASCII, but inadvertently skipped
error handling for 0 bytes. Don't take the optimized path when
encountering 0.

Fixes #46855

Change-Id: Ic584e077eb74c012611fefa20eb71ca09c81b3c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/329790
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
3 years agodatabase/sql: fix deadlock test in prepare statement
Daniel Theophanes [Mon, 21 Jun 2021 16:11:20 +0000 (11:11 -0500)]
database/sql: fix deadlock test in prepare statement

The issue go#46783 correctly diagnosed the context timeout
caused an intermittent failure when the context was canceled
prior to the BeginTx call. However due to the asynchronous nature
of canceling a Tx through a context on fast systems, the tx.Prepare
also succeeded. On slower systems or if a time.Sleep was inserted
between the BeginTx and Prepare, the Prepare would fail.

Resolve this by moving the context cancel after the Prepare.
This will still trigger the deadlock which I tested locally.
In addition, I interspersed multiple time.Sleep calls and the
test still functioned.

Fixes #46852

Change-Id: I9cbf90d3c12b2555493a37799738772b615ae39d
Reviewed-on: https://go-review.googlesource.com/c/go/+/329830
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Bryan C. Mills <bcmills@google.com>

3 years ago[dev.typeparams] all: merge master (16e82be) into dev.typeparams
Cuong Manh Le [Mon, 21 Jun 2021 17:35:07 +0000 (00:35 +0700)]
[dev.typeparams] all: merge master (16e82be) into dev.typeparams

Merge List:

+ 2021-06-21 16e82be454 runtime: fix crash during VDSO calls on PowerPC
+ 2021-06-21 2e542c3c06 runtime/pprof: deflake TestMorestack more
+ 2021-06-21 ced0fdbad0 doc/go1.17: note deprecation of 'go get' for installing commands
+ 2021-06-21 7a5e7047a4 doc/go1.17: add Go 1.18 pre-announcements
+ 2021-06-21 85a2e24afd doc/go1.17: add security-related release notes
+ 2021-06-21 1de332996c doc/go1.17: document go/parser.SkipObjectResolution
+ 2021-06-21 117ebe0f52 cmd/go: do not require the module cache to exist for 'go mod edit'
+ 2021-06-20 460900a7b5 os/signal: test with a significantly longer fatal timeout
+ 2021-06-19 b73cc4b02b database/sql: do not rely on timeout for deadlock test
+ 2021-06-18 86743e7d86 image: add RGBA64Image interface
+ 2021-06-18 9401172166 runtime: clarify Frames.Next documentation
+ 2021-06-18 57aaa19aae runtime: disable CPU profiling before removing the SIGPROF handler
+ 2021-06-18 6f22d2c682 doc/go1.17: fix typo
+ 2021-06-17 45f251ad6c cmd/pprof,runtime/pprof: disable test on more broken platforms
+ 2021-06-17 ed834853ad cmd/go: replace a TODO with an explanatory comment
+ 2021-06-17 4dede02550 cmd/pprof: make ObjAddr a no-op
+ 2021-06-17 97cee43c93 testing: drop unusual characters from TempDir directory name
+ 2021-06-17 b0355a3e72 time: fix receiver for Time.IsDST method
+ 2021-06-17 881b6ea7ba doc/go1.17: fix redundant space
+ 2021-06-16 0e67ce3d28 cmd/go: in lazy modules, add transitive imports for 'go get' arguments
+ 2021-06-16 6ea2af0890 cmd/go: add a regression test for #45979
+ 2021-06-16 a294e4e798 math/rand: mention half-open intervals explicitly
+ 2021-06-16 a6a853f94c cmd/asm: restore supporting of *1 scaling on ARM64

Change-Id: Ifdcb817fd44b4fa9c477042b41da55d1d769b016

3 years agoruntime: fix crash during VDSO calls on PowerPC
Derek Parker [Thu, 17 Jun 2021 20:22:40 +0000 (20:22 +0000)]
runtime: fix crash during VDSO calls on PowerPC

This patch reinstates a fix for PowerPC with regard to making VDSO calls
while receiving a signal, and subsequently crashing. The crash happens
because certain VDSO calls can modify the r30 register, which is where g
is stored. This change was reverted for PowerPC because r30 is supposed
to be a non-volatile register. This is true, but that only makes a
guarantee across function calls, but not "within" a function call. This
patch was seemingly fine before because the Linux kernel still had hand
rolled assembly VDSO function calls, however with a recent change to C
function calls it seems the compiler used can generate instructions
which temporarily clobber r30. This means that when we receive a signal
during one of these calls the value of r30 will not be the g as the
runtime expects, causing a segfault.

You can see from this assembly dump how the register is clobbered during
the call:

(the following is from a 5.13rc2 kernel)

```
Dump of assembler code for function __cvdso_clock_gettime_data:
   0x00007ffff7ff0700 <+0>:     cmplwi  r4,15
   0x00007ffff7ff0704 <+4>:     bgt     0x7ffff7ff07f0 <__cvdso_clock_gettime_data+240>
   0x00007ffff7ff0708 <+8>:     li      r9,1
   0x00007ffff7ff070c <+12>:    slw     r9,r9,r4
   0x00007ffff7ff0710 <+16>:    andi.   r10,r9,2179
   0x00007ffff7ff0714 <+20>:    beq     0x7ffff7ff0810 <__cvdso_clock_gettime_data+272>
   0x00007ffff7ff0718 <+24>:    rldicr  r10,r4,4,59
   0x00007ffff7ff071c <+28>:    lis     r9,32767
   0x00007ffff7ff0720 <+32>:    std     r30,-16(r1)
   0x00007ffff7ff0724 <+36>:    std     r31,-8(r1)
   0x00007ffff7ff0728 <+40>:    add     r6,r3,r10
   0x00007ffff7ff072c <+44>:    ori     r4,r9,65535
   0x00007ffff7ff0730 <+48>:    lwz     r8,0(r3)
   0x00007ffff7ff0734 <+52>:    andi.   r9,r8,1
   0x00007ffff7ff0738 <+56>:    bne     0x7ffff7ff07d0 <__cvdso_clock_gettime_data+208>
   0x00007ffff7ff073c <+60>:    lwsync
   0x00007ffff7ff0740 <+64>:    mftb    r30      <---- RIGHT HERE
=> 0x00007ffff7ff0744 <+68>:    ld      r12,40(r6)
```

What I believe is happening is that the kernel changed the PowerPC VDSO
calls to use standard C calls instead of using hand rolled assembly. The
hand rolled assembly calls never touched r30, so this change was safe to
roll back. That does not seem to be the case anymore as on the 5.13rc2
kernel the compiler *is* generating assembly which modifies r30, making
this change again unsafe and causing a crash when the program receives a
signal during these calls (which will happen often due to async
preempt). This change happened here:
https://lwn.net/ml/linux-kernel/235e5571959cfa89ced081d7e838ed5ff38447d2.1601365870.git.christophe.leroy@csgroup.eu/.

I realize this was reverted due to unexplained hangs in PowerPC
builders, but I think we should reinstate this change and investigate
those issues separately:
https://github.com/golang/go/commit/f4ca3c1e0a2066ca4f7bd6203866d282ed34acf2

Fixes #46803

Change-Id: Ib18d7bbfc80a1a9cb558f0098878d41081324b52
GitHub-Last-Rev: c3002bcfca3ef58b27485e31328e6297b7a9dfe7
GitHub-Pull-Request: golang/go#46767
Reviewed-on: https://go-review.googlesource.com/c/go/+/328110
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Trust: Lynn Boger <laboger@linux.vnet.ibm.com>

3 years agoruntime/pprof: deflake TestMorestack more
Cherry Mui [Mon, 21 Jun 2021 16:09:43 +0000 (12:09 -0400)]
runtime/pprof: deflake TestMorestack more

Apparently, TestMorestack is still flaky on darwin/arm64 builder
after CL 307730. Let it spend more time in copying the stack.
With this CL, on my Apple M1 machine it passes reliably in short
mode for 1000 runs, and reliably gets 250+ samples in the 5-second
interval in long mode.

May fix #46755.

Change-Id: I07b36c1cf63ad35f7820e1f8e837e29376a37b2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/329869
Trust: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
3 years agodoc/go1.17: note deprecation of 'go get' for installing commands
Jay Conrod [Fri, 18 Jun 2021 22:07:00 +0000 (15:07 -0700)]
doc/go1.17: note deprecation of 'go get' for installing commands

Fixes #43684

Change-Id: I8982f6816c002c71e62f37a926c8543e34b8b785
Reviewed-on: https://go-review.googlesource.com/c/go/+/329549
Trust: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
3 years agodoc/go1.17: add Go 1.18 pre-announcements
Filippo Valsorda [Tue, 15 Jun 2021 14:59:58 +0000 (10:59 -0400)]
doc/go1.17: add Go 1.18 pre-announcements

Updates #41682
Updates #45428

Change-Id: Ia31d454284f0e114bd29ba398a2858fc90454032
Reviewed-on: https://go-review.googlesource.com/c/go/+/327811
Trust: Filippo Valsorda <filippo@golang.org>
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
3 years agodoc/go1.17: add security-related release notes
Filippo Valsorda [Tue, 15 Jun 2021 11:13:08 +0000 (07:13 -0400)]
doc/go1.17: add security-related release notes

Change-Id: I573def0f48fe66a1bc60fff321ab007c76b47ef0
Reviewed-on: https://go-review.googlesource.com/c/go/+/327810
Reviewed-by: Katie Hockman <katie@golang.org>
Trust: Katie Hockman <katie@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>

3 years agodoc/go1.17: document go/parser.SkipObjectResolution
Rob Findley [Thu, 17 Jun 2021 16:28:53 +0000 (12:28 -0400)]
doc/go1.17: document go/parser.SkipObjectResolution

Documents the mode added in CL 306149 to skip object resolution.

Fixes #46298

Change-Id: I6a14aaa00790f9f7e4e4ba17033355f5e878d74b
Reviewed-on: https://go-review.googlesource.com/c/go/+/329009
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
3 years agocmd/go: do not require the module cache to exist for 'go mod edit'
Bryan C. Mills [Wed, 16 Jun 2021 18:48:45 +0000 (14:48 -0400)]
cmd/go: do not require the module cache to exist for 'go mod edit'

Updates #46695

Change-Id: I4afbc1401ef4183d94c1ac6271394fac1fff95ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/328769
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
3 years ago[dev.typeparams] cmd/compile: simplify import* functions
Cuong Manh Le [Sun, 20 Jun 2021 14:33:54 +0000 (21:33 +0700)]
[dev.typeparams] cmd/compile: simplify import* functions

CL 280634 remove Sym.Importdef, so ipkg in importsym is not used
anymore. So we can remove it from importsym and all other import*
functions, which just call importsym internally.

Change-Id: I15b9d11c4445dbe40982f7ff2a33a2116705e790
Reviewed-on: https://go-review.googlesource.com/c/go/+/329573
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: explain why reader.funcExt need to set n.Defn
Cuong Manh Le [Sun, 20 Jun 2021 15:17:19 +0000 (22:17 +0700)]
[dev.typeparams] cmd/compile: explain why reader.funcExt need to set n.Defn

Change-Id: I1a7d669879af57a1c1f48ce63ff0d214b694e680
Reviewed-on: https://go-review.googlesource.com/c/go/+/329572
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: fold reader checking type params logic to separate...
Cuong Manh Le [Sun, 20 Jun 2021 07:33:51 +0000 (14:33 +0700)]
[dev.typeparams] cmd/compile: fold reader checking type params logic to separate method

So making it less verbose and clearer to the reader what that check means.

Change-Id: I41587aab399e63600356c5cecec64978048bed36
Reviewed-on: https://go-review.googlesource.com/c/go/+/329571
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years agoos/signal: test with a significantly longer fatal timeout
Andrew G. Morgan [Sat, 19 Jun 2021 16:46:52 +0000 (09:46 -0700)]
os/signal: test with a significantly longer fatal timeout

We've observed some occasional os-arch specific timeouts
in signal.TestSignalTrace(). While the main purpose of a
short timeout is to ensure the passing tests complete
quickly, the unexpected failure path can tolerate waiting
longer (the test is not intended to test how slow or
overloaded the OS is at the time it is run).

Fixes #46736
Change-Id: Ib392fc6ce485a919612784ca88ed76c30f4898e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/329502
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>

3 years ago[dev.typeparams] cmd/compile: explain how pkgReader.typIdx handles alias cyclic
Cuong Manh Le [Sat, 19 Jun 2021 19:06:45 +0000 (02:06 +0700)]
[dev.typeparams] cmd/compile: explain how pkgReader.typIdx handles alias cyclic

Change-Id: Ib9357c21bb010abf0d5fd17c3bee3197854c3a8a
Reviewed-on: https://go-review.googlesource.com/c/go/+/329570
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years agodatabase/sql: do not rely on timeout for deadlock test
Daniel Theophanes [Wed, 16 Jun 2021 16:46:00 +0000 (11:46 -0500)]
database/sql: do not rely on timeout for deadlock test

Fixes #46783

Change-Id: I8a8d1716279a041a7411c0c47a440a7997b39c80
Reviewed-on: https://go-review.googlesource.com/c/go/+/328649
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Trust: Carlos Amedee <carlos@golang.org>

3 years agoimage: add RGBA64Image interface
Nigel Tao [Sun, 18 Apr 2021 03:38:34 +0000 (13:38 +1000)]
image: add RGBA64Image interface

The new RGBA64At method is equivalent to the existing At method (and the
new SetRGBA64 method is equivalent to the existing Set method in the
image/draw package), but they can avoid allocations from converting
concrete color types to the color.Color interface type.

Also update api/go1.17.txt and doc/go1.17.html

Fixes #44808

Change-Id: I8671f3144512b1200fa373840ed6729a5d61bc35
Reviewed-on: https://go-review.googlesource.com/c/go/+/311129
Trust: Nigel Tao <nigeltao@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
3 years ago[dev.typeparams] cmd/compile: add "toolstash -cmp"-like test of -d=unified
Matthew Dempsky [Fri, 18 Jun 2021 10:21:43 +0000 (03:21 -0700)]
[dev.typeparams] cmd/compile: add "toolstash -cmp"-like test of -d=unified

This CL adds a longtest test to make sure -d=unified=1 produces output
identical to -d=unified=0.

Change-Id: I2c5d38f67dbc8fecd8332a91ba7cae22225b090c
Reviewed-on: https://go-review.googlesource.com/c/go/+/329429
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
3 years agoruntime: clarify Frames.Next documentation
Matthew Dempsky [Fri, 18 Jun 2021 05:12:24 +0000 (22:12 -0700)]
runtime: clarify Frames.Next documentation

I wrote code that relied on this API, but I misunderstood the original
description of the "more" result. As a consequence, my code always
stopped one frame early.

This CL expands the documentation to be more explicit and specifically
call out my confusion (i.e., that the "more" result indicates whether
the *next* Next call will return a valid Frame, and not whether this
call did).

Change-Id: If135f8f8c05425073d45377c4179e4f79e6bd6ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/329389
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>

3 years agoruntime: disable CPU profiling before removing the SIGPROF handler
Ian Lance Taylor [Fri, 18 Jun 2021 00:34:18 +0000 (17:34 -0700)]
runtime: disable CPU profiling before removing the SIGPROF handler

Otherwise, in c-archive or c-shared mode, there is the chance of
getting a SIGPROF just after the signal handler is removed but before
profiling is disabled, in which case the program will die.

Fixes #46498

Change-Id: I5492beef45fec9fb9a7f58724356d6aedaf799ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/329290
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
3 years ago[dev.typeparams] cmd/compile: add missing copy of Field.Embedded in type substituter.
Dan Scales [Sun, 13 Jun 2021 18:05:45 +0000 (11:05 -0700)]
[dev.typeparams] cmd/compile: add missing copy of Field.Embedded in type substituter.

Change-Id: I876933370a6bcb6586eda9d8fc28a081bf31b1cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/328511
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Dan Scales <danscales@google.com>

3 years agodoc/go1.17: fix typo
Nick Miyake [Fri, 18 Jun 2021 00:19:10 +0000 (00:19 +0000)]
doc/go1.17: fix typo

Change-Id: Ie8629e0f710d3eb95b4bbcc9c680ffc5004c2f15
GitHub-Last-Rev: 52f48f429cdd65692b88f724f757efed1fa39cd2
GitHub-Pull-Request: golang/go#46812
Reviewed-on: https://go-review.googlesource.com/c/go/+/329289
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Tobias Klauser <tobias.klauser@gmail.com>

3 years ago[dev.typeparams] cmd/compile: add documentation for unified IR pipeline
Cuong Manh Le [Thu, 17 Jun 2021 15:51:45 +0000 (22:51 +0700)]
[dev.typeparams] cmd/compile: add documentation for unified IR pipeline

While at it, also rename "useUnifiedIR" to "unified", to be consistent
with "-d=unified" and "GOEXPERIMENT=unified".

Change-Id: I48ffdb4b36368343893b74f174608f5f59278249
Reviewed-on: https://go-review.googlesource.com/c/go/+/328989
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] cmd/compile: record writer's stack at export data sync points
Matthew Dempsky [Thu, 17 Jun 2021 08:12:23 +0000 (01:12 -0700)]
[dev.typeparams] cmd/compile: record writer's stack at export data sync points

This CL extends the unified export data format's existing sync
mechanism to save writer stacks, controlled by the -d=syncframes debug
flag. This allows readers to provide more details when reporting
desync errors, which should simplify development of the data format
and the various reader/writer implementations.

For example, CL 328051 updated reader and writer, but missed making a
similar change to the linker (fix in CL 328054). Re-reviewing the CL
in isolation after the failure, it was not immediately obvious what
was going wrong. But the pair of stack traces below identifies exactly
what happened: it should have updated linker.relocFuncExt to write out
the new sync marker too.

```
data sync error: package "internal/abi", section 6, index 4, offset 536

found UseReloc, written at:
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/encoder.go:221: (*encoder).reloc +0x44
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/linker.go:214: (*linker).relocFuncExt +0x580
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/linker.go:233: (*linker).relocTypeExt +0x234
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/linker.go:161: (*linker).relocObj +0x2198
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/linker.go:64: (*linker).relocIdx +0x196

expected ImplicitTypes, reading at:
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/reader.go:796: (*reader).implicitTypes +0x36
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/reader.go:810: (*reader).addBody +0x81
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/reader.go:727: (*reader).funcExt +0x542
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/reader.go:651: (*reader).method +0x324
/home/mdempsky/wd/go/src/cmd/compile/internal/noder/reader.go:557: (*pkgReader).objIdx +0x2704
```

Change-Id: I911193edd2a965f81b7459f15fb613a773584685
Reviewed-on: https://go-review.googlesource.com/c/go/+/328909
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
3 years ago[dev.typeparams] cmd/go: include new internal packages in TestNewReleaseRebuildsStale...
Michael Anthony Knyszek [Fri, 18 Jun 2021 03:14:09 +0000 (03:14 +0000)]
[dev.typeparams] cmd/go: include new internal packages in TestNewReleaseRebuildsStalePackagesInGOPATH

CL 328336 introduced two new packages that the runtime and other
low-level packages depend on. Include them as targets to copy in this
test with other such packages.

Fixes the dev.typeparams longtest builders.

Change-Id: Ia886f0264962a68acd06ebca002eef8515f06487
Reviewed-on: https://go-review.googlesource.com/c/go/+/329251
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
3 years ago[dev.typeparams] go/types: report better error for invalid untyped operation
Cuong Manh Le [Thu, 17 Jun 2021 03:14:07 +0000 (10:14 +0700)]
[dev.typeparams] go/types: report better error for invalid untyped operation

This is port of CL 328053 for types2 to go/type.

The change is identical, but for some tweaks to the error positions in
tests.

Updates #46749

Change-Id: I8d34c5b1669e59e4ec7d91f81dcf655b2bfd89a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/328869
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
3 years ago[dev.typeparams] cmd/compile/internal/syntax: convert (most) parser tests to new...
Robert Griesemer [Wed, 16 Jun 2021 00:48:31 +0000 (17:48 -0700)]
[dev.typeparams] cmd/compile/internal/syntax: convert (most) parser tests to new type set syntax

Left a couple of tests with old notation so that we keep testing it
while the notation is still supported.

Change-Id: Ia6a3e7911af87eaccc7b06189c10f79789575a98
Reviewed-on: https://go-review.googlesource.com/c/go/+/328256
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
3 years ago[dev.typeparams] cmd/compile: skip types2 GC test during bootstrapping
Matthew Dempsky [Thu, 17 Jun 2021 22:36:23 +0000 (15:36 -0700)]
[dev.typeparams] cmd/compile: skip types2 GC test during bootstrapping

Unified includes a check to make sure that types2 memory has been
garbage collected, but it relies on precise finalization, which we
provide (for dynamically allocated objects, at least) but isn't
guaranteed by the Go spec. In particular, Go 1.4 doesn't provide this.

The check is strictly unnecessary and only exists to make sure we
don't regress and start holding onto types2 memory accidentally. So
just disable the check during bootstrap builds.

Change-Id: Ie54fe53b2edba02c0b0b1e5ae39d81be8a0ace8d
Reviewed-on: https://go-review.googlesource.com/c/go/+/329269
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
3 years agocmd/pprof,runtime/pprof: disable test on more broken platforms
Michael Pratt [Thu, 17 Jun 2021 19:23:53 +0000 (15:23 -0400)]
cmd/pprof,runtime/pprof: disable test on more broken platforms

runtime/pprof has a more complete list of platforms with broken
profiling than I used in cmd/pprof in https://golang.org/cl/325809.
Duplicate that list in cmd/pprof and clean it up a bit in runtime/pprof
for easier reference.

Change-Id: I8f2580aac223de9b73cfff4355f49916f7b76493
Reviewed-on: https://go-review.googlesource.com/c/go/+/329149
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>

3 years ago[dev.typeparams] reflect: support big endian architectures in callMethod
Michael Anthony Knyszek [Wed, 16 Jun 2021 19:08:52 +0000 (19:08 +0000)]
[dev.typeparams] reflect: support big endian architectures in callMethod

Currently, callMethod has some ABI translation code that is not agnostic
of endianness. This change rectifies that by adding a method to
internal/abi.RegArgs for safely returning an offset into a register slot
that's endianness-dependent.

No tests for this because it's just best-effort. There's no actual way
to test this because we don't support a register ABI on any big endian
architectures yet.

Change-Id: Ic68d9ee1bfdea0fc2992d467d749e2b083e92de3
Reviewed-on: https://go-review.googlesource.com/c/go/+/328348
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
3 years ago[dev.typeparams] internal/reflectlite: remove unused ptrSize
Michael Anthony Knyszek [Wed, 16 Jun 2021 23:29:40 +0000 (23:29 +0000)]
[dev.typeparams] internal/reflectlite: remove unused ptrSize

Change-Id: Ia0da5e5d1e8d20327690cb53c9df067401f3428c
Reviewed-on: https://go-review.googlesource.com/c/go/+/328812
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>