Mac backtrace from near-master:

7  swift                    0x0000000104cc3401 filterForEnumElement(swift::TypeChecker&, swift::DeclContext*, swift::SourceLoc, bool, swift::LookupResult) (.cold.9) + 33
8  swift                    0x0000000101b02291 filterForEnumElement(swift::TypeChecker&, swift::DeclContext*, swift::SourceLoc, bool, swift::LookupResult) + 801
9  swift                    0x0000000101b01d67 lookupEnumMemberElement(swift::TypeChecker&, swift::DeclContext*, swift::Type, swift::Identifier, swift::SourceLoc) + 167
10 swift                    0x0000000101aff9fb swift::TypeChecker::coercePatternToType(swift::Pattern*&, swift::TypeResolution, swift::Type, swift::TypeResolutionOptions, swift::TypeLoc) + 267
11 swift                    0x0000000101b44e81 (anonymous namespace)::StmtChecker::checkCaseLabelItemPattern(swift::CaseStmt*, swift::CaseLabelItem&, bool&, swift::Type, llvm::SmallVectorImpl<swift::VarDecl*>**, llvm::SmallVectorImpl<swift::VarDecl*>**) + 177
12 swift                    0x0000000101b4213e swift::ASTVisitor<(anonymous namespace)::StmtChecker, void, swift::Stmt*, void, void, void, void>::visit(swift::Stmt*) + 4046

@swift-ci create

I think this is probably StarterBug-simple: lookupEnumMemberElement should take a DeclName instead of just an Identifier.

Comment by Andrew Litteken (JIRA)

I'd like to take a try at figuring out this bug. I did a little bit of work on this and it seems like this is not the only problem. Based on an assertion that is thrown, there is a collision between the two enum cases and not having the information of the parameter list on-hand to figure this out. Is this something that would be contained in a DeclName?

Yeah, a DeclName is a base name (an Identifier) plus argument labels (several more Identifiers). My hope is that by doing a full-name lookup instead of a base-name lookup we won't ever find multiple enum cases except if the user types a plain let .a.

cc also CodaFi (JIRA User)

Comment by Andrew Litteken (JIRA)

Maybe I'm missing something, but I'm not sure the parser is picking up the arguments for use in this situation when used as a case in the switch statement.

The reasoning behind this that I'm trying to get access to the parameters in `TypeChecker::coercePatternToType` before we call `lookupEnumMemberElement` to create a DeclName since I can't extract it from the EnumElementPattern as the it contains an UnresolvedMemberExpr rather an EnumElementDecl. However, when I use `hasArguments()` on the Expr, it returns false. Additionally, when examining the AST visitor for these EnumElement, the no arguments are not registered at that stage either.

Potentially related, this example works

enum Foo 
{
    case a(y:Int)
}
func switcher(_ foo:Foo) 
{
    switch foo 
    {   
    case .a(x:):
        print("here")
        break 
    }   
}

But seems like it shouldn't based on my understanding of the expected solution to this bug. It seems like it should fail based on the different argument names in the enum member.

hasArguments checks for actual arguments, not just argument labels. What does the DeclName inside the UnresolvedMemberExpr look like?

Comment by Andrew Litteken (JIRA)

The DeclNames do have the arguments, and once a NameDecl is passed, the assertion failure doesn't happen. But with the given test case the warning `warning: case is already handled by previous patterns; consider removing it` is thrown. Is this the expected behavior for this?

Comment by Andrew Litteken (JIRA)

I have some additional examples that I'm not sure should be covered by this bug or not, but seem releated:

Contrary to the last comment if you have:

enum Foo 
{   
    case a(x:Int)
}
func switcher(_ foo:Foo) 
{
    switch foo
    {
    case .a(y:):
        break 
    }   
}   

You get the error:

error: type 'Foo' has no member 'a(y:)' case .a(y:):

Which seems like they are in contradiction with one another. But I can see where the original version throwing the "case is already handled by previous patterns" warning is more focusing on the type and not the name of the variable.

With the code:

enum Foo 
{
    case a(x:Int)
    case a(y:Float)
}


func switcher(_ foo:Foo) 
{
    switch foo 
    {   
    case .a(x:Int):
        break 
    case .a(y:Float):
        break 
    }   
}

There is a similar error to the initial bug, where the assertion about ambiguity in the enum is thrown. In these tests. In this case when the string of the DeclName is asked for, where (I think) it should contain something like a(x:Int), for the case statement it only shows a and then random characters following.

I mean, it's true that Foo has no member a(y:) in that new example. But yeah, we need to treat a(x:) and a(y:) as different cases, and the exhaustivity checking logic probably isn't doing that yet either. Still, maybe that can just be plumbed through by changing the Identifier in the Space class to a DeclName as well (and then fixing up all the call sites, of course).

Comment by Andrew Litteken (JIRA)

I tried doing this, and just using DeclNames in the Space class doesn't clear that up. But, before I get into the exhaustiveness checking, I'm trying to clear up some ambiguities when parsing the enums.

I have made some progress in recognizing when there are arguments. I have added to the pattern building s that when arguments are specified, they are added and matched correctly (although still with some exhaustiveness errors). Previously, they were just being matched against the .a, but now they, using the original as an example, they check for .a(x: ) or .a(y: ), avoiding the ambiguity case. However, when we have cases like:

enum Foo 
{   
    case a(x:Int)
    case a(y:Int)
}

func switcher(_ foo:Foo) 
{
    switch foo
    {
    case .a(let x):
        break
    }   
}

There are ambiguities in which case to take and crash the compiler currently. What should happen here? I feel like it should match something, but I'm not sure what.

This is a problem for other Patterns, like the Typed and Is patterns.

Edit: formatting

The document to reference here is SE-0155, which introduced enum elements with the same base name. That document describes what should happen in cases like the one you've brought up here.

Comment by Andrew Litteken (JIRA)

Thank you, I think that clears up most of my questions. What's the best way to get the error messages right for these?