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
cmd/compile: "UNREVIEWED" headers scattered around the tree #48194
Comments
Files marked as
The importer test files should match the corresponding test files elsewhere and should be straight-forward. We review them by comparing them against existing reviewed files and pointing out the differences, if any. The noder files are related to @mdempsky 's work on the unified IR and need to be reviewed in full. This is planned to happen by the end of October (before the freeze). |
Checking in on this issue as it's labeled a release blocker for Go 1.18. Is there any update? |
Several of these files have been reviewed and now we have a shorter list remaining:
These are all in new compiler files with functionality that may not be enabled by default for cc: @mdempsky |
Yeah, all of those files are only used for |
There are a handful of unreviewed files remaining. Given that we're going to enable unified IR soon, can we get a quick status update on reviewing these?
|
@aclements My understanding is that @mdempsky is working on getting all the remaining prerequisites in place (such as a compiler-matching importer for go/types, which I reviewed yesterday). We will review these files soon. |
Change https://go.dev/cl/407614 mentions this issue: |
Change https://go.dev/cl/408234 mentions this issue: |
This CL adds documentation for all exported pkgbits APIs, and removes its UNREVIEWED comments. Updates #48194. Change-Id: Ifff548cd9f31a5c5cc5f400a6dae5c98c46ec4ca Reviewed-on: https://go-review.googlesource.com/c/go/+/407614 Reviewed-by: Robert Griesemer <gri@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com>
If I'm not mistaken, once CL 408234 lands, the only remaining unreviewed file should be cmd/compile/internal/importer/ureader.go Given that we're not enabling unified IR for 1.19, it seems like it should be okay to mark this okay-after-beta1 (or possibly even drop release-blocker?) (This is great progress!) (Edited: I meant "1.19", not "1.20" above.) |
Yeah, we can bump the issue for 1.20. |
This comment was marked as resolved.
This comment was marked as resolved.
Change https://go.dev/cl/411917 mentions this issue: |
This adds more documentation throughout the core Unified IR logic and removes their UNREVIEWED notices. Updates #48194. Change-Id: Iddd30edaee1c6ea8a05a5a7e013480e02be00d29 Reviewed-on: https://go-review.googlesource.com/c/go/+/411917 Auto-Submit: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: David Chase <drchase@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
This adds more documentation throughout the core Unified IR logic and removes their UNREVIEWED notices. Updates golang#48194. Change-Id: Iddd30edaee1c6ea8a05a5a7e013480e02be00d29 Reviewed-on: https://go-review.googlesource.com/c/go/+/411917 Auto-Submit: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: David Chase <drchase@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
Just checking in, it looks like cmd/compile/internal/importer/ureader.go is the only remaining UNREVIEWED file? |
Change https://go.dev/cl/422617 mentions this issue: |
I think we just forgot to remove the UNREVIEWED header for those files, I sent https://go-review.googlesource.com/c/go/+/422617 for this. |
A bunch of files in the tree start:
I assume this is left over from generics work. Can those all be removed now?
cc @ianlancetaylor @griesemer @mdempsky
The text was updated successfully, but these errors were encountered: