[RFC] Allow #warning and #error directives in case position of a switch

[nenadvulic]

Summary

#warning and #error directives placed in case position of a switch
are currently diagnosed as errors by SwiftSyntax's parser, even though the
Swift compiler accepts them. This change makes the parser accept these
freestanding macro expansions as elements of the switch's case list, matching
compiler behavior and fixing issue #3251.

As a result, SwitchCaseListSyntax.Element gains a new macroExpansionDecl
case.

Link to PR

Allow `#warning` and `#error` directives in case position of a switch by nenadvulic · Pull Request #3363 · swiftlang/swift-syntax · GitHub

Swift Interface

Existing API (before):

  public enum SwitchCaseListSyntax.Element {
    case switchCase(SwitchCaseSyntax)
    case ifConfigDecl(IfConfigDeclSyntax)
  }

Changed API (after):

  public enum SwitchCaseListSyntax.Element {
    case switchCase(SwitchCaseSyntax)
    case ifConfigDecl(IfConfigDeclSyntax)

    /// The expansion of a freestanding macro in a position that expects a declaration.
    case macroExpansionDecl(MacroExpansionDeclSyntax)
  }

(The associated init(_:), is/as/cast helpers and structure are
extended accordingly, as generated for all collection element choices.)

For existing API

This modifies the existing SwitchCaseListSyntax.Element enum.

• Clients affected: Clients that exhaustively switch over
  SwitchCaseListSyntax.Element will get a non-exhaustive-switch error and
  must handle the new macroExpansionDecl case. Clients that don't switch
  exhaustively over this enum are unaffected.
• Migration: In exhaustive switches over SwitchCaseListSyntax.Element,
  cover the new macroExpansionDecl case.

Release note added in Release Notes/605.md.

[allevato (Tony Allevato)]

If I'm being completely honest, I'd rather we just remove the ability to do this from the language entirely. I'd love to know if the original support for #error/#warning was ever intentional or if it just fell out of some coincidental parsing logic in the C++ parser.

Is there actually any value to allowing this? Would we actually break any real code if we just dropped this? I suspect the answer to both questions is "no."

If there is some hard requirement imposed that we must support anything the C++ parser does, then... fine, I guess. But considering this impacts the user-facing APIs for syntax nodes, if there's any chance we can just drop this instead, that would be much cleaner.

[hamishknight]

@allevato

I'd love to know if the original support for #error/#warning was ever intentional or if it just fell out of some coincidental parsing logic in the C++ parser.

It was intentionally added as part of the implementation of SE-0196, though it isn't explicitly written in the proposal.

@allevato

Is there actually any value to allowing this? Would we actually break any real code if we just dropped this? I suspect the answer to both questions is "no."

I think the main benefit here comes when it's combined with #if, e.g:

switch x {
#if FOO
    #error("foo")
#else
    case .bar:
        ...
#endif
}

In the syntax tree this produces RawIfConfigClauseSyntax.switchCases(RawSwitchCaseListSyntax).

@allevato

If there is some hard requirement imposed that we must support anything the C++ parser does, then... fine, I guess.

We eventually want to switch over to using ASTGen in the compiler, so need to be able to switch over to the new swift-syntax based parser without source breakage. If there's some aspect of the parser we think ought to be reconsidered before we make this transition we ought to pitch the change to Swift evolution, but I think we should keep whatever behavior we decide on consistent between the two parsers.

[allevato (Tony Allevato)]

@hamishknight

It was intentionally added as part of the implementation of SE-0196, though it isn't explicitly written in the proposal.

It may actually be subtly written in the proposal, since the grammar changes include

compiler-control-statement → warning-directive
compiler-control-statement → error-directive

and so switch statements containing compiler-control-statement would pick that up as well. (Interestingly, the TSPL grammar does not list any compiler control statements as possible children of switch bodies! But we know that's not the case.)

Alright, I'm convinced, and the #if-surrounded case you showed is compelling; it's the unconditional warning/error that looks busted. I just wanted to make sure this wasn't something that fell out accidentally that we'd regret keeping, but it's a consequence of a grammar change that went through evolution so it should definitely stick.

[nenadvulic]

Thanks for digging into this!
Agreed, it falls out of the compiler-control-statement grammar rule that came in through Swift Evolution, so SwiftSyntax accepting it (rather than diagnosing it) just brings the parser in line with the compiler. I'll keep both the #if-nested and the unconditional forms, since both are valid per that grammar.

[Cyberbeni (Benedek Kozma)]

I use freestanding warning for "todo as part of current task"

[allevato (Tony Allevato)]

@Cyberbeni

I use freestanding warning for "todo as part of current task"

To be clear, I'm not talking about #warning in general, which is very valuable. My question was about #warning specifically between switch and the first case:

switch foo {
#warning "blah blah"
case .bar: ...
}

I can't think of a reason that anyone would ever want to do that, but as @hamishknight convinced me above, it falls out of the other approved grammar rules regarding compiler conditionals.