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.

Sign up for GitHub

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

test: cleanup fixedbugs/notinheap.go #54846

Closed
cuonglm opened this issue Sep 2, 2022 · 4 comments
Closed

test: cleanup fixedbugs/notinheap.go #54846

cuonglm opened this issue Sep 2, 2022 · 4 comments
Assignees
@cuonglm
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.23

Comments

@cuonglm
Copy link
Member

cuonglm commented Sep 2, 2022

After #46731, //go:notinheap is gone, however, there's still test/fixedbugs/notinheap.go which is using it. There's no problem with trybot, because the file is ignored:

go/test/run.go

Line 1975 in 34f0029

"fixedbugs/notinheap.go", // types2 doesn't report errors about conversions that are invalid due to //go:notinheap

We should revise those test cases:

  • Writing the equivalent ones, replacing //go:notinheap by cgo.Incomplete.
  • Remove the file entirely.

cc @mdempsky @randall77
@cuonglm cuonglm self-assigned this Sep 2, 2022
@cuonglm cuonglm added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 2, 2022
@cuonglm cuonglm added this to the Go1.20 milestone Sep 2, 2022
@cuonglm
Copy link
Member Author

cuonglm commented Sep 2, 2022

Taking a quick look, not all the cases can be rewrite, for example, there's no thing like this anymore:

//go:notinheap
type t3 byte

@gopherbot gopherbot modified the milestones: Go1.20, Go1.21 Feb 1, 2023
@gopherbot gopherbot modified the milestones: Go1.21, Go1.22 Aug 8, 2023
@gopherbot gopherbot modified the milestones: Go1.22, Go1.23 Feb 6, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/597535 mentions this issue: cmd/compile: restore not-in-heap check for map/channel type

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/597536 mentions this issue: test: re-enabled fixedbugs/notinheap.go test

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/597537 mentions this issue: test: add test cases for local not-in-heap types

gopherbot pushed a commit that referenced this issue Jul 24, 2024
@cuonglm @gopherbot
cmd/compile: restore not-in-heap check for map/channel type
88833c9 
CL 388538 removed unused -G=0 node types.

However, the code for checking not-in-heap types for map and channel
type was also removed, which is likely not intentional.

This CL restores the check, porting removed code to noder.

Updates #54846

Change-Id: I2995836b90e36d2684197fefc9829fddfffe8585
Reviewed-on: https://go-review.googlesource.com/c/go/+/597535
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@google.com>
gopherbot pushed a commit that referenced this issue Jul 24, 2024
@cuonglm @gopherbot
test: add test cases for local not-in-heap types
72cc769 
Follow up review in CL 597535.

Updates #54846

Change-Id: Idf586552a22bbc6710c0888f046df0a5d1f0ce4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/597537
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cuonglm cuonglm

Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.23
Development

No branches or pull requests
2 participants
@cuonglm @gopherbot