Skip to content
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

Jump to bottom

cmd/compile: panic message for failed assertion of generic type mentions shape type, not user type #53276

Closed
mdempsky opened this issue Jun 7, 2022 · 5 comments
Closed

cmd/compile: panic message for failed assertion of generic type mentions shape type, not user type #53276

mdempsky opened this issue Jun 7, 2022 · 5 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.20

Comments

@mdempsky
Copy link
Member

mdempsky commented Jun 7, 2022

https://go.dev/play/p/3Ru8nGnvTQY

The above program reports at run-time "panic: interface conversion: interface { T() go.shape.int_0 } is nil, not int".

I think the correct error is "panic: interface conversion: interface { T() int } is nil, not int".

The problem is the call to s.reflectType(src) in ssagen/ssa.go. This is too late to do dictionary lookups, so we end up using the *runtime._type for the shape type, rather than the user type.

We should push all the reflectdata.TypePtr / reflectdata.TypeLinksym calls earlier into the frontend, so they can be replaced with dictionary accesses when appropriate. Same applies to reflectdata.ITabAddr and reflectdata.ITabLSym.

/cc @randall77 @dr2chase @cuonglm
@mdempsky mdempsky added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 7, 2022
@mdempsky mdempsky modified the milestones: Go1.19, Go1.20 Jun 7, 2022
@gopherbot
Copy link

Change https://go.dev/cl/411695 mentions this issue: [dev.unified] test: add regress tests for #53276 and #53328

gopherbot pushed a commit that referenced this issue Jun 10, 2022
@mdempsky @gopherbot
[dev.unified] test: add regress tests for #53276 and #53328
f73ad3d 
These two tests fail with the 1.18 compiler frontend, because of
incomplete dictionary support. This CL adds the tests for Unified IR,
which currently handles them correctly, to make sure it doesn't repeat
the same errors.

Updates #53276.
Updates #53328.

Change-Id: I9f436495d28f2bc5707a17bd2527c86abacf91f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/411695
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/413357 mentions this issue: [dev.unified] cmd/compile: avoid reflectType in ssagen

gopherbot pushed a commit that referenced this issue Jun 21, 2022
@mdempsky
[dev.unified] cmd/compile: avoid reflectType in ssagen
5e0258c 
This CL adds alternate code paths for the frontend to plumb through
rtypes to package ssagen, so the latter doesn't have to use
reflectType (which in general will only have access to shape types).

Note: This CL doesn't yet plumb through the rtypes for variables that
escape to the heap. However, those rtypes are only used for calling
runtime.newobject, and the status quo as of Go 1.18 is already to use
shape rtypes for most runtime.newobject calls. (Longer term though, I
would like to get rid of shape rtypes altogether.)

Passes toolstash -cmp.

Updates #53276.

Change-Id: I76a281eca8300de2e701fbac89ead32f8568a5f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/413357
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
@gopherbot
Copy link

Change https://go.dev/cl/421374 mentions this issue: all: REVERSE MERGE dev.unified (d558507) into master

gopherbot pushed a commit that referenced this issue Aug 4, 2022
@mdempsky
all: REVERSE MERGE dev.unified (d558507) into master
a10afb1 
This commit is a REVERSE MERGE.
It merges dev.unified back into its parent branch, master.
This marks the end of development on dev.unified.

Merge List:

+ 2022-08-04 d558507 [dev.unified] all: merge master (85d87b9) into dev.unified
+ 2022-08-03 c9f2150 [dev.unified] cmd/compile: start using runtime dictionaries
+ 2022-07-30 994ff78 [dev.unified] go/internal: set underlying types in proper order
+ 2022-07-28 23554d4 [dev.unified] all: merge master (462b78f) into dev.unified
+ 2022-07-28 c8d5ccf [dev.unified] go/internal/gcimporter: flatten imports
+ 2022-07-28 ac0844e [dev.unified] cmd/compile: move "has init" to private metadata
+ 2022-07-28 f995946 [dev.unified] cmd/compile: implement simple inline body pruning heuristic
+ 2022-07-28 f2851c6 [dev.unified] cmd/compile: allow inlining to fail gracefully
+ 2022-07-28 831fdf1 [dev.unified] cmd/compile: extract nil handling from exprType
+ 2022-07-28 9279817 [dev.unified] cmd/compile: write iface conversion RTTI into unified IR
+ 2022-07-28 9b70178 [dev.unified] cmd/compile: write RTTI into unified IR export data
+ 2022-07-25 fc72b77 [dev.unified] cmd/compile: add method expressions to dictionaries
+ 2022-07-25 f48fa64 [dev.unified] cmd/compile: remove obsolete RTTI wiring
+ 2022-07-22 131f981 [dev.unified] cmd/compile: make Unified IR always writes concrete type for const exprs
+ 2022-07-20 ae43bdc Merge "[dev.unified] all: merge master (8e1e64c) into dev.unified" into dev.unified
+ 2022-07-19 7a8ba83 [dev.unified] cmd/compile/internal/reflectdata: remove hasRType's `required` param
+ 2022-07-19 64cd6fa [dev.unified] cmd/compile/internal/noder: simplify mixed tag/case RTTI wiring
+ 2022-07-19 a4c5198 [dev.unified] cmd/compile/internal/noder: better switch statements
+ 2022-07-19 3180270 [dev.unified] cmd/compile/internal/noder: explicit nil handling
+ 2022-07-19 e971b6a [dev.unified] test: add switch test case for tricky nil handling
+ 2022-07-19 878439c [dev.unified] cmd/compile/internal/noder: preserve RTTI for select statements
+ 2022-07-19 e376746 [dev.unified] cmd/compile/internal/noder: wire RTTI for implicit conversions
+ 2022-07-19 c846fd8 [dev.unified] cmd/compile/internal/noder: implicit conversions for binary exprs
+ 2022-07-19 ebd34e3 [dev.unified] test: relax panic message expectations
+ 2022-07-19 76a82f0 [dev.unified] cmd/compile/internal/noder: prefer *At functions
+ 2022-07-19 de649a2 [dev.unified] all: merge master (8e1e64c) into dev.unified
+ 2022-07-19 055a5e5 [dev.unified] test: change Unicode file/package name to use characters not translated by macOS.
+ 2022-07-18 2cf632c [dev.unified] cmd/compile/internal/reflectdata: prefer ITabAddrAt in ConvIfaceTypeWord
+ 2022-07-12 9371a65 internal/pkgbits: change EnableSync into a dynamic knob
+ 2022-07-01 d667be8 [dev.unified] cmd/compile/internal/walk: RType fields for range assignments
+ 2022-06-30 1b838e9 [dev.unified] all: merge master (993c387) into dev.unified
+ 2022-06-30 0a503cf [dev.unified] cmd/compile: refactor `range` desugaring
+ 2022-06-30 3635b07 [dev.unified] cmd/compile/internal/noder: implicit conversions for multi-valued expressions
+ 2022-06-30 e7219cc [dev.unified] cmd/compile/internal/noder: refactor N:1 expression handling
+ 2022-06-30 2f3ef73 [dev.unified] test: tweak nilcheck test
+ 2022-06-30 95d7ce9 [dev.unified] test: break escape_iface.go into unified/nounified variants
+ 2022-06-30 f751319 [dev.unified] test: relax live_regabi.go
+ 2022-06-30 e3cdc98 [dev.unified] cmd/compile/internal/walk: fix typo in debug print
+ 2022-06-29 2280d89 [dev.unified] test: add regress test for generic select statements
+ 2022-06-27 4b78ece [dev.unified] cmd/compile: drop package height from Unified IR importer
+ 2022-06-27 398d46d [dev.unified] cmd/compile/internal/types2: remove package height
+ 2022-06-24 e7100ad [dev.unified] all: merge master (5a1c5b8) into dev.unified
+ 2022-06-23 09a838a [dev.unified] cmd/compile: rename haveRType and implicitExpr
+ 2022-06-23 421e9e9 [dev.unified] cmd/compile: implicit conversions for return statements
+ 2022-06-23 a3fea77 [dev.unified] cmd/compile/internal/noder: implicit conversions for writer.assignStmt
+ 2022-06-23 82a958a [dev.unified] cmd/compile/internal/noder: refactor stmtAssign generation
+ 2022-06-23 711dacd [dev.unified] cmd/compile/internal/noder: implicit conversion of call arguments
+ 2022-06-23 46b01ec [dev.unified] cmd/compile/internal/noder: remove needType logic
+ 2022-06-23 a3e474f [dev.unified] cmd/compile/internal/noder: implicit conversions for complits
+ 2022-06-23 5f5422a [dev.unified] cmd/compile/internal/noder: start writing implicit conversions
+ 2022-06-23 9cb784a [dev.unified] cmd/compile/internal/noder: add pkgWriter.typeOf helper
+ 2022-06-23 c70e93f [dev.unified] cmd/compile/internal/typecheck: replace unreachable code with assert
+ 2022-06-23 20e1d5a [dev.unified] cmd/compile: special case f(g()) calls in Unified IR
+ 2022-06-23 61ae2b7 [dev.unified] cmd/compile: plumb rtype through OSWITCH/OCASE clauses
+ 2022-06-23 3d432b6 [dev.unified] cmd/compile: plumb rtype through for OMAPLIT
+ 2022-06-23 7368647 [dev.unified] cmd/compile: start setting RType fields for Unified IR
+ 2022-06-23 5960f4e [dev.unified] cmd/compile: add RType fields
+ 2022-06-21 5e0258c [dev.unified] cmd/compile: avoid reflectType in ssagen
+ 2022-06-21 93833cd [dev.unified] cmd/compile: extract rtype code from walk
+ 2022-06-21 f70775f [dev.unified] cmd/compile: refactor reflectdata.{TypePtr,ITabAddr}
+ 2022-06-21 fc5dad6 [dev.unified] cmd/compile/internal/walk: minor prep refactoring
+ 2022-06-16 1f4e8af [dev.unified] all: merge master (635b124) into dev.unified
+ 2022-06-15 8a9485c [dev.unified] test: extract different inline test between unified and non-unified
+ 2022-06-14 394ea70 [dev.unified] cmd/compile: more Unified IR docs and review
+ 2022-06-10 f73ad3d [dev.unified] test: add regress tests for #53276 and #53328
+ 2022-06-09 8ef8b60 [dev.unified] cmd/compile/internal/noder: stop handling type expressions as expressions
+ 2022-06-09 1a6c96b [dev.unified] test: relax issue7921.go diagnostic message
+ 2022-06-09 c50c6bb [dev.unified] cmd/compile: set base.Pos when process assignDef in Unified IR
+ 2022-06-09 d6df086 [dev.unified] cmd/compile: fix unified IR don't report type size too large error
+ 2022-06-08 e7ef585 [dev.unified] cmd/compile: restore Unified IR linkname pragma diagnostic
+ 2022-06-07 9e5c968 [dev.unified] cmd/compile: visit LHS before RHS/X in assign/for statement
+ 2022-06-06 46ddf08 [dev.unified] cmd/compile: export/import implicit attribute for conversion exprs
+ 2022-06-06 a8780f9 [dev.unified] cmd/compile: fix missing method value wrapper in unified IR
+ 2022-06-06 3a1f1e1 [dev.unified] cmd/compile: remove package height
+ 2022-06-06 df7cb59 [dev.unified] cmd/compile: only sort symbols by name and package path
+ 2022-06-06 b39ac80 [dev.unified] cmd/compile/internal/noder: push exprBlank up into assignment handling
+ 2022-06-06 55fc07e [dev.unified] cmd/compile/internal/noder: add optExpr for optional expressions
+ 2022-06-06 6c33f1d [dev.unified] cmd/compile/internal/noder: rename exprName to exprGlobal
+ 2022-06-06 4d28fca [dev.unified] all: update codereview.cfg for dev.unified branch

Change-Id: I604d057735e8a365621c91c206f9e46eabb4679b
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
@mdempsky @jproberts
[dev.unified] test: add regress tests for golang#53276 and golang#53328
62690ce 
These two tests fail with the 1.18 compiler frontend, because of
incomplete dictionary support. This CL adds the tests for Unified IR,
which currently handles them correctly, to make sure it doesn't repeat
the same errors.

Updates golang#53276.
Updates golang#53328.

Change-Id: I9f436495d28f2bc5707a17bd2527c86abacf91f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/411695
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
@mdempsky @jproberts
[dev.unified] cmd/compile: avoid reflectType in ssagen
f5ff8c7 
This CL adds alternate code paths for the frontend to plumb through
rtypes to package ssagen, so the latter doesn't have to use
reflectType (which in general will only have access to shape types).

Note: This CL doesn't yet plumb through the rtypes for variables that
escape to the heap. However, those rtypes are only used for calling
runtime.newobject, and the status quo as of Go 1.18 is already to use
shape rtypes for most runtime.newobject calls. (Longer term though, I
would like to get rid of shape rtypes altogether.)

Passes toolstash -cmp.

Updates golang#53276.

Change-Id: I76a281eca8300de2e701fbac89ead32f8568a5f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/413357
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
@mdempsky @jproberts
[dev.unified] test: add regress tests for golang#53276 and golang#53328
09737ca 
These two tests fail with the 1.18 compiler frontend, because of
incomplete dictionary support. This CL adds the tests for Unified IR,
which currently handles them correctly, to make sure it doesn't repeat
the same errors.

Updates golang#53276.
Updates golang#53328.

Change-Id: I9f436495d28f2bc5707a17bd2527c86abacf91f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/411695
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
@cuonglm
Copy link
Member

cuonglm commented Sep 30, 2022

@mdempsky seems we can close this now?

@mdempsky
Copy link
Member Author

@cuonglm Yes, thanks.

@golang golang locked and limited conversation to collaborators Sep 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.20
Development

No branches or pull requests
3 participants
@mdempsky @cuonglm @gopherbot