cmd/compile: failed to build github.com/hashicorp/cap/oidc due to internal compiler error: unexpected Defn: #45743
Comments
|
Small reproduction package main
type opt func(interface{})
type a struct {
s0 []string
s1 []string
f func() t
}
type b struct {
s0 []string
s1 []string
f func() t
}
type t struct {
wall uint64
ext int64
t *t
}
func fn(f func() t) opt {
return func(o interface{}) {
switch v := o.(type) {
case *a:
v.f = f
case *b:
v.f = f
}
}
}
func test(opt) {}
func main() {
test(fn(func() t { return t{} }))
} |
ALTree
added
NeedsInvestigation
|
Change https://golang.org/cl/313289 mentions this issue: |
|
Thanks for the test case and bisection. This was already incorrectly handled before d310b21, but that commit added additional internal checks which are failing on this test case. It looks like we need to extend the AssignStmt/AssignListStmt rewriting to also handle SwitchStmt+TypeSwitchGuard. I uploaded a partial fix for this at golang.org/cl/313289, but I'm suspicious because it's finding switch variables with /cc @danscales |
|
@mdempsky I think that the non-setting of Defn is unrelated to closures. The nil value for Defn is happening for functions without any closures (for example, when inlining os.underlyingError). I debugged it a bit, and see that Defn is not being set at all in inlvar(), even if the input node var_ has var_.Defn set. So, when the name nodes for the per-case variables (named 'err' in the example of os.underlyingError) in the Dcl list are copied during inlining via inlvar, they are not getting their Defn set. Not quite sure why this isn't causing problems, or whether this glitch was a result of the big re-factoring. |
|
@danscales is that also the reason we have the |
|
@cuonglm, Besides the possible bug in inlvar(), I think nameNode.Defn can be legitimately nil for param name nodes. |
|
@danscales Hmm, I see for param name nodes, we always set its Defn in I see inlvar behavior seems correct, we don't want new node Defn point to the same Defn with old node, we want it point to the right node in inlined form (that's d310b2a for). |
|
Yes, but for parameters of normal functions (i.e. not in the process of being inlined), nameNode.Defn is not set (because there is no assignment to refer back to for a function input parameter). |
|
Probably it's enough for now to just change |
|
Change https://golang.org/cl/319192 mentions this issue: |
|
Change https://golang.org/cl/319191 mentions this issue: |
|
Change https://golang.org/cl/324573 mentions this issue: |
This CL adds a new unified IR construction mode to the frontend. It's purely additive, and all files include "UNREVIEWED" at the top, like how types2 was initially imported. The next CL adds a -d=unified flag to actually enable unified IR mode. See below for more details, but some highlights: 1. It adds ~6kloc (excluding enum listings and stringer output), but I estimate it will allow removing ~14kloc (see CL 324670, including its commit message); 2. When enabled by default, it passes more tests than -G=3 does (see CL 325213 and CL 324673); 3. Without requiring any new code, it supports inlining of more code than the current inliner (see CL 324574; contrast CL 283112 and CL 266203, which added support for inlining function literals and type switches, respectively); 4. Aside from dictionaries (which I intend to add still), its support for generics is more complete (e.g., it fully supports local types, including local generic types within generic functions and instantiating generic types with local types; see test/typeparam/nested.go); 5. It supports lazy loading of types and objects for types2 type checking; 6. It supports re-exporting of types, objects, and inline bodies without needing to parse them into IR; 7. The new export data format has extensive support for debugging with "sync" markers, so mistakes during development are easier to catch; 8. When compiling with -d=inlfuncswithclosures=0, it enables "quirks mode" where it generates output that passes toolstash -cmp. -- The new unified IR pipeline combines noding, stenciling, inlining, and import/export into a single, shared code path. Previously, IR trees went through multiple phases of copying during compilation: 1. "Noding": the syntax AST is copied into the initial IR form. To support generics, there's now also "irgen", which implements the same idea, but takes advantage of types2 type-checking results to more directly construct IR. 2. "Stenciling": generic IR forms are copied into instantiated IR forms, substituting type parameters as appropriate. 3. "Inlining": the inliner made backup copies of inlinable functions, and then copied them again when inlining into a call site, with some modifications (e.g., updating position information, rewriting variable references, changing "return" statements into "goto"). 4. "Importing/exporting": the exporter wrote out the IR as saved by the inliner, and then the importer read it back as to be used by the inliner again. Normal functions are imported/exported "desugared", while generic functions are imported/exported in source form. These passes are all conceptually the same thing: make a copy of a function body, maybe with some minor changes/substitutions. However, they're all completely separate implementations that frequently run into the same issues because IR has many nuanced corner cases. For example, inlining currently doesn't support local defined types, "range" loops, or labeled "for"/"switch" statements, because these require special handling around Sym references. We've recently extended the inliner to support new features like inlining type switches and function literals, and they've had issues. The exporter only knows how to export from IR form, so when re-exporting inlinable functions (e.g., methods on imported types that are exposed via exported APIs), these functions may need to be imported as IR for the sole purpose of being immediately exported back out again. By unifying all of these modes of copying into a single code path that cleanly separates concerns, we eliminate many of these possible issues. Some recent examples: 1. Issues #45743 and #46472 were issues where type switches were mishandled by inlining and stenciling, respectively; but neither of these affected unified IR, because it constructs type switches using the exact same code as for normal functions. 2. CL 325409 fixes an issue in stenciling with implicit conversion of values of type-parameter type to variables of interface type, but this issue did not affect unified IR. Change-Id: I5a05991fe16d68bb0f712503e034cb9f2d19e296 Reviewed-on: https://go-review.googlesource.com/c/go/+/324573 Trust: Matthew Dempsky <[email protected]> Trust: Robert Griesemer <[email protected]> Run-TryBot: Matthew Dempsky <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Robert Griesemer <[email protected]>
What version of Go are you using (
go version)?Does this issue reproduce with the latest release?
No, gotip only.
What operating system and processor architecture are you using (
go env)?go envOutputWhat did you do?
What did you expect to see?
Build succeds
What did you see instead?
The text was updated successfully, but these errors were encountered: