[PlaygroundLogger] Implemented a check to avoid trying to log nil-con… #40
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ChrisBrough
approved these changes
Jan 27, 2020
…taining non-Optional objects. It is undefined behavior for a non-Optional object to be nil in Swift, and at present, PlaygroundLogger triggers crashes in the runtime when trying to log such objects. The most common way for such an object to exist is for an Objective-C API to be annotated as returning nonnull but in fact returns nil. In a regular Swift program, this can be worked around by explicitly assigning the value to an Optional. PlaygroundLogger cannot do that as it only receives an Any, and assigning to Any? does not recover the nil. As a result, inspect the contents of the Any's existential container (i.e. the thing in memory that is the Any itself, not the value inside of the Any) to determine if it contains a non-Optional object whose value is nil. This commit includes additional tests to ensure that we can log this broken value without crashing and that we can log a nil Optional object correctly. This addresses <rdar://problem/56370098>.
cwakamo
force-pushed
the
eng/PR-56370098-avoid-crashing-on-nil
branch
from
a643349 to
1cad754
Compare
|
@jonathanpenn I made the changes you requested, would you mind taking another look at this PR? |
|
Looks great! |
jonathanpenn
approved these changes
Feb 3, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…taining non-Optional objects.
It is undefined behavior for a non-Optional object to be nil in Swift, and at present, PlaygroundLogger triggers crashes in the runtime when trying to log such objects.
The most common way for such an object to exist is for an Objective-C API to be annotated as returning nonnull but in fact returns nil.
In a regular Swift program, this can be worked around by explicitly assigning the value to an Optional.
PlaygroundLogger cannot do that as it only receives an Any, and assigning to Any? does not recover the nil.
As a result, inspect the contents of the Any's existential container (i.e. the thing in memory that is the Any itself, not the value inside of the Any) to determine if it contains a non-Optional object whose value is nil.
This commit includes additional tests to ensure that we can log this broken value without crashing and that we can log a nil Optional object correctly.
This addresses rdar://problem/56370098.