[SR-1827] #selector not working in Xcode 8 Swift 3

[swift-ci]

Previous ID	SR-1827
Radar	None
Original Reporter	maury.markowitz@gmail.com (JIRA User)
Type	Bug
Status	Resolved
Resolution	Done

Environment

MacOS X 10.11.5 (15F34), Xcode Version 8.0 beta (8S128d)

Additional Detail from JIRA

| | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug, CompilerCrash | |Assignee | @ahoppen | |Priority | Medium | md5: 83f099abd7f19440c9f109e51e844166

Issue Description:

Here is some macOS code that was working under Xcode7/Swift 2.2:

override func validate(_ menuItem: NSMenuItem) -> Bool {
    let c = cards.count
    switch menuItem.action {
        case #selector(showRescale) :
            return c > 0
        case #selector(showUnits) :
            return true
        default:
            return true // there is no super.validate since 10.3, it's "validateUserInterfaceItem"
    }
}

This does not work under Xcode 8.0 beta (8S128d):

/Developer/SwiftNEC 3/SwiftNEC/NECDocumentVC.swift:78:18: Expected '(' following '#selector'

I have tried several different syntax modifications in an attempt to get it to work:

case #selector(showRescale):
case #selector(showRescale)?:
case #selector(showRescale)? :
case #selector(showRescale)!:
case #selector(showRescale)! :
case #selector!(showRescale):
case #selector!(showRescale) :

All of these fail with different errors, either the one reported above, or about a missing , so I assume this is the blessed format, but it does not work.This, however, does work:

case Optional.some(#selector(showRescale)):

But shortened versions do not. This version:

case (#selector(showRescale))?:

causes the compiler to crash. So does this version:

case .some(#selector(showRescale)):

[ahoppen]

I tried to reproduce the issue and got the following results:
case (#selector(showRescale))?: works. As does case Optional.some(#selector(showRescale)): and case .some(#selector(showRescale)):.

I guess the parenthesis around #selector should not be necessary and case #selector(showRescale)?: should work as well. After this change the syntax of optional #selector aligns with the one of any other optional type and all other versions are expected to be incorrect.

Secondly, applying the fix-its result in invalid code, so they should be fixed to append a question mark at the end of the expression.

I could, however, not reproduce the compiler crash. Could you maybe add more context, in particular method stubs for showRescale and showUnits and the class declaration? A small example file or project that crashes the compiler would be best.

[belkadan]

As discussed on swift-users, this is not a Swift regression but an SDK change. (That doesn't mean we shouldn't improve things here, just that there's not going to be some particular Swift commit to blame.)

[belkadan]

I believe the ? shorthand only works when doing a pattern-match-style case, not arbitrary expression matching.

[ahoppen]

I will look into it. My current status is that it is easy to remove the current fix-it which generates malformatted source code. Adding the parenthesis and question mark as a fix-it seems tricky but I will investigate if it is possible.

I think the syntax with the question mark without parenthesis should be supported since you can also use case foo()?. But maybe I'm missing some crucial fact.

[belkadan]

That makes sense. @jckarter?

[jckarter]

? is part of the pattern grammar so it seems like it should work with an expression as a subpattern to me. We definitely shouldn't crash.

[ahoppen]

I just looked into the grammar and it seems like the pattern is eventually only mapped to expr-unary which only supports expr-selector without any postfix symbols, whereas otherwise expr-unary is mapped to expr-postfix that handles trailing questions marks. I would suggest to modify the grammar rule that maps expr-unary to expr-selector from
expr-unary(Mode): expr-selector
to
expr-unary(Mode): expr-selector '?'?

AFAIK the trailing exclamation mark or multiple question marks do not make any sense, so I would leave them out.

[jckarter]

I think a better fix would be to make selector a primary-expression, so that postfix operations can chain off of it normally.

[ahoppen]

I have one implementation question so that Swift no longer provides a fix-it after #selector.
First little background: The case statement is resolved to a pattern matching expression $match ~= toMatch where the source location of $match is set to the beginning of #selector, which AFAIK is sensible so that general errors about this pattern matching expression are reported in the right source line.
If, however, $match is an Optional and toMatch isn’t, the generic fix-it is to force unwrap $match. Because of the implicit source location of $match this inserts an ! after the first token following case, which is definitely incorrect. I am currently thinking about the following resolutions to this problem:

1. Annotate the type variable of $match as implicit, so that force unwrapping it is not considered in ConstraintSystem::matchTypes

   • Currently my favourite approach, since I guess that such implicit types that cannot be changed using fix-its could come in handy for other features as well. On the downside, we cannot provide a fix-it that appends a ? using this approach, but I can’t think of any way to get such a fix-it right without doing some special casing in the constraint system that AFAIK can’t tell the difference between an expression with operator ~= (where the trailing ? is not allowed) and a pattern matching operation (where a trailing ? is allowed).

2. Special case the constraint system to provide a fix-it for a trailing ?

   • If this is the correct way to go, someone else would have to do the implementation

3. Set the source location of $match to the expression to be switched on

   • I am not a huge fan of this approach since adding an ! to the switch variable may break other case statements in the same switch and probably isn’t what the user wanted.

4. Provide the same error message (value of optional type 'Int?' not unwrapped; did you mean to use '!' or '?'?) without a fixit

   • Is pretty easy to implement but the error message is IMO misleading

Which approach do you think is best? And if 1 is the way to go: Can you think of any other AST nodes with implicit types except from DeclRefExpr that reference an implicit declaration?

Btw the problem with the fix-its ins’t just about case #selector but occurs with every type, e.g. in this sample:

let myOpt: Int? = 2
let myNonOpt: Int = 5

switch myOpt {
case myNonOpt:
  break
}