Merge pull request #34715 from Jumhyn/SR-13815

[Sema] Always look through optionals for unresolved member lookup

Author: xedin

include/swift/Sema/ConstraintSystem.h

@@ -785,6 +785,8 @@ enum ScoreKind {
   SK_ForwardTrailingClosure,
   /// A use of a disfavored overload.
   SK_DisfavoredOverload,
+  /// A member for an \c UnresolvedMemberExpr found via unwrapped optional base.
+  SK_UnresolvedMemberViaOptional,
   /// An implicit force of an implicitly unwrapped optional value.
   SK_ForceUnchecked,
   /// A user-defined conversion.

lib/Sema/CSRanking.cpp

@@ -52,6 +52,9 @@ static StringRef getScoreKindName(ScoreKind kind) {
   case SK_DisfavoredOverload:
     return "disfavored overload";
 
+  case SK_UnresolvedMemberViaOptional:
+    return "unwrapping optional at unresolved member base";
+
   case SK_ForceUnchecked:
     return "force of an implicitly unwrapped optional";
 

lib/Sema/CSSimplify.cpp

@@ -6777,13 +6777,12 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
   // through optional types.
   //
   // FIXME: Unify with the above code path.
-  if (result.ViableCandidates.empty() &&
-      baseObjTy->is<AnyMetatypeType>() &&
+  if (baseObjTy->is<AnyMetatypeType>() &&
       constraintKind == ConstraintKind::UnresolvedValueMember) {
     if (auto objectType = instanceTy->getOptionalObjectType()) {
       // If we don't have a wrapped type yet, we can't look through the optional
       // type.
-      if (objectType->getAs<TypeVariableType>()) {
+      if (objectType->getAs<TypeVariableType>() && result.ViableCandidates.empty()) {
         MemberLookupResult result;
         result.OverallResult = MemberLookupResult::Unsolved;
         return result;

lib/Sema/ConstraintSystem.cpp

@@ -2802,6 +2802,11 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
       choice.getDecl()->getAttrs().hasAttribute<DisfavoredOverloadAttr>()) {
     increaseScore(SK_DisfavoredOverload);
   }
+
+  if (choice.getKind() == OverloadChoiceKind::DeclViaUnwrappedOptional &&
+      locator->isLastElement<LocatorPathElt::UnresolvedMember>()) {
+    increaseScore(SK_UnresolvedMemberViaOptional);
+  }
 }
 
 Type ConstraintSystem::simplifyTypeImpl(Type type,

test/expr/delayed-ident/optional_overload.swift

@@ -0,0 +1,47 @@
+// RUN: %target-typecheck-verify-swift -dump-ast > %t.dump
+// RUN: %FileCheck %s < %t.dump
+
+// SR-13815
+extension Optional {
+    func sr13815() -> SR13815? { SR13815() }
+    static func sr13815_2() -> SR13815? { SR13815() }
+    static func sr13815_3() -> SR13815? { SR13815() }
+    static var sr13815_wrongType: Int { 0 }
+    static var sr13815_overload: SR13815 { SR13815() }
+    init(overloaded: Void) { self = nil }
+}
+
+struct SR13815 {
+    static var sr13815: SR13815? = SR13815()
+    static var sr13815_2: SR13815? = SR13815()
+    static var sr13815_wrongType: SR13815? { SR13815() }
+    static var p_SR13815: SR13815? { SR13815() }
+    static func sr13815_3() -> SR13815? { SR13815() }
+    static var sr13815_overload: SR13815? { SR13815() }
+    init(overloaded: Void) {}
+    init?(failable: Void) {}
+    init() {}
+}
+
+protocol P_SR13815 {}
+extension Optional: P_SR13815 where Wrapped: Equatable {
+    static func p_SR13815() {}
+}
+
+let _: SR13815? = .sr13815
+let _: SR13815? = .sr13815_wrongType
+let _: SR13815? = .init()
+let _: SR13815? = .sr13815() // expected-error {{instance member 'sr13815' cannot be used on type 'SR13815?'}}
+let _: SR13815? = .sr13815_2()
+let _: SR13815? = .init(SR13815())
+let _: SR13815? = .init(overloaded: ())
+// If members exist on Optional and Wrapped, always choose the one on optional
+// CHECK: declref_expr {{.*}} location={{.*}}optional_overload.swift:37
+// CHECK-SAME: decl=optional_overload.(file).Optional extension.init(overloaded:)
+let _: SR13815? = .sr13815_overload
+// Should choose the overload from Optional even if the Wrapped overload would otherwise have a better score
+// CHECK: member_ref_expr {{.*}} location={{.*}}optional_overload.swift:41
+// CHECK-SAME: decl=optional_overload.(file).Optional extension.sr13815_overload
+let _: SR13815? = .init(failable: ())
+let _: SR13815? = .sr13815_3()
+let _: SR13815? = .p_SR13815

unittests/Sema/CMakeLists.txt

@@ -2,7 +2,8 @@
 add_swift_unittest(swiftSemaTests
   SemaFixture.cpp
   BindingInferenceTests.cpp
-  ConstraintSimplificationTests.cpp)
+  ConstraintSimplificationTests.cpp
+  UnresolvedMemberLookupTests.cpp)
 
 target_link_libraries(swiftSemaTests
   PRIVATE

unittests/Sema/SemaFixture.cpp

@@ -76,6 +76,39 @@ Type SemaTest::getStdlibType(StringRef name) const {
   return Type();
 }
 
+NominalTypeDecl *SemaTest::getStdlibNominalTypeDecl(StringRef name) const {
+  auto typeName = Context.getIdentifier(name);
+
+  auto *stdlib = Context.getStdlibModule();
+
+  llvm::SmallVector<ValueDecl *, 4> results;
+  stdlib->lookupValue(typeName, NLKind::UnqualifiedLookup, results);
+
+  if (results.size() != 1)
+    return nullptr;
+
+  return dyn_cast<NominalTypeDecl>(results.front());
+}
+
+VarDecl *SemaTest::addExtensionVarMember(NominalTypeDecl *decl,
+                                         StringRef name, Type type) const {
+  auto *ext = ExtensionDecl::create(Context, SourceLoc(), nullptr, { }, DC,
+                                    nullptr);
+  decl->addExtension(ext);
+  ext->setExtendedNominal(decl);
+
+  auto *VD = new (Context) VarDecl(/*isStatic=*/ true, VarDecl::Introducer::Var,
+                                   /*nameLoc=*/ SourceLoc(),
+                                   Context.getIdentifier(name), ext);
+
+  ext->addMember(VD);
+  auto *pat = new (Context) NamedPattern(VD);
+  VD->setNamingPattern(pat);
+  pat->setType(type);
+
+  return VD;
+}
+
 ProtocolType *SemaTest::createProtocol(llvm::StringRef protocolName,
                                        Type parent) {
   auto *PD = new (Context)

unittests/Sema/SemaFixture.h

@@ -67,6 +67,11 @@ class SemaTest : public SemaTestBase {
 protected:
   Type getStdlibType(StringRef name) const;
 
+  NominalTypeDecl *getStdlibNominalTypeDecl(StringRef name) const;
+
+  VarDecl *addExtensionVarMember(NominalTypeDecl *decl, StringRef name,
+                                 Type type) const;
+
   ProtocolType *createProtocol(llvm::StringRef protocolName,
                                Type parent = Type());
 

unittests/Sema/UnresolvedMemberLookupTests.cpp

@@ -0,0 +1,90 @@
+//===--- UnresolvedMemberLookupTests.cpp --------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+#include "SemaFixture.h"
+#include "swift/Sema/ConstraintSystem.h"
+
+using namespace swift;
+using namespace swift::unittest;
+using namespace swift::constraints;
+
+TEST_F(SemaTest, TestLookupAlwaysLooksThroughOptionalBase) {
+  auto *intTypeDecl = getStdlibNominalTypeDecl("Int");
+  auto *optTypeDecl = getStdlibNominalTypeDecl("Optional");
+  auto intType = intTypeDecl->getDeclaredType();
+  auto intOptType = OptionalType::get(intType);
+  auto stringType = getStdlibType("String");
+
+  auto *intMember = addExtensionVarMember(intTypeDecl, "test", intOptType);
+  addExtensionVarMember(optTypeDecl, "test", stringType);
+
+  auto *UME = new (Context)
+      UnresolvedMemberExpr(SourceLoc(), DeclNameLoc(),
+                           DeclNameRef(Context.getIdentifier("test")), true);
+  auto *UMCRE = new (Context) UnresolvedMemberChainResultExpr(UME, UME);
+
+  ConstraintSystem cs(DC, ConstraintSystemOptions());
+  cs.generateConstraints(UMCRE, DC);
+  cs.addConstraint(
+      ConstraintKind::Conversion, cs.getType(UMCRE), intOptType,
+      cs.getConstraintLocator(UMCRE, ConstraintLocator::ContextualType));
+  SmallVector<Solution, 2> solutions;
+  cs.solve(solutions);
+
+  // We should have a solution.
+  ASSERT_EQ(solutions.size(), 1);
+
+  auto &solution = solutions[0];
+  auto *locator = cs.getConstraintLocator(UME,
+                                          ConstraintLocator::UnresolvedMember);
+  auto choice = solution.getOverloadChoice(locator).choice;
+
+  // The `test` member on `Int` should be selected.
+  ASSERT_EQ(choice.getDecl(), intMember);
+}
+
+TEST_F(SemaTest, TestLookupPrefersResultsOnOptionalRatherThanBase) {
+  auto *intTypeDecl = getStdlibNominalTypeDecl("Int");
+  auto *optTypeDecl = getStdlibNominalTypeDecl("Optional");
+  auto intType = intTypeDecl->getDeclaredType();
+  auto intOptType = OptionalType::get(intType);
+
+  addExtensionVarMember(intTypeDecl, "test", intOptType);
+  auto *optMember = addExtensionVarMember(optTypeDecl, "test", intType);
+
+  auto *UME = new (Context)
+      UnresolvedMemberExpr(SourceLoc(), DeclNameLoc(),
+                           DeclNameRef(Context.getIdentifier("test")), true);
+  auto *UMCRE = new (Context) UnresolvedMemberChainResultExpr(UME, UME);
+
+  ConstraintSystem cs(DC, ConstraintSystemOptions());
+  cs.generateConstraints(UMCRE, DC);
+  cs.addConstraint(
+      ConstraintKind::Conversion, cs.getType(UMCRE), intOptType,
+      cs.getConstraintLocator(UMCRE, ConstraintLocator::ContextualType));
+  SmallVector<Solution, 2> solutions;
+  cs.solve(solutions);
+
+  // We should have a solution.
+  ASSERT_EQ(solutions.size(), 1);
+
+  auto &solution = solutions[0];
+  auto *locator = cs.getConstraintLocator(UME,
+                                          ConstraintLocator::UnresolvedMember);
+  auto choice = solution.getOverloadChoice(locator).choice;
+  auto score = solution.getFixedScore();
+
+  // The `test` member on `Optional` should be chosen over the member on `Int`,
+  // even though the score is otherwise worse.
+  ASSERT_EQ(score.Data[SK_ValueToOptional], 1);
+  ASSERT_EQ(choice.getDecl(), optMember);
+}