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

cmd/compile: error message should not mention -lang when //go:build set file version #63489

Closed
rsc opened this issue Oct 10, 2023 · 5 comments
Closed

cmd/compile: error message should not mention -lang when //go:build set file version #63489

rsc opened this issue Oct 10, 2023 · 5 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.22

Comments

@rsc
Copy link
Contributor

rsc commented Oct 10, 2023 

% cat go.mod
go 1.21

module m
% cat x.go
//go:build go1.4

package p

const c = 0o123
% go build
# m
./x.go:5:11: 0o/0O-style octal literals requires go1.13 or later (-lang was set to go1.21; check go.mod)
%

The parenthetical misleads. It should say something like "(file declares //go:build go1.4)" or just be removed unconditionally.
@rsc rsc added NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Oct 10, 2023
@rsc rsc added this to the Go1.22 milestone Oct 10, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Oct 10, 2023
@cuonglm
Copy link
Member

cuonglm commented Oct 10, 2023

How about making the error more generic, something like:

./x.go:5:11: 0o/0O-style octal literals requires go1.13 or later (check go.mod or //go:build tag)

@mdempsky
Copy link
Member

@cuonglm I think it's worthwhile to keep the current language version in the error message. For example, if the error message for Russ's example mentioned "go1.4", then users can grep for occurrences of that string to find immediately what needs to be changed.

Also, the compiler knows whether the version was set by -lang or //go:build, so I think we should just tell users which file it came from, rather than making them look in both places.

@dmitshur dmitshur mentioned this issue Oct 11, 2023
Open
@cuonglm
Copy link
Member

cuonglm commented Oct 12, 2023 •
edited

Also, the compiler knows whether the version was set by -lang or //go:build, so I think we should just tell users which file it came from, rather than making them look in both places.

Yeah, I realized that now we have types2.Info.FileVersions after my comment.

@gopherbot
Copy link

Change https://go.dev/cl/534755 mentions this issue: cmd/compile: report mismatched version set by //go:build

@gopherbot
Copy link

Change https://go.dev/cl/536055 mentions this issue: cmd/internal/testdir: accept build go1.x build tag

gopherbot pushed a commit that referenced this issue Oct 17, 2023
@cuonglm @gopherbot
cmd/internal/testdir: accept build go1.x build tag
860c255 
While at it, also using "slices" package to simplify code.

For #63489

Change-Id: I72b325f6ad379b996c108145885fa71706f6659f
Reviewed-on: https://go-review.googlesource.com/c/go/+/536055
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
yunginnanet pushed a commit to yunginnanet/go that referenced this issue Oct 20, 2023
@cuonglm @yunginnanet
cmd/internal/testdir: accept build go1.x build tag
fa91aec 
While at it, also using "slices" package to simplify code.

For golang#63489

Change-Id: I72b325f6ad379b996c108145885fa71706f6659f
Reviewed-on: https://go-review.googlesource.com/c/go/+/536055
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
yunginnanet pushed a commit to yunginnanet/go that referenced this issue Oct 20, 2023
@cuonglm @yunginnanet
cmd/compile: report mismatched version set by //go:build
7345676 
Fixes golang#63489

Change-Id: I5e02dc5165ada7f5c292d56203dc670e96eaf2c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/534755
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.22
Development

No branches or pull requests
4 participants
@mdempsky @rsc @cuonglm @gopherbot