Merge pull request #61042 from nate-chandler/fix-lexical-recursive-properties-bug

Fix bug with lexical recursive property.

Author: nate-chandler

include/swift/SIL/SILType.h

@@ -20,6 +20,7 @@
 
 #include "swift/AST/SILLayout.h"
 #include "swift/AST/Types.h"
+#include "swift/SIL/AbstractionPattern.h"
 #include "swift/SIL/Lifetime.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/PointerIntPair.h"
@@ -753,25 +754,6 @@ class SILType {
   void dump() const;
   void print(raw_ostream &OS,
              const PrintOptions &PO = PrintOptions::printSIL()) const;
-
-#ifndef NDEBUG
-  /// Visit the distinct types of the fields out of which a type is aggregated.
-  ///
-  /// As we walk into the field types, if an aggregate is encountered, it may
-  /// still be a leaf.  It is a leaf if the \p isLeafAggregate predicate
-  /// returns true.
-  ///
-  /// Returns false if the leaves cannot be visited or if any invocation of the
-  /// visitor returns false.
-  ///
-  /// NOTE: This function is meant for use in verification.  For real use-cases,
-  ///       recursive walks of type leaves should be done via
-  ///       TypeLowering::RecursiveProperties.
-  bool visitAggregateLeaves(
-      Lowering::TypeConverter &TC, TypeExpansionContext context,
-      std::function<bool(SILType, SILType, VarDecl *)> isLeafAggregate,
-      std::function<bool(SILType, SILType, VarDecl *)> visit) const;
-#endif
 };
 
 // Statically prevent SILTypes from being directly cast to a type

include/swift/SIL/TypeLowering.h

@@ -1250,6 +1250,15 @@ class TypeConverter {
   /// getTypeLowering(AbstractionPattern,Type,TypeExpansionContext).
   void verifyLowering(const TypeLowering &, AbstractionPattern origType,
                       Type origSubstType, TypeExpansionContext forExpansion);
+  bool
+  visitAggregateLeaves(Lowering::AbstractionPattern origType, Type substType,
+                       TypeExpansionContext context,
+                       std::function<bool(Type, Lowering::AbstractionPattern,
+                                          ValueDecl *, Optional<unsigned>)>
+                           isLeafAggregate,
+                       std::function<bool(Type, Lowering::AbstractionPattern,
+                                          ValueDecl *, Optional<unsigned>)>
+                           visit);
 #endif
 };
 

lib/SIL/IR/SILType.cpp

@@ -942,66 +942,3 @@ bool SILType::isMoveOnly() const {
       return true;
   return isMoveOnlyWrapped();
 }
-
-#ifndef NDEBUG
-bool SILType::visitAggregateLeaves(
-    Lowering::TypeConverter &TC, TypeExpansionContext context,
-    std::function<bool(SILType, SILType, VarDecl *)> isLeaf,
-    std::function<bool(SILType, SILType, VarDecl *)> visit) const {
-
-  llvm::SmallSet<std::tuple<SILType::ValueType, SILType::ValueType, VarDecl *>,
-                 16>
-      visited;
-  llvm::SmallVector<
-      std::tuple<SILType::ValueType, SILType::ValueType, VarDecl *>, 16>
-      worklist;
-  auto insertIntoWorklist = [&visited, &worklist](SILType parent, SILType type,
-                                                  VarDecl *decl) -> bool {
-    if (!visited.insert({parent.value, type.value, decl}).second) {
-      return false;
-    }
-    worklist.push_back({parent.value, type.value, decl});
-    return true;
-  };
-  auto popFromWorklist =
-      [&worklist]() -> std::tuple<SILType, SILType, VarDecl *> {
-    SILType::ValueType parentOpaqueType;
-    SILType::ValueType opaqueType;
-    VarDecl *decl;
-    std::tie(parentOpaqueType, opaqueType, decl) = worklist.pop_back_val();
-    return {parentOpaqueType, opaqueType, decl};
-  };
-  insertIntoWorklist(SILType(), *this, nullptr);
-  while (!worklist.empty()) {
-    SILType parent;
-    SILType ty;
-    VarDecl *decl;
-    std::tie(parent, ty, decl) = popFromWorklist();
-    if (ty.isAggregate() && !isLeaf(parent, ty, decl)) {
-      if (auto tupleTy = ty.getAs<TupleType>()) {
-        for (unsigned index = 0, num = tupleTy->getNumElements(); index < num;
-             ++index) {
-          insertIntoWorklist(ty, ty.getTupleElementType(index), nullptr);
-        }
-      } else if (auto *decl = ty.getStructOrBoundGenericStruct()) {
-        for (auto *field : decl->getStoredProperties()) {
-          insertIntoWorklist(ty, ty.getFieldType(field, TC, context), field);
-        }
-      } else if (auto *decl = ty.getEnumOrBoundGenericEnum()) {
-        for (auto *field : decl->getStoredProperties()) {
-          insertIntoWorklist(ty, ty.getFieldType(field, TC, context), field);
-        }
-      } else {
-        llvm_unreachable("unknown aggregate kind!");
-      }
-      continue;
-    }
-
-    // This type is a leaf.  Visit it.
-    auto success = visit(parent, ty, decl);
-    if (!success)
-      return false;
-  }
-  return true;
-}
-#endif

lib/SIL/IR/TypeLowering.cpp

@@ -2110,6 +2110,7 @@ namespace {
       if (D->isCxxNonTrivial()) {
         properties.setAddressOnly();
         properties.setNonTrivial();
+        properties.setLexical(IsLexical);
       }
 
       auto subMap = structType->getContextSubstitutionMap(&TC.M, D);
@@ -2169,6 +2170,9 @@ namespace {
       // may be added resiliently later.
       if (D->isIndirect()) {
         properties.setNonTrivial();
+        properties.setLexical(IsLexical);
+        properties =
+            applyLifetimeAnnotation(D->getLifetimeAnnotation(), properties);
         return new (TC) LoadableEnumTypeLowering(enumType, properties,
                                                  Expansion);
       }
@@ -2184,6 +2188,9 @@ namespace {
         // Indirect elements only make the type nontrivial.
         if (elt->isIndirect()) {
           properties.setNonTrivial();
+          properties.setLexical(IsLexical);
+          properties =
+              applyLifetimeAnnotation(elt->getLifetimeAnnotation(), properties);
           continue;
         }
 
@@ -2433,74 +2440,160 @@ TypeConverter::getTypeLowering(AbstractionPattern origType,
   }
 
 #ifndef NDEBUG
-  verifyLowering(*lowering, origType, origSubstType, forExpansion);
+  verifyLowering(*lowering, origType, loweredSubstType, forExpansion);
 #endif
 
   return *lowering;
 }
 
 #ifndef NDEBUG
+bool TypeConverter::visitAggregateLeaves(
+    Lowering::AbstractionPattern origType, Type substType,
+    TypeExpansionContext context,
+    std::function<bool(Type, Lowering::AbstractionPattern, ValueDecl *,
+                       Optional<unsigned>)>
+        isLeafAggregate,
+    std::function<bool(Type, Lowering::AbstractionPattern, ValueDecl *,
+                       Optional<unsigned>)>
+        visit) {
+  llvm::SmallSet<std::tuple<TypeBase *, ValueDecl *, unsigned>, 16> visited;
+  llvm::SmallVector<
+      std::tuple<TypeBase *, AbstractionPattern, ValueDecl *, unsigned>, 16>
+      worklist;
+  auto insertIntoWorklist = [&visited,
+                             &worklist](Type substTy, AbstractionPattern origTy,
+                                        ValueDecl *field,
+                                        Optional<unsigned> maybeIndex) -> bool {
+    unsigned index = maybeIndex.getValueOr(UINT_MAX);
+    if (!visited.insert({substTy.getPointer(), field, index}).second)
+      return false;
+    worklist.push_back({substTy.getPointer(), origTy, field, index});
+    return true;
+  };
+  auto popFromWorklist = [&worklist]()
+      -> std::tuple<Type, AbstractionPattern, ValueDecl *, Optional<unsigned>> {
+    TypeBase *ty;
+    AbstractionPattern origTy = AbstractionPattern::getOpaque();
+    ValueDecl *field;
+    unsigned index;
+    std::tie(ty, origTy, field, index) = worklist.pop_back_val();
+    Optional<unsigned> maybeIndex;
+    if (index != UINT_MAX)
+      maybeIndex = {index};
+    return {ty->getCanonicalType(), origTy, field, index};
+  };
+  auto isAggregate = [](Type ty) {
+    return ty->is<TupleType>() || ty->getEnumOrBoundGenericEnum() ||
+           ty->getStructOrBoundGenericStruct();
+  };
+  insertIntoWorklist(substType, origType, nullptr, llvm::None);
+  while (!worklist.empty()) {
+    Type ty;
+    AbstractionPattern origTy = AbstractionPattern::getOpaque();
+    ValueDecl *field;
+    Optional<unsigned> index;
+    std::tie(ty, origTy, field, index) = popFromWorklist();
+    if (isAggregate(ty) && !isLeafAggregate(ty, origTy, field, index)) {
+      if (auto tupleTy = ty->getAs<TupleType>()) {
+        for (unsigned tupleIndex = 0, num = tupleTy->getNumElements();
+             tupleIndex < num; ++tupleIndex) {
+          auto origElementTy = origTy.getTupleElementType(tupleIndex);
+          auto substElementTy =
+              tupleTy->getElementType(tupleIndex)->getCanonicalType();
+          substElementTy =
+              computeLoweredRValueType(context, origElementTy, substElementTy);
+          insertIntoWorklist(substElementTy, origElementTy, nullptr,
+                             tupleIndex);
+        }
+      } else if (auto *decl = ty->getStructOrBoundGenericStruct()) {
+        for (auto *structField : decl->getStoredProperties()) {
+          auto subMap = ty->getContextSubstitutionMap(&M, decl);
+          auto substFieldTy =
+              structField->getInterfaceType().subst(subMap)->getCanonicalType();
+          auto sig =
+              structField->getDeclContext()->getGenericSignatureOfContext();
+          auto interfaceTy =
+              structField->getInterfaceType()->getReducedType(sig);
+          auto origFieldType =
+              origTy.unsafeGetSubstFieldType(structField, interfaceTy);
+          insertIntoWorklist(substFieldTy, origFieldType, structField,
+                             llvm::None);
+        }
+      } else if (auto *decl = ty->getEnumOrBoundGenericEnum()) {
+        auto subMap = ty->getContextSubstitutionMap(&M, decl);
+        for (auto *element : decl->getAllElements()) {
+          if (!element->hasAssociatedValues())
+            continue;
+          // TODO: Callback for indirect elements.
+          if (element->isIndirect())
+            continue;
+          auto substElementType = element->getArgumentInterfaceType()
+                                      .subst(subMap)
+                                      ->getCanonicalType();
+          auto origElementTy = origTy.unsafeGetSubstFieldType(
+              element, element->getArgumentInterfaceType()->getReducedType(
+                           decl->getGenericSignature()));
+
+          insertIntoWorklist(substElementType, origElementTy, element,
+                             llvm::None);
+        }
+      } else {
+        llvm_unreachable("unknown aggregate kind!");
+      }
+      continue;
+    }
+
+    // This type is a leaf.  Visit it.
+    auto success = visit(ty, origTy, field, index);
+    if (!success)
+      return false;
+  }
+  return true;
+}
+
 void TypeConverter::verifyLowering(const TypeLowering &lowering,
-                                   AbstractionPattern origType,
-                                   Type origSubstType,
+                                   AbstractionPattern origType, Type substType,
                                    TypeExpansionContext forExpansion) {
   // Non-trivial lowerings should always be lexical unless all non-trivial
   // fields are eager move.
   if (!lowering.isTrivial() && !lowering.isLexical()) {
-    auto getLifetimeAnnotation = [](SILType ty) -> LifetimeAnnotation {
+    if (lowering.getRecursiveProperties().isInfinite())
+      return;
+    auto getLifetimeAnnotation = [](Type ty) -> LifetimeAnnotation {
       NominalTypeDecl *nominal;
-      if (!(nominal = ty.getASTType().getAnyNominal()))
+      if (!(nominal = ty->getAnyNominal()))
         return LifetimeAnnotation::None;
       return nominal->getLifetimeAnnotation();
     };
-    auto loweredType = lowering.getLoweredType();
-    bool hasNoNontrivialLexicalLeaf = loweredType.visitAggregateLeaves(
-        *this, forExpansion,
+    bool hasNoNontrivialLexicalLeaf = visitAggregateLeaves(
+        origType, substType, forExpansion,
         /*isLeaf=*/
-        [&](auto parent, auto ty, auto *fieldDecl) -> bool {
+        [&](auto ty, auto origTy, auto *field, auto index) -> bool {
           // The field's type is an aggregate.  Treat it as a leaf if it
           // has a lifetime annotation.
 
-          // If we don't have a field decl, it's either a field of a tuple
-          // or the top-level type.  Either way, there's no var decl on
-          // which to look for an attribute.
-          //
-          // It's a leaf if the type has a lifetime annotation.
-          if (!fieldDecl)
+          // If it's a field of a tuple or the top-level type, there's no value
+          // decl on which to look for an attribute.  It's a leaf iff the type
+          // has a lifetime annotation.
+          if (index || !field)
             return getLifetimeAnnotation(ty).isSome();
 
           // It's a field of a struct or an enum.  It's a leaf if the type
           // or the var decl has a lifetime annotation.
-          return fieldDecl->getLifetimeAnnotation().isSome() ||
+          return field->getLifetimeAnnotation().isSome() ||
                  getLifetimeAnnotation(ty);
         },
         /*visit=*/
-        [&](auto parent, auto ty, auto *fieldDecl) -> bool {
+        [&](auto ty, auto origTy, auto *field, auto index) -> bool {
           // Look at each leaf: if it is non-trivial, verify that it is
           // attributed @_eagerMove.
 
           // If the leaf is the whole type, verify that it is annotated
           // @_eagerMove.
-          if (ty == loweredType)
+          if (ty->getCanonicalType() == substType->getCanonicalType())
             return getLifetimeAnnotation(ty) == LifetimeAnnotation::EagerMove;
 
-          // Get ty's lowering.
-          CanGenericSignature sig;
-          if (fieldDecl) {
-            AbstractionPattern origFieldTy = getAbstractionPattern(fieldDecl);
-            CanType substFieldTy;
-            if (fieldDecl->hasClangNode()) {
-              substFieldTy = origFieldTy.getType();
-            } else {
-              substFieldTy = parent.getASTType()
-                                 ->getTypeOfMember(&M, fieldDecl)
-                                 ->getCanonicalType();
-            }
-            sig = getAbstractionPattern(fieldDecl).getGenericSignatureOrNull();
-          } else {
-            sig = CanGenericSignature();
-          }
-          auto &tyLowering = getTypeLowering(ty, forExpansion, sig);
+          auto &tyLowering = getTypeLowering(origTy, ty, forExpansion);
 
           // Leaves which are trivial aren't of interest.
           if (tyLowering.isTrivial())
@@ -2510,17 +2603,16 @@ void TypeConverter::verifyLowering(const TypeLowering &lowering,
           // not lexical.  The leaf must be annotated @_eagerMove.
           // Otherwise, the whole type would be lexical.
 
-          if (!fieldDecl) {
-            // The field is non-trivial and the whole type is non-lexical.
-            // If there's no field decl which might be annotated
-            // @_eagerMove, we have a problem.
-            return false;
+          if (index || !field) {
+            // There is no field decl that might be annotated @_eagerMove.  The
+            // field is @_eagerMove iff its type is annotated @_eagerMove.
+            return getLifetimeAnnotation(ty) == LifetimeAnnotation::EagerMove;
           }
 
           // The field is non-trivial and the whole type is non-lexical.
           // That's fine as long as the field or its type is annotated
           // @_eagerMove.
-          return fieldDecl->getLifetimeAnnotation() ==
+          return field->getLifetimeAnnotation() ==
                      LifetimeAnnotation::EagerMove ||
                  getLifetimeAnnotation(ty) == LifetimeAnnotation::EagerMove;
         });