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: //go:nointerface mishandled for promoted methods with -G=3 #47928

Closed
mdempsky opened this issue Aug 24, 2021 · 2 comments
Closed

cmd/compile: //go:nointerface mishandled for promoted methods with -G=3 #47928

mdempsky opened this issue Aug 24, 2021 · 2 comments
Assignees
Labels
Milestone

Comments

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Aug 24, 2021

The test program below has type T that embeds type U, which has a go:nointerface method M. When compiled with GOEXPERIMENT=fieldtrack, the type assertion should fail. It correctly fails with -G=0 and GOEXPERIMENT=fieldtrack,unified; but it incorrectly succeeds with -G=3.

package main

func main() {
	var i interface{} = new(T)
	if _, ok := i.(interface{ Bad() }); ok {
		panic("FAIL")
	}
}

type T struct{ U }

type U struct{}

//go:nointerface
func (*U) Bad() {}

Notably, the test passes if func main is moved after the Bad method. Presumably the issue is that we don't call irgen.funcDecl (which currently handles setting types.Field.Nointerface) until after types.ResumeCheckSize. Probably we need to change irgen.typeDecl to set types.Field.Nointerface instead, so that it's already set before any method promotion happens.

Alternatively, maybe we can just get rid of Field.Nointerface in favor of checking Func.Pragma directly. If this works, it seems preferable.

/cc @cuonglm @danscales @randall77

@mdempsky mdempsky added this to the Go1.18 milestone Aug 24, 2021
@mdempsky mdempsky self-assigned this Aug 24, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Aug 24, 2021

Change https://golang.org/cl/344617 mentions this issue: cmd/compile: replace types.Field.Nointerface with ir.IsNoInterfaceMethod

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 24, 2021

Change https://golang.org/cl/344649 mentions this issue: cmd/compile: change irgen to generate exprs/stmts after decls processed

Loading

@gopherbot gopherbot closed this in 4a9f0ce Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants