Fixes build logger call crashing with ImplicitOpenExistentials. #77350
Conversation
chrismiles
added
the
playground transform
|
@swift-ci Please smoke test |
lib/Sema/PlaygroundTransform.cpp
Outdated
| } | ||
|
|
||
| Added<Expr *> AddedLogger(E); | ||
| if (*AddedLogger == nullptr) { |
There was a problem hiding this comment.
Just curious if you ever saw this be nullptr here. By my reading it should not be possible for doTypeCheckExpr() to have returned true but then the expression to have been set to nullptr (and it looks as if Added<T> just wraps whatever is given).
This isn't a request for change — it's great to be defensive here (and in fact I think the playground transform should be a lot more defensive in general, so that failures result in missing logger calls rather than compiler crashes!
Just curious as I read through the code in detail and tested out the changes locally.
There was a problem hiding this comment.
Good call out. You're right, this check should be unnecessary. Originally, doTypeCheck() was setting Added.Contents to null by a failed dyn_cast, which was the cause of the crash. I had added this check to fail gracefully in that case, before applying the full fix. This check is no longer needed, as the Added constructor shouldn't fail to wrap the expression, and doTypeCheckExpr() will return false if expression is null after type checking.
There was a problem hiding this comment.
Great, thanks for clarifying and confirming. To be clear, I don't think it hurts, just in case we ever do end up with nullptr here.
When ImplicitOpenExistentials was enabled (default in Swift language mode 6) the Instrumenter would crash the compiler when building logger calls. This was due to an incorrect assumption that the newly created apply expr wouldn't change type when type-checked. However, the type checker is free to change the kind of expression and did so in circumstances where the call expr was wrapped in an open existential expr.
chrismiles
force-pushed
the
eng/chrismiles/Instrumenter-buildPatternAndVariable-crash
branch
from
d898a62 to
5380d8c
Compare
|
@swift-ci Please smoke test |
chrismiles
deleted the
eng/chrismiles/Instrumenter-buildPatternAndVariable-crash
branch
When ImplicitOpenExistentials was enabled (default in Swift language mode 6) the Instrumenter would crash the compiler while building logger calls. This was due to an incorrect assumption that the newly created apply expr wouldn't change type when type-checked. However, the type checker is free to change the kind of expression and did so in circumstances where the call expr was wrapped in an open existential expr.
This change fixes that erroneous assumption and type-checks the new call expr before wrapping it in Added<Expr *>, removing another assumption that only Apply expressions are being added.
rdar://136459076