[StrictExhaustiveSwitches] Replace default switch case with known enumerated cases #85168

AdamCmiel · 2025-10-28T02:23:14Z

Implementing the default behavior from this forum thread, similar to -Wswitch-enum

Optionally (with -enable-experimental-feature StrictExhaustiveSwitches) replace a default case label with the known set of exhaustive cases if the compiler is able to determine that set of cases. This works great for the simple case of an enum, with @unknown default able to still handle the resilient (or NS_ENUM) case.

AdamCmiel · 2025-10-28T02:30:12Z

@swift-ci Please test

an0 · 2025-10-28T13:25:26Z

Thanks for implementing my feature request!

I see this in your test case

  // exhaustive enum
  switch ReasonableEnum.zero {
  case .zero, .one: return true
  @unknown default: return false
  }

I'm wondering whether we could also warn against using @unknown default for frozen enums (which is majority), even if the switch is exhaustive, to catch call sites that pass in invalid values with a hard crash.

For example, ObjC could pass an int 3 to a Swift function that takes an enum with 2 defined cases (with raw values 0 and 1). If the switch has @unknown default the invalid value 3 will fall into @unknown default branch but it is better to remove @unknown default and let the switch crash so that we could catch the bad ObjC call site.

AdamCmiel · 2025-10-28T15:12:23Z

@an0 I specifically didn't tackle the @unkonwn default case in this PR because of the feedback I got on this other forums post that discussed rejecting code based on the presence or lack of resiliency. As this change merely adds warnings, I don't think it would be unreasonable to add a second that warns if:

a switch statement is using an unknown default case when:
- the type of the switch's subject is defined in the same module
- the type of the switch's subject is defined in a non-resilient module
- the type of the switch's subject is defined by a clang decl closed enum in a non-resilient module

The last case is likely the most tricky because of the vulnerability you described above, as NS_ENUM - declared values are merely typedef for a number and are as such unbound.

That said, because the (missing) unknown default warning becomes an error in Swift 6 mode, this becomes less about adding an opt-in warning than it does diverging language features, the pain of which has been discussed. That said there's another source change available:

If the enum is typedef with the macro NS_CLOSED_ENUM instead, the swift compiler will not warn on unexpected cases but as you pointed out, it's still possible to call an @objc func from the clang side with a value outside of the enum - perhaps we should warn in clang here instead if that can be statically determined, but yes I do think this should crash for a NS_CLOSED_ENUM on the swift side if it's not possible to typecheck, perhaps with some synthesized version of:

// for a given closed enum decl
typedef NS_CLOSED_ENUM(NSInteger, ObjCEnum) ...

extension ObjCEnum: CaseIterable {}
func takesAnObjCEnum(value: ObjCEnum) {
  // inserted prologue assert that the value is of a known case:
  assert(ObjCEnum.allCases.contains(value))

  // user-defined function body...
}

but that is also outside the scope of this PR

That said, I'll leave the unknown default case outside the scope of this PR and follow up with another where we can discuss the impact of resiliency as that opens up the exhaustiveness of the enum and with it another can of worms.

NSProgrammer

This looks like a solid change, thank you for submitting.

AdamCmiel · 2025-10-28T15:35:33Z

lib/Sema/TypeCheckSwitchStmt.cpp

      int diagnosedCases = 0;

-      processUncoveredSpaces([&](const Space &space,
-                                 bool onlyOneUncoveredSpace) {


param was unused, removed here

AdamCmiel · 2025-10-29T18:11:43Z

@xwu do you mind reviewing from https://forums.swift.org/t/allow-unknown-default-on-all-enum-switching-to-enforce-default-case-handling/82469/16?

xwu

Lovely, in my view.

xwu · 2025-10-29T20:19:34Z

include/swift/AST/DiagnosticsSema.def

     "handle unknown values using \"@unknown default\"", ())

+GROUPED_WARNING(decompose_default_case, StrictExhaustiveSwitches,none,
+  "switch has known exhaustive cases, remove default case", ())


I'd fiddle with the wording: switches are exhaustive, not cases. Perhaps something like, "switch can be made exhaustive without use of 'default'; replace with known cases".

xwu · 2025-10-29T20:19:48Z

include/swift/Basic/Features.def

 EXPERIMENTAL_FEATURE(ImplicitLastExprResults, false)

+/// Decompose `default` case in a `switch` statement into missing
+/// known iteratable `case` statements.


Nit: "iterable"

xwu · 2025-10-29T20:23:47Z

test/stmt/strict_exhaustive_switches.swift

+  case 2...10: return true
+  }
+
+  // Unkonwn default


Suggested change

// Unkonwn default

// Unknown default

The one @unknown default case I'd like to see handled right off the bat (IMO), perhaps even without the new compiler option, is unambiguously unnecessary use of non-@unknown default when an @unknown default will do. That is, if all known cases of a resilient enum are already handled, it should be at least a warning to unwittingly defeat future diagnostics by writing plain default. (This may already be diagnosed--I'm not sure. I'd imagine it was not possible to have these diagnostics in place upon first introduction for source compatibility reasons but at least in Swift 6 mode that shouldn't be a problem anymore.)

Ah yeah good catch. I'll check

drodriguez

Why the new experimental feature? The warning group should give everyone control about the new warning, doesn't it? If you want to default it to be a warning, and people are using -warnings-as-errors, they can demote it back to warning with -Wwarning StrictExhaustiveSwitches.

xwu · 2025-10-29T22:47:46Z

Hmm, I would disagree with that. An opt-in mode for an explicit language subset (like opt-in strict memory safety, or embeeded mode) has its uses for the reasons outlined on the forums, but I don't see how using a feature exactly as designed can be made something warned about by default. It should remain now and forever the case that using default to mean default—that is, to cover all leftover cases without uttering them—is expressly permitted without warning. This should not require toggling anything or even silencing the warning in source.

drodriguez · 2025-10-29T23:08:09Z

My problem is with the concept of "feature". This is not a feature of the language, just a new warning.

If you want the warning disabled by default, so people need to opt in with -Wwarning or -Werror, why not use the DiagnosticOptions::DefaultIgnore in this diagnostic? This will avoid adding yet another feature that (if I understand correctly your position) will never be possible to remove.

AdamCmiel · 2025-10-30T21:29:08Z

@drodriguez getting a little semantic but I'm not married to using an experimental feature. I do think it would be better to just use DefaultIgnore and enable with -Wwarning but I couldn't find how to check if the diagnostic was enabled, which you need to check to do the exhaustiveness checking.

If the user doesn't want these warnings, and I expect if they are DefaultIgnore to be the majority of cases, we probably shouldn't do the exhaustiveness checking here but I needed some way to "turn on" the checks. If we use an experimental feature AND DefaultIgnore, then you get into a weird case where the diagnostics are weird. So either setting the diagnostic level should enable the feature (by whatever mechanism) or vice-versa. There was a note in Features.def about a feature enabling other diagnostics but I couldn't find where it was actually applied (looks out of date).

@xwu any suggestions?

drodriguez · 2025-10-30T21:33:21Z

@AdamCmiel ctx.Diags.isIgnoredDiagnostic(diag::YourNameHere.ID) is what other DefaultIgnore warnings do.

Additionally, it seems that if your warning is DefaultIgnore, even if you emit the diagnostic, the emission doesn't mean that it will be shown to the user. The Diagnostic Manager seems to check for the level of the diagnostic before "printing" it.

xwu · 2025-10-30T23:49:16Z

@xwu any suggestions?

Implementation details about the diagnostics engine are less my jam; as long as banning default is opt-in and not opt-out, whether it's considered a feature, language subset, warning toggle, it's all the same to me :)

drodriguez

Minor feedback, but the bunch of the implementation looks good to me.

drodriguez · 2025-10-31T18:40:49Z

include/swift/AST/DiagnosticsSema.def


 GROUPED_WARNING(unsafe_superclass,StrictMemorySafety,none,
-     "%kindbase0 has superclass involving unsafe type %1", 
+     "%kindbase0 has superclass involving unsafe type %1",


Minor: you might want to remove the whitespace changes to make the commit more clear.

drodriguez · 2025-10-31T18:41:42Z

lib/Sema/TypeCheckSwitchStmt.cpp

    }

+    static bool hasStrictExhaustiveChecksEnabled(ASTContext &Context) {
+      return !(Context.Diags.isIgnoredDiagnostic(diag::decompose_default_case.ID));


Nit: If you run clang-format does it add the parenthesis? They are not really necessary.

drodriguez · 2025-10-31T18:54:36Z

lib/Sema/TypeCheckSwitchStmt.cpp

+        } else if (!defaultCase) {
          diagnoseMissingCases(Switch->getCases().empty()
                                 ? RequiresDefault::EmptySwitchBody
                                 : RequiresDefault::UncoveredSwitch,
-                               uncovered, unknownCase);
+                               uncovered, unknownCase, defaultCase);
        }
        return;


Maybe the comment in 1082-1084 should change to say what happens when the exhaustive switch is requested. It looks like the comment is saying that the code cannot reach here with a default: case, but now it can, and it seems that it is simply accepted (which was the behaviour before, IIUC).

drodriguez · 2025-10-31T19:01:19Z

lib/Sema/TypeCheckSwitchStmt.cpp

+      std::optional<decltype(diag::non_exhaustive_switch)> mainDiagType =
+          diag::non_exhaustive_switch;
+
+      bool downgrade = false;


Nit: you might want to leave it where it was, close to where it is used.

drodriguez · 2025-10-31T19:05:04Z

lib/Sema/TypeCheckSwitchStmt.cpp

+        // Switch actually is exhaustive with known cases,
+        // so don't emit exhaustive diagnostic
+        mainDiagType = std::nullopt;


mainDiagType.reset();

drodriguez · 2025-10-31T19:06:04Z

test/stmt/strict_exhaustive_switches.swift

+// RUN: %target-swift-frontend    %t/Enums.swift %t/NewDiagnostics.swift %t/PreventRegressions.swift %t/WithError.swift -swift-version 6 -Wwarning StrictExhaustiveSwitches -typecheck -verify
+// RUN: %target-swift-emit-silgen %t/Enums.swift %t/NewDiagnostics.swift %t/PreventRegressions.swift %t/SILVerify.swift -swift-version 6 -Wwarning StrictExhaustiveSwitches -o /dev/null -verify
+
+// REQUIRES: swift_feature_StrictExhaustiveSwitches


Not longer a feature?

drodriguez · 2025-10-31T19:09:13Z

@swift-ci please smoke test

AdamCmiel requested review from DougGregor, hborla, slavapestov and xedin as code owners October 28, 2025 02:23

AdamCmiel changed the title ~~Handle switch default~~ [StrictExhaustiveSwitches] Replace default switch case with known enumerated cases Oct 28, 2025

NSProgrammer approved these changes Oct 28, 2025

View reviewed changes

AdamCmiel commented Oct 28, 2025

View reviewed changes

xwu reviewed Oct 29, 2025

View reviewed changes

drodriguez reviewed Oct 29, 2025

View reviewed changes

AdamCmiel force-pushed the handle-switch-default branch from f21b7e9 to 1fd4444 Compare October 31, 2025 18:22

drodriguez approved these changes Oct 31, 2025

View reviewed changes

[StrictExhaustiveSwitches] flow through default case to space checking

61d6823

AdamCmiel force-pushed the handle-switch-default branch from 1fd4444 to 61d6823 Compare October 31, 2025 20:00

[StrictExhaustiveSwitches] Replace default switch case with known enumerated cases #85168

Are you sure you want to change the base?

[StrictExhaustiveSwitches] Replace default switch case with known enumerated cases #85168

Conversation

AdamCmiel commented Oct 28, 2025

Uh oh!

AdamCmiel commented Oct 28, 2025

Uh oh!

an0 commented Oct 28, 2025

Uh oh!

AdamCmiel commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NSProgrammer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AdamCmiel commented Oct 29, 2025

Uh oh!

xwu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xwu Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

drodriguez left a comment

Choose a reason for hiding this comment

Uh oh!

xwu commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

drodriguez commented Oct 29, 2025

Uh oh!

AdamCmiel commented Oct 30, 2025

Uh oh!

drodriguez commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xwu commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

drodriguez left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

drodriguez commented Oct 31, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AdamCmiel commented Oct 28, 2025 •

edited

Loading

xwu Oct 29, 2025 •

edited

Loading

xwu commented Oct 29, 2025 •

edited

Loading

drodriguez commented Oct 30, 2025 •

edited

Loading

xwu commented Oct 30, 2025 •

edited

Loading