Skip to content

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

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

Merged
merged 1 commit into from
Aug 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 42 additions & 15 deletions lib/Sema/TypeCheckPattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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;
}
}
}
Expand All @@ -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(
Expand Down
41 changes: 41 additions & 0 deletions test/Constraints/patterns.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
Expand Down
24 changes: 24 additions & 0 deletions test/SILGen/enum.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down