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: possible miscompilation comparing interface to concrete value #44051
Comments
Here's a simple reproducer:
This prints I'm actually not sure what the right behavior actually is. The fact that directional channels exist at compile time but don't really exist at runtime is probably part of this. |
It seems like the spec's definition of equality is non-transitive for channels stored in interfaces:
I think this behavior is surprising but correct per https://golang.org/ref/spec#Comparison_operators. "A value x of non-interface type X and a value t of interface type T are comparable when values of type X are comparable and X implements T. They are equal if t's dynamic type is identical to X and t's dynamic value is equal to x." |
The spec says
and
So, when comparing as interface values, the direction matters, and When comparing as concrete channel types, the spec says
without mentioning direction. So it seems reasonable for them to be equal. I agree this is confusing... |
That's horrifying. |
Maybe there's something to do here, but not for 1.16. |
This also happens for non-channel cases:
prints |
So it seems to me that the code in Line 641 in 0b6cfea
|
@cuonglm That quote was about channel values, not channel types. Channel type comparisons do take into account direction. Channel value comparisons do not (not sure how that would work anyway). |
That may be, but equality is defined in terms of assignability (https://golang.org/ref/spec#Comparison_operators):
And assignability is already not transitive — both because of the “at least one of |
I noticed this while modifying package
context
in CL 288193.To reproduce:
Note that the package context tests pass.
Now at
context/context.go:306
, replace:with
The tests fail.
Those two formulations should be identical (given that
p.done.Load()
is achan struct{}
, which it always is).That suggests to me that we might be miscompiling the comparison
p.done.Load() != done
.I've been unable to reproduce in a more minimized form, although I haven't tried very hard.
Tentatively marking as release blocker until diagnosed.
cc @mdempsky @cuonglm @randall77
The text was updated successfully, but these errors were encountered: