Skip to content

Commit cf809ce

Browse files
authored
Merge pull request #68452 from tshortli/lazy-typecheck-vars
ModuleInterface: Fix PatternBindingDecl printing in lazy mode
2 parents d971f12 + 5e76f76 commit cf809ce

File tree

10 files changed

+196
-37
lines changed

10 files changed

+196
-37
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: 24 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,
@@ -2258,6 +2270,13 @@ void PrintAST::printAccessors(const AbstractStorageDecl *ASD) {
22582270
return;
22592271
}
22602272

2273+
// Force implicit accessors to be created if they haven't been already.
2274+
if (shouldTypeCheck(Options)) {
2275+
ASD->visitEmittedAccessors([](AccessorDecl *accessor) {
2276+
(void)accessor;
2277+
});
2278+
}
2279+
22612280
// Collect the accessor declarations that we should print.
22622281
SmallVector<AccessorDecl *, 4> accessorsToPrint;
22632282
auto AddAccessorToPrint = [&](AccessorKind kind) {
@@ -3799,6 +3818,11 @@ void PrintAST::visitPatternBindingDecl(PatternBindingDecl *decl) {
37993818
bool isFirst = true;
38003819
for (auto idx : range(decl->getNumPatternEntries())) {
38013820
auto *pattern = decl->getPattern(idx);
3821+
3822+
// Force the entry to be typechecked before attempting to print.
3823+
if (shouldTypeCheck(Options) && !pattern->hasType())
3824+
(void)decl->getCheckedPatternBindingEntry(idx);
3825+
38023826
if (!shouldPrintPattern(pattern))
38033827
continue;
38043828
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: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,38 @@ public func publicFuncWithOpaqueReturnType() -> some PublicProto { // expected-n
4747
}
4848
}
4949

50+
// MARK: - Property wrappers
51+
52+
@propertyWrapper
53+
public struct PublicWrapper<T> {
54+
public var wrappedValue: T {
55+
get {
56+
_ = DoesNotExist() // expected-error {{cannot find 'DoesNotExist' in scope}}
57+
}
58+
set {
59+
_ = DoesNotExist() // expected-error {{cannot find 'DoesNotExist' in scope}}
60+
}
61+
}
62+
63+
public var projectedValue: PublicWrapper { self }
64+
65+
public init(wrappedValue value: T) {
66+
_ = DoesNotExist() // expected-error {{cannot find 'DoesNotExist' in scope}}
67+
}
68+
}
69+
70+
@propertyWrapper
71+
struct InternalWrapper<T> {} // expected-error {{property wrapper type 'InternalWrapper' does not contain a non-static property named 'wrappedValue'}}
72+
73+
// MARK: - Global vars
74+
75+
public var publicGlobalVar: Int = 0
76+
public var publicGlobalVarInferredType = ""
77+
public var (publicGlobalVarInferredTuplePatX, publicGlobalVarInferredTuplePatY) = (0, 1)
78+
79+
var internalGlobalVar: DoesNotExist // expected-error {{cannot find type 'DoesNotExist' in scope}}
80+
var internalGlobalVarInferredType = DoesNotExist() // expected-error {{cannot find 'DoesNotExist' in scope}}
81+
5082
// MARK: - Nominal types
5183

5284
public protocol EmptyPublicProto {}
@@ -69,7 +101,9 @@ protocol InternalProtoConformingToPublicProto: PublicProto {
69101
}
70102

71103
public struct PublicStruct {
72-
// FIXME: Test properties
104+
public var publicProperty: Int
105+
public var publicPropertyInferredType = ""
106+
@PublicWrapper public var publicWrappedProperty = 3.14
73107

74108
public init(x: Int) {
75109
_ = DoesNotExist() // expected-error {{cannot find 'DoesNotExist' in scope}}
@@ -105,7 +139,8 @@ struct InternalStruct: DoesNotExist { // expected-error {{cannot find type 'Does
105139
}
106140

107141
public class PublicClass {
108-
// FIXME: Test properties
142+
public var publicProperty: Int
143+
public var publicPropertyInferredType = ""
109144

110145
public init(x: Int) {
111146
_ = DoesNotExist() // expected-error {{cannot find 'DoesNotExist' in scope}}
@@ -188,4 +223,3 @@ extension PublicGenericStruct where T == InternalStructForConstraint {}
188223
extension PublicGenericStruct: EmptyPublicProto where T == InternalStructForConstraint {}
189224

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

test/Inputs/lazy_typecheck_client.swift

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,33 @@ 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
34+
let _: Double = s.publicWrappedProperty
35+
let _: Double = s.$publicWrappedProperty.wrappedValue
2636
PublicStruct.publicStaticMethod()
2737
}
2838

2939
func testPublicClass() {
3040
let c = PublicClass(x: 2)
31-
_ = c.publicMethod()
41+
let _: Int = c.publicMethod()
42+
let _: Int = c.publicProperty
43+
let _: String = c.publicPropertyInferredType
3244
PublicClass.publicClassMethod()
3345

3446
let d = PublicDerivedClass(x: 3)
35-
_ = d.publicMethod()
47+
let _: Int = d.publicMethod()
48+
let _: Int = d.publicProperty
49+
let _: String = d.publicPropertyInferredType
3650
PublicDerivedClass.publicClassMethod()
3751
}
3852

test/ModuleInterface/lazy-typecheck.swift

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// RUN: %empty-directory(%t)
22

3-
// RUN: %target-swift-frontend -swift-version 5 %S/../Inputs/lazy_typecheck.swift -module-name lazy_typecheck -typecheck -emit-module-interface-path %t/lazy_typecheck.swiftinterface -enable-library-evolution -parse-as-library -package-name Package -experimental-lazy-typecheck -experimental-skip-all-function-bodies -experimental-serialize-external-decls-only
3+
// RUN: %target-swift-frontend -swift-version 5 %S/../Inputs/lazy_typecheck.swift -module-name lazy_typecheck -typecheck -emit-module-interface-path %t/lazy_typecheck.swiftinterface -enable-library-evolution -parse-as-library -package-name Package -experimental-lazy-typecheck
44
// RUN: %FileCheck %s < %t/lazy_typecheck.swiftinterface
55

66
// RUN: rm -rf %t/*.swiftmodule
@@ -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,16 @@
3740
// CHECK: }
3841
// CHECK: #endif
3942
// CHECK: public struct PublicStruct {
43+
// CHECK: public var publicProperty: Swift.Int
44+
// CHECK: public var publicPropertyInferredType: Swift.String
45+
// CHECK: @lazy_typecheck.PublicWrapper @_projectedValueProperty($publicWrappedProperty) public var publicWrappedProperty: Swift.Double {
46+
// CHECK-NEXT: get
47+
// CHECK-NEXT: set
48+
// CHECK-NEXT: _modify
49+
// CHECK-NEXT: }
50+
// CHECK: public var $publicWrappedProperty: lazy_typecheck.PublicWrapper<Swift.Double> {
51+
// CHECK-NEXT: get
52+
// CHECK-NEXT: }
4053
// CHECK: public init(x: Swift.Int)
4154
// CHECK: public func publicMethod() -> Swift.Int
4255
// CHECK: public static func publicStaticMethod()
@@ -45,6 +58,8 @@
4558
// CHECK: public func publicMethod() -> T
4659
// CHECK: }
4760
// CHECK: public class PublicClass {
61+
// CHECK: public var publicProperty: Swift.Int
62+
// CHECK: public var publicPropertyInferredType: Swift.String
4863
// CHECK: public init(x: Swift.Int)
4964
// CHECK: public func publicMethod() -> Swift.Int
5065
// CHECK: public class func publicClassMethod()

0 commit comments

Comments
 (0)