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/api: API checker doesn't seem to handle Alias uses correctly #65437

Open
griesemer opened this issue Feb 1, 2024 · 4 comments
Open

cmd/api: API checker doesn't seem to handle Alias uses correctly #65437

griesemer opened this issue Feb 1, 2024 · 4 comments
Assignees
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@griesemer
Copy link
Contributor

Background: API check failure in https://go-review.git.corp.google.com/c/go/+/559435.

@griesemer griesemer added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 1, 2024
@griesemer griesemer added this to the Go1.23 milestone Feb 1, 2024
@griesemer griesemer self-assigned this Feb 1, 2024
@randall77
Copy link
Contributor

I think this might not be a bug in the API checker. I think it might actually be a problem with whether we can move types.

Suppose package a has a type T.
Then if we do fmt.Printf("%v\n", reflect.ValueOf(a.T{}).Type()), it prints a.T.
Then if we move T to package b and replace the declaration of T in a with type T = b.T, then that same print statement will now print b.T.
So that move isn't undetectable by callers.

@randall77
Copy link
Contributor

@qiulaidongfeng

@qiulaidongfeng
Copy link
Contributor

I think it might actually be a problem with whether we can move types.

Then if we do fmt.Printf("%v\n", reflect.ValueOf(a.T{}).Type()), it prints a.T.
Then if we move T to package b and replace the declaration of T in a with type T = b.T, then that same print statement will now print b.T.

See https://go.dev/doc/go1compat , Compatibility is at the source level.
See https://go.dev/blog/compat , Perhaps the clearest fact about compatibility is that we can’t take away API, or else programs using it will break.

From the above blog about compatibility, I can't tell if fmt.Printf("%v\n", reflect.ValueOf(a.T{}).Type()) output a.T backward compatibility is guaranteed to not change it?

@griesemer
Copy link
Contributor Author

@randall77 I seems to me that explicit Alias type nodes should allow us to fix this: that is, if package a; type T = b.T, printing an a.T value could still print a.T with reflect. But I haven't thought it through in detail.

That said, per the original alias proposal, we already have issue with reflect (link).
Also, in the document it seems that we don't mention the reflect name of alias types (not just anonymous field names) - possibly an oversight. In any case, I don't think we must require unchanged reflect type names with the backward-compatibility guarantee.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants