Define NamingPatternRequest

Use it to provide an idealized API for the VarDecl case in validateDecl.

In reality, a lot of work is needed to rationalize the dependency
structure of this request.  To start, the callers of
typeCheckPatternBinding must be eliminated piecemeal.  Once that is
done, the AST should introduce pattern binding decls along all the
places where getParentStmt() used to apply.

Author: CodaFi

include/swift/AST/ASTTypeIDZone.def

@@ -33,6 +33,7 @@ SWIFT_TYPEID_NAMED(GenericTypeParamType *, GenericTypeParamType)
 SWIFT_TYPEID_NAMED(InfixOperatorDecl *, InfixOperatorDecl)
 SWIFT_TYPEID_NAMED(IterableDeclContext *, IterableDeclContext)
 SWIFT_TYPEID_NAMED(ModuleDecl *, ModuleDecl)
+SWIFT_TYPEID_NAMED(NamedPattern *, NamedPattern)
 SWIFT_TYPEID_NAMED(NominalTypeDecl *, NominalTypeDecl)
 SWIFT_TYPEID_NAMED(OpaqueTypeDecl *, OpaqueTypeDecl)
 SWIFT_TYPEID_NAMED(OperatorDecl *, OperatorDecl)

include/swift/AST/ASTTypeIDs.h

@@ -32,6 +32,7 @@ class GenericTypeParamType;
 class InfixOperatorDecl;
 class IterableDeclContext;
 class ModuleDecl;
+class NamedPattern;
 class NominalTypeDecl;
 class OperatorDecl;
 class OpaqueTypeDecl;

include/swift/AST/Decl.h

