Strict safety improvements #78332
Open
Strict safety improvements #78332
+751
−290
Conversation
This file contains 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
DougGregor
requested review from
AnthonyLatsis,
hborla,
slavapestov,
tshortli,
xedin and
artemcm
as code owners
|
@swift-ci please smoke test |
|
@swift-ci please smoke test |
I'm confused, is using |
Ah, you're right, of course. We've already indicated that it can't be checked at the definition site. Strict memory safety checking doesn't need to diagnose either of these. I'll revert that bit and remove it from the proposal, thanks! |
DougGregor
requested review from
ktoso,
xymus,
ahoppen,
bnbarham,
hamishknight,
rintaro,
zoecarver,
hyp,
egorzhdan,
beccadax,
ian-twilightcoder,
Xazax-hun and
CodaFi
as code owners
…te associated with its type
Under strict concurrency and memory safety, uses of `@unchecked Sendable` conformances are considered unsafe. Diagnose the use sites, not the declaration site.
@preconcurrency imports disable Sendable checking, which can lead to data races that undermine memory safety. Diagnose such imports, and require `@safe(unchecked)` to suppress the diagnostic.
Aligns with the current proposal and fixes rdar://127128995
…o deal with the warnings
…orporate @unsafe Protocol conformances have a handful attributes that can apply to them directly, including @unchecked (for Sendable), @preconcurrency, and @retroactive. Generalize this into an option set that we carry around, so it's a bit easier to add them, as well as reworking the serialization logic to deal with an arbitrary number of such options. Use this generality to add support for @unsafe conformances, which are needed when unsafe witnesses are used to conform to safe requirements. Implement general support for @unsafe conformances, including producing a single diagnostic per missing @unsafe that provides a Fix-It and collects together all of the unsafe witnesses as notes.
…n't need to be @unsafe What we should actually do with SerialExecutor conformances is not yet clear, though.
The `@unchecked` conformance is effectively the same as `@safe(unchecked)`, in that it asserts memory safety in a place where it cannot be automatically checked. But once that has been asserted, there is no reason to diagnose anywhere else. While here, drop the "unsafe declaration here" note, which isn't adding value but did add noise. Thanks, Alex!
DougGregor
force-pushed
the
strict-safety-improvements
branch
from
8cc3ebe to
419c477
Compare
|
@swift-ci please smoke test |
|
@swift-ci please build toolchain |
|
@swift-ci please build toolchain macOS |
This was referenced Dec 21, 2024
As we do when referencing other kinds of declarations, if a typealias isn't `@unsafe`, but it involves unsafe types, diagnose the non-safety at the point of reference. Fixes swiftlang#78220
When a witness is unsafe and its corresponding requirement is not unsafe, the conformance itself needs to be determined to be unsafe. When doing this check, account for the fact that the requirement might be implicitly unsafe, i.e., it uses unsafe types. In such cases, the requirement is effectively unsafe, so the unsafe witness to it does not make the conformance unsafe. Therefore, suppress the diagnostic. This accounts for cases where the protocol comes from a module that didn't enable strict memory safety checking, or didn't suppress all warnings related to it, as well as cases where substitution of type witnesses introduces the unsafe type. The same thing occurs with overriding a method: an unsafe override of a method that involves unsafe types (but was not marked @unsafe for whatever reason) does not make the subclassing itself unsafe, so don't diagnose it as such.
|
@swift-ci please smoke test |
|
@swift-ci please build toolchain |
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.
Collected improvements to the strict memory-safety mode to better align with the proposal:
@preconcurrency import, since it admits data races that can undermine memory safety-Ounchecked, which disables safety checks@unsafeconformances, diagnose when we need them (unsafe witnesses for safe requirements, collated nicely), and diagnose when using them@unsafe(e.g., because they're in a module that doesn't use strict safety) in type aliases (@unsafe attribute is not propagated through type alias declarations #78220), witnessed requirements, and overridden declarations.This third one required a significant refactoring of how we keep track of the attributes that apply to a conformance, like
@unchecked,@preconcurrency, and so on. That's the bulk of the code change here.Fixes rdar://127128995