Skip to content

Suppress strict safety diagnostics in @unsafe declarations #78157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 13, 2024

Conversation

DougGregor
Copy link
Member

When a declaration is @unsafe, don't emit strict safety diagnostics for uses of unsafe entities, constructs, or types within it. This allows one to account for all unsafe behavior in a module using strict memory safety by marking the appropriate declarations @unsafe.

Enhance the strict-safety diagnostics to suggest the addition of @unsafe where it is needed to suppress them, with a Fix-It. Ensure that all such diagnostics can be suppressed via @unsafe so it's possible to get to the above state.

Also includes a drive-by bug fix where we weren't diagnosing unsafe methods overriding safe ones in some cases.

Fixes rdar://139467327.

When a declaration is `@unsafe`, don't emit strict safety diagnostics
for uses of unsafe entities, constructs, or types within it. This
allows one to account for all unsafe behavior in a module using strict
memory safety by marking the appropriate declarations `@unsafe`.

Enhance the strict-safety diagnostics to suggest the addition of
`@unsafe` where it is needed to suppress them, with a Fix-It. Ensure
that all such diagnostics can be suppressed via `@unsafe` so it's
possible to get to the above state.

Also includes a drive-by bug fix where we weren't diagnosing unsafe
methods overriding safe ones in some cases.

Fixes rdar://139467327.
@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@@ -170,6 +174,10 @@ bool AvailabilityContext::isUnavailableInEmbedded() const {
return Info->Platform.IsUnavailableInEmbedded;
}

bool AvailabilityContext::allowsUnsafe() const {
return Info->Platform.AllowsUnsafe;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be good to add this bit to the AvailabilityScope dump output for debugging.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea, done in the last commit. Thanks!

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor DougGregor enabled auto-merge December 13, 2024 07:04
@Xazax-hun
Copy link
Contributor

Xazax-hun commented Dec 13, 2024

The proposed design here is similar to what Rust is doing, i.e., the meaning of:

unsafe fn foo() {
 // content
}

is:

unsafe fn foo() {
  unsafe {
   // content
  }
}

This is a design decision that the Rust community regrets. My understanding is they would prefer unsafe functions to not permit calls to unsafe by default. This would make it more explicit where exactly the unsafe content is within an unsafe function, and help enforcing some policies like adding an informal argument as a form of comment to each unsafe block why the unsafe operation is actually safe in that context:

// comment with preconditions not checked by the compiler
unsafe fn foo() {
  safe_operation();
  unsafe {
    // comment why is this fine
    unsafe_operation();
  }
  safe_operation();
}

All that being said, I understand that Swift does not have unsafe blocks yet, but I am wondering if this is an alternative design we could consider to enable users to define more narrow scopes where unsafe operations are permitted.

@DougGregor DougGregor merged commit cb05f22 into swiftlang:main Dec 13, 2024
3 checks passed
@DougGregor DougGregor deleted the unsafe-suppressed-unsafe-diags branch December 13, 2024 14:57
@DougGregor
Copy link
Member Author

All that being said, I understand that Swift does not have unsafe blocks yet, but I am wondering if this is an alternative design we could consider to enable users to define more narrow scopes where unsafe operations are permitted.

Personally, I don't think that unsafe blocks are the right language design. They encourage folks to write the minimal amount of code within unsafe blocks, which sounds like a good thing, but I think it's a false metric. What we care about is encapsulating the unsafety and documenting how it interacts with safe code that uses it, and that's best done at a function boundary where all of your inputs and outputs are clearly documented.

All of this is going into my upcoming pitch on introducing this strict safety mode. I've avoided it in the vision because I don't want this particular design point to distract from the discussion of the overall direction.

@DougGregor
Copy link
Member Author

I'd prefer to have this discussion on the forums. I'm currently doing the experimental implementation of what I'll be pitching, which I hope will happen soon (next few days).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants