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/go: pass -gcflags after other flags generated by the go command #47682

Closed
go101 opened this issue Aug 13, 2021 · 7 comments
Closed

cmd/go: pass -gcflags after other flags generated by the go command #47682

go101 opened this issue Aug 13, 2021 · 7 comments
Labels
FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@go101
Copy link

go101 commented Aug 13, 2021

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

$ go version
go version go1.17rc2 linux/amd64

What did you do?

https://groups.google.com/g/golang-nuts/c/lhedc8YaWlM/m/R4Q3xuhHBAAJ

What did you expect to see?

Use go1.17 as feature set instead.

What did you see instead?

gc chooses go1.16 as language version finally.

@dmitshur dmitshur added this to the Go1.17 milestone Aug 14, 2021
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 14, 2021
@dmitshur
Copy link
Contributor

dmitshur commented Aug 14, 2021

If I understand right, the exact issue this report is about is as follows.

What did you do?

$ cd $(mktemp -d)
$ cat <<EOF > main.go
package main

func main() {
    var s = []int{1, 2, 3}
    var pa = (*[2]int)(s[1:])
    println(pa[1])
}
EOF
$ cat <<EOF > go.mod
module example.com

go 1.16
EOF
$ go run -gcflags="-lang=go1.17" main.go

What did you expect to see?

For -gcflags="-lang=go1.17" to take higher precedence over the language version specified in go.mod, and the program to compile and run successfully:

$ go run -gcflags="-lang=go1.17" main.go
3

What did you see instead?

Setting -lang via -gcflags has no effect:

$ go run -gcflags="-lang=go1.17" main.go
# command-line-arguments
./main.go:5:23: cannot convert s[1:] (type []int) to type *[2]int:
	conversion of slices to array pointers only supported as of -lang=go1.17

Including -x flag in go run invocation shows that lang flag is included twice, once from -gcflags, and another time presumably by cmd/go via go.mod file. So:

/usr/local/go/pkg/tool/darwin_amd64/compile -o $WORK/b001/_pkg_.a -trimpath "$WORK/b001=>" -lang=go1.17 -p main -lang=go1.16 -complete -buildid uk9j_aZadleRNjlcL9DK/uk9j_aZadleRNjlcL9DK -dwarf=false -goversion go1.17rc2 -D _/var/folders/zb/5p8cwfhj29gf_m8vdy8ylmlr00jwcj/T/tmp.TN1gRr9t -importcfg $WORK/b001/importcfg -pack ./main.go $WORK/b001/_gomod_.go

The later of the two -lang takes precedence.

In this message, @ianlancetaylor suggested the order may be a bug.

If I understand right, this issue is about cmd/go, not cmd/compile, since it's the go command that decides the order of arguments for cmd/compile.

CC @bcmills, @ianlancetaylor, @mknyszek.

@dmitshur dmitshur changed the title go/compile: -gcflags -lang option should shadow default 1.16 value cmd/go: providing custom -lang via -gcflag doesn't have effect Aug 14, 2021
@dmitshur dmitshur modified the milestones: Go1.17, Backlog Aug 14, 2021
@dmitshur dmitshur added the GoCommand cmd/go label Aug 14, 2021
@cuonglm
Copy link
Member

cuonglm commented Aug 14, 2021

Can we just simply swap the order of gcflags vs gcargs in

args := []interface{}{cfg.BuildToolexec, base.Tool("compile"), "-o", ofile, "-trimpath", a.trimpath(), gcflags, gcargs}

The name gcargs is somewhat misleading to me, though.

@jayconrod
Copy link
Contributor

I don't think overriding the language version with -gcflags=-lang is something we really want to support. It's okay if it works, I just don't think we should add a special case for it or promise it will always work.

In this message, @ianlancetaylor suggested the order may be a bug.

Do we know if the order causes problems anywhere else? Changing this kind of thing may break someone, so I'm not sure we should change it unless passing -gcflags first is clearly the right thing to do.

Can we just simply swap the order of gcflags vs gcargs in ... The name gcargs is somewhat misleading to me, though.

The flag package requires that flags appear before positional arguments. gcargs has to come last because of that. If we changed this, I think we'd switch the order of forcedGcflags and p.Internal.Gcflags right above that.

@ianlancetaylor
Copy link
Contributor

My opinion is that people in general shouldn't use -gcflags, but if they do use -gcflags it should override everything that the go tool passes. That is, -gcflags should always come last. That's how these kinds of options behave in other build systems, and it's the only rational choice. If we give the user a way to control how the compiler is invoked, they should have full control, and it's up to them to make things work.

@jayconrod jayconrod changed the title cmd/go: providing custom -lang via -gcflag doesn't have effect cmd/go: pass -gcflags after other flags generated by the go command Aug 16, 2021
@jayconrod jayconrod 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 Aug 16, 2021
@jayconrod
Copy link
Contributor

@ianlancetaylor That sounds reasonable. I've changed the issue title to match.

@gopherbot
Copy link

Change https://golang.org/cl/344574 mentions this issue: cmd/go: pass -gcflags after other flags generated by the go command

@gopherbot
Copy link

Change https://golang.org/cl/344909 mentions this issue: cmd/go: pass -gcflags after other flags generated by the go command

@bcmills bcmills modified the milestones: Backlog, Go1.18 Oct 1, 2021
@golang golang locked and limited conversation to collaborators Oct 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

7 participants