-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
[SR-11159] Typechecker crash on overloaded enum case #53556
Comments
Mac backtrace from near-master:
@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 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. |
|
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 |
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 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? |
Environment
$ swift --version
Swift version 5.1-dev (LLVM 200186e28b, Swift 75a5496)
Target: x86_64-unknown-linux-gnu
Additional Detail from JIRA
md5: 045414bcf57bf8b343a071693e9843bc
Parent-Task:
is blocked by:
relates to:
Issue Description:
The text was updated successfully, but these errors were encountered: