Merge pull request #26159 from slavapestov/do-not-finalize-structs-and-enums

Struct and enum declarations no longer go through finalizeDecl()

Author: slavapestov

lib/IRGen/GenEnum.cpp

@@ -5823,14 +5823,7 @@ EnumImplStrategy::get(TypeConverter &TC, SILType type, EnumDecl *theEnum) {
   for (auto elt : theEnum->getAllElements()) {
     numElements++;
 
-    // Compute whether this gives us an apparent payload or dynamic layout.
-    // Note that we do *not* apply substitutions from a bound generic instance
-    // yet. We want all instances of a generic enum to share an implementation
-    // strategy. If the abstract layout of the enum is dependent on generic
-    // parameters, then we additionally need to constrain any layout
-    // optimizations we perform to things that are reproducible by the runtime.
-    Type origArgType = elt->getArgumentInterfaceType();
-    if (origArgType.isNull()) {
+    if (!elt->hasAssociatedValues()) {
       elementsWithNoPayload.push_back({elt, nullptr, nullptr});
       continue;
     }
@@ -5843,6 +5836,13 @@ EnumImplStrategy::get(TypeConverter &TC, SILType type, EnumDecl *theEnum) {
       continue;
     }
 
+    // Compute whether this gives us an apparent payload or dynamic layout.
+    // Note that we do *not* apply substitutions from a bound generic instance
+    // yet. We want all instances of a generic enum to share an implementation
+    // strategy. If the abstract layout of the enum is dependent on generic
+    // parameters, then we additionally need to constrain any layout
+    // optimizations we perform to things that are reproducible by the runtime.
+    Type origArgType = elt->getArgumentInterfaceType();
     origArgType = theEnum->mapTypeIntoContext(origArgType);
 
     auto origArgLoweredTy = TC.IGM.getLoweredType(origArgType);

lib/SIL/TypeLowering.cpp

@@ -237,7 +237,7 @@ namespace {
       llvm_unreachable("shouldn't get an inout type here");
     }
     RetTy visitErrorType(CanErrorType type) {
-      llvm_unreachable("shouldn't get an error type here");
+      return asImpl().handleTrivial(type);
     }
 
     // Dependent types should be contextualized before visiting.
@@ -501,9 +501,6 @@ namespace {
 static RecursiveProperties classifyType(CanType type, SILModule &M,
                                         CanGenericSignature sig,
                                         ResilienceExpansion expansion) {
-  assert(!type->hasError() &&
-         "Error types should not appear in type-checked AST");
-
   return TypeClassifier(M, sig, expansion).visit(type);
 }
 
@@ -1559,9 +1556,6 @@ TypeConverter::getTypeLowering(AbstractionPattern origType,
 
 CanType TypeConverter::computeLoweredRValueType(AbstractionPattern origType,
                                                 CanType substType) {
-  assert(!substType->hasError() &&
-         "Error types should not appear in type-checked AST");
-
   // AST function types are turned into SIL function types:
   //   - the type is uncurried as desired
   //   - types are turned into their unbridged equivalents, depending

lib/Sema/CodeSynthesis.cpp

@@ -2012,7 +2012,6 @@ void swift::triggerAccessorSynthesis(TypeChecker &TC,
     if (!accessor->hasBody()) {
       maybeMarkTransparent(accessor, TC.Context);
       accessor->setBodySynthesizer(&synthesizeAccessorBody);
-      TC.DeclsToFinalize.insert(accessor);
     }
   });
 }

lib/Sema/TypeCheckDecl.cpp

@@ -2319,7 +2319,9 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
       (void) VD->isDynamic();
 
       // Make sure we finalize this declaration.
-      TC.DeclsToFinalize.insert(VD);
+      if (isa<ClassDecl>(VD) || isa<ProtocolDecl>(VD) ||
+          isa<AbstractStorageDecl>(VD))
+        TC.DeclsToFinalize.insert(VD);
 
       // If this is a member of a nominal type, don't allow it to have a name of
       // "Type" or "Protocol" since we reserve the X.Type and X.Protocol
@@ -2873,11 +2875,11 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
     TC.validateDecl(SD);
     checkGenericParams(SD->getGenericParams(), SD, TC);
 
-    TC.addImplicitConstructors(SD);
-
     // Force lowering of stored properties.
     (void) SD->getStoredProperties();
 
+    TC.addImplicitConstructors(SD);
+
     for (Decl *Member : SD->getMembers())
       visit(Member);
 
@@ -3008,9 +3010,11 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
     // Force lowering of stored properties.
     (void) CD->getStoredProperties();
 
-    for (Decl *Member : CD->getMembers()) {
+    TC.addImplicitConstructors(CD);
+    CD->addImplicitDestructor();
+
+    for (Decl *Member : CD->getMembers())
       visit(Member);
-    }
 
     TC.checkPatternBindingCaptures(CD);
 
@@ -3019,9 +3023,6 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
     if (CD->requiresStoredPropertyInits())
       checkRequiredInClassInits(CD);
 
-    TC.addImplicitConstructors(CD);
-    CD->addImplicitDestructor();
-
     // Compute @objc for each superclass member, to catch selector
     // conflicts resulting from unintended overrides.
     //
@@ -3913,9 +3914,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
         checkEnumRawValues(*this, ED);
     }
 
-    if (!isa<ClassDecl>(nominal))
-      requestNominalLayout(nominal);
-
     break;
   }
 
@@ -4457,38 +4455,6 @@ void TypeChecker::validateDeclForNameLookup(ValueDecl *D) {
   }
 }
 
-static bool shouldValidateMemberDuringFinalization(NominalTypeDecl *nominal,
-                                                   ValueDecl *VD) {
-  // For enums, we only need to validate enum elements to know
-  // the layout.
-  if (isa<EnumDecl>(nominal) &&
-      isa<EnumElementDecl>(VD))
-    return true;
-
-  // For structs, we only need to validate stored properties to
-  // know the layout.
-  if (isa<StructDecl>(nominal) &&
-      (isa<VarDecl>(VD) &&
-       !cast<VarDecl>(VD)->isStatic() &&
-       (cast<VarDecl>(VD)->hasStorage() ||
-        VD->getAttrs().hasAttribute<LazyAttr>() ||
-        cast<VarDecl>(VD)->hasAttachedPropertyWrapper())))
-    return true;
-
-  // For classes, we need to validate properties and functions,
-  // but skipping nested types is OK.
-  if (isa<ClassDecl>(nominal) &&
-      !isa<TypeDecl>(VD))
-    return true;
-
-  // For protocols, skip nested typealiases and nominal types.
-  if (isa<ProtocolDecl>(nominal) &&
-      !isa<GenericTypeDecl>(VD))
-    return true;
-
-  return false;
-}
-
 void TypeChecker::requestMemberLayout(ValueDecl *member) {
   auto *dc = member->getDeclContext();
   if (auto *classDecl = dyn_cast<ClassDecl>(dc))
@@ -4531,13 +4497,11 @@ static void finalizeType(TypeChecker &TC, NominalTypeDecl *nominal) {
   // Force lowering of stored properties.
   (void) nominal->getStoredProperties();
 
-  for (auto *D : nominal->getMembers()) {
-    auto VD = dyn_cast<ValueDecl>(D);
-    if (!VD)
-      continue;
-
-    if (shouldValidateMemberDuringFinalization(nominal, VD))
-      TC.DeclsToFinalize.insert(VD);
+  if (isa<ClassDecl>(nominal) || isa<ProtocolDecl>(nominal)) {
+    for (auto *D : nominal->getMembers()) {
+      if (auto *ASD = dyn_cast<AbstractStorageDecl>(D))
+        TC.DeclsToFinalize.insert(ASD);
+    }
   }
 
   if (auto *CD = dyn_cast<ClassDecl>(nominal)) {

lib/Sema/TypeCheckProtocol.cpp

@@ -3703,7 +3703,8 @@ void ConformanceChecker::resolveValueWitnesses() {
 
       // Make sure that we finalize the witness, so we can emit this
       // witness table.
-      TC.DeclsToFinalize.insert(witness);
+      if (isa<AbstractStorageDecl>(witness))
+        TC.DeclsToFinalize.insert(witness);
 
       // Objective-C checking for @objc requirements.
       if (requirement->isObjC() &&

test/decl/class/override.swift

@@ -265,8 +265,8 @@ class Base24646184 {
   func foo(ok: SubTy) { }
 }
 class Derived24646184 : Base24646184 {
-  override init(_: Ty) { } // expected-note {{'init(_:)' previously overridden here}}
-  override init(_: SubTy) { } // expected-error {{'init(_:)' has already been overridden}}
+  override init(_: Ty) { } // expected-error {{'init(_:)' has already been overridden}}
+  override init(_: SubTy) { } // expected-note {{'init(_:)' previously overridden here}}
   override func foo(_: Ty) { } // expected-note {{'foo' previously overridden here}}
   override func foo(_: SubTy) { } // expected-error {{'foo' has already been overridden}}
 

test/decl/init/Inputs/memberwise-init-multifile-other.swift

@@ -1,3 +1,3 @@
 struct S {
-  var a = S() // expected-error {{'S' cannot be constructed because it has no accessible initializers}}
+  var a = S()
 }

test/decl/init/basic_init.swift

@@ -23,14 +23,13 @@ class InitClass {
   @objc dynamic init(bar: Int) {}
 }
 class InitSubclass: InitClass {}
-// expected-note@-1 {{'init(bar:)' previously overridden here}}
-// expected-note@-2 {{'init(baz:)' previously overridden here}}
+// expected-error@-1 {{'init(bar:)' has already been overridden}}
+// expected-error@-2 {{'init(baz:)' has already been overridden}}
 extension InitSubclass {
   convenience init(arg: Bool) {} // expected-error{{overriding non-@objc declarations from extensions is not supported}}
-  convenience override init(baz: Int) {}
+  convenience override init(baz: Int) {} // expected-note{{'init(baz:)' previously overridden here}}
   // expected-error@-1 {{cannot override a non-dynamic class declaration from an extension}}
-  // expected-error@-2 {{'init(baz:)' has already been overridden}}
-  convenience override init(bar: Int) {} // expected-error{{'init(bar:)' has already been overridden}}
+  convenience override init(bar: Int) {} // expected-note{{'init(bar:)' previously overridden here}}
 }
 
 struct InitStruct {

test/multifile/Inputs/require-layout-enum-other.swift

@@ -0,0 +1,10 @@
+public indirect enum E {
+  case foo(Any)
+  case bar(EE)
+  case blah
+}
+
+public enum EE {
+  case x(Int)
+  case y(String)
+}

test/multifile/Inputs/require-layout-error-other.swift

@@ -0,0 +1,3 @@
+public struct S {
+  var x: DoesNotExist // expected-error {{use of undeclared type 'DoesNotExist'}}
+}

test/multifile/require-layout-enum.swift

@@ -0,0 +1,3 @@
+// RUN: %target-swift-frontend -emit-ir -primary-file %s %S/Inputs/require-layout-enum-other.swift -module-name layout
+
+public func returnE() -> E { return .blah }

test/multifile/require-layout-error.swift

@@ -0,0 +1,3 @@
+// RUN: %target-swift-frontend -verify -emit-ir -primary-file %s %S/Inputs/require-layout-error-other.swift -module-name layout
+
+public func identity(_ s: S) -> S { return s }

validation-test/compiler_crashers_2_fixed/0124-sr5825.swift

@@ -1,4 +1,4 @@
-// RUN: not %target-swift-frontend %s -emit-ir
+// RUN: %target-swift-frontend %s -emit-ir
 
 struct DefaultAssociatedType {
 }

validation-test/compiler_crashers_2_fixed/0199-sr9583.swift

@@ -0,0 +1,14 @@
+// RUN: %target-swift-frontend -emit-ir -primary-file %s %S/Inputs/sr9583-other.swift -module-name foo
+
+protocol P {
+    associatedtype A
+    typealias T = S1<Self>
+}
+
+struct S1<G: P>: Sequence, IteratorProtocol {
+    mutating func next() -> G.A? {
+        return nil
+    }
+}
+
+func f(_: LazyFilterSequence<S3.T>) { }

validation-test/compiler_crashers_2_fixed/Inputs/sr9583-other.swift

@@ -0,0 +1,7 @@
+struct S2 {
+    let v: Int
+}
+
+struct S3: P {
+    typealias A = S2
+}

validation-test/compiler_scale/bind_extension_decl.gyb

@@ -1,4 +1,4 @@
-// RUN: %scale-test --sum-multi --typecheck --begin 5 --end 16 --step 5 --select NumDeclsValidated %s
+// RUN: %scale-test --sum-multi --begin 5 --end 16 --step 5 --select NumDeclsValidated %s
 // REQUIRES: OS=macosx
 // REQUIRES: asserts
 

validation-test/compiler_scale/bind_nested_extension_decl.gyb

@@ -1,4 +1,4 @@
-// RUN: %scale-test --sum-multi --typecheck --begin 5 --end 16 --step 5 --select NumDeclsValidated %s
+// RUN: %scale-test --sum-multi --begin 5 --end 16 --step 5 --select NumDeclsValidated %s
 // REQUIRES: OS=macosx
 // REQUIRES: asserts
 

validation-test/compiler_scale/class_members.gyb

@@ -1,4 +1,4 @@
-// RUN: %scale-test --sum-multi --typecheck --begin 5 --end 16 --step 5 --select NumDeclsValidated %s
+// RUN: %scale-test --sum-multi --begin 5 --end 16 --step 5 --select NumDeclsValidated %s
 // REQUIRES: OS=macosx
 // REQUIRES: asserts
 

validation-test/compiler_scale/enum_indirect.gyb

@@ -0,0 +1,11 @@
+// RUN: %scale-test --sum-multi --begin 5 --end 16 --step 5 --select NumDeclsValidated %s
+// REQUIRES: OS=macosx
+// REQUIRES: asserts
+
+indirect enum Enum${N} {
+    case OK
+
+% if int(N) > 1:
+    case next(Enum${int(N)-1})
+% end
+}

validation-test/compiler_scale/enum_members.gyb

@@ -1,4 +1,4 @@
-// RUN: %scale-test --sum-multi --typecheck --begin 5 --end 16 --step 5 --select NumDeclsValidated %s
+// RUN: %scale-test --sum-multi --begin 5 --end 16 --step 5 --select NumDeclsValidated %s
 // REQUIRES: OS=macosx
 // REQUIRES: asserts
 

validation-test/compiler_scale/function_bodies.gyb

@@ -1,4 +1,4 @@
-// RUN: %scale-test --sum-multi --typecheck --begin 5 --end 16 --step 5 --select NumFunctionsParsed %s
+// RUN: %scale-test --sum-multi --begin 5 --end 16 --step 5 --select NumFunctionsParsed %s
 // REQUIRES: OS=macosx
 // REQUIRES: asserts
 
@@ -34,4 +34,4 @@ protocol P${N} {
 
 extension P${N} {
     func otherMethod${N}() {}
-}
+}

validation-test/compiler_scale/protocol_members.gyb

@@ -1,4 +1,4 @@
-// RUN: %scale-test --sum-multi --typecheck --begin 5 --end 16 --step 5 --select NumDeclsValidated %s
+// RUN: %scale-test --sum-multi --begin 5 --end 16 --step 5 --select NumDeclsValidated %s
 // REQUIRES: OS=macosx
 // REQUIRES: asserts
 

validation-test/compiler_scale/scale_neighbouring_getset.gyb

@@ -1,4 +1,4 @@
-// RUN: %scale-test --sum-multi --typecheck --begin 5 --end 16 --step 5 --select NumFunctionsTypechecked %s
+// RUN: %scale-test --sum-multi --begin 5 --end 16 --step 5 --select NumFunctionsTypechecked %s
 // REQUIRES: OS=macosx
 // REQUIRES: asserts
 

validation-test/compiler_scale/struct_members.gyb

@@ -1,4 +1,4 @@
-// RUN: %scale-test --sum-multi --typecheck --begin 5 --end 16 --step 5 --select NumDeclsValidated %s
+// RUN: %scale-test --sum-multi --begin 5 --end 16 --step 5 --select NumDeclsValidated %s
 // REQUIRES: OS=macosx
 // REQUIRES: asserts
 

validation-test/compiler_scale/struct_nested.gyb

@@ -0,0 +1,10 @@
+// RUN: %scale-test --sum-multi --begin 5 --end 16 --step 5 --select NumDeclsValidated %s
+// REQUIRES: OS=macosx
+// REQUIRES: asserts
+
+struct Struct${N} {
+% if int(N) > 1:
+    var next: Struct${int(N)-1}.Nested? = nil
+% end
+    struct Nested {}
+}

validation-test/compiler_scale/used_conformances.gyb

@@ -1,4 +1,4 @@
-// RUN: %scale-test --sum-multi --typecheck --begin 5 --end 16 --step 5 --select NumDeclsValidated %s
+// RUN: %scale-test --sum-multi --begin 5 --end 16 --step 5 --select NumDeclsValidated %s
 // REQUIRES: OS=macosx
 // REQUIRES: asserts