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

cmd/compile: double error when break is not in a loop #51456

Closed
alicebob opened this issue Mar 3, 2022 · 4 comments
Closed

cmd/compile: double error when break is not in a loop #51456

alicebob opened this issue Mar 3, 2022 · 4 comments
Assignees
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@alicebob
Copy link
Contributor

alicebob commented Mar 3, 2022

What version of Go are you using (go version)?

go version devel go1.19-d3fe4e193e Thu Mar 3 16:02:44 2022 +0000 linux/amd64

and

go version go1.18rc1 linux/amd64

Does this issue reproduce with the latest release?

(edit) No, on 1.17.7 it's a single error.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/harmen/.cache/go-build"
GOENV="/home/harmen/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/harmen/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/harmen/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/harmen/src/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/harmen/src/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel go1.19-d3fe4e193e Thu Mar 3 16:02:44 2022 +0000"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2287321463=/tmp/go-build -gno-record-gcc-switches"

What did you do?

package main

func main() {
	break
}

https://go.dev/play/p/pyIH7X9EZMT?v=gotip

What did you expect to see?

A single error

What did you see instead?

harmen@umount:/tmp$ go run foo.go
# command-line-arguments
./foo.go:4:2: break is not in a loop, switch, or select
./foo.go:4:2: break is not in a loop, switch, or select statement
harmen@umount:/tmp$ 
@alicebob alicebob changed the title affected/package: double error when break is not in a loop cmd/compile: double error when break is not in a loop Mar 3, 2022
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 3, 2022
@ALTree ALTree added this to the Go1.19 milestone Mar 3, 2022
@ALTree
Copy link
Member

ALTree commented Mar 3, 2022

Thanks for reporting this. Tentatively putting it in the 1.19 milestone because it's a (non-serious) 1.18 regression.

@ianlancetaylor
Copy link
Contributor

CC @griesemer

Rolling forward to 1.20. Please comment if you disagree. Thanks.

@ianlancetaylor ianlancetaylor modified the milestones: Go1.19, Go1.20 Jun 24, 2022
@griesemer griesemer self-assigned this Jun 24, 2022
@gopherbot
Copy link

Change https://go.dev/cl/414134 mentions this issue: cmd/compile/internal/syntax: check fallthrough in CheckBranches mode

@gopherbot
Copy link

Change https://go.dev/cl/414135 mentions this issue: cmd/compile: do branch/label checks only once

gopherbot pushed a commit that referenced this issue Jun 26, 2022
The parser CheckBranches mode checked correct use of break, continue,
and labels, but not of fallthrough statements.

This CL adds checking of fallthrough statements as well.

For #51456.

Change-Id: I5000388011973724f80c59a6aaf015e3bb70faea
Reviewed-on: https://go-review.googlesource.com/c/go/+/414134
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
The parser CheckBranches mode checked correct use of break, continue,
and labels, but not of fallthrough statements.

This CL adds checking of fallthrough statements as well.

For golang#51456.

Change-Id: I5000388011973724f80c59a6aaf015e3bb70faea
Reviewed-on: https://go-review.googlesource.com/c/go/+/414134
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
The previous change implemented the missing fallthrough checking
in the parser. Therefore we can now disable the duplicate check
in the type checker:

- rename (types2.Config.)IngoreLabels to IgnoreBranches to more
  accurately reflect its functionality

- now also ignore break/continue/fallthroughs, not just labels

The IgnoreBranches flag only exists for types2, for use with
the compiler. There's no need to port this code to go/types.

Note: An alternative (and perhaps better) approach would be
to not use the the parser's CheckBranches mode and instead
enable (i.e. not disable) the branch/label checking in the
type checker. However, this requires a bit more work because
the type checker's error messages about goto's jumping over
variables don't have access to the variable names, which are
desired in the error messages.

Fixes golang#51456.

Change-Id: Ib2e71e811d4e84e4895b729646e879fd43b12dcd
Reviewed-on: https://go-review.googlesource.com/c/go/+/414135
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
@golang golang locked and limited conversation to collaborators Jun 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Development

No branches or pull requests

5 participants