Merge pull request swiftlang#68452 from tshortli/lazy-typecheck-vars

ModuleInterface: Fix PatternBindingDecl printing in lazy mode

Author: tshortli

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 {
 

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,
@@ -2258,6 +2270,13 @@ void PrintAST::printAccessors(const AbstractStorageDecl *ASD) {
     return;
   }
 
+  // Force implicit accessors to be created if they haven't been already.
+  if (shouldTypeCheck(Options)) {
+    ASD->visitEmittedAccessors([](AccessorDecl *accessor) {
+      (void)accessor;
+    });
+  }
+
   // Collect the accessor declarations that we should print.
   SmallVector<AccessorDecl *, 4> accessorsToPrint;
   auto AddAccessorToPrint = [&](AccessorKind kind) {
@@ -3799,6 +3818,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)

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) {

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);

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);

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.
     }
   }

test/Inputs/lazy_typecheck.swift

@@ -47,6 +47,38 @@ public func publicFuncWithOpaqueReturnType() -> some PublicProto { // expected-n
   }
 }
 
+// MARK: - Property wrappers
+
+@propertyWrapper
+public struct PublicWrapper<T> {
+  public var wrappedValue: T {
+    get {
+      _ = DoesNotExist() // expected-error {{cannot find 'DoesNotExist' in scope}}
+    }
+    set {
+      _ = DoesNotExist() // expected-error {{cannot find 'DoesNotExist' in scope}}
+    }
+  }
+
+  public var projectedValue: PublicWrapper { self }
+
+  public init(wrappedValue value: T) {
+    _ = DoesNotExist() // expected-error {{cannot find 'DoesNotExist' in scope}}
+  }
+}
+
+@propertyWrapper
+struct InternalWrapper<T> {} // expected-error {{property wrapper type 'InternalWrapper' does not contain a non-static property named 'wrappedValue'}}
+
+// 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 +101,9 @@ protocol InternalProtoConformingToPublicProto: PublicProto {
 }
 
 public struct PublicStruct {
-  // FIXME: Test properties
+  public var publicProperty: Int
+  public var publicPropertyInferredType = ""
+  @PublicWrapper public var publicWrappedProperty = 3.14
 
   public init(x: Int) {
     _ = DoesNotExist() // expected-error {{cannot find 'DoesNotExist' in scope}}
@@ -105,7 +139,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 +223,3 @@ extension PublicGenericStruct where T == InternalStructForConstraint {}
 extension PublicGenericStruct: EmptyPublicProto where T == InternalStructForConstraint {}
 
 // FIXME: Test enums
-// FIXME: Test global vars

test/Inputs/lazy_typecheck_client.swift

@@ -20,19 +20,33 @@ 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
+  let _: Double = s.publicWrappedProperty
+  let _: Double = s.$publicWrappedProperty.wrappedValue
   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()
 }
 

test/ModuleInterface/lazy-typecheck.swift

@@ -1,6 +1,6 @@
 // RUN: %empty-directory(%t)
 
-// 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
+// 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
 // RUN: %FileCheck %s < %t/lazy_typecheck.swiftinterface
 
 // RUN: rm -rf %t/*.swiftmodule
@@ -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,16 @@
 // CHECK:       }
 // CHECK:       #endif
 // CHECK:       public struct PublicStruct {
+// CHECK:         public var publicProperty: Swift.Int
+// CHECK:         public var publicPropertyInferredType: Swift.String
+// CHECK:         @lazy_typecheck.PublicWrapper @_projectedValueProperty($publicWrappedProperty) public var publicWrappedProperty: Swift.Double {
+// CHECK-NEXT:      get
+// CHECK-NEXT:      set
+// CHECK-NEXT:      _modify
+// CHECK-NEXT:    }
+// CHECK:         public var $publicWrappedProperty: lazy_typecheck.PublicWrapper<Swift.Double> {
+// CHECK-NEXT:      get
+// CHECK-NEXT:    }
 // CHECK:         public init(x: Swift.Int)
 // CHECK:         public func publicMethod() -> Swift.Int
 // CHECK:         public static func publicStaticMethod()
@@ -45,6 +58,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()