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: fix Unified IR known test failures in test/run.go #53058

Open
mdempsky opened this issue May 24, 2022 · 15 comments
Open

cmd/compile: fix Unified IR known test failures in test/run.go #53058

mdempsky opened this issue May 24, 2022 · 15 comments
Labels
help wanted NeedsFix
Milestone

Comments

@mdempsky
Copy link
Member

@mdempsky mdempsky commented May 24, 2022

There are still a handful of known failures for Unified IR in test/run.go:

go/test/run.go

Line 1995 in 0a30cf9

var unifiedFailures = setOf(

The tests where diagnostics spelling has changed are fine.

But the tests for linkname errors and channel/array type size issues need to be fixed. Off hand I'm not sure why the existing typecheck code isn't just doing the right thing for Unified IR, but I expect it should be easy to massage into working now. (There's no -G=0 mode anymore, so it's fine to move the existing error logic around, if that's the easiest solution.)

/cc @cuonglm @dr2chase

@mdempsky mdempsky added help wanted NeedsFix labels May 24, 2022
@mdempsky mdempsky added this to the Go1.20 milestone May 24, 2022
@cuonglm
Copy link
Member

@cuonglm cuonglm commented May 25, 2022

The fix for channel/array type size is trivial, I have it in my local stack for a long time. Do you want me to send CL now or wait until we have dev.unified branch?

@mdempsky
Copy link
Member Author

@mdempsky mdempsky commented May 25, 2022

Oh great. I think now is fine, since they're bug fixes and should be small.

FYI, I'm out of office until next Tuesday though. David or Keith may be able to review though.

@gopherbot
Copy link

@gopherbot gopherbot commented May 25, 2022

Change https://go.dev/cl/408594 mentions this issue: cmd/compile: fix unified IR don't report type size too large error

@gopherbot
Copy link

@gopherbot gopherbot commented May 27, 2022

Change https://go.dev/cl/409034 mentions this issue: cmd/compile: restore Unified IR linkname pragma diagnostic

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 6, 2022

Change https://go.dev/cl/410341 mentions this issue: [dev.unified] cmd/compile: restore Unified IR linkname pragma diagnostic

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 6, 2022

Change https://go.dev/cl/410340 mentions this issue: [dev.unified] cmd/compile: fix unified IR don't report type size too large error

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 6, 2022

Change https://go.dev/cl/410345 mentions this issue: [dev.unified] cmd/compile: fix unified IR don't report type size too large error

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 6, 2022

Change https://go.dev/cl/410346 mentions this issue: [dev.unified] cmd/compile: restore Unified IR linkname pragma diagnostic

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 6, 2022

Change https://go.dev/cl/410574 mentions this issue: [dev.unified] cmd/compile: export/import implicit attribute for conversion exprs

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 6, 2022

Change https://go.dev/cl/410575 mentions this issue: test: enable test issue42284.go in Unified IR

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 6, 2022

Change https://go.dev/cl/410595 mentions this issue: [dev.unified] test: enable test issue42284.go in Unified IR

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 6, 2022

Change https://go.dev/cl/410594 mentions this issue: [dev.unified] cmd/compile: export/import implicit attribute for conversion exprs

gopherbot pushed a commit that referenced this issue Jun 6, 2022
…rsion exprs

So they can be formatted more presicely, and make it easier in the
transition to Unified IR.

Updates #53058

Change-Id: I8b5a46db05a2e2822289458995b8653f0a3ffbbe
Reviewed-on: https://go-review.googlesource.com/c/go/+/410594
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 7, 2022

Change https://go.dev/cl/410794 mentions this issue: [dev.unified] cmd/compile: set base.Pos when process assignDef in Unified IR

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 7, 2022

Change https://go.dev/cl/410795 mentions this issue: [dev.unified] test: relax issue7921.go diagnostic message

gopherbot pushed a commit that referenced this issue Jun 8, 2022
CL 333109 restore the diagnostic for irgen, now it's safe to restore for
Unified IR, too.

Updates #53058

Change-Id: I467902c0e9fa451aaa78cf0813231f14d9d7a3a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/410346
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
gopherbot pushed a commit that referenced this issue Jun 9, 2022
…large error

For error reported during type size calculation, base.Pos needs to be
set, otherwise, the compiler will treat them as the same error and only
report once. Old typechecker and irgen all set base.Pos before
processing types, this CL do the same thing for unified IR.

Updates #53058

Change-Id: I686984ffe4aca3e8b14d2103018c8d3c7d71fb02
Reviewed-on: https://go-review.googlesource.com/c/go/+/410345
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
gopherbot pushed a commit that referenced this issue Jun 9, 2022
…fied IR

CL 410343 changes Unified IR to visit LHS before RHS/X in assign/for
statement. Thus, it needs to set base.Pos before processing assignee
expression, so invalid type can be reported with correct position.

Updates #53058

Change-Id: Ic9f60cbf35c8bd71cb391e806396572c37811af7
Reviewed-on: https://go-review.googlesource.com/c/go/+/410794
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Jun 9, 2022
For constants literal, iimport/iexport read/write them as basic literal
nodes. So they are printed in diagnostic message as Go syntax. So "foo"
will be reported as string("foo").

Unified IR read/write the raw expression as string value, and when
printed in diagnostic, the string value is written out exactly as-is, so
"foo" will be written as "foo".

Thus, this CL relax the test in issue7921.go to match the string value only.

Updates #53058

Change-Id: I6fcf4fdcfc4b3be91cb53b081c48bd57186d8f35
Reviewed-on: https://go-review.googlesource.com/c/go/+/410795
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 14, 2022

Change https://go.dev/cl/411935 mentions this issue: [dev.unified] test: split inline.go into unified and non-unified tests

gopherbot pushed a commit that referenced this issue Jun 15, 2022
… non-unified

Unified IR records the inline nodes position right at the position of
the inline call, while the old inliner always records at the position of
the original nodes.

We want to keep non-unified working up through go 1.20, thus this CL
extract the inline test case that is different in Unified IR and the old
inliner.

Updates #53058

Change-Id: I14b0ee99fe797d34f27cfec068982790c64ac263
Reviewed-on: https://go-review.googlesource.com/c/go/+/411935
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted NeedsFix
Projects
None yet
Development

No branches or pull requests

3 participants