Fix a comparison in an assertion #80347
Conversation
This was being caught by a newer clang in -Wparentheses by default: warning: comparisons like 'X<=Y<=Z' don't have their mathematical meaning [-Wparentheses]
|
@swift-ci please smoke test |
| @@ -857,7 +857,7 @@ void swift::assertRequiredSynthesizedPropertyOrder(ASTContext &Context, | |||
| } | |||
| if (idIdx + actorSystemIdx + unownedExecutorIdx >= 0 + 1 + 2) { | |||
There was a problem hiding this comment.
It's unclear what this if condition means. The logic above doesn't differentiate between the id field being missing, and the id being the first field. In both cases, idIdx is 0.
Now if all three fields are missing for example, we skip the assert. So we're only asserting that they have the correct order when present?
@ktoso Can you please explain the intent behind this code?
There was a problem hiding this comment.
It’s a bug if this order ever not as expected, I wanted to fix the warning earlier #79146 but yeah it’s a real bug that will need some investigation.
every distributed actor gets those fields synthesized. They MUST be in the specific order (ID then system) because IRGen will use them in this order in remote call accessors AFAIR. I’ll have to dig up the exact place.
We can close this PR as I have the other one I’ll get back to after some other bug fixes
|
The three test failures are from this assertion failing in distributed actor tests, so it definitely needs some further feedback. |
Pull request was closed
This was being caught by a newer clang in -Wparentheses by default: