Skip to content

Struct and enum declarations no longer go through finalizeDecl() #26159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions lib/IRGen/GenEnum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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);
Expand Down
8 changes: 1 addition & 7 deletions lib/SIL/TypeLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are error types wandering into SIL type lowering?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type checking is now sufficiently lazy that if a struct has a field with an invalid type, and you don't directly reference the invalid field from source, SILGen might end up lazily validating the stored properties of the struct anyway, if you nevertheless emit an expression involving a value of the struct's type.

return asImpl().handleTrivial(type);
}

// Dependent types should be contextualized before visiting.
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion lib/Sema/CodeSynthesis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2012,7 +2012,6 @@ void swift::triggerAccessorSynthesis(TypeChecker &TC,
if (!accessor->hasBody()) {
maybeMarkTransparent(accessor, TC.Context);
accessor->setBodySynthesizer(&synthesizeAccessorBody);
TC.DeclsToFinalize.insert(accessor);
}
});
}
Expand Down
64 changes: 14 additions & 50 deletions lib/Sema/TypeCheckDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand All @@ -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.
//
Expand Down Expand Up @@ -3913,9 +3914,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
checkEnumRawValues(*this, ED);
}

if (!isa<ClassDecl>(nominal))
requestNominalLayout(nominal);

break;
}

Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)) {
Expand Down
3 changes: 2 additions & 1 deletion lib/Sema/TypeCheckProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() &&
Expand Down
4 changes: 2 additions & 2 deletions test/decl/class/override.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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}}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
struct S {
var a = S() // expected-error {{'S' cannot be constructed because it has no accessible initializers}}
var a = S()
}
9 changes: 4 additions & 5 deletions test/decl/init/basic_init.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 10 additions & 0 deletions test/multifile/Inputs/require-layout-enum-other.swift
Original file line number Diff line number Diff line change
@@ -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)
}
3 changes: 3 additions & 0 deletions test/multifile/Inputs/require-layout-error-other.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
public struct S {
var x: DoesNotExist // expected-error {{use of undeclared type 'DoesNotExist'}}
}
3 changes: 3 additions & 0 deletions test/multifile/require-layout-enum.swift
Original file line number Diff line number Diff line change
@@ -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 }
3 changes: 3 additions & 0 deletions test/multifile/require-layout-error.swift
Original file line number Diff line number Diff line change
@@ -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 }
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: not %target-swift-frontend %s -emit-ir
// RUN: %target-swift-frontend %s -emit-ir

struct DefaultAssociatedType {
}
Expand Down
14 changes: 14 additions & 0 deletions validation-test/compiler_crashers_2_fixed/0199-sr9583.swift
Original file line number Diff line number Diff line change
@@ -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>) { }
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
struct S2 {
let v: Int
}

struct S3: P {
typealias A = S2
}
2 changes: 1 addition & 1 deletion validation-test/compiler_scale/bind_extension_decl.gyb
Original file line number Diff line number Diff line change
@@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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

Expand Down
2 changes: 1 addition & 1 deletion validation-test/compiler_scale/class_members.gyb
Original file line number Diff line number Diff line change
@@ -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

Expand Down
11 changes: 11 additions & 0 deletions validation-test/compiler_scale/enum_indirect.gyb
Original file line number Diff line number Diff line change
@@ -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
}
2 changes: 1 addition & 1 deletion validation-test/compiler_scale/enum_members.gyb
Original file line number Diff line number Diff line change
@@ -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

Expand Down
4 changes: 2 additions & 2 deletions validation-test/compiler_scale/function_bodies.gyb
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -34,4 +34,4 @@ protocol P${N} {

extension P${N} {
func otherMethod${N}() {}
}
}
2 changes: 1 addition & 1 deletion validation-test/compiler_scale/protocol_members.gyb
Original file line number Diff line number Diff line change
@@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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

Expand Down
2 changes: 1 addition & 1 deletion validation-test/compiler_scale/struct_members.gyb
Original file line number Diff line number Diff line change
@@ -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

Expand Down
10 changes: 10 additions & 0 deletions validation-test/compiler_scale/struct_nested.gyb
Original file line number Diff line number Diff line change
@@ -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 {}
}
2 changes: 1 addition & 1 deletion validation-test/compiler_scale/used_conformances.gyb
Original file line number Diff line number Diff line change
@@ -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

Expand Down