Make value capture of optionals more robust. #277
Merged
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
When an expression is optional and is passed through `#require()`, and that
requirement fails, we capture the expression's subexpressions and record them as
an expectation-failed event that is eventually propagated to `stderr`. Yay.
The underlying value-capturing machinery accepts optional values as input so
that they can be lazily evaluated: if a value is `nil` at this layer, that means
that it wasn't evaluated and we present `"<not evaluated>"` instead of a
description of it or `"nil"`.
In the case of `#require()` unwrapping optionals, these values need to be boxed
in a second optional (i.e. `T??` instead of `T?`, or `Optional<Optional<T>>`
instead of `Optional<T>`) so that if the value is `nil`, it isn't confused with
"no value". See the following table:
| Expression | Value of `x: String?` | Presented As |
|-|-|-|
| `try #require(x)` | `.some("Hello World")` | `"Hello World"` |
| `try #require(x)` | `.some(.none)` | `nil` |
| `try #require("Goodbye Venus" ?? x)` | `.none` | `"<not evaluated>"` |
(This maybe suggests we should use a different enum than `Optional` here, but
that's a debate for another PR I think…)
This PR fixes a couple of spots where we weren't casting our values, known to be
optionals, to double-optionals, resulting in incorrect output that incorrectly
included `"<not evaluated>"`.
grynspan
requested review from
stmontgomery,
dennisweissmann,
briancroom,
SeanROlszewski and
suzannaratcliff
as code owners
|
@swift-ci please test |
briancroom
approved these changes
Mar 11, 2024
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.
When an expression is optional and is passed through
#require(), and that requirement fails, we capture the expression's subexpressions and record them as an expectation-failed event that is eventually propagated tostderr. Yay.The underlying value-capturing machinery accepts optional values as input so that they can be lazily evaluated: if a value is
nilat this layer, that means that it wasn't evaluated and we present"<not evaluated>"instead of a description of it or"nil".In the case of
#require()unwrapping optionals, these values need to be boxed in a second optional (i.e.T??instead ofT?, orOptional<Optional<T>>instead ofOptional<T>) so that if the value isnil, it isn't confused with "no value". See the following table:x as String??try #require(x).some(.some("Hello World"))"Hello World"try #require(x).some(.none)niltry #require("Goodbye Venus" ?? x).none"<not evaluated>"(This maybe suggests we should use a different enum than
Optionalhere, but that's a debate for another PR I think…)This PR fixes a couple of spots where we weren't casting our values, known to be optionals, to double-optionals, resulting in incorrect output that incorrectly included
"<not evaluated>".Checklist: