Merge pull request #71933 from jckarter/conditional-copyable-trivial-type-lowering

jckarter · web-flow · commit 64be3978c788 · 2024-02-28T07:31:06.000-08:00
SIL: More accurate for type lowering whether a type is trivial based on conditional Copyable requirements.
diff --git a/include/swift/AST/Module.h b/include/swift/AST/Module.h
@@ -846,6 +846,7 @@ class ModuleDecl
                                            bool allowMissing = false);
 
   /// Global conformance lookup, checks conditional requirements.
+  /// Requires a contextualized type.
   ///
   /// \param type The type for which we are computing conformance. Must not
   /// contain type parameters.
@@ -859,8 +860,32 @@ class ModuleDecl
   /// \returns An invalid conformance if the search failed, otherwise an
   /// abstract, concrete or pack conformance, depending on the lookup type.
   ProtocolConformanceRef checkConformance(Type type, ProtocolDecl *protocol,
-                                          // Note: different default than above
-                                          bool allowMissing = true);
+                              // Note: different default from lookupConformance
+                              bool allowMissing = true);
+
+  /// Global conformance lookup, checks conditional requirements.
+  /// Accepts interface types without context. If the conformance cannot be
+  /// definitively established without the missing context, returns \c nullopt.
+  ///
+  /// \param type The type for which we are computing conformance. Must not
+  /// contain type parameters.
+  ///
+  /// \param protocol The protocol to which we are computing conformance.
+  ///
+  /// \param allowMissing When \c true, the resulting conformance reference
+  /// might include "missing" conformances, which are synthesized for some
+  /// protocols as an error recovery mechanism.
+  ///
+  /// \returns An invalid conformance if the search definitively failed. An
+  /// abstract, concrete or pack conformance, depending on the lookup type,
+  /// if the search succeeded. `std::nullopt` if the type could have
+  /// conditionally conformed depending on the context of the interface types.
+  std::optional<ProtocolConformanceRef>
+  checkConformanceWithoutContext(Type type,
+                                 ProtocolDecl *protocol,
+                                 // Note: different default from lookupConformance
+                                 bool allowMissing = true);
+
 
   /// Look for the conformance of the given existential type to the given
   /// protocol.
diff --git a/include/swift/AST/Requirement.h b/include/swift/AST/Requirement.h
@@ -223,6 +223,12 @@ enum class CheckRequirementsResult : uint8_t {
 /// not contain any type parameters.
 CheckRequirementsResult checkRequirements(ArrayRef<Requirement> requirements);
 
+/// Check if each substituted requirement is satisfied. If the requirement
+/// contains type parameters, and the answer would depend on the context of
+/// those type parameters, then `nullopt` is returned.
+std::optional<CheckRequirementsResult>
+checkRequirementsWithoutContext(ArrayRef<Requirement> requirements);
+
 /// Check if each requirement is satisfied after applying the given
 /// substitutions. The substitutions must replace all type parameters that
 /// appear in the requirement with concrete types or archetypes.
diff --git a/include/swift/SIL/AbstractionPattern.h b/include/swift/SIL/AbstractionPattern.h
@@ -1014,6 +1014,7 @@ class AbstractionPattern {
 
   bool requiresClass() const;
   LayoutConstraint getLayoutConstraint() const;
+  bool isNoncopyable(CanType substTy) const;
 
   /// Return the Swift type which provides structure for this
   /// abstraction pattern.
diff --git a/lib/AST/ConformanceLookup.cpp b/lib/AST/ConformanceLookup.cpp
@@ -796,8 +796,20 @@ LookupConformanceInModuleRequest::evaluate(
 ProtocolConformanceRef
 ModuleDecl::checkConformance(Type type, ProtocolDecl *proto,
                              bool allowMissing) {
-  assert(!type->hasTypeParameter());
+  assert(!type->hasTypeParameter()
+         && "must take a contextual type. if you really are ok with an "
+            "indefinite answer (and usually YOU ARE NOT), then consider whether "
+            "you really, definitely are ok with an indefinite answer, and "
+            "use `checkConformanceWithoutContext` instead");
+
+  // With no type parameter in the type, we should always get a definite answer
+  // from the underlying test.
+  return checkConformanceWithoutContext(type, proto, allowMissing).value();
+}
 
+std::optional<ProtocolConformanceRef>
+ModuleDecl::checkConformanceWithoutContext(Type type, ProtocolDecl *proto,
+                                           bool allowMissing) {
   auto lookupResult = lookupConformance(type, proto, allowMissing);
   if (lookupResult.isInvalid()) {
     return ProtocolConformanceRef::forInvalid();
@@ -807,7 +819,11 @@ ModuleDecl::checkConformance(Type type, ProtocolDecl *proto,
 
   // If we have a conditional requirements that we need to check, do so now.
   if (!condReqs.empty()) {
-    switch (checkRequirements(condReqs)) {
+    auto reqResult = checkRequirementsWithoutContext(condReqs);
+    if (!reqResult.has_value()) {
+      return std::nullopt;
+    }
+    switch (*reqResult) {
     case CheckRequirementsResult::Success:
       break;
 
diff --git a/lib/AST/Requirement.cpp b/lib/AST/Requirement.cpp
@@ -246,7 +246,9 @@ int Requirement::compare(const Requirement &other) const {
   return compareProtos;
 }
 
-CheckRequirementsResult swift::checkRequirements(ArrayRef<Requirement> requirements) {
+static std::optional<CheckRequirementsResult>
+checkRequirementsImpl(ArrayRef<Requirement> requirements,
+                      bool allowTypeParameters) {
   SmallVector<Requirement, 4> worklist(requirements.begin(), requirements.end());
 
   bool hadSubstFailure = false;
@@ -258,12 +260,20 @@ CheckRequirementsResult swift::checkRequirements(ArrayRef<Requirement> requireme
 #ifndef NDEBUG
   {
     auto firstType = req.getFirstType();
-    assert(!firstType->hasTypeParameter());
+    assert((allowTypeParameters || !firstType->hasTypeParameter())
+           && "must take a contextual type. if you really are ok with an "
+            "indefinite answer (and usually YOU ARE NOT), then consider whether "
+            "you really, definitely are ok with an indefinite answer, and "
+            "use `checkRequirementsWithoutContext` instead");
     assert(!firstType->hasTypeVariable());
 
     if (req.getKind() != RequirementKind::Layout) {
       auto secondType = req.getSecondType();
-      assert(!secondType->hasTypeParameter());
+      assert((allowTypeParameters || !secondType->hasTypeParameter())
+             && "must take a contextual type. if you really are ok with an "
+              "indefinite answer (and usually YOU ARE NOT), then consider whether "
+              "you really, definitely are ok with an indefinite answer, and "
+              "use `checkRequirementsWithoutContext` instead");
       assert(!secondType->hasTypeVariable());
     }
   }
@@ -276,6 +286,12 @@ CheckRequirementsResult swift::checkRequirements(ArrayRef<Requirement> requireme
       break;
 
     case CheckRequirementResult::RequirementFailure:
+      // If a requirement failure was caused by a context-free type parameter,
+      // then we can't definitely know whether it would have satisfied the
+      // requirement without context.
+      if (req.getFirstType()->isTypeParameter()) {
+        return std::nullopt;
+      }
       return CheckRequirementsResult::RequirementFailure;
 
     case CheckRequirementResult::SubstitutionFailure:
@@ -290,6 +306,19 @@ CheckRequirementsResult swift::checkRequirements(ArrayRef<Requirement> requireme
   return CheckRequirementsResult::Success;
 }
 
+CheckRequirementsResult
+swift::checkRequirements(ArrayRef<Requirement> requirements) {
+  // This entry point requires that there are no type parameters in any of the
+  // requirements, so the underlying check should always produce a result.
+  return checkRequirementsImpl(requirements, /*allow type parameters*/ false)
+    .value();
+}
+
+std::optional<CheckRequirementsResult>
+swift::checkRequirementsWithoutContext(ArrayRef<Requirement> requirements) {
+  return checkRequirementsImpl(requirements, /*allow type parameters*/ true);
+}
+
 CheckRequirementsResult swift::checkRequirements(
     ModuleDecl *module, ArrayRef<Requirement> requirements,
     TypeSubstitutionFn substitutions, SubstOptions options) {
@@ -332,4 +361,4 @@ void InverseRequirement::expandDefaults(
                          SourceLoc()});
     }
   }
-}
+}
diff --git a/lib/SIL/IR/AbstractionPattern.cpp b/lib/SIL/IR/AbstractionPattern.cpp
@@ -284,6 +284,83 @@ LayoutConstraint AbstractionPattern::getLayoutConstraint() const {
   }
 }
 
+bool AbstractionPattern::isNoncopyable(CanType substTy) const {
+  auto copyable
+    = substTy->getASTContext().getProtocol(KnownProtocolKind::Copyable);
+    
+  auto isDefinitelyCopyable = [&](CanType t) -> bool {
+    auto result = copyable->getParentModule()
+      ->checkConformanceWithoutContext(substTy, copyable,
+                                       /*allowMissing=*/false);
+    return result.has_value() && !result.value().isInvalid();
+  };
+    
+  // If the substituted type definitely conforms, that's authoritative.
+  if (isDefinitelyCopyable(substTy)) {
+    return false;
+  }
+
+  // If the substituted type is fully concrete, that's it. If there are unbound
+  // type variables in the type, then we may have to account for the upper
+  // abstraction bound from the abstraction pattern.
+  if (!substTy->hasTypeParameter()) {
+    return true;
+  }
+  
+  switch (getKind()) {
+  case Kind::Opaque: {
+    // The abstraction pattern doesn't provide any more specific bounds.
+    return true;
+  }
+  case Kind::Type:
+  case Kind::Discard:
+  case Kind::ClangType: {
+    // See whether the abstraction pattern's context gives us an upper bound
+    // that ensures the type is copyable.
+    auto type = getType();
+    if (hasGenericSignature() && getType()->hasTypeParameter()) {
+      type = GenericEnvironment::mapTypeIntoContext(
+        getGenericSignature().getGenericEnvironment(), getType())
+        ->getReducedType(getGenericSignature());
+    }
+    
+    return !isDefinitelyCopyable(type);
+  }
+  case Kind::Tuple: {
+    // A tuple is noncopyable if any element is.
+    if (doesTupleVanish()) {
+      return getVanishingTupleElementPatternType().value()
+        .isNoncopyable(substTy);
+    }
+    auto substTupleTy = cast<TupleType>(substTy);
+  
+    for (unsigned i = 0, e = getNumTupleElements(); i < e; ++i) {
+      if (getTupleElementType(i).isNoncopyable(substTupleTy.getElementType(i))){
+        return true;
+      }
+    }
+    return false;
+  }
+  // Functions are, at least for now, always copyable.
+  case Kind::CurriedObjCMethodType:
+  case Kind::PartialCurriedObjCMethodType:
+  case Kind::CFunctionAsMethodType:
+  case Kind::CurriedCFunctionAsMethodType:
+  case Kind::PartialCurriedCFunctionAsMethodType:
+  case Kind::ObjCMethodType:
+  case Kind::ObjCCompletionHandlerArgumentsType:
+  case Kind::CXXMethodType:
+  case Kind::CurriedCXXMethodType:
+  case Kind::PartialCurriedCXXMethodType:
+  case Kind::OpaqueFunction:
+  case Kind::OpaqueDerivativeFunction:
+    return false;
+  
+  case Kind::Invalid:
+    llvm_unreachable("asking invalid abstraction pattern");
+  }
+}
+
 bool AbstractionPattern::matchesTuple(CanType substType) const {
   switch (getKind()) {
   case Kind::Invalid:
diff --git a/lib/SIL/IR/TypeLowering.cpp b/lib/SIL/IR/TypeLowering.cpp
@@ -2353,7 +2353,7 @@ namespace {
 
       return handleReference(classType, properties);
     }
-
+    
     // WARNING: when the specification of trivial types changes, also update
     // the isValueTrivial() API used by SILCombine.
     TypeLowering *visitAnyStructType(CanType structType,
@@ -2433,7 +2433,7 @@ namespace {
       properties =
           applyLifetimeAnnotation(D->getLifetimeAnnotation(), properties);
 
-      if (D->canBeCopyable() != TypeDecl::CanBeInvertible::Always) {
+      if (origType.isNoncopyable(structType)) {
         properties.setNonTrivial();
         properties.setLexical(IsLexical);
         if (properties.isAddressOnly())
@@ -2530,7 +2530,7 @@ namespace {
       properties =
           applyLifetimeAnnotation(D->getLifetimeAnnotation(), properties);
 
-      if (D->canBeCopyable() != TypeDecl::CanBeInvertible::Always) {
+      if (origType.isNoncopyable(enumType)) {
         properties.setNonTrivial();
         properties.setLexical(IsLexical);
         if (properties.isAddressOnly())
diff --git a/test/SILGen/typelowering_inverses.swift b/test/SILGen/typelowering_inverses.swift
@@ -1,7 +1,5 @@
 // RUN: %target-swift-frontend -emit-silgen -enable-experimental-feature NoncopyableGenerics -disable-availability-checking -module-name main %s | %FileCheck %s
 
-
-
 struct NC: ~Copyable {}
 
 struct RudeStruct<T: ~Copyable>: Copyable {
@@ -34,13 +32,13 @@ func check(_ t: RudeEnum<Int>) {}
 // CHECK: sil hidden [ossa] @$s4main5checkyyAA8RudeEnumOyAA2NCVGF : $@convention(thin) (RudeEnum<NC>) -> () {
 func check(_ t: RudeEnum<NC>) {}
 
-// CHECK: sil hidden [ossa] @$s4main5checkyyAA18CondCopyableStructVySiGF : $@convention(thin) (@guaranteed CondCopyableStruct<Int>) -> () {
+// CHECK: sil hidden [ossa] @$s4main5checkyyAA18CondCopyableStructVySiGF : $@convention(thin) (CondCopyableStruct<Int>) -> () {
 func check(_ t: CondCopyableStruct<Int>) {}
 
 // CHECK: sil hidden [ossa] @$s4main5checkyyAA18CondCopyableStructVyAA2NCVGF : $@convention(thin) (@guaranteed CondCopyableStruct<NC>) -> () {
 func check(_ t: borrowing CondCopyableStruct<NC>) {}
 
-// CHECK: sil hidden [ossa] @$s4main5checkyyAA16CondCopyableEnumOySiGF : $@convention(thin) (@guaranteed CondCopyableEnum<Int>) -> () {
+// CHECK: sil hidden [ossa] @$s4main5checkyyAA16CondCopyableEnumOySiGF : $@convention(thin) (CondCopyableEnum<Int>) -> () {
 func check(_ t: CondCopyableEnum<Int>) {}
 
 // CHECK: sil hidden [ossa] @$s4main5checkyyAA16CondCopyableEnumOyAA2NCVGF : $@convention(thin) (@guaranteed CondCopyableEnum<NC>) -> () {
@@ -51,3 +49,51 @@ func check<T>(_ t: CondCopyableEnum<T>) {}
 
 // CHECK: sil hidden [ossa] @$s4main13check_noClashyyAA16CondCopyableEnumOyxGlF : $@convention(thin) <U where U : ~Copyable> (@in_guaranteed CondCopyableEnum<U>) -> () {
 func check_noClash<U: ~Copyable>(_ t: borrowing CondCopyableEnum<U>) {}
+
+struct MyStruct<T: ~Copyable>: ~Copyable {
+    var x: T
+}
+
+extension MyStruct: Copyable where T: Copyable {}
+
+enum MyEnum<T: ~Copyable>: ~Copyable {
+    case x(T)
+    case knoll
+}
+
+extension MyEnum: Copyable where T: Copyable {}
+
+enum Trivial {
+    case a, b, c
+}
+
+// CHECK-LABEL: sil{{.*}} @{{.*}}13trivialStruct
+func trivialStruct() -> Int {
+    // CHECK: [[ALLOC:%.*]] = alloc_stack $MyStruct<Trivial>
+    // CHECK-NOT: destroy_addr [[ALLOC]] :
+    // CHECK: dealloc_stack [[ALLOC]] :
+     return MemoryLayout.size(ofValue: MyStruct(x: Trivial.a))
+}
+// CHECK-LABEL: sil{{.*}} @{{.*}}11trivialEnum
+func trivialEnum() -> Int {
+    // CHECK: [[ALLOC:%.*]] = alloc_stack $MyEnum<Trivial>
+    // CHECK-NOT: destroy_addr [[ALLOC]] :
+    // CHECK: dealloc_stack [[ALLOC]] :
+     return MemoryLayout.size(ofValue: MyEnum.x(Trivial.a))
+}
+
+struct MyAssortment {
+    var a: MyStruct<Trivial>
+    var b: MyEnum<Trivial>
+}
+
+// CHECK-LABEL: sil{{.*}} @{{.*}}4frob
+func frob(x: MyAssortment) -> Int {
+    // CHECK: [[ALLOC:%.*]] = alloc_stack $MyAssortment
+    // CHECK-NOT: destroy_addr [[ALLOC]] :
+    // CHECK: dealloc_stack [[ALLOC]] :
+    return MemoryLayout.size(ofValue: x)
+}
+
+extension MyEnum: _BitwiseCopyable
+    where T: Copyable & _BitwiseCopyable {}