@@ -1926,8 +1926,20 @@ class PatternBindingEntry {
 
   friend class PatternBindingInitializer;
 
+  // FIXME: This API is transitional. Once the callers of
+  // typeCheckPatternBinding are requestified, merge this bit with
+  // Flags::Checked.
+  friend class PatternBindingEntryRequest;
+
   bool IsFullyValidated = false;
 
+  bool isFullyValidated() const {
+    return IsFullyValidated;
+  }
+  void setFullyValidated() {
+    IsFullyValidated = true;
+  }
+
 public:
   /// \p E is the initializer as parsed.
   PatternBindingEntry(Pattern *P, SourceLoc EqualLoc, Expr *E,
@@ -1997,13 +2009,6 @@ class PatternBindingEntry {
     PatternAndFlags.setInt(PatternAndFlags.getInt() | Flags::Checked);
   }
 
-  bool isFullyValidated() const {
-    return IsFullyValidated;
-  }
-  void setFullyValidated() {
-    IsFullyValidated = true;
-  }
-
   bool isInitializerSubsumed() const {
     return PatternAndFlags.getInt().contains(Flags::Subsumed);
   }
@@ -4800,6 +4805,7 @@ enum class PropertyWrapperSynthesizedPropertyKind {
 
 /// VarDecl - 'var' and 'let' declarations.
 class VarDecl : public AbstractStorageDecl {
+  friend class NamingPatternRequest;
   NamedPattern *NamingPattern = nullptr;
 
 public:
@@ -4921,8 +4927,8 @@ class VarDecl : public AbstractStorageDecl {
     Parent = v;
   }
 
-  NamedPattern *getNamingPattern() const { return NamingPattern; }
-  void setNamingPattern(NamedPattern *Pat) { NamingPattern = Pat; }
+  NamedPattern *getNamingPattern() const;
+  void setNamingPattern(NamedPattern *Pat);
 
   /// If this is a VarDecl that does not belong to a CaseLabelItem's pattern,
   /// return this. Otherwise, this VarDecl must belong to a CaseStmt's

include/swift/AST/TypeCheckRequests.h

@@ -1407,6 +1407,26 @@ class PatternBindingEntryRequest
   void cacheResult(const PatternBindingEntry *value) const;
 };
 
+class NamingPatternRequest
+    : public SimpleRequest<NamingPatternRequest, NamedPattern *(VarDecl *),
+                           CacheKind::SeparatelyCached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  // Evaluation.
+  llvm::Expected<NamedPattern *> evaluate(Evaluator &evaluator,
+                                          VarDecl *VD) const;
+
+public:
+  // Separate caching.
+  bool isCached() const { return true; }
+  Optional<NamedPattern *> getCachedResult() const;
+  void cacheResult(NamedPattern *P) const;
+};
+
 // Allow AnyValue to compare two Type values, even though Type doesn't
 // support ==.
 template<>

include/swift/AST/TypeCheckerTypeIDZone.def

@@ -85,6 +85,8 @@ SWIFT_REQUEST(TypeChecker, LazyStoragePropertyRequest, VarDecl *(VarDecl *),
               Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, MangleLocalTypeDeclRequest,
               std::string(const TypeDecl *), Cached, NoLocationInfo)
+SWIFT_REQUEST(TypeChecker, NamingPatternRequest,
+              NamedPattern *(VarDecl *), SeparatelyCached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, OpaqueReadOwnershipRequest,
               OpaqueReadOwnership(AbstractStorageDecl *), SeparatelyCached,
               NoLocationInfo)

lib/AST/Decl.cpp

@@ -5309,6 +5309,17 @@ Pattern *VarDecl::getParentPattern() const {
   return nullptr;
 }
 
+NamedPattern *VarDecl::getNamingPattern() const {
+  return evaluateOrDefault(getASTContext().evaluator,
+                           NamingPatternRequest{const_cast<VarDecl *>(this)},
+                           nullptr);
+}
+
+void VarDecl::setNamingPattern(NamedPattern *Pat) {
+  getASTContext().evaluator.cacheOutput(NamingPatternRequest{this},
+                                        std::move(Pat));
+}
+
 TypeRepr *VarDecl::getTypeReprOrParentPatternTypeRepr() const {
   if (auto *param = dyn_cast<ParamDecl>(this))
     return param->getTypeRepr();

lib/AST/TypeCheckRequests.cpp

@@ -984,3 +984,20 @@ void PatternBindingEntryRequest::cacheResult(
   auto idx = std::get<1>(getStorage());
   PBD->getMutablePatternList()[idx].setFullyValidated();
 }
+
+//----------------------------------------------------------------------------//
+// NamingPatternRequest computation.
+//----------------------------------------------------------------------------//
+
+Optional<NamedPattern *> NamingPatternRequest::getCachedResult() const {
+  auto *VD = std::get<0>(getStorage());
+  if (auto *Pat = VD->NamingPattern) {
+    return Pat;
+  }
+  return None;
+}
+
+void NamingPatternRequest::cacheResult(NamedPattern *value) const {
+  auto *VD = std::get<0>(getStorage());
+  VD->NamingPattern = value;
+}

lib/Sema/ConstraintSystem.cpp

@@ -1461,6 +1461,16 @@ Type ConstraintSystem::getEffectiveOverloadType(const OverloadChoice &overload,
   if (decl->isImplicitlyUnwrappedOptional())
     return Type();
 
+  // In a pattern binding initializer, all of its bound variables have no
+  // effective overload type.
+  if (auto *PBI = dyn_cast<PatternBindingInitializer>(useDC)) {
+    if (auto *VD = dyn_cast<VarDecl>(decl)) {
+      if (PBI->getBinding() == VD->getParentPatternBinding()) {
+        return Type();
+      }
+    }
+  }
+
   // Retrieve the interface type.
   auto type = decl->getInterfaceType();
   if (!type || type->hasError()) {

lib/Sema/TypeCheckDecl.cpp

@@ -2410,9 +2410,20 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
       entry->getPattern()->forEachVariable([&](VarDecl *var) {
         this->visitBoundVariable(var);
 
+        if (entry->isInitialized()) {
+          // Add the attribute that preserves the "has an initializer" value
+          // across module generation, as required for TBDGen.
+          if (var->hasStorage() &&
+              !var->getAttrs().hasAttribute<HasInitialValueAttr>()) {
+            var->getAttrs().add(new (TC.Context)
+                                    HasInitialValueAttr(/*IsImplicit=*/true));
+          }
+          return;
+        }
+
         // If this is a declaration without an initializer, reject code if
         // uninitialized vars are not allowed.
-        if (entry->isInitialized() || isInSILMode) return;
+        if (isInSILMode) return;
 
         // If the variable has no storage, it never needs an initializer.
         if (!var->hasStorage())
@@ -4187,52 +4198,10 @@ void TypeChecker::validateDecl(ValueDecl *D) {
 
   case DeclKind::Var: {
     auto *VD = cast<VarDecl>(D);
-    auto *PBD = VD->getParentPatternBinding();
-    if (PBD) {
-      // FIXME: Sink this into a request for the naming pattern.
-      unsigned i = PBD->getPatternEntryIndexForVarDecl(VD);
-      (void)evaluateOrDefault(Context.evaluator,
-                              PatternBindingEntryRequest{PBD, i},
-                              nullptr);
-      if (PBD->isInvalid()) {
-        VD->getParentPattern()->setType(ErrorType::get(Context));
-        setBoundVarsTypeError(VD->getParentPattern(), Context);
-        break;
-      }
-    } else if (!VD->getParentPatternStmt() && !VD->getParentVarDecl()) {
-      // No parent?  That's an error.
-      VD->setInterfaceType(ErrorType::get(Context));
-      break;
-    }
-
-    // Go digging for the named pattern that declares this variable.
     auto *namingPattern = VD->getNamingPattern();
     if (!namingPattern) {
-      auto *canVD = VD->getCanonicalVarDecl();
-      namingPattern = canVD->getNamingPattern();
-
-      // HACK: If no other diagnostic applies, emit a generic diagnostic about
-      // a variable being unbound. We can't do better than this at the
-      // moment because TypeCheckPattern does not reliably invalidate parts of
-      // the pattern AST on failure.
-      //
-      // Once that's through, this will only fire during circular validation.
-      if (!namingPattern) {
-        if (!VD->isInvalid() && !VD->getParentPattern()->isImplicit()) {
-          VD->diagnose(diag::variable_bound_by_no_pattern, VD->getName());
-        }
-
-        VD->getParentPattern()->setType(ErrorType::get(Context));
-        setBoundVarsTypeError(VD->getParentPattern(), Context);
-        VD->setInterfaceType(ErrorType::get(Context));
-        break;
-      }
-    }
-    assert(namingPattern && "Bound variable with no naming pattern!");
-
-    if (!namingPattern->hasType()) {
-      namingPattern->setType(ErrorType::get(Context));
-      setBoundVarsTypeError(namingPattern, Context);
+      VD->setInterfaceType(ErrorType::get(Context));
+      break;
     }
 
     Type interfaceType = namingPattern->getType();
@@ -4312,6 +4281,63 @@ void TypeChecker::validateDecl(ValueDecl *D) {
   assert(D->hasInterfaceType());
 }
 
+llvm::Expected<NamedPattern *>
+NamingPatternRequest::evaluate(Evaluator &evaluator, VarDecl *VD) const {
+  auto &Context = VD->getASTContext();
+  auto *PBD = VD->getParentPatternBinding();
+  // FIXME: In order for this request to properly express its dependencies,
+  // all of the places that allow variable bindings need to also use pattern
+  // binding decls. Otherwise, we'll have to go digging around in case
+  // statements and patterns to find named patterns.
+  if (PBD) {
+    // FIXME: For now, this works because PatternBindingEntryRequest fills in
+    // 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},
+                            nullptr);
+    if (PBD->isInvalid()) {
+      VD->getParentPattern()->setType(ErrorType::get(Context));
+      setBoundVarsTypeError(VD->getParentPattern(), Context);
+      return nullptr;
+    }
+  } else if (!VD->getParentPatternStmt() && !VD->getParentVarDecl()) {
+    // No parent?  That's an error.
+    return nullptr;
+  }
+
+  // Go digging for the named pattern that declares this variable.
+  auto *namingPattern = VD->NamingPattern;
+  if (!namingPattern) {
+    auto *canVD = VD->getCanonicalVarDecl();
+    namingPattern = canVD->NamingPattern;
+
+    // HACK: If no other diagnostic applies, emit a generic diagnostic about
+    // a variable being unbound. We can't do better than this at the
+    // moment because TypeCheckPattern does not reliably invalidate parts of
+    // the pattern AST on failure.
+    //
+    // Once that's through, this will only fire during circular validation.
+    if (!namingPattern) {
+      if (!VD->isInvalid() && !VD->getParentPattern()->isImplicit()) {
+        VD->diagnose(diag::variable_bound_by_no_pattern, VD->getName());
+      }
+
+      VD->getParentPattern()->setType(ErrorType::get(Context));
+      setBoundVarsTypeError(VD->getParentPattern(), Context);
+      return nullptr;
+    }
+  }
+
+  if (!namingPattern->hasType()) {
+    namingPattern->setType(ErrorType::get(Context));
+    setBoundVarsTypeError(namingPattern, Context);
+  }
+
+  return namingPattern;
+}
+
 llvm::Expected<DeclRange>
 EmittedMembersRequest::evaluate(Evaluator &evaluator,
                                 ClassDecl *CD) const {

lib/Sema/TypeCheckStorage.cpp

@@ -237,18 +237,6 @@ PatternBindingEntryRequest::evaluate(Evaluator &eval,
     }
   }
 
-  if (pbe.isInitialized()) {
-    // Add the attribute that preserves the "has an initializer" value across
-    // module generation, as required for TBDGen.
-    pattern->forEachVariable([&](VarDecl *VD) {
-      if (VD->hasStorage() &&
-          !VD->getAttrs().hasAttribute<HasInitialValueAttr>()) {
-        auto *attr = new (Context) HasInitialValueAttr(/*IsImplicit=*/true);
-        VD->getAttrs().add(attr);
-      }
-    });
-  }
-
   // If the pattern didn't get a type or if it contains an unbound generic type,
   // we'll need to check the initializer.
   if (!pattern->hasType() || pattern->getType()->hasUnboundGenericType()) {

test/ModuleInterface/access-filter.swift

@@ -109,7 +109,7 @@ extension UFIProto {
 
 // CHECK: extension PublicStruct {{[{]$}}
 extension PublicStruct {
-  // CHECK: @_hasInitialValue @_hasStorage public static var secretlySettable: Swift.Int {
+  // CHECK: @_hasInitialValue public static var secretlySettable: Swift.Int {
   // CHECK-NEXT: get
   // CHECK-NEXT: }
   public private(set) static var secretlySettable: Int = 0

test/ModuleInterface/stored-properties.swift

@@ -53,7 +53,7 @@ public struct HasStoredProperties {
   // RESILIENT-NOT: private var privateVar: Swift.Bool
   private var privateVar: Bool
 
-  // CHECK: {{@_hasInitialValue @_hasStorage|@_hasStorage @_hasInitialValue}} public var storedWithObserversInitialValue: Swift.Int {
+  // CHECK: @_hasStorage @_hasInitialValue public var storedWithObserversInitialValue: Swift.Int {
   // RESILIENT: {{^}}  public var storedWithObserversInitialValue: Swift.Int {
   // COMMON-NEXT: get
   // COMMON-NEXT: set

test/NameBinding/name_lookup.swift

@@ -628,6 +628,6 @@ struct PatternBindingWithTwoVars3 { var x = y, y = x }
 
 // https://bugs.swift.org/browse/SR-9015
 func sr9015() {
-  let closure1 = { closure2() } // expected-error {{circular reference}} expected-error{{variable 'closure1' is not bound by any pattern}}
-  let closure2 = { closure1() } // expected-note {{through reference here}}
+  let closure1 = { closure2() } // expected-error {{circular reference}} expected-error{{variable 'closure1' is not bound by any pattern}} expected-note {{through reference here}}
+  let closure2 = { closure1() } // expected-note {{through reference here}} expected-note {{through reference here}}
 }

test/SIL/Parser/final.swift

@@ -1,14 +1,14 @@
 // RUN: %target-swift-frontend %s -emit-silgen | %FileCheck %s
 
 // CHECK: final class Rect
-// CHECK: @_hasInitialValue @_hasStorage final var orgx: Double
+// CHECK: @_hasStorage @_hasInitialValue final var orgx: Double
 final class Rect {
   var orgx = 0.0
 }
 
 protocol P { }
 // CHECK: struct Rect2 : P {
-// CHECK: @_hasInitialValue @_hasStorage var orgx: Double
+// CHECK: @_hasStorage @_hasInitialValue var orgx: Double
 struct Rect2 : P {
   var orgx = 0.0
 }

test/SILGen/implicit_property_initializers.swift

@@ -1,10 +1,10 @@
 // RUN: %target-swift-emit-silgen -module-name implicit_property_initializers -Xllvm -sil-full-demangle -enable-testing %s | %FileCheck %s
 
 // CHECK: struct HasDefaultTupleOfNils {
-// CHECK:   @_hasInitialValue @_hasStorage var x: (Int?, Int?)
-// CHECK:   @_hasInitialValue @_hasStorage var y: Int?
+// CHECK:   @_hasStorage @_hasInitialValue var x: (Int?, Int?)
+// CHECK:   @_hasStorage @_hasInitialValue var y: Int?
 // CHECK:   @_hasStorage var z: Int
-// CHECK:   @_hasInitialValue @_hasStorage var w: ((Int?, (), Int?), (Int?, Int?))
+// CHECK:   @_hasStorage @_hasInitialValue var w: ((Int?, (), Int?), (Int?, Int?))
 // CHECK:   init(x: (Int?, Int?) = (nil, nil),
 // CHECK-SAME:   y: Int? = nil,
 // CHECK-SAME:   z: Int,

test/SILOptimizer/access_dom_call.sil

@@ -98,7 +98,7 @@ bb0:
 // Accessors
 
 public class C {
-  @_hasInitialValue @_hasStorage public var field: Int64 { get set }
+  @_hasStorage @_hasInitialValue public var field: Int64 { get set }
 }
 
 sil_property #C.field ()

test/SILOptimizer/access_enforcement_opts.sil

@@ -1596,7 +1596,7 @@ bb3(%1 : $*X):
 // cannot be merged without introducing a false conflict.
 
 public class TestClass {
-  @_hasInitialValue @_hasStorage var flags: Int64 { get set }
+  @_hasStorage @_hasInitialValue var flags: Int64 { get set }
 }
 
 // CHECK-LABEL: sil hidden [noinline] @readFlags : $@convention(method) (Int64, @guaranteed TestClass) -> Bool {