-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merge upstream master
into microsoft/main
#185
Merged
microsoft-golang-review-bot
merged 15 commits into
microsoft:microsoft/main
from
microsoft-golang-bot:auto-merge/microsoft/main
Aug 19, 2021
Merged
Merge upstream master
into microsoft/main
#185
microsoft-golang-review-bot
merged 15 commits into
microsoft:microsoft/main
from
microsoft-golang-bot:auto-merge/microsoft/main
Aug 19, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixed a problem where an error would not occur when a negative value was specified for the p flag. `go build -p=0` now should throw an error. this is my first pr to this project. If there's anything I'm missing, please let me know 🙏 Fixes #46686 Change-Id: I3b19773ef095fad0e0419100d317727c2268699a GitHub-Last-Rev: e5c57804d9995f5c858aa42d9de21b25de246eb5 GitHub-Pull-Request: golang/go#47360 Reviewed-on: https://go-review.googlesource.com/c/go/+/336751 Reviewed-by: Jay Conrod <jayconrod@google.com> Trust: Jay Conrod <jayconrod@google.com> Trust: Bryan C. Mills <bcmills@google.com> Run-TryBot: Jay Conrod <jayconrod@google.com> TryBot-Result: Go Bot <gobot@golang.org>
Clean up a few issues related to panicking during invalid instantiation. - Panic early in instantiateLazy when check == nil and verify == true. Otherwise, we would panic at check.later. - Always panic when check == nil and verify == true, even if targs is of incorrect length. This is more consistent behavior. - Lift the check for len(posList) <= len(targs) out of Checker.instantiate. This is the only reason why posList is passed to that function, and doing this allows us to eliminate posList from instance. At this point instance is close to being unnecessary, so update a TODO to this effect. Change-Id: Id5f44cbb1a5897aef10ce2a573aa78acd7ae4026 Reviewed-on: https://go-review.googlesource.com/c/go/+/341862 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>
Change an internal call of instantiateLazy to call Instantiate, so that we can consolidate the logic for invoking verification. This made verification of signatures lazy, which is not necessary but should be harmless. Change-Id: I2e59b04ac859e08c2e2910ded3c183093d1e34a7 Reviewed-on: https://go-review.googlesource.com/c/go/+/342149 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>
…dquirks These sorts are only important for 'toolstash -cmp' testing of unified IR against -G=0 mode, but they were added before I added -d=unifiedquirks to allow altering small "don't care" output details like this. This CL should help mitigate issues with #44195 until package objectpath is updated and deployed. Change-Id: Ia3dcf359481ff7abad5ddfca8e673fd2bb30ae01 Reviewed-on: https://go-review.googlesource.com/c/go/+/343390 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>
When substituting a type instance, we rely on the instance being expanded and do not call validType, so there is need to depend on subster.pos for error reporting or to use subst.check for creating the new Named type. Errors will be reported for the unsubstituted instance. This is a superficial change, but justifies some later simplification where we don't have access to pos or check. Change-Id: I1f3f12aa245d821512c6242ad829c940f20afae4 Reviewed-on: https://go-review.googlesource.com/c/go/+/342150 Trust: Robert Findley <rfindley@google.com> Run-TryBot: Robert Findley <rfindley@google.com> Reviewed-by: Robert Griesemer <gri@golang.org> TryBot-Result: Go Bot <gobot@golang.org>
In preparation for upcoming API changes, change the internal API for verification of type arguments to return an error and argument index, and use this to lift up error reporting into Instantiate. Change-Id: I88b1e64dd9055c4c20c0db49c96c79c5da894450 Reviewed-on: https://go-review.googlesource.com/c/go/+/342151 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>
Conversions to regular concrete types should not be rewritten during stenciling. Fixes #47740 Change-Id: I2b45e22f962dcd2e18bd6cc876ebc0f850860822 Reviewed-on: https://go-review.googlesource.com/c/go/+/342989 Trust: Keith Randall <khr@golang.org> Trust: Dan Scales <danscales@google.com> Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com> Run-TryBot: Keith Randall <khr@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Dan Scales <danscales@google.com>
…sistently It appears that this code predates golang.org/cl/96535, which added RelCol to support /*line*/ directives. Change-Id: Ib79cebc1be53af706e84e8799eeea81ef8c81c8b Reviewed-on: https://go-review.googlesource.com/c/go/+/343430 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
This is used within Google's internal code repo, so getting it working is a pre-req for enabling -G=3 by default. Change-Id: Icbc570948c852ca09cdb2a59f778140f620244b2 Reviewed-on: https://go-review.googlesource.com/c/go/+/343429 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>
Many of the methods inside the archive/tar package don't need to be exported. Doing so sets a bad precedent that it's OK to export methods to indicate an internal public API. That's not a good idea in general, because exported methods increase cognitive load when reading code: the reader needs to consider whether the exported method might be used via some external interface or reflection. This CL should have no externally visible behaviour changes at all. Change-Id: I94a63de5e6a28e9ac8a283325217349ebce4f308 Reviewed-on: https://go-review.googlesource.com/c/go/+/341410 Reviewed-by: Joe Tsai <joetsai@digital-static.net> Trust: Joe Tsai <joetsai@digital-static.net> Trust: Michael Knyszek <mknyszek@google.com>
The methods on the pipe type don't need to be exported. Doing so sets a bad precedent that it's OK to export methods to indicate an internal public API. That's not a good idea in general, because exported methods increase cognitive load when reading code: the reader needs to consider whether the exported method might be used via some external interface or reflection. Change-Id: Ib13f1b3f9fe0ff251628f31b776182a0953268ee Reviewed-on: https://go-review.googlesource.com/c/go/+/341409 Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com> Reviewed-by: Joe Tsai <joetsai@digital-static.net> Trust: Daniel Martí <mvdan@mvdan.cc>
Change Instantiate to be a function (not a method) and return an error. Introduce an ArgumentError type to report information about which type argument led to an error during verification. This resolves a few concerns with the current API: - The Checker method set was previously just Files. It is somewhat odd to add an additional method for instantiation. Passing the checker as an argument seems cleaner. - pos, posList, and verify were bound together. In cases where no verification is required, the call site was somewhat cluttered. - Callers will likely want to access structured information about why type information is invalid, and also may not have access to position information. Returning an argument index solves both these problems; if callers want to associate errors with an argument position, they can do this via the resulting index. We may want to make the first argument an opaque environment rather than a Checker. Change-Id: I3bc56d205c13d832b538401a4c91d3917c041225 Reviewed-on: https://go-review.googlesource.com/c/go/+/342152 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>
Conversion between slices with different element types is not allowed. Previously (1.8 <= goversion <= 1.16), this conversion was allowed if the base types were from different packages and had identical names. Update #47785 Change-Id: I359de5b6fe3ff35bdbf9ab5a13902a0f820cac66 Reviewed-on: https://go-review.googlesource.com/c/go/+/343329 Trust: Keith Randall <khr@golang.org> Run-TryBot: Keith Randall <khr@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
While processing signatset, the entry is deleted immediately after being pushed to signatslice. Then calling writeType may add the same type to signatset again. That would add more works, though not a big impact to the performace, since when writeType is guarded by s.Siggen() check. Instead, we should keep the entry in signatset, so written type will never be added again. This change does not affect compiler performace, but help debugging issue like one in #46386 easier. Change-Id: Iddafe773885fa21cb7003ba27ddf9554fc3f297d Reviewed-on: https://go-review.googlesource.com/c/go/+/326029 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: Keith Randall <khr@golang.org>
microsoft-golang-bot
requested review from
microsoft-golang-review-bot and
a team
as code owners
August 19, 2021 17:27
microsoft-golang-review-bot
approved these changes
Aug 19, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Auto-approving.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🔃 This is an automatically generated PR merging upstream
master
intomicrosoft/main
.This PR should auto-merge itself when PR validation passes. If CI fails and you need to make fixups, be sure to use a merge commit, not a squash or rebase!
After these changes, the difference between upstream and the branch is: