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

gccgo: rejects valid program #56109

Closed
ALTree opened this issue Oct 8, 2022 · 3 comments
Closed

gccgo: rejects valid program #56109

ALTree opened this issue Oct 8, 2022 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Gccgo

Comments

@ALTree
Copy link
Member

ALTree commented Oct 8, 2022 

$ gccgo --version
gccgo (Debian 12.2.0-3) 12.2.0

The following valid Go program:

package main

import "math"

func main() {
	f := func(p bool) {
		if p {
			println("hi")
		}
	}
	go f(true || math.Sqrt(2) > 1)
}

compiles and runs on gc, but is rejected by gccgo

$ gccgo main.go 
main.go:11:9: error: too few expressions for struct
   11 |         go f(true || math.Sqrt(2) > 1)
      |         ^

cc @ianlancetaylor
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 8, 2022
@ALTree ALTree added this to the Gccgo milestone Oct 8, 2022
@ianlancetaylor
Copy link
Contributor

Just a note that the problem is that when the thunk is created the argument is not seen as constant, but when the thunk is later simplified the argument is constant (thanks to the remove_deadcode pass, which changes true || x to simply true). So we build a field in the thunk argument for the struct, but then we don't fill it in. The code needs to be consistent as to whether the argument is constant or not.

@gopherbot
Copy link

Change https://go.dev/cl/440297 mentions this issue: test: add test case that caused a bogus error from gofrontend

@gopherbot
Copy link

Change https://go.dev/cl/440298 mentions this issue: compiler: only build thunk struct type when it is needed

nstester pushed a commit to nstester/gcc that referenced this issue Oct 10, 2022
@ianlancetaylor
compiler: only build thunk struct type when it is needed
0f6efd3 
Instead of building the thunk struct type in the determine_types pass,
build it when we need it.  That ensures that we are consistent in
determining whether an argument is constant.

We no longer need to add a field for a call to recover, as the
simplify_thunk_statements pass runs after the build_recover_thunks pass,
so the additional argument will already have been added to the call.

The test case is https://go.dev/cl/440297.

Fixes golang/go#56109

Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/440298
gopherbot pushed a commit that referenced this issue Oct 10, 2022
@ianlancetaylor @gopherbot
test: add test case that caused a bogus error from gofrontend
bb2a96b 
For #56109

Change-Id: I999763e463fac57732a92f5e396f8fa8c35bd2e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/440297
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
@ianlancetaylor @romaindoumenc
test: add test case that caused a bogus error from gofrontend
f7febe7 
For golang#56109

Change-Id: I999763e463fac57732a92f5e396f8fa8c35bd2e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/440297
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Oct 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Gccgo
Development

No branches or pull requests
3 participants
@ianlancetaylor @ALTree @gopherbot