Skip to content

Commit 9b641f9

Browse files
tshortlitshortli
committed
ModuleInterface: Typecheck PatternBindingDecls lazily before printing.
Previously, fully qualified types would be missing for global vars and properties in `.swiftinterface` files that were emitted lazily. Adding the test case also revealed that PatternBindingDecls needed to be typechecked before lazy module serialization as well.
1 parent f78b60c commit 9b641f9

File tree

10 files changed

+137
-30
lines changed

10 files changed

+137
-30
lines changed

‎include/swift/AST/Decl.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2116,6 +2116,11 @@ class PatternBindingDecl final : public Decl,
21162116
return const_cast<PatternBindingDecl*>(this)->getMutablePatternList();
21172117
}
21182118

2119+
/// Returns the typechecked binding entry at the given index.
2120+
const PatternBindingEntry *
2121+
getCheckedPatternBindingEntry(unsigned i,
2122+
bool leaveClosureBodiesUnchecked = false) const;
2123+
21192124
/// Clean up walking the initializers for the pattern
21202125
class InitIterator {
21212126

‎lib/AST/ASTPrinter.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,18 @@ static bool isInObjCImpl(const ValueDecl *VD) {
142142
return ED && ED->isObjCImplementation();
143143
}
144144

145+
/// Triggering type checking requests while printing is desirable in compiler
146+
/// modes in which type checking is lazy and the printed content is expected to
147+
/// be complete (for example, when printing a .swiftinterface). In other
148+
/// contexts, though, triggering type checking could cause re-entrancy and
149+
/// should be avoided.
150+
static bool shouldTypeCheck(const PrintOptions &options) {
151+
if (options.IsForSwiftInterface)
152+
return true;
153+
154+
return false;
155+
}
156+
145157
PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *ModuleToPrint,
146158
bool preferTypeRepr,
147159
bool printFullConvention,
@@ -3799,6 +3811,11 @@ void PrintAST::visitPatternBindingDecl(PatternBindingDecl *decl) {
37993811
bool isFirst = true;
38003812
for (auto idx : range(decl->getNumPatternEntries())) {
38013813
auto *pattern = decl->getPattern(idx);
3814+
3815+
// Force the entry to be typechecked before attempting to print.
3816+
if (shouldTypeCheck(Options) && !pattern->hasType())
3817+
(void)decl->getCheckedPatternBindingEntry(idx);
3818+
38023819
if (!shouldPrintPattern(pattern))
38033820
continue;
38043821
if (isFirst)

‎lib/AST/Decl.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2075,6 +2075,15 @@ bool PatternBindingDecl::hasStorage() const {
20752075
return false;
20762076
}
20772077

2078+
const PatternBindingEntry *PatternBindingDecl::getCheckedPatternBindingEntry(
2079+
unsigned i, bool leaveClosureBodiesUnchecked) const {
2080+
return evaluateOrDefault(
2081+
getASTContext().evaluator,
2082+
PatternBindingEntryRequest{const_cast<PatternBindingDecl *>(this), i,
2083+
leaveClosureBodiesUnchecked},
2084+
nullptr);
2085+
}
2086+
20782087
void PatternBindingDecl::setPattern(unsigned i, Pattern *P,
20792088
DeclContext *InitContext,
20802089
bool isFullyValidated) {

‎lib/Sema/TypeCheckDecl.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2625,10 +2625,7 @@ NamingPatternRequest::evaluate(Evaluator &evaluator, VarDecl *VD) const {
26252625
// the naming pattern as a side effect in this case, and TypeCheckStmt
26262626
// and TypeCheckPattern handle the others. But that's all really gross.
26272627
unsigned i = PBD->getPatternEntryIndexForVarDecl(VD);
2628-
(void)evaluateOrDefault(evaluator,
2629-
PatternBindingEntryRequest{
2630-
PBD, i, /*LeaveClosureBodiesUnchecked=*/false},
2631-
nullptr);
2628+
(void)PBD->getCheckedPatternBindingEntry(i);
26322629
if (PBD->isInvalid()) {
26332630
VD->getParentPattern()->setType(ErrorType::get(Context));
26342631
setBoundVarsTypeError(VD->getParentPattern(), Context);

‎lib/Sema/TypeCheckDeclPrimary.cpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2300,13 +2300,10 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
23002300

23012301
auto &Ctx = getASTContext();
23022302
for (auto i : range(PBD->getNumPatternEntries())) {
2303-
const auto *entry =
2304-
PBD->isFullyValidated(i)
2305-
? &PBD->getPatternList()[i]
2306-
: evaluateOrDefault(Ctx.evaluator,
2307-
PatternBindingEntryRequest{
2308-
PBD, i, LeaveClosureBodiesUnchecked},
2309-
nullptr);
2303+
const auto *entry = PBD->isFullyValidated(i)
2304+
? &PBD->getPatternList()[i]
2305+
: PBD->getCheckedPatternBindingEntry(
2306+
i, LeaveClosureBodiesUnchecked);
23102307
assert(entry && "No pattern binding entry?");
23112308

23122309
const auto *Pat = PBD->getPattern(i);

‎lib/Serialization/Serialization.cpp

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3284,8 +3284,6 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
32843284
/// anything needed by a client as unsafe as the client will reject reading
32853285
/// it, but at the same time keep the safety checks precise to avoid
32863286
/// XRef errors and such.
3287-
///
3288-
/// \p decl should be either an \c ExtensionDecl or a \c ValueDecl.
32893287
static bool isDeserializationSafe(const Decl *decl) {
32903288
if (auto ext = dyn_cast<ExtensionDecl>(decl)) {
32913289
// Consider extensions as safe as their extended type.
@@ -3308,8 +3306,11 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
33083306
// We can mark the extension unsafe only if it has no public
33093307
// conformances.
33103308
auto protocols = ext->getLocalProtocols(ConformanceLookupKind::All);
3311-
bool hasSafeConformances = std::any_of(protocols.begin(), protocols.end(),
3312-
isDeserializationSafe);
3309+
bool hasSafeConformances = std::any_of(
3310+
protocols.begin(), protocols.end(), [](ProtocolDecl *protocol) {
3311+
return isDeserializationSafe(protocol);
3312+
});
3313+
33133314
if (hasSafeConformances)
33143315
return true;
33153316

@@ -3320,11 +3321,26 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
33203321
return false;
33213322
}
33223323

3323-
auto value = cast<ValueDecl>(decl);
3324+
if (auto pbd = dyn_cast<PatternBindingDecl>(decl)) {
3325+
// Pattern bindings are safe if any of their var decls are safe.
3326+
for (auto i : range(pbd->getNumPatternEntries())) {
3327+
if (auto *varDecl = pbd->getAnchoringVarDecl(i)) {
3328+
if (isDeserializationSafe(varDecl))
3329+
return true;
3330+
}
3331+
}
33243332

3333+
return false;
3334+
}
3335+
3336+
return isDeserializationSafe(cast<ValueDecl>(decl));
3337+
}
3338+
3339+
static bool isDeserializationSafe(const ValueDecl *value) {
33253340
// A decl is safe if formally accessible publicly.
3326-
auto accessScope = value->getFormalAccessScope(/*useDC=*/nullptr,
3327-
/*treatUsableFromInlineAsPublic=*/true);
3341+
auto accessScope =
3342+
value->getFormalAccessScope(/*useDC=*/nullptr,
3343+
/*treatUsableFromInlineAsPublic=*/true);
33283344
if (accessScope.isPublic() || accessScope.isPackage())
33293345
return true;
33303346

@@ -3351,7 +3367,7 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
33513367
}
33523368

33533369
// Paramters don't have meaningful access control.
3354-
if (isa<ParamDecl>(decl) || isa<GenericTypeParamDecl>(decl))
3370+
if (isa<ParamDecl>(value) || isa<GenericTypeParamDecl>(value))
33553371
return true;
33563372

33573373
return false;
@@ -3919,7 +3935,13 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
39193935
initContextIDs);
39203936

39213937
for (auto entryIdx : range(binding->getNumPatternEntries())) {
3922-
writePattern(binding->getPattern(entryIdx));
3938+
auto pattern = binding->getPattern(entryIdx);
3939+
3940+
// Force the entry to be typechecked before attempting to serialize.
3941+
if (!pattern->hasType())
3942+
(void)binding->getCheckedPatternBindingEntry(entryIdx);
3943+
3944+
writePattern(pattern);
39233945
// Ignore initializer; external clients don't need to know about it.
39243946
}
39253947
}

‎test/Inputs/lazy_typecheck.swift

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,15 @@ public func publicFuncWithOpaqueReturnType() -> some PublicProto { // expected-n
4747
}
4848
}
4949

50+
// MARK: - Global vars
51+
52+
public var publicGlobalVar: Int = 0
53+
public var publicGlobalVarInferredType = ""
54+
public var (publicGlobalVarInferredTuplePatX, publicGlobalVarInferredTuplePatY) = (0, 1)
55+
56+
var internalGlobalVar: DoesNotExist // expected-error {{cannot find type 'DoesNotExist' in scope}}
57+
var internalGlobalVarInferredType = DoesNotExist() // expected-error {{cannot find 'DoesNotExist' in scope}}
58+
5059
// MARK: - Nominal types
5160

5261
public protocol EmptyPublicProto {}
@@ -69,7 +78,8 @@ protocol InternalProtoConformingToPublicProto: PublicProto {
6978
}
7079

7180
public struct PublicStruct {
72-
// FIXME: Test properties
81+
public var publicProperty: Int
82+
public var publicPropertyInferredType = ""
7383

7484
public init(x: Int) {
7585
_ = DoesNotExist() // expected-error {{cannot find 'DoesNotExist' in scope}}
@@ -105,7 +115,8 @@ struct InternalStruct: DoesNotExist { // expected-error {{cannot find type 'Does
105115
}
106116

107117
public class PublicClass {
108-
// FIXME: Test properties
118+
public var publicProperty: Int
119+
public var publicPropertyInferredType = ""
109120

110121
public init(x: Int) {
111122
_ = DoesNotExist() // expected-error {{cannot find 'DoesNotExist' in scope}}
@@ -188,4 +199,3 @@ extension PublicGenericStruct where T == InternalStructForConstraint {}
188199
extension PublicGenericStruct: EmptyPublicProto where T == InternalStructForConstraint {}
189200

190201
// FIXME: Test enums
191-
// FIXME: Test global vars

‎test/Inputs/lazy_typecheck_client.swift

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,31 @@ func testGlobalFunctions() {
2020
}
2121
}
2222

23+
func testGobalVars() {
24+
let _: Int = publicGlobalVar
25+
let _: String = publicGlobalVarInferredType
26+
let _: (Int, Int) = (publicGlobalVarInferredTuplePatX, publicGlobalVarInferredTuplePatY)
27+
}
28+
2329
func testPublicStruct() {
2430
let s = PublicStruct(x: 1)
25-
_ = s.publicMethod()
31+
let _: Int = s.publicMethod()
32+
let _: Int = s.publicProperty
33+
let _: String = s.publicPropertyInferredType
2634
PublicStruct.publicStaticMethod()
2735
}
2836

2937
func testPublicClass() {
3038
let c = PublicClass(x: 2)
31-
_ = c.publicMethod()
39+
let _: Int = c.publicMethod()
40+
let _: Int = c.publicProperty
41+
let _: String = c.publicPropertyInferredType
3242
PublicClass.publicClassMethod()
3343

3444
let d = PublicDerivedClass(x: 3)
35-
_ = d.publicMethod()
45+
let _: Int = d.publicMethod()
46+
let _: Int = d.publicProperty
47+
let _: String = d.publicPropertyInferredType
3648
PublicDerivedClass.publicClassMethod()
3749
}
3850

‎test/ModuleInterface/lazy-typecheck.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@
2626
// CHECK-NEXT: }
2727
// CHECK-NEXT: }
2828

29+
// CHECK: public var publicGlobalVar: Swift.Int
30+
// CHECK: public var publicGlobalVarInferredType: Swift.String
31+
// CHECK: public var (publicGlobalVarInferredTuplePatX, publicGlobalVarInferredTuplePatY): (Swift.Int, Swift.Int)
2932
// CHECK: public protocol EmptyPublicProto {
3033
// CHECK: }
3134
// CHECK: public protocol PublicProto {
@@ -37,6 +40,8 @@
3740
// CHECK: }
3841
// CHECK: #endif
3942
// CHECK: public struct PublicStruct {
43+
// CHECK: public var publicProperty: Swift.Int
44+
// CHECK: public var publicPropertyInferredType: Swift.String
4045
// CHECK: public init(x: Swift.Int)
4146
// CHECK: public func publicMethod() -> Swift.Int
4247
// CHECK: public static func publicStaticMethod()
@@ -45,6 +50,8 @@
4550
// CHECK: public func publicMethod() -> T
4651
// CHECK: }
4752
// CHECK: public class PublicClass {
53+
// CHECK: public var publicProperty: Swift.Int
54+
// CHECK: public var publicPropertyInferredType: Swift.String
4855
// CHECK: public init(x: Swift.Int)
4956
// CHECK: public func publicMethod() -> Swift.Int
5057
// CHECK: public class func publicClassMethod()

‎test/TBD/lazy-typecheck.swift

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,19 +59,42 @@ exports:
5959
'_$s14lazy_typecheck10publicFuncSiyF', '_$s14lazy_typecheck11PublicClassC06publicD6MethodyyFZTj',
6060
'_$s14lazy_typecheck11PublicClassC06publicD6MethodyyFZTq',
6161
'_$s14lazy_typecheck11PublicClassC12publicMethodSiyFTj', '_$s14lazy_typecheck11PublicClassC12publicMethodSiyFTq',
62+
'_$s14lazy_typecheck11PublicClassC14publicPropertySivMTj',
63+
'_$s14lazy_typecheck11PublicClassC14publicPropertySivMTq',
64+
'_$s14lazy_typecheck11PublicClassC14publicPropertySivgTj',
65+
'_$s14lazy_typecheck11PublicClassC14publicPropertySivgTq',
66+
'_$s14lazy_typecheck11PublicClassC14publicPropertySivpMV',
67+
'_$s14lazy_typecheck11PublicClassC14publicPropertySivsTj',
68+
'_$s14lazy_typecheck11PublicClassC14publicPropertySivsTq',
6269
'_$s14lazy_typecheck11PublicClassC1xACSi_tcfC', '_$s14lazy_typecheck11PublicClassC1xACSi_tcfCTj',
6370
'_$s14lazy_typecheck11PublicClassC1xACSi_tcfCTq', '_$s14lazy_typecheck11PublicClassC1xACSi_tcfc',
71+
'_$s14lazy_typecheck11PublicClassC26publicPropertyInferredTypeSSvMTj',
72+
'_$s14lazy_typecheck11PublicClassC26publicPropertyInferredTypeSSvMTq',
73+
'_$s14lazy_typecheck11PublicClassC26publicPropertyInferredTypeSSvgTj',
74+
'_$s14lazy_typecheck11PublicClassC26publicPropertyInferredTypeSSvgTq',
75+
'_$s14lazy_typecheck11PublicClassC26publicPropertyInferredTypeSSvpMV',
76+
'_$s14lazy_typecheck11PublicClassC26publicPropertyInferredTypeSSvsTj',
77+
'_$s14lazy_typecheck11PublicClassC26publicPropertyInferredTypeSSvsTq',
6478
'_$s14lazy_typecheck11PublicClassCMa', '_$s14lazy_typecheck11PublicClassCMm',
6579
'_$s14lazy_typecheck11PublicClassCMn', '_$s14lazy_typecheck11PublicClassCMo',
6680
'_$s14lazy_typecheck11PublicClassCMu', '_$s14lazy_typecheck11PublicClassCN',
6781
'_$s14lazy_typecheck11PublicClassCfD', '_$s14lazy_typecheck11PublicClassCfd',
6882
'_$s14lazy_typecheck11PublicProtoMp', '_$s14lazy_typecheck11PublicProtoP3reqSiyFTj',
6983
'_$s14lazy_typecheck11PublicProtoP3reqSiyFTq', '_$s14lazy_typecheck11PublicProtoTL',
7084
'_$s14lazy_typecheck11packageFuncSiyF', '_$s14lazy_typecheck12PublicStructV12publicMethodSiyF',
85+
'_$s14lazy_typecheck12PublicStructV14publicPropertySivM',
86+
'_$s14lazy_typecheck12PublicStructV14publicPropertySivg',
87+
'_$s14lazy_typecheck12PublicStructV14publicPropertySivpMV',
88+
'_$s14lazy_typecheck12PublicStructV14publicPropertySivs',
7189
'_$s14lazy_typecheck12PublicStructV18publicStaticMethodyyFZ',
72-
'_$s14lazy_typecheck12PublicStructV1xACSi_tcfC', '_$s14lazy_typecheck12PublicStructVMa',
73-
'_$s14lazy_typecheck12PublicStructVMn', '_$s14lazy_typecheck12PublicStructVN',
74-
'_$s14lazy_typecheck13inlinableFuncSiyF', '_$s14lazy_typecheck16EmptyPublicProtoMp',
90+
'_$s14lazy_typecheck12PublicStructV1xACSi_tcfC', '_$s14lazy_typecheck12PublicStructV26publicPropertyInferredTypeSSvM',
91+
'_$s14lazy_typecheck12PublicStructV26publicPropertyInferredTypeSSvg',
92+
'_$s14lazy_typecheck12PublicStructV26publicPropertyInferredTypeSSvpMV',
93+
'_$s14lazy_typecheck12PublicStructV26publicPropertyInferredTypeSSvs',
94+
'_$s14lazy_typecheck12PublicStructVMa', '_$s14lazy_typecheck12PublicStructVMn',
95+
'_$s14lazy_typecheck12PublicStructVN', '_$s14lazy_typecheck13inlinableFuncSiyF',
96+
'_$s14lazy_typecheck15publicGlobalVarSivM', '_$s14lazy_typecheck15publicGlobalVarSivg',
97+
'_$s14lazy_typecheck15publicGlobalVarSivs', '_$s14lazy_typecheck16EmptyPublicProtoMp',
7598
'_$s14lazy_typecheck16EmptyPublicProtoTL', '_$s14lazy_typecheck18PublicDerivedClassC1xACSi_tcfC',
7699
'_$s14lazy_typecheck18PublicDerivedClassC1xACSi_tcfc', '_$s14lazy_typecheck18PublicDerivedClassCMa',
77100
'_$s14lazy_typecheck18PublicDerivedClassCMm', '_$s14lazy_typecheck18PublicDerivedClassCMn',
@@ -83,9 +106,17 @@ exports:
83106
'_$s14lazy_typecheck19PublicGenericStructVyxGAA05EmptyC5ProtoA2A08InternalE13ForConstraintVRszlWP',
84107
'_$s14lazy_typecheck19PublicRethrowsProtoMp', '_$s14lazy_typecheck19PublicRethrowsProtoP3reqSiyKFTj',
85108
'_$s14lazy_typecheck19PublicRethrowsProtoP3reqSiyKFTq', '_$s14lazy_typecheck19PublicRethrowsProtoTL',
86-
'_$s14lazy_typecheck24publicFuncWithDefaultArgyS2iF', '_$s14lazy_typecheck30publicFuncWithOpaqueReturnTypeQryF',
109+
'_$s14lazy_typecheck24publicFuncWithDefaultArgyS2iF', '_$s14lazy_typecheck27publicGlobalVarInferredTypeSSvM',
110+
'_$s14lazy_typecheck27publicGlobalVarInferredTypeSSvg', '_$s14lazy_typecheck27publicGlobalVarInferredTypeSSvs',
111+
'_$s14lazy_typecheck30publicFuncWithOpaqueReturnTypeQryF',
87112
'_$s14lazy_typecheck30publicFuncWithOpaqueReturnTypeQryFQOMQ',
88113
'_$s14lazy_typecheck32constrainedGenericPublicFunctionyyxAA0E5ProtoRzlF',
114+
'_$s14lazy_typecheck32publicGlobalVarInferredTuplePatXSivM',
115+
'_$s14lazy_typecheck32publicGlobalVarInferredTuplePatXSivg',
116+
'_$s14lazy_typecheck32publicGlobalVarInferredTuplePatXSivs',
117+
'_$s14lazy_typecheck32publicGlobalVarInferredTuplePatYSivM',
118+
'_$s14lazy_typecheck32publicGlobalVarInferredTuplePatYSivg',
119+
'_$s14lazy_typecheck32publicGlobalVarInferredTuplePatYSivs',
89120
'_$sSS14lazy_typecheck11PublicProtoAAMc', '_$sSS14lazy_typecheck11PublicProtoAAWP',
90121
'_$sSS14lazy_typecheckE3reqSiyF', '_$sSi14lazy_typecheck19PublicRethrowsProtoAAMc',
91122
'_$sSi14lazy_typecheck19PublicRethrowsProtoAAWP', '_$sSi14lazy_typecheckE3reqSiyKF' ]

0 commit comments

Comments
 (0)