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: compiler errors should quote user-provided names where they are used "in sentence" of an error message #65790

Closed
seankhliao opened this issue Feb 19, 2024 · 8 comments
Assignees
Labels
BadErrorMessage Issues related compiler error messages that should be better. compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@seankhliao
Copy link
Member

seankhliao commented Feb 19, 2024

Go version

go version devel go1.23-4a7f3ac8eb Sat Feb 10 02:14:22 2024 +0000 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/user/.cache/go-build'
GOENV='/home/user/.config/go/env'
GOEXE=''
GOEXPERIMENT='cacheprog'
GOFLAGS='-trimpath "-ldflags=-s -w"'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/user/.data/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/user/.data/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/user/sdk/gotip'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='path'
GOTOOLDIR='/home/user/sdk/gotip/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.23-4a7f3ac8eb Sat Feb 10 02:14:22 2024 +0000'
GCCGO='gccgo'
GOAMD64='v3'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='0'
GOMOD='/home/user/tmp/testrepo0548/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2290460180=/tmp/go-build -gno-record-gcc-switches'

What did you do?

https://go.dev/play/p/cUZM3KyGHda

package main

import (
        "fmt"
)

func main() {
        int status
        fmt.Println(status)
}

What did you see happen?

# go.seankhliao.com/testrepo0548
./main.go:8:13: syntax error: unexpected status at end of statement

What did you expect to see?

"status" quoted as a user provided token, separating it from compiler status.

from #65788

cc @golang/compiler

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Feb 19, 2024
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 19, 2024
@griesemer griesemer self-assigned this Feb 20, 2024
@griesemer griesemer added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 20, 2024
@griesemer
Copy link
Contributor

Fair enough. We've been trying to avoid "punctuation" but it probably will be clearer to quote user-provided names (at least in some cases like this).

@griesemer griesemer added this to the Go1.23 milestone Feb 20, 2024
@griesemer griesemer added the BadErrorMessage Issues related compiler error messages that should be better. label Feb 20, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/565518 mentions this issue: cmd/compile: use quotes to wrap user-supplied token

gopherbot pushed a commit that referenced this issue Feb 27, 2024
Use quotes to wrap user-supplied token in the syntax error message.
Updates #65790

Change-Id: I631a63df4a6bb8615b7850a324d812190bc15f30
GitHub-Last-Rev: f291e1d
GitHub-Pull-Request: #65840
Reviewed-on: https://go-review.googlesource.com/c/go/+/565518
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@fukanchik
Copy link

Testdata is called testdata/issue65970.go while this issue is #65790

@griesemer
Copy link
Contributor

@fukanchik Thanks for pointing this out. Will fix.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/567875 mentions this issue: cmd/compile/internal/syntax: rename test file to match issue

@griesemer
Copy link
Contributor

Leaving this issue open to review other locations (such as the type checker) where a name is used within an error sentence and where it may be confusing.

@griesemer griesemer changed the title cmd/compile: syntax errors should quote user provided values cmd/compile: compiler errors should quote user-provided names where they are used "in sentence" of an error message Feb 28, 2024
@griesemer griesemer added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed NeedsFix The path to resolution is known, but the work has not been done. labels Feb 28, 2024
gopherbot pushed a commit that referenced this issue Feb 28, 2024
Follow-up on CL 565518 which addressed issue #65790 but added
testdata/issue65970.go instead of testdata/issue65790.go.
Rename that file to match the issue.

For #65790.

Change-Id: I647c297286355137fa950fb6722e31ae4340393b
Reviewed-on: https://go-review.googlesource.com/c/go/+/567875
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/571396 mentions this issue: go/types, types2: quote user-supplied names in error messages

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/589118 mentions this issue: cmd/compile: remove quoting in favor of clearer prose in error messages

gopherbot pushed a commit that referenced this issue May 30, 2024
In an attempt to address issue #65790 (confusing error messages),
quoting of names was introduced for some (but not all) names used
in error messages.

That CL solved the issue at hand at the cost of extra punctuation
(the quotes) plus some inconsistency (not all names were quoted).

This CL removes the quoting again in favor or adding a qualifying noun
(such as "name", "label", "package", "built-in" etc.) before a user-
specified name where needed.

For instance, instead of

        invalid argument to `max'

we now say

        invalid argument to built-in max

There's still a chance for confusion. For instance, before an error
might have been

        `sadly' not exported by package X

and now it would be

        name sadly not exported by package X

but adverbs (such as "sadly") seem unlikely names in programs.

This change touches a lot of files but only affects error messages.

Fixes #67685.

Change-Id: I95435b388f92cade316e2844d59ecf6953b178bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/589118
Auto-Submit: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BadErrorMessage Issues related compiler error messages that should be better. compiler/runtime Issues related to the Go compiler and/or runtime. 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

4 participants