Merge pull request swiftlang#20419 from jrose-apple/resolution-dissolution

[ParseableInterfaces] Skip value witnesses of resilient conformances

Author: jrose-apple

lib/Sema/TypeCheckProtocol.cpp

@@ -1173,23 +1173,6 @@ bool WitnessChecker::findBestWitness(
   }
 
   if (numViable == 0) {
-    // Assume any missing value witnesses for a conformance in a parseable
-    // interface can be treated as opaque.
-    // FIXME: ...but we should do something better about types.
-    if (conformance && !conformance->isInvalid()) {
-      if (auto *SF = DC->getParentSourceFile()) {
-        if (SF->Kind == SourceFileKind::Interface) {
-          auto match = matchWitness(TC, ReqEnvironmentCache, Proto,
-                                    conformance, DC, requirement, requirement);
-          assert(match.isViable());
-          numViable = 1;
-          bestIdx = matches.size();
-          matches.push_back(std::move(match));
-          return true;
-        }
-      }
-    }
-
     if (anyFromUnconstrainedExtension &&
         conformance != nullptr &&
         conformance->isInvalid()) {
@@ -2914,6 +2897,23 @@ void ConformanceChecker::checkNonFinalClassWitness(ValueDecl *requirement,
   }
 }
 
+ResolveWitnessResult
+ConformanceChecker::resolveWitnessAsOpaque(ValueDecl *requirement) {
+  assert(!isa<AssociatedTypeDecl>(requirement) && "Use resolveTypeWitnessVia*");
+  auto match = matchWitness(TC, ReqEnvironmentCache, Proto,
+                            Conformance, DC, requirement, requirement);
+  recordWitness(requirement, match);
+  return ResolveWitnessResult::Success;
+}
+
+static bool isConformanceFromParseableInterface(
+    const NormalProtocolConformance *conformance) {
+  auto *containingSF = conformance->getDeclContext()->getParentSourceFile();
+  if (!containingSF)
+    return false;
+  return containingSF->Kind == SourceFileKind::Interface;
+}
+
 ResolveWitnessResult
 ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
   assert(!isa<AssociatedTypeDecl>(requirement) && "Use resolveTypeWitnessVia*");
@@ -2931,25 +2931,33 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
     }
   }
 
-  // Determine whether we can derive a witness for this requirement.
-  bool canDerive = false;
-
-  // Can a witness for this requirement be derived for this nominal type?
-  if (auto derivable = DerivedConformance::getDerivableRequirement(
-                         TC,
-                         nominal,
-                         requirement)) {
-    if (derivable == requirement) {
-      // If it's the same requirement, we can derive it here.
-      canDerive = true;
-    } else {
-      // Otherwise, go satisfy the derivable requirement, which can introduce
-      // a member that could in turn satisfy *this* requirement.
-      auto derivableProto = cast<ProtocolDecl>(derivable->getDeclContext());
-      if (auto conformance =
-            TC.conformsToProtocol(Adoptee, derivableProto, DC, None)) {
-        if (conformance->isConcrete())
-          (void)conformance->getConcrete()->getWitnessDecl(derivable, &TC);
+  // Determine whether we can get a witness for this requirement some other way.
+  bool hasFallbacks = false;
+  if (!hasFallbacks)
+    hasFallbacks = requirement->getAttrs().hasAttribute<OptionalAttr>();
+  if (!hasFallbacks)
+    hasFallbacks = requirement->getAttrs().isUnavailable(TC.Context);
+  if (!hasFallbacks)
+    hasFallbacks = isConformanceFromParseableInterface(Conformance);
+
+  if (!hasFallbacks) {
+    // Can a witness for this requirement be derived for this nominal type?
+    if (auto derivable = DerivedConformance::getDerivableRequirement(
+                           TC,
+                           nominal,
+                           requirement)) {
+      if (derivable == requirement) {
+        // If it's the same requirement, we can derive it here.
+        hasFallbacks = true;
+      } else {
+        // Otherwise, go satisfy the derivable requirement, which can introduce
+        // a member that could in turn satisfy *this* requirement.
+        auto derivableProto = cast<ProtocolDecl>(derivable->getDeclContext());
+        if (auto conformance =
+              TC.conformsToProtocol(Adoptee, derivableProto, DC, None)) {
+          if (conformance->isConcrete())
+            (void)conformance->getConcrete()->getWitnessDecl(derivable, &TC);
+        }
       }
     }
   }
@@ -2960,11 +2968,7 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
   unsigned bestIdx = 0;
   bool doNotDiagnoseMatches = false;
   bool ignoringNames = false;
-  bool considerRenames =
-    !canDerive && !requirement->getAttrs().hasAttribute<OptionalAttr>() &&
-    !requirement->getAttrs().isUnavailable(TC.Context);
-  if (findBestWitness(requirement,
-                      considerRenames ? &ignoringNames : nullptr,
+  if (findBestWitness(requirement, hasFallbacks ? nullptr : &ignoringNames,
                       Conformance,
                       /* out parameters: */
                       matches, numViable, bestIdx, doNotDiagnoseMatches)) {
@@ -3182,19 +3186,7 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
   // We have either no matches or an ambiguous match.
 
   // If we can derive a definition for this requirement, just call it missing.
-  if (canDerive) {
-    return ResolveWitnessResult::Missing;
-  }
-
-  // If the requirement is optional, it's okay. We'll satisfy this via
-  // our handling of default definitions.
-  //
-  // FIXME: revisit this once we get default definitions in protocol bodies.
-  //
-  // Treat 'unavailable' implicitly as if it were 'optional'.
-  // The compiler will reject actual uses.
-  auto Attrs = requirement->getAttrs();
-  if (Attrs.hasAttribute<OptionalAttr>() || Attrs.isUnavailable(TC.Context)) {
+  if (hasFallbacks) {
     return ResolveWitnessResult::Missing;
   }
 
@@ -3355,6 +3347,47 @@ CheckTypeWitnessResult swift::checkTypeWitness(TypeChecker &tc, DeclContext *dc,
   return CheckTypeWitnessResult();
 }
 
+ResolveWitnessResult
+ConformanceChecker::resolveWitnessTryingAllStrategies(ValueDecl *requirement) {
+  using ResolveWitnessStrategy =
+      decltype(&ConformanceChecker::resolveWitnessViaLookup);
+  static const constexpr ResolveWitnessStrategy defaultStrategies[] = {
+      &ConformanceChecker::resolveWitnessViaLookup,
+      &ConformanceChecker::resolveWitnessViaDerivation,
+      &ConformanceChecker::resolveWitnessViaDefault};
+  ArrayRef<ResolveWitnessStrategy> strategies = defaultStrategies;
+
+  // Don't try any sort of derivation when processing a parseable interface.
+  if (isConformanceFromParseableInterface(Conformance)) {
+    if (Conformance->isResilient()) {
+      // Resilient conformances don't allow any sort of devirtualization, so
+      // don't bother looking up witnesses at all.
+      static const constexpr ResolveWitnessStrategy resilientStrategies[] = {
+          &ConformanceChecker::resolveWitnessAsOpaque};
+      strategies = resilientStrategies;
+    } else {
+      static const constexpr ResolveWitnessStrategy interfaceStrategies[] = {
+          &ConformanceChecker::resolveWitnessViaLookup,
+          &ConformanceChecker::resolveWitnessAsOpaque};
+      strategies = interfaceStrategies;
+    }
+  }
+
+  for (auto strategy : strategies) {
+    ResolveWitnessResult result = (this->*strategy)(requirement);
+    switch (result) {
+    case ResolveWitnessResult::Success:
+    case ResolveWitnessResult::ExplicitFailed:
+      return result;
+    case ResolveWitnessResult::Missing:
+      // Continue trying.
+      break;
+    }
+  }
+
+  return ResolveWitnessResult::Missing;
+}
+
 /// Attempt to resolve a type witness via member name lookup.
 ResolveWitnessResult ConformanceChecker::resolveTypeWitnessViaLookup(
                        AssociatedTypeDecl *assocType) {
@@ -3625,60 +3658,14 @@ void ConformanceChecker::ensureRequirementsAreSatisfied(
 }
 
 #pragma mark Protocol conformance checking
-void ConformanceChecker::checkConformance(MissingWitnessDiagnosisKind Kind) {
-  assert(!Conformance->isComplete() && "Conformance is already complete");
-
-  FrontendStatsTracer statsTracer(TC.Context.Stats, "check-conformance",
-                                  Conformance);
 
-  llvm::SaveAndRestore<bool> restoreSuppressDiagnostics(SuppressDiagnostics);
-  SuppressDiagnostics = false;
-
-  // FIXME: Caller checks that this type conforms to all of the
-  // inherited protocols.
-
-  // Emit known diags for this conformance.
-  emitDelayedDiags();
-
-  // If delayed diags have already complained, return.
-  if (AlreadyComplained) {
-    Conformance->setInvalid();
-    return;
-  }
-
-  // Resolve all of the type witnesses.
-  resolveTypeWitnesses();
-
-  // Diagnose missing type witnesses for now.
-  diagnoseMissingWitnesses(Kind);
-
-  // Ensure the conforming type is used.
-  //
-  // FIXME: This feels like the wrong place for this, but if we don't put
-  // it here, extensions don't end up depending on the extended type.
-  recordConformanceDependency(DC, Adoptee->getAnyNominal(), Conformance, false);
-
-  // If we complain about any associated types, there is no point in continuing.
-  // FIXME: Not really true. We could check witnesses that don't involve the
-  // failed associated types.
-  if (AlreadyComplained) {
-    Conformance->setInvalid();
-    return;
-  }
-
-  // Diagnose missing value witnesses later.
-  SWIFT_DEFER { diagnoseMissingWitnesses(Kind); };
-
-  // Ensure the associated type conformances are used.
-  addUsedConformances(Conformance);
-
-  // Check non-type requirements.
+void ConformanceChecker::resolveValueWitnesses() {
   for (auto member : Proto->getMembers()) {
     auto requirement = dyn_cast<ValueDecl>(member);
     if (!requirement)
       continue;
 
-    // Associated type requirements handled above.
+    // Associated type requirements handled elsewhere.
     if (isa<TypeDecl>(requirement))
       continue;
 
@@ -3829,9 +3816,9 @@ void ConformanceChecker::checkConformance(MissingWitnessDiagnosisKind Kind) {
     // If this is an accessor for a storage decl, ignore it.
     if (isa<AccessorDecl>(requirement))
       continue;
-    
-    // Try to resolve the witness via explicit definitions.
-    switch (resolveWitnessViaLookup(requirement)) {
+
+    // Try to resolve the witness.
+    switch (resolveWitnessTryingAllStrategies(requirement)) {
     case ResolveWitnessResult::Success:
       finalizeWitness();
       continue;
@@ -3841,41 +3828,62 @@ void ConformanceChecker::checkConformance(MissingWitnessDiagnosisKind Kind) {
       continue;
 
     case ResolveWitnessResult::Missing:
-      // Continue trying below.
+      // Let it get diagnosed later.
       break;
     }
+  }
+}
 
-    // Try to resolve the witness via derivation.
-    switch (resolveWitnessViaDerivation(requirement)) {
-    case ResolveWitnessResult::Success:
-      finalizeWitness();
-      continue;
+void ConformanceChecker::checkConformance(MissingWitnessDiagnosisKind Kind) {
+  assert(!Conformance->isComplete() && "Conformance is already complete");
 
-    case ResolveWitnessResult::ExplicitFailed:
-      Conformance->setInvalid();
-      continue;
+  FrontendStatsTracer statsTracer(TC.Context.Stats, "check-conformance",
+                                  Conformance);
 
-    case ResolveWitnessResult::Missing:
-      // Continue trying below.
-      break;
-    }
+  llvm::SaveAndRestore<bool> restoreSuppressDiagnostics(SuppressDiagnostics);
+  SuppressDiagnostics = false;
 
-    // Try to resolve the witness via defaults.
-    switch (resolveWitnessViaDefault(requirement)) {
-    case ResolveWitnessResult::Success:
-      finalizeWitness();
-      continue;
+  // FIXME: Caller checks that this type conforms to all of the
+  // inherited protocols.
 
-    case ResolveWitnessResult::ExplicitFailed:
-      Conformance->setInvalid();
-      continue;
+  // Emit known diags for this conformance.
+  emitDelayedDiags();
 
-    case ResolveWitnessResult::Missing:
-      // Continue trying below.
-      break;
-    }
+  // If delayed diags have already complained, return.
+  if (AlreadyComplained) {
+    Conformance->setInvalid();
+    return;
   }
 
+  // Resolve all of the type witnesses.
+  resolveTypeWitnesses();
+
+  // Diagnose missing type witnesses for now.
+  diagnoseMissingWitnesses(Kind);
+
+  // Ensure the conforming type is used.
+  //
+  // FIXME: This feels like the wrong place for this, but if we don't put
+  // it here, extensions don't end up depending on the extended type.
+  recordConformanceDependency(DC, Adoptee->getAnyNominal(), Conformance, false);
+
+  // If we complain about any associated types, there is no point in continuing.
+  // FIXME: Not really true. We could check witnesses that don't involve the
+  // failed associated types.
+  if (AlreadyComplained) {
+    Conformance->setInvalid();
+    return;
+  }
+
+  // Diagnose missing value witnesses later.
+  SWIFT_DEFER { diagnoseMissingWitnesses(Kind); };
+
+  // Ensure the associated type conformances are used.
+  addUsedConformances(Conformance);
+
+  // Check non-type requirements.
+  resolveValueWitnesses();
+
   emitDelayedDiags();
 
   // Except in specific hardcoded cases for Foundation/Swift

lib/Sema/TypeCheckProtocol.h

@@ -620,6 +620,9 @@ class ConformanceChecker : public WitnessChecker {
   void checkNonFinalClassWitness(ValueDecl *requirement,
                                  ValueDecl *witness);
 
+  /// Resolve the (non-type) witness as requiring dynamic dispatch.
+  ResolveWitnessResult resolveWitnessAsOpaque(ValueDecl *requirement);
+
   /// Resolve a (non-type) witness via name lookup.
   ResolveWitnessResult resolveWitnessViaLookup(ValueDecl *requirement);
 
@@ -629,6 +632,11 @@ class ConformanceChecker : public WitnessChecker {
   /// Resolve a (non-type) witness via default definition or optional.
   ResolveWitnessResult resolveWitnessViaDefault(ValueDecl *requirement);
 
+  /// Resolve a (non-type) witness by trying each standard strategy until one
+  /// of them produces a result.
+  ResolveWitnessResult
+  resolveWitnessTryingAllStrategies(ValueDecl *requirement);
+
   /// Attempt to resolve a type witness via member name lookup.
   ResolveWitnessResult resolveTypeWitnessViaLookup(
                          AssociatedTypeDecl *assocType);
@@ -675,6 +683,9 @@ class ConformanceChecker : public WitnessChecker {
   /// Resolve all of the type witnesses.
   void resolveTypeWitnesses();
 
+  /// Resolve all of the non-type witnesses.
+  void resolveValueWitnesses();
+
   /// Resolve the witness for the given non-type requirement as
   /// directly as possible, only resolving other witnesses if
   /// needed, e.g., to determine type witnesses used within the