Skip to content

Commit db1fc95

Browse files
jrose-applejrose-apple
committed
[Serialization] Drop subscripts and initializers with problem types.
Finishes up the "big four" non-type decl kinds. Unfortunately, indiscriminately dropping members from a class affects the layout of its vtable. That issue is tracked by rdar://problem/31878396.
1 parent 59a2545 commit db1fc95

File tree

3 files changed

+106
-15
lines changed

3 files changed

+106
-15
lines changed

‎lib/Serialization/Deserialization.cpp

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2525,13 +2525,20 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
25252525
Optional<swift::CtorInitializerKind> initKind =
25262526
getActualCtorInitializerKind(storedInitKind);
25272527

2528+
auto errorKind = DeclDeserializationError::Normal;
2529+
if (initKind == CtorInitializerKind::Designated)
2530+
errorKind = DeclDeserializationError::DesignatedInitializer;
2531+
25282532
auto overridden = getDeclChecked(overriddenID);
25292533
if (!overridden) {
25302534
llvm::consumeError(overridden.takeError());
2531-
auto kind = OverrideError::Normal;
2532-
if (initKind == CtorInitializerKind::Designated)
2533-
kind = OverrideError::DesignatedInitializer;
2534-
return llvm::make_error<OverrideError>(name, kind);
2535+
return llvm::make_error<OverrideError>(name, errorKind);
2536+
}
2537+
2538+
auto canonicalType = getTypeChecked(canonicalTypeID);
2539+
if (!canonicalType) {
2540+
return llvm::make_error<TypeError>(
2541+
name, takeErrorInfo(canonicalType.takeError()), errorKind);
25352542
}
25362543

25372544
auto parent = getDeclContext(contextID);
@@ -2629,8 +2636,10 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
26292636
}
26302637

26312638
auto canonicalType = getTypeChecked(canonicalTypeID);
2632-
if (!canonicalType)
2633-
return llvm::make_error<TypeError>(name, takeErrorInfo(canonicalType.takeError()));
2639+
if (!canonicalType) {
2640+
return llvm::make_error<TypeError>(
2641+
name, takeErrorInfo(canonicalType.takeError()));
2642+
}
26342643

26352644
auto DC = ForcedContext ? *ForcedContext : getDeclContext(contextID);
26362645
if (declOrOffset.isComplete())
@@ -2757,13 +2766,21 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
27572766
}
27582767

27592768
auto canonicalType = getTypeChecked(canonicalTypeID);
2760-
if (!canonicalType)
2761-
return llvm::make_error<TypeError>(name, takeErrorInfo(canonicalType.takeError()));
2769+
if (!canonicalType) {
2770+
return llvm::make_error<TypeError>(
2771+
name, takeErrorInfo(canonicalType.takeError()));
2772+
}
27622773

27632774
auto DC = getDeclContext(contextID);
27642775
if (declOrOffset.isComplete())
27652776
return declOrOffset;
27662777

2778+
// If we are an accessor on a var or subscript, make sure it is deserialized
2779+
// first.
2780+
auto accessor = getDeclChecked(accessorStorageDeclID);
2781+
if (!accessor)
2782+
return accessor.takeError();
2783+
27672784
// Read generic params before reading the type, because the type may
27682785
// reference generic parameters, and we want them to have a dummy
27692786
// DeclContext for now.
@@ -2844,10 +2861,6 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
28442861
fn->setImplicit();
28452862
fn->setMutating(isMutating);
28462863
fn->setDynamicSelf(hasDynamicSelf);
2847-
2848-
// If we are an accessor on a var or subscript, make sure it is deserialized
2849-
// too.
2850-
getDecl(accessorStorageDeclID);
28512864
break;
28522865
}
28532866

@@ -3289,6 +3302,12 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
32893302
return llvm::make_error<OverrideError>(name);
32903303
}
32913304

3305+
auto canonicalType = getTypeChecked(canonicalTypeID);
3306+
if (!canonicalType) {
3307+
return llvm::make_error<TypeError>(
3308+
name, takeErrorInfo(canonicalType.takeError()));
3309+
}
3310+
32923311
auto parent = getDeclContext(contextID);
32933312
if (declOrOffset.isComplete())
32943313
return declOrOffset;
@@ -4342,6 +4361,9 @@ void ModuleFile::loadAllMembers(Decl *container, uint64_t contextData) {
43424361
fatal(next.takeError());
43434362

43444363
// Drop the member if it had a problem.
4364+
// FIXME: If this was a non-final, non-dynamic, non-@objc-overriding
4365+
// member, it's going to affect the vtable layout, and /every single call/
4366+
// will be wrong.
43454367
if (auto *containingClass = dyn_cast<ClassDecl>(container)) {
43464368
auto handleMissingDesignatedInit =
43474369
[containingClass](const DeclDeserializationError &error) {

‎lib/Serialization/DeserializationErrors.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,8 +256,11 @@ class TypeError : public llvm::ErrorInfo<TypeError, DeclDeserializationError> {
256256
DeclName name;
257257
std::unique_ptr<ErrorInfoBase> underlyingReason;
258258
public:
259-
explicit TypeError(DeclName name, std::unique_ptr<ErrorInfoBase> reason)
260-
: name(name), underlyingReason(std::move(reason)) {}
259+
explicit TypeError(DeclName name, std::unique_ptr<ErrorInfoBase> reason,
260+
Kind kind = Normal)
261+
: name(name), underlyingReason(std::move(reason)) {
262+
this->kind = kind;
263+
}
261264

262265
void log(raw_ostream &OS) const override {
263266
OS << "could not deserialize type for '" << name << "'";

‎test/Serialization/Recovery/typedefs.swift

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,78 @@ let _ = unwrapped // okay
3939
_ = usesWrapped(nil) // expected-error {{use of unresolved identifier 'usesWrapped'}}
4040
_ = usesUnwrapped(nil) // expected-error {{nil is not compatible with expected argument type 'Int32'}}
4141

42+
public class UserSub: User {
43+
override init() {}
44+
}
45+
// FIXME: Bad error message; really it's that the convenience init hasn't been
46+
// inherited.
47+
_ = UserSub(conveniently: 0) // expected-error {{argument passed to call that takes no arguments}}
48+
49+
public class UserConvenienceSub: UserConvenience {
50+
override init() {}
51+
}
52+
_ = UserConvenienceSub(conveniently: 0)
53+
4254
#endif // VERIFY
4355

4456
#else // TEST
4557

4658
import Typedefs
4759

60+
// CHECK-LABEL: class User {
61+
// CHECK-RECOVERY-LABEL: class User {
62+
open class User {
63+
// CHECK: var unwrappedProp: UnwrappedInt?
64+
// CHECK-RECOVERY: var unwrappedProp: Int32?
65+
public var unwrappedProp: UnwrappedInt?
66+
// CHECK: var wrappedProp: WrappedInt?
67+
// CHECK-RECOVERY-NEGATIVE-NOT: var wrappedProp:
68+
public var wrappedProp: WrappedInt?
69+
70+
// CHECK: func returnsUnwrappedMethod() -> UnwrappedInt
71+
// CHECK-RECOVERY: func returnsUnwrappedMethod() -> Int32
72+
public func returnsUnwrappedMethod() -> UnwrappedInt { fatalError() }
73+
// CHECK: func returnsWrappedMethod() -> WrappedInt
74+
// CHECK-RECOVERY-NEGATIVE-NOT: func returnsWrappedMethod(
75+
public func returnsWrappedMethod() -> WrappedInt { fatalError() }
76+
77+
// CHECK: subscript(_: WrappedInt) -> () { get }
78+
// CHECK-RECOVERY-NEGATIVE-NOT: subscript(
79+
public subscript(_: WrappedInt) -> () { return () }
80+
81+
// CHECK: init()
82+
// CHECK-RECOVERY: init()
83+
public init() {}
84+
85+
// CHECK: init(wrapped: WrappedInt)
86+
// CHECK-RECOVERY-NEGATIVE-NOT: init(wrapped:
87+
public init(wrapped: WrappedInt) {}
88+
89+
// CHECK: convenience init(conveniently: Int)
90+
// CHECK-RECOVERY: convenience init(conveniently: Int)
91+
public convenience init(conveniently: Int) { self.init() }
92+
}
93+
// CHECK: {{^}$}}
94+
// CHECK-RECOVERY: {{^}$}}
95+
96+
// CHECK-LABEL: class UserConvenience
97+
// CHECK-RECOVERY-LABEL: class UserConvenience
98+
open class UserConvenience {
99+
// CHECK: init()
100+
// CHECK-RECOVERY: init()
101+
public init() {}
102+
103+
// CHECK: convenience init(wrapped: WrappedInt)
104+
// CHECK-RECOVERY-NEGATIVE-NOT: init(wrapped:
105+
public convenience init(wrapped: WrappedInt) { self.init() }
106+
107+
// CHECK: convenience init(conveniently: Int)
108+
// CHECK-RECOVERY: convenience init(conveniently: Int)
109+
public convenience init(conveniently: Int) { self.init() }
110+
}
111+
// CHECK: {{^}$}}
112+
// CHECK-RECOVERY: {{^}$}}
113+
48114
// CHECK-DAG: let x: MysteryTypedef
49115
// CHECK-RECOVERY-DAG: let x: Int32
50116
public let x: MysteryTypedef = 0
@@ -139,7 +205,7 @@ public func usesUnwrapped(_ unwrapped: UnwrappedInt) {}
139205
public func returnsWrapped() -> WrappedInt { fatalError() }
140206

141207
// CHECK-DAG: func returnsWrappedGeneric<T>(_: T.Type) -> WrappedInt
142-
// CHECK-RECOVERY-NEGATIVE-NOT: func returnsWrappedGeneric
208+
// CHECK-RECOVERY-NEGATIVE-NOT: func returnsWrappedGeneric<
143209
public func returnsWrappedGeneric<T>(_: T.Type) -> WrappedInt { fatalError() }
144210

145211
#endif // TEST

0 commit comments

Comments
 (0)