ModuleInterface: Typecheck PatternBindingDecls lazily before printing.

tshortli · tshortli · commit 9b641f9ba34d · 2023-09-12T09:08:10.000-07:00
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.
diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h
@@ -2116,6 +2116,11 @@ class PatternBindingDecl final : public Decl,
     return const_cast<PatternBindingDecl*>(this)->getMutablePatternList();
   }
 
+  /// Returns the typechecked binding entry at the given index.
+  const PatternBindingEntry *
+  getCheckedPatternBindingEntry(unsigned i,
+                                bool leaveClosureBodiesUnchecked = false) const;
+
   /// Clean up walking the initializers for the pattern
   class InitIterator {
 
diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp
@@ -142,6 +142,18 @@ static bool isInObjCImpl(const ValueDecl *VD) {
   return ED && ED->isObjCImplementation();
 }
 
+/// Triggering type checking requests while printing is desirable in compiler
+/// modes in which type checking is lazy and the printed content is expected to
+/// be complete (for example, when printing a .swiftinterface). In other
+/// contexts, though, triggering type checking could cause re-entrancy and
+/// should be avoided.
+static bool shouldTypeCheck(const PrintOptions &options) {
+  if (options.IsForSwiftInterface)
+    return true;
+
+  return false;
+}
+
 PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *ModuleToPrint,
                                                    bool preferTypeRepr,
                                                    bool printFullConvention,
@@ -3799,6 +3811,11 @@ void PrintAST::visitPatternBindingDecl(PatternBindingDecl *decl) {
   bool isFirst = true;
   for (auto idx : range(decl->getNumPatternEntries())) {
     auto *pattern = decl->getPattern(idx);
+
+    // Force the entry to be typechecked before attempting to print.
+    if (shouldTypeCheck(Options) && !pattern->hasType())
+      (void)decl->getCheckedPatternBindingEntry(idx);
+
     if (!shouldPrintPattern(pattern))
       continue;
     if (isFirst)
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -2075,6 +2075,15 @@ bool PatternBindingDecl::hasStorage() const {
   return false;
 }
 
+const PatternBindingEntry *PatternBindingDecl::getCheckedPatternBindingEntry(
+    unsigned i, bool leaveClosureBodiesUnchecked) const {
+  return evaluateOrDefault(
+      getASTContext().evaluator,
+      PatternBindingEntryRequest{const_cast<PatternBindingDecl *>(this), i,
+                                 leaveClosureBodiesUnchecked},
+      nullptr);
+}
+
 void PatternBindingDecl::setPattern(unsigned i, Pattern *P,
                                     DeclContext *InitContext,
                                     bool isFullyValidated) {
diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp
@@ -2625,10 +2625,7 @@ NamingPatternRequest::evaluate(Evaluator &evaluator, VarDecl *VD) const {
     // the naming pattern as a side effect in this case, and TypeCheckStmt
     // and TypeCheckPattern handle the others. But that's all really gross.
     unsigned i = PBD->getPatternEntryIndexForVarDecl(VD);
-    (void)evaluateOrDefault(evaluator,
-                            PatternBindingEntryRequest{
-                                PBD, i, /*LeaveClosureBodiesUnchecked=*/false},
-                            nullptr);
+    (void)PBD->getCheckedPatternBindingEntry(i);
     if (PBD->isInvalid()) {
       VD->getParentPattern()->setType(ErrorType::get(Context));
       setBoundVarsTypeError(VD->getParentPattern(), Context);
diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp
@@ -2300,13 +2300,10 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
 
     auto &Ctx = getASTContext();
     for (auto i : range(PBD->getNumPatternEntries())) {
-      const auto *entry =
-          PBD->isFullyValidated(i)
-              ? &PBD->getPatternList()[i]
-              : evaluateOrDefault(Ctx.evaluator,
-                                  PatternBindingEntryRequest{
-                                      PBD, i, LeaveClosureBodiesUnchecked},
-                                  nullptr);
+      const auto *entry = PBD->isFullyValidated(i)
+                              ? &PBD->getPatternList()[i]
+                              : PBD->getCheckedPatternBindingEntry(
+                                    i, LeaveClosureBodiesUnchecked);
       assert(entry && "No pattern binding entry?");
 
       const auto *Pat = PBD->getPattern(i);
diff --git a/lib/Serialization/Serialization.cpp b/lib/Serialization/Serialization.cpp
@@ -3284,8 +3284,6 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
   /// anything needed by a client as unsafe as the client will reject reading
   /// it, but at the same time keep the safety checks precise to avoid
   /// XRef errors and such.
-  ///
-  /// \p decl should be either an \c ExtensionDecl or a \c ValueDecl.
   static bool isDeserializationSafe(const Decl *decl) {
     if (auto ext = dyn_cast<ExtensionDecl>(decl)) {
       // Consider extensions as safe as their extended type.
@@ -3308,8 +3306,11 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
       // We can mark the extension unsafe only if it has no public
       // conformances.
       auto protocols = ext->getLocalProtocols(ConformanceLookupKind::All);
-      bool hasSafeConformances = std::any_of(protocols.begin(), protocols.end(),
-                                             isDeserializationSafe);
+      bool hasSafeConformances = std::any_of(
+          protocols.begin(), protocols.end(), [](ProtocolDecl *protocol) {
+            return isDeserializationSafe(protocol);
+          });
+
       if (hasSafeConformances)
         return true;
 
@@ -3320,11 +3321,26 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
       return false;
     }
 
-    auto value = cast<ValueDecl>(decl);
+    if (auto pbd = dyn_cast<PatternBindingDecl>(decl)) {
+      // Pattern bindings are safe if any of their var decls are safe.
+      for (auto i : range(pbd->getNumPatternEntries())) {
+        if (auto *varDecl = pbd->getAnchoringVarDecl(i)) {
+          if (isDeserializationSafe(varDecl))
+            return true;
+        }
+      }
 
+      return false;
+    }
+
+    return isDeserializationSafe(cast<ValueDecl>(decl));
+  }
+
+  static bool isDeserializationSafe(const ValueDecl *value) {
     // A decl is safe if formally accessible publicly.
-    auto accessScope = value->getFormalAccessScope(/*useDC=*/nullptr,
-                       /*treatUsableFromInlineAsPublic=*/true);
+    auto accessScope =
+        value->getFormalAccessScope(/*useDC=*/nullptr,
+                                    /*treatUsableFromInlineAsPublic=*/true);
     if (accessScope.isPublic() || accessScope.isPackage())
       return true;
 
@@ -3351,7 +3367,7 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
     }
 
     // Paramters don't have meaningful access control.
-    if (isa<ParamDecl>(decl) || isa<GenericTypeParamDecl>(decl))
+    if (isa<ParamDecl>(value) || isa<GenericTypeParamDecl>(value))
       return true;
 
     return false;
@@ -3919,7 +3935,13 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
         initContextIDs);
 
     for (auto entryIdx : range(binding->getNumPatternEntries())) {
-      writePattern(binding->getPattern(entryIdx));
+      auto pattern = binding->getPattern(entryIdx);
+
+      // Force the entry to be typechecked before attempting to serialize.
+      if (!pattern->hasType())
+        (void)binding->getCheckedPatternBindingEntry(entryIdx);
+
+      writePattern(pattern);
       // Ignore initializer; external clients don't need to know about it.
     }
   }
diff --git a/test/Inputs/lazy_typecheck.swift b/test/Inputs/lazy_typecheck.swift
@@ -47,6 +47,15 @@ public func publicFuncWithOpaqueReturnType() -> some PublicProto { // expected-n
   }
 }
 
+// MARK: - Global vars
+
+public var publicGlobalVar: Int = 0
+public var publicGlobalVarInferredType = ""
+public var (publicGlobalVarInferredTuplePatX, publicGlobalVarInferredTuplePatY) = (0, 1)
+
+var internalGlobalVar: DoesNotExist // expected-error {{cannot find type 'DoesNotExist' in scope}}
+var internalGlobalVarInferredType = DoesNotExist() // expected-error {{cannot find 'DoesNotExist' in scope}}
+
 // MARK: - Nominal types
 
 public protocol EmptyPublicProto {}
@@ -69,7 +78,8 @@ protocol InternalProtoConformingToPublicProto: PublicProto {
 }
 
 public struct PublicStruct {
-  // FIXME: Test properties
+  public var publicProperty: Int
+  public var publicPropertyInferredType = ""
 
   public init(x: Int) {
     _ = DoesNotExist() // expected-error {{cannot find 'DoesNotExist' in scope}}
@@ -105,7 +115,8 @@ struct InternalStruct: DoesNotExist { // expected-error {{cannot find type 'Does
 }
 
 public class PublicClass {
-  // FIXME: Test properties
+  public var publicProperty: Int
+  public var publicPropertyInferredType = ""
 
   public init(x: Int) {
     _ = DoesNotExist() // expected-error {{cannot find 'DoesNotExist' in scope}}
@@ -188,4 +199,3 @@ extension PublicGenericStruct where T == InternalStructForConstraint {}
 extension PublicGenericStruct: EmptyPublicProto where T == InternalStructForConstraint {}
 
 // FIXME: Test enums
-// FIXME: Test global vars
diff --git a/test/Inputs/lazy_typecheck_client.swift b/test/Inputs/lazy_typecheck_client.swift
@@ -20,19 +20,31 @@ func testGlobalFunctions() {
   }
 }
 
+func testGobalVars() {
+  let _: Int = publicGlobalVar
+  let _: String = publicGlobalVarInferredType
+  let _: (Int, Int) = (publicGlobalVarInferredTuplePatX, publicGlobalVarInferredTuplePatY)
+}
+
 func testPublicStruct() {
   let s = PublicStruct(x: 1)
-  _ = s.publicMethod()
+  let _: Int = s.publicMethod()
+  let _: Int = s.publicProperty
+  let _: String = s.publicPropertyInferredType
   PublicStruct.publicStaticMethod()
 }
 
 func testPublicClass() {
   let c = PublicClass(x: 2)
-  _ = c.publicMethod()
+  let _: Int = c.publicMethod()
+  let _: Int = c.publicProperty
+  let _: String = c.publicPropertyInferredType
   PublicClass.publicClassMethod()
 
   let d = PublicDerivedClass(x: 3)
-  _ = d.publicMethod()
+  let _: Int = d.publicMethod()
+  let _: Int = d.publicProperty
+  let _: String = d.publicPropertyInferredType
   PublicDerivedClass.publicClassMethod()
 }
 
diff --git a/test/ModuleInterface/lazy-typecheck.swift b/test/ModuleInterface/lazy-typecheck.swift
@@ -26,6 +26,9 @@
 // CHECK-NEXT:    }
 // CHECK-NEXT:  }
 
+// CHECK:       public var publicGlobalVar: Swift.Int
+// CHECK:       public var publicGlobalVarInferredType: Swift.String
+// CHECK:       public var (publicGlobalVarInferredTuplePatX, publicGlobalVarInferredTuplePatY): (Swift.Int, Swift.Int)
 // CHECK:       public protocol EmptyPublicProto {
 // CHECK:       }
 // CHECK:       public protocol PublicProto {
@@ -37,6 +40,8 @@
 // CHECK:       }
 // CHECK:       #endif
 // CHECK:       public struct PublicStruct {
+// CHECK:         public var publicProperty: Swift.Int
+// CHECK:         public var publicPropertyInferredType: Swift.String
 // CHECK:         public init(x: Swift.Int)
 // CHECK:         public func publicMethod() -> Swift.Int
 // CHECK:         public static func publicStaticMethod()
@@ -45,6 +50,8 @@
 // CHECK:         public func publicMethod() -> T
 // CHECK:       }
 // CHECK:       public class PublicClass {
+// CHECK:         public var publicProperty: Swift.Int
+// CHECK:         public var publicPropertyInferredType: Swift.String
 // CHECK:         public init(x: Swift.Int)
 // CHECK:         public func publicMethod() -> Swift.Int
 // CHECK:         public class func publicClassMethod()
diff --git a/test/TBD/lazy-typecheck.swift b/test/TBD/lazy-typecheck.swift
@@ -59,19 +59,42 @@ exports:
                        '_$s14lazy_typecheck10publicFuncSiyF', '_$s14lazy_typecheck11PublicClassC06publicD6MethodyyFZTj',
                        '_$s14lazy_typecheck11PublicClassC06publicD6MethodyyFZTq',
                        '_$s14lazy_typecheck11PublicClassC12publicMethodSiyFTj', '_$s14lazy_typecheck11PublicClassC12publicMethodSiyFTq',
+                       '_$s14lazy_typecheck11PublicClassC14publicPropertySivMTj',
+                       '_$s14lazy_typecheck11PublicClassC14publicPropertySivMTq',
+                       '_$s14lazy_typecheck11PublicClassC14publicPropertySivgTj',
+                       '_$s14lazy_typecheck11PublicClassC14publicPropertySivgTq',
+                       '_$s14lazy_typecheck11PublicClassC14publicPropertySivpMV',
+                       '_$s14lazy_typecheck11PublicClassC14publicPropertySivsTj',
+                       '_$s14lazy_typecheck11PublicClassC14publicPropertySivsTq',
                        '_$s14lazy_typecheck11PublicClassC1xACSi_tcfC', '_$s14lazy_typecheck11PublicClassC1xACSi_tcfCTj',
                        '_$s14lazy_typecheck11PublicClassC1xACSi_tcfCTq', '_$s14lazy_typecheck11PublicClassC1xACSi_tcfc',
+                       '_$s14lazy_typecheck11PublicClassC26publicPropertyInferredTypeSSvMTj',
+                       '_$s14lazy_typecheck11PublicClassC26publicPropertyInferredTypeSSvMTq',
+                       '_$s14lazy_typecheck11PublicClassC26publicPropertyInferredTypeSSvgTj',
+                       '_$s14lazy_typecheck11PublicClassC26publicPropertyInferredTypeSSvgTq',
+                       '_$s14lazy_typecheck11PublicClassC26publicPropertyInferredTypeSSvpMV',
+                       '_$s14lazy_typecheck11PublicClassC26publicPropertyInferredTypeSSvsTj',
+                       '_$s14lazy_typecheck11PublicClassC26publicPropertyInferredTypeSSvsTq',
                        '_$s14lazy_typecheck11PublicClassCMa', '_$s14lazy_typecheck11PublicClassCMm',
                        '_$s14lazy_typecheck11PublicClassCMn', '_$s14lazy_typecheck11PublicClassCMo',
                        '_$s14lazy_typecheck11PublicClassCMu', '_$s14lazy_typecheck11PublicClassCN',
                        '_$s14lazy_typecheck11PublicClassCfD', '_$s14lazy_typecheck11PublicClassCfd',
                        '_$s14lazy_typecheck11PublicProtoMp', '_$s14lazy_typecheck11PublicProtoP3reqSiyFTj',
                        '_$s14lazy_typecheck11PublicProtoP3reqSiyFTq', '_$s14lazy_typecheck11PublicProtoTL',
                        '_$s14lazy_typecheck11packageFuncSiyF', '_$s14lazy_typecheck12PublicStructV12publicMethodSiyF',
+                       '_$s14lazy_typecheck12PublicStructV14publicPropertySivM',
+                       '_$s14lazy_typecheck12PublicStructV14publicPropertySivg',
+                       '_$s14lazy_typecheck12PublicStructV14publicPropertySivpMV',
+                       '_$s14lazy_typecheck12PublicStructV14publicPropertySivs',
                        '_$s14lazy_typecheck12PublicStructV18publicStaticMethodyyFZ',
-                       '_$s14lazy_typecheck12PublicStructV1xACSi_tcfC', '_$s14lazy_typecheck12PublicStructVMa',
-                       '_$s14lazy_typecheck12PublicStructVMn', '_$s14lazy_typecheck12PublicStructVN',
-                       '_$s14lazy_typecheck13inlinableFuncSiyF', '_$s14lazy_typecheck16EmptyPublicProtoMp',
+                       '_$s14lazy_typecheck12PublicStructV1xACSi_tcfC', '_$s14lazy_typecheck12PublicStructV26publicPropertyInferredTypeSSvM',
+                       '_$s14lazy_typecheck12PublicStructV26publicPropertyInferredTypeSSvg',
+                       '_$s14lazy_typecheck12PublicStructV26publicPropertyInferredTypeSSvpMV',
+                       '_$s14lazy_typecheck12PublicStructV26publicPropertyInferredTypeSSvs',
+                       '_$s14lazy_typecheck12PublicStructVMa', '_$s14lazy_typecheck12PublicStructVMn',
+                       '_$s14lazy_typecheck12PublicStructVN', '_$s14lazy_typecheck13inlinableFuncSiyF',
+                       '_$s14lazy_typecheck15publicGlobalVarSivM', '_$s14lazy_typecheck15publicGlobalVarSivg',
+                       '_$s14lazy_typecheck15publicGlobalVarSivs', '_$s14lazy_typecheck16EmptyPublicProtoMp',
                        '_$s14lazy_typecheck16EmptyPublicProtoTL', '_$s14lazy_typecheck18PublicDerivedClassC1xACSi_tcfC',
                        '_$s14lazy_typecheck18PublicDerivedClassC1xACSi_tcfc', '_$s14lazy_typecheck18PublicDerivedClassCMa',
                        '_$s14lazy_typecheck18PublicDerivedClassCMm', '_$s14lazy_typecheck18PublicDerivedClassCMn',
@@ -83,9 +106,17 @@ exports:
                        '_$s14lazy_typecheck19PublicGenericStructVyxGAA05EmptyC5ProtoA2A08InternalE13ForConstraintVRszlWP',
                        '_$s14lazy_typecheck19PublicRethrowsProtoMp', '_$s14lazy_typecheck19PublicRethrowsProtoP3reqSiyKFTj',
                        '_$s14lazy_typecheck19PublicRethrowsProtoP3reqSiyKFTq', '_$s14lazy_typecheck19PublicRethrowsProtoTL',
-                       '_$s14lazy_typecheck24publicFuncWithDefaultArgyS2iF', '_$s14lazy_typecheck30publicFuncWithOpaqueReturnTypeQryF',
+                       '_$s14lazy_typecheck24publicFuncWithDefaultArgyS2iF', '_$s14lazy_typecheck27publicGlobalVarInferredTypeSSvM',
+                       '_$s14lazy_typecheck27publicGlobalVarInferredTypeSSvg', '_$s14lazy_typecheck27publicGlobalVarInferredTypeSSvs',
+                       '_$s14lazy_typecheck30publicFuncWithOpaqueReturnTypeQryF',
                        '_$s14lazy_typecheck30publicFuncWithOpaqueReturnTypeQryFQOMQ',
                        '_$s14lazy_typecheck32constrainedGenericPublicFunctionyyxAA0E5ProtoRzlF',
+                       '_$s14lazy_typecheck32publicGlobalVarInferredTuplePatXSivM',
+                       '_$s14lazy_typecheck32publicGlobalVarInferredTuplePatXSivg',
+                       '_$s14lazy_typecheck32publicGlobalVarInferredTuplePatXSivs',
+                       '_$s14lazy_typecheck32publicGlobalVarInferredTuplePatYSivM',
+                       '_$s14lazy_typecheck32publicGlobalVarInferredTuplePatYSivg',
+                       '_$s14lazy_typecheck32publicGlobalVarInferredTuplePatYSivs',
                        '_$sSS14lazy_typecheck11PublicProtoAAMc', '_$sSS14lazy_typecheck11PublicProtoAAWP',
                        '_$sSS14lazy_typecheckE3reqSiyF', '_$sSi14lazy_typecheck19PublicRethrowsProtoAAMc',
                        '_$sSi14lazy_typecheck19PublicRethrowsProtoAAWP', '_$sSi14lazy_typecheckE3reqSiyKF' ]
