Rewrite LoadBorrowImmutabilityChecker #34385
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
Limit names to a straightforward and unambiguous statement of purpose. They should not pose additional questions which can only be answered by reading the code. Nuanced meaning belongs in descriptions and code comments. These are all examples that legitimately made reading the code very difficult for me: - LoadBorrowInvalidationChecker: what does "invalidation" mean in this context? How does that extend the meaning of "checker"? How can something ever pass a checker and not be invalid? - constructValuesForKey outside of an ADT does not state purpose at all. - wellBehavedWriteAccumulator: Raises questions about what writes are included and the broader semantics of the parent function. It turns out that well-behavedness is handled by the function's return value and has nothing to do with the accumulator.
|
@swift-ci test |
|
@swift-ci test source compatibility |
atrick
force-pushed
the
rewrite-loadborrowchecker
branch
2 times, most recently
from
3fda11b to
7a6e914
Compare
|
@swift-ci test |
|
@swift-ci test source compatibility |
The verification will now be as complete as it can be within the capability of our SIL utilities. It is much more aggressive with respect to boxes, references, and pointers. It's more efficient in that it only considers "overlapping" uses. It is also now wholly consistent with the utilities that it uses, so can be reenabled. We could probably go even further and remove the switch statement entirely, relying on AccessPath to recognize any operations that propagate addresses, boxes, or pointers. But I didn't want to potentially weaken enforcement without more careful consideration.
atrick
force-pushed
the
rewrite-loadborrowchecker
branch
from
7a6e914 to
0f1beed
Compare
|
@swift-ci test |
|
@swift-ci test source compatibility |
|
SCK Evergreen is failing on the main branch |
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.
AccessPath allows us to discard a lot of the original implementation.
The verification will now be as complete as it can be within the
capability of our SIL utilities. It is much more aggressive with
respect to boxes, references, and pointers. It's more efficient in
that it only considers "overlapping" uses.
It is also now wholly consistent with the utilities that it uses, so
can be reenabled.
We could probably go even further and remove the switch statement
entirely, relying on AccessPath to recognize any operations that
propagate addresses, boxes, or pointers. But I didn't want to
potentially weaken enforcement without more careful consideration.