[TypeCheckPattern] Attempt ExprPattern conversion before failing pattern coercion to optional

Author: Jumhyn

Diff for: lib/Sema/TypeCheckPattern.cpp

@@ -128,25 +128,49 @@ lookupUnqualifiedEnumMemberElement(DeclContext *DC, DeclNameRef name,
                               /*unqualifiedLookup=*/true, lookup);
 }
 
-/// Find an enum element in an enum type.
-static EnumElementDecl *
-lookupEnumMemberElement(DeclContext *DC, Type ty,
-                        DeclNameRef name, SourceLoc UseLoc) {
+static LookupResult lookupMembers(DeclContext *DC, Type ty, DeclNameRef name,
+                                  SourceLoc UseLoc) {
   if (!ty->mayHaveMembers())
-    return nullptr;
+    return LookupResult();
 
   // FIXME: We should probably pay attention to argument labels someday.
   name = name.withoutArgumentLabels();
 
   // Look up the case inside the enum.
   // FIXME: We should be able to tell if this is a private lookup.
   NameLookupOptions lookupOptions = defaultMemberLookupOptions;
-  LookupResult foundElements =
-      TypeChecker::lookupMember(DC, ty, name, UseLoc, lookupOptions);
+  return TypeChecker::lookupMember(DC, ty, name, UseLoc, lookupOptions);
+}
+
+/// Find an enum element in an enum type.
+static EnumElementDecl *lookupEnumMemberElement(DeclContext *DC, Type ty,
+                                                DeclNameRef name,
+                                                SourceLoc UseLoc) {
+  LookupResult foundElements = lookupMembers(DC, ty, name, UseLoc);
   return filterForEnumElement(DC, UseLoc,
                               /*unqualifiedLookup=*/false, foundElements);
 }
 
+/// Whether the type contains an enum element or static var member with the
+/// given name. Used for potential ambiguity diagnostics when matching against
+/// \c .none on \c Optional since users might be trying to match against an
+/// underlying \c .none member on the wrapped type.
+static bool hasEnumElementOrStaticVarMember(DeclContext *DC, Type ty,
+                                            DeclNameRef name,
+                                            SourceLoc UseLoc) {
+  LookupResult foundElements = lookupMembers(DC, ty, name, UseLoc);
+  return llvm::any_of(foundElements, [](const LookupResultEntry &result) {
+    auto *VD = result.getValueDecl();
+    if (isa<VarDecl>(VD) && VD->isStatic())
+      return true;
+
+    if (isa<EnumElementDecl>(VD))
+      return true;
+
+    return false;
+  });
+}
+
 namespace {
 /// Build up an \c DeclRefTypeRepr and see what it resolves to.
 /// FIXME: Support DeclRefTypeRepr nodes with non-identifier base components.
@@ -1480,13 +1504,10 @@ Pattern *TypeChecker::coercePatternToType(
               return coercePatternToType(
                   pattern.forSubPattern(P, /*retainTopLevel=*/true), type,
                   options, tryRewritePattern);
-            } else {
-              diags.diagnose(EEP->getLoc(),
-                             diag::enum_element_pattern_member_not_found,
-                             EEP->getName(), type);
-              return nullptr;
             }
-          } else if (EEP->hasUnresolvedOriginalExpr()) {
+          }
+
+          if (EEP->hasUnresolvedOriginalExpr()) {
             // If we have the original expression parse tree, try reinterpreting
             // it as an expr-pattern if enum element lookup failed, since `.foo`
             // could also refer to a static member of the context type.
@@ -1495,6 +1516,12 @@ Pattern *TypeChecker::coercePatternToType(
             return coercePatternToType(
                 pattern.forSubPattern(P, /*retainTopLevel=*/true), type,
                 options, tryRewritePattern);
+          } else {
+            // Otherwise, treat this as a failed enum element lookup.
+            diags.diagnose(EEP->getLoc(),
+                           diag::enum_element_pattern_member_not_found,
+                           EEP->getName(), type);
+            return nullptr;
           }
         }
       }
@@ -1510,8 +1537,8 @@ Pattern *TypeChecker::coercePatternToType(
           type->getOptionalObjectType()) {
         SmallVector<Type, 4> allOptionals;
         auto baseTyUnwrapped = type->lookThroughAllOptionalTypes(allOptionals);
-        if (lookupEnumMemberElement(dc, baseTyUnwrapped, EEP->getName(),
-                                    EEP->getLoc())) {
+        if (hasEnumElementOrStaticVarMember(dc, baseTyUnwrapped, EEP->getName(),
+                                            EEP->getLoc())) {
           auto baseTyName = type->getCanonicalType().getString();
           auto baseTyUnwrappedName = baseTyUnwrapped->getString();
           diags.diagnoseWithNotes(

Diff for: test/Constraints/patterns.swift

@@ -412,13 +412,34 @@ do {
   enum E {
    case baz
    case bar
+
+      static var member: Self {
+          .baz
+      }
+
+      static func method() -> Self {
+          .bar
+      }
+
+      static func methodArg(_: Void) -> Self {
+          .baz
+      }
+
+      static var none: Self {
+          .baz
+      }
   }
 
   let oe: E? = .bar
 
   switch oe {
    case .bar?: break // Ok
    case .baz: break // Ok
+   case .member: break // Ok
+   case .missingMember: break // expected-error {{type 'E?' has no member 'missingMember'}}
+   case .method(): break // Ok
+   case .methodArg(()): break // Ok
+   case .none: break // expected-warning {{assuming you mean 'Optional<E>.none'; did you mean 'E.none' instead?}} expected-note {{use 'nil' to silence this warning}} expected-note {{use 'none?' instead}}
    default: break
   }
 
@@ -427,11 +448,31 @@ do {
   switch ooe {
    case .bar?: break // Ok
    case .baz: break // Ok
+   case .member: break // Ok
+   case .missingMember: break // expected-error {{type 'E??' has no member 'missingMember'}}
+   case .method(): break // Ok
+   case .methodArg(()): break // Ok
    default: break
   }
 
   if case .baz = ooe {} // Ok
   if case .bar? = ooe {} // Ok
+  if case .member = ooe {} // Ok
+  if case .missingMember = ooe {} // expected-error {{type 'E??' has no member 'missingMember'}}
+  if case .method() = ooe {} // Ok
+  if case .methodArg(()) = ooe {} // Ok
+
+  enum M {
+    case m
+    static func `none`(_: Void) -> Self { .m }
+  }
+
+  let om: M? = .m
+
+  switch om {
+  case .none: break // Ok
+  default: break
+  }
 }
 
 // rdar://problem/60048356 - `if case` fails when `_` pattern doesn't have a label

Diff for: test/SILGen/enum.swift

@@ -222,6 +222,30 @@ func matchOptionalEnum2(bar: OneOrTwo??) {
   }
 }
 
+enum OneTwoOrNone {
+    case one
+    case two
+
+    var none: Self {
+        .one
+    }
+}
+
+// CHECK-LABEL: sil hidden [ossa] @$s4enum18matchOptionalEnum33baryAA12OneTwoOrNoneOSg_tF : $@convention(thin) (Optional<OneTwoOrNone>) -> () {
+// CHECK: bb0(%0 : $Optional<OneTwoOrNone>):
+// CHECK-NEXT:  debug_value %0 : $Optional<OneTwoOrNone>, let, name "bar", argno 1
+// CHECK-NEXT:  switch_enum %0 : $Optional<OneTwoOrNone>, case #Optional.some!enumelt: bb1, case #Optional.none!enumelt: bb4
+// CHECK: bb1([[PHI_ARG:%.*]] : $OneTwoOrNone):
+// CHECK-NEXT:  switch_enum [[PHI_ARG]] : $OneTwoOrNone, case #OneTwoOrNone.one!enumelt: bb2, case #OneTwoOrNone.two!enumelt: bb3
+func matchOptionalEnum3(bar: OneTwoOrNone?) {
+  switch bar {
+  case .one: print("one")
+  case .two?: print("two")
+  case .none: print("none")
+  default: print("default")
+  }
+}
+
 // Make sure that we handle enum, tuple initialization composed
 // correctly. Previously, we leaked down a failure path due to us misusing
 // scopes.