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

go/types: missing error on an edge-case with continue/break with labels #70974

Open
mateusz834 opened this issue Dec 23, 2024 · 5 comments · May be fixed by #70976
Open

go/types: missing error on an edge-case with continue/break with labels #70974

mateusz834 opened this issue Dec 23, 2024 · 5 comments · May be fixed by #70976
Assignees
@mateusz834
Labels
NeedsFix The path to resolution is known, but the work has not been done.

Comments

@mateusz834
Copy link
Member

mateusz834 commented Dec 23, 2024 •
edited
Loading

Once again, because i am poking around with go/types internals, i have spotted a bug in the type-checker.

The code below type-checks without any error:

package test

func test() {
outer:
	for range "aa" {
		break outer
	}

	for range "aa" {
		break outer
	}
}

This happens because lstmt:

go/src/go/types/labels.go

Line 169 in b9955f0

lstmt = s

is assigned and never cleared, thus:

go/src/go/types/labels.go

Lines 234 to 236 in b9955f0

case *ast.BlockStmt:
blockBranches(lstmt, s.List)

is executed with a wrong label (for the second for statement).

CC @adonovan @griesemer
@mateusz834 mateusz834 added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 23, 2024
@mateusz834
Copy link
Member Author

I will send a CL for this.

@mateusz834 mateusz834 changed the title go/types: missing error on an edge-case with continue/break go/types: missing error on an edge-case with continue/break with labels Dec 23, 2024
@adonovan
Copy link
Member

I'm curious: how are you finding these interesting bugs? Are you using a fuzzer, or just reading skeptically? Either way, impressive.

@mateusz834 mateusz834 linked a pull request Dec 23, 2024 that will close this issue
Open
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/638257 mentions this issue: go/types: propagate *ast.LabeledStmt in blockBranches properly

@mateusz834
Copy link
Member Author

@adonovan I am working on a POC go/* fork tgoast, with a HTML-like syntax (it is a superset of Go, Go is a valid tgo (working name)). As I try to get the fork of types working, something told me that the current logic seemed wrong, after a few attempts, this bug came to my mind.

@mateusz834 mateusz834 self-assigned this Dec 24, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/640895 mentions this issue: cmd/compile/internal/syntax: add test case for invalid label use

gopherbot pushed a commit that referenced this issue Jan 7, 2025
@griesemer @gopherbot
cmd/compile/internal/syntax: add test case for invalid label use
9d0772b 
This case is not properly handled by the type checkers (see issue)
but the compiler uses the parser's label checking so it works as
expected.

For #70974.

Change-Id: I0849376bf7514a9a7730846649c3fe28c91f44ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/640895
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>
Auto-Submit: Robert Griesemer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mateusz834 mateusz834

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

Successfully merging a pull request may close this issue.

go/types: propagate *ast.LabeledStmt in blockBranches properly mateusz834/go
3 participants
@adonovan @gopherbot @mateusz834