[AST] Remove stored TypeLoc from TypedPattern (#19175)

* [AST] Remove stored TypeLoc from TypedPattern

TypedPattern was only using this TypeLoc as a means to a TypeRepr, which
caused it to store the pattern type twice (through the superclass and through
the TypeLoc itself.)

This also fixes a bug where deserializing a TypedPattern doesn't store
the type correctly and generally cleans up TypedPattern initialization.

Resolves rdar://44144435

* Address review comments

include/swift/AST/Pattern.h

@@ -383,14 +383,25 @@ public:
 /// dynamic type match.
 class TypedPattern : public Pattern {
   Pattern *SubPattern;
-  mutable TypeLoc PatType;
+  TypeRepr *PatTypeRepr;
 
 public:
-  TypedPattern(Pattern *pattern, TypeLoc tl, Optional<bool> implicit = None)
-    : Pattern(PatternKind::Typed), SubPattern(pattern), PatType(tl) {
-    if (implicit.hasValue() ? *implicit : !tl.hasLocation())
-      setImplicit();
-    Bits.TypedPattern.IsPropagatedType = false;
+  /// Creates a new TypedPattern which annotates the provided sub-pattern with
+  /// the provided TypeRepr. If 'implicit' is true, the pattern will be
+  /// set to implicit. If false, it will not. If 'implicit' is not provided,
+  /// then the pattern will be set to 'implicit' if there is a provided TypeRepr
+  /// which has a valid SourceRange.
+  TypedPattern(Pattern *pattern, TypeRepr *tr, Optional<bool> implicit = None);
+
+  /// Creates an implicit typed pattern annotating the provided sub-pattern
+  /// with a given type.
+  static TypedPattern *
+  createImplicit(ASTContext &ctx, Pattern *pattern, Type type) {
+    auto tp = new (ctx) TypedPattern(pattern, /*typeRepr*/nullptr,
+                                     /*implicit*/true);
+    if (!type.isNull())
+      tp->setType(type);
+    return tp;
   }
 
   /// True if the type in this \c TypedPattern was propagated from a different
@@ -412,27 +423,10 @@ public:
   const Pattern *getSubPattern() const { return SubPattern; }
   void setSubPattern(Pattern *p) { SubPattern = p; }
 
-  TypeLoc &getTypeLoc() {
-    // If we have a delayed interface type, set our type from that.
-    if (getDelayedInterfaceType())
-      PatType.setType(getType());
+  TypeRepr *getTypeRepr() const { return PatTypeRepr; }
 
-    return PatType;
-  }
-  TypeLoc getTypeLoc() const {
-    // If we have a delayed interface type, set our type from that.
-    if (getDelayedInterfaceType())
-      PatType.setType(getType());
-
-    return PatType;
-  }
-
-  SourceLoc getLoc() const {
-    if (SubPattern->isImplicit())
-      return PatType.getSourceRange().Start;
-
-    return SubPattern->getLoc();
-  }
+  TypeLoc getTypeLoc() const;
+  SourceLoc getLoc() const;
   SourceRange getSourceRange() const;
 
   static bool classof(const Pattern *P) {

lib/AST/ASTWalker.cpp

@@ -1571,8 +1571,9 @@ Pattern *Traversal::visitTypedPattern(TypedPattern *P) {
   else
     return nullptr;
   if (!P->isImplicit())
-    if (doIt(P->getTypeLoc()))
-      return nullptr;
+    if (auto *TR = P->getTypeRepr())
+      if (doIt(TR))
+        return nullptr;
   return P;
 }
 

lib/AST/Pattern.cpp

@@ -400,6 +400,30 @@ SourceRange TuplePattern::getSourceRange() const {
            Fields.back().getPattern()->getEndLoc() };
 }
 
+TypedPattern::TypedPattern(Pattern *pattern, TypeRepr *tr,
+                           Optional<bool> implicit)
+  : Pattern(PatternKind::Typed), SubPattern(pattern), PatTypeRepr(tr) {
+  if (implicit ? *implicit : tr && !tr->getSourceRange().isValid())
+    setImplicit();
+  Bits.TypedPattern.IsPropagatedType = false;
+}
+
+TypeLoc TypedPattern::getTypeLoc() const {
+  TypeLoc loc = TypeLoc(PatTypeRepr);
+
+  if (hasType())
+    loc.setType(getType());
+
+  return loc;
+}
+
+SourceLoc TypedPattern::getLoc() const {
+  if (SubPattern->isImplicit() && PatTypeRepr)
+    return PatTypeRepr->getSourceRange().Start;
+
+  return SubPattern->getLoc();
+}
+
 SourceRange TypedPattern::getSourceRange() const {
   if (isImplicit() || isPropagatedType()) {
     // If a TypedPattern is implicit, then its type is definitely implicit, so
@@ -408,10 +432,14 @@ SourceRange TypedPattern::getSourceRange() const {
     return SubPattern->getSourceRange();
   }
 
-  if (SubPattern->isImplicit())
-    return PatType.getSourceRange();
+  if (!PatTypeRepr)
+    return SourceRange();
 
-  return { SubPattern->getSourceRange().Start, PatType.getSourceRange().End };
+  if (SubPattern->isImplicit())
+    return PatTypeRepr->getSourceRange();
+
+  return { SubPattern->getSourceRange().Start,
+           PatTypeRepr->getSourceRange().End };
 }
 
 /// Construct an ExprPattern.

lib/ClangImporter/ImportDecl.cpp

@@ -117,10 +117,7 @@ static Pattern *createTypedNamedPattern(VarDecl *decl) {
   Pattern *P = new (Ctx) NamedPattern(decl);
   P->setType(ty);
   P->setImplicit();
-  P = new (Ctx) TypedPattern(P, TypeLoc::withoutLoc(ty));
-  P->setType(ty);
-  P->setImplicit();
-  return P;
+  return TypedPattern::createImplicit(Ctx, P, ty);
 }
 
 /// Create a var member for this struct, along with its pattern binding, and add

lib/Parse/ParseDecl.cpp

@@ -4502,8 +4502,8 @@ VarDecl *Parser::parseDeclVarGetSet(Pattern *pattern,
     storage->setInvalid(true);
 
     Pattern *pattern =
-      new (Context) TypedPattern(new (Context) NamedPattern(storage),
-                                 TypeLoc::withoutLoc(ErrorType::get(Context)));
+      TypedPattern::createImplicit(Context, new (Context) NamedPattern(storage),
+                                   ErrorType::get(Context));
     PatternBindingEntry entry(pattern, /*EqualLoc*/ SourceLoc(),
                               /*Init*/ nullptr, /*InitContext*/ nullptr);
     auto binding = PatternBindingDecl::create(Context, StaticLoc,
@@ -5135,7 +5135,7 @@ Parser::parseDeclVar(ParseDeclOptions Flags,
           }
 
           TypedPattern *NewTP = new (Context) TypedPattern(PrevPat,
-                                                           TP->getTypeLoc());
+                                                           TP->getTypeRepr());
           NewTP->setPropagatedType();
           PBDEntries[i-1].setPattern(NewTP);
         }

lib/Sema/CodeSynthesis.cpp

@@ -1271,6 +1271,7 @@ static void synthesizeLazyGetterBody(TypeChecker &TC, AccessorDecl *Get,
                                    Ctx.getIdentifier("tmp2"),
                                    Get);
   Tmp2VD->setType(VD->getType());
+  Tmp2VD->setInterfaceType(VD->getInterfaceType());
   Tmp2VD->setImplicit();
 
 
@@ -1297,9 +1298,8 @@ static void synthesizeLazyGetterBody(TypeChecker &TC, AccessorDecl *Get,
   }
 
   Pattern *Tmp2PBDPattern = new (Ctx) NamedPattern(Tmp2VD, /*implicit*/true);
-  Tmp2PBDPattern = new (Ctx) TypedPattern(Tmp2PBDPattern,
-                                          TypeLoc::withoutLoc(VD->getType()),
-                                          /*implicit*/true);
+  Tmp2PBDPattern =
+    TypedPattern::createImplicit(Ctx, Tmp2PBDPattern, VD->getType());
 
   auto *Tmp2PBD = PatternBindingDecl::createImplicit(
       Ctx, StaticSpellingKind::None, Tmp2PBDPattern, InitValue, Get,
@@ -1447,9 +1447,8 @@ void TypeChecker::completePropertyBehaviorStorage(VarDecl *VD,
   // Create the pattern binding decl for the storage decl.  This will get
   // default initialized using the protocol's initStorage() method.
   Pattern *PBDPattern = new (Context) NamedPattern(Storage, /*implicit*/true);
-  PBDPattern = new (Context) TypedPattern(PBDPattern,
-                                  TypeLoc::withoutLoc(SubstStorageContextTy),
-                                  /*implicit*/true);
+  PBDPattern = TypedPattern::createImplicit(Context, PBDPattern,
+                                            SubstStorageContextTy);
   auto *PBD = PatternBindingDecl::createImplicit(
       Context, VD->getParentPatternBinding()->getStaticSpelling(), PBDPattern,
       InitStorageExpr, VD->getDeclContext(), /*VarLoc*/ VD->getLoc());
@@ -1740,9 +1739,7 @@ void TypeChecker::completeLazyVarImplementation(VarDecl *VD) {
   // Create the pattern binding decl for the storage decl.  This will get
   // default initialized to nil.
   Pattern *PBDPattern = new (Context) NamedPattern(Storage, /*implicit*/true);
-  PBDPattern = new (Context) TypedPattern(PBDPattern,
-                                          TypeLoc::withoutLoc(StorageTy),
-                                          /*implicit*/true);
+  PBDPattern = TypedPattern::createImplicit(Context, PBDPattern, StorageTy);
   auto *PBD = PatternBindingDecl::createImplicit(
       Context, StaticSpellingKind::None, PBDPattern, /*init*/ nullptr,
       VD->getDeclContext(), /*VarLoc*/ VD->getLoc());

lib/Sema/DerivedConformanceEquatableHashable.cpp

@@ -221,7 +221,7 @@ static DeclRefExpr *convertEnumToIndex(SmallVectorImpl<ASTNode> &stmts,
   // generate: var indexVar
   Pattern *indexPat = new (C) NamedPattern(indexVar, /*implicit*/ true);
   indexPat->setType(intType);
-  indexPat = new (C) TypedPattern(indexPat, TypeLoc::withoutLoc(intType));
+  indexPat = TypedPattern::createImplicit(C, indexPat, intType);
   indexPat->setType(intType);
   auto *indexBind = PatternBindingDecl::createImplicit(
       C, StaticSpellingKind::None, indexPat, /*InitExpr*/ nullptr, funcDecl);
@@ -1105,9 +1105,7 @@ static ValueDecl *deriveHashable_hashValue(DerivedConformance &derived) {
 
   Pattern *hashValuePat = new (C) NamedPattern(hashValueDecl, /*implicit*/true);
   hashValuePat->setType(intType);
-  hashValuePat
-    = new (C) TypedPattern(hashValuePat, TypeLoc::withoutLoc(intType),
-                           /*implicit*/ true);
+  hashValuePat = TypedPattern::createImplicit(C, hashValuePat, intType);
   hashValuePat->setType(intType);
 
   auto *patDecl = PatternBindingDecl::createImplicit(

lib/Sema/DerivedConformances.cpp

@@ -340,9 +340,7 @@ DerivedConformance::declareDerivedProperty(Identifier name,
   Pattern *propPat = new (C) NamedPattern(propDecl, /*implicit*/ true);
   propPat->setType(propertyContextType);
 
-  propPat = new (C) TypedPattern(propPat,
-                                 TypeLoc::withoutLoc(propertyContextType),
-                                 /*implicit*/ true);
+  propPat = TypedPattern::createImplicit(C, propPat, propertyContextType);
   propPat->setType(propertyContextType);
 
   auto *pbDecl = PatternBindingDecl::createImplicit(

lib/Sema/TypeCheckPattern.cpp

@@ -699,7 +699,7 @@ static bool validateTypedPattern(TypeChecker &TC,
   if (TP->hasType())
     return TP->getType()->hasError();
 
-  TypeLoc &TL = TP->getTypeLoc();
+  TypeLoc TL = TP->getTypeLoc();
   bool hadError = TC.validateType(TL, resolution, options);
 
   if (hadError) {

lib/Serialization/Deserialization.cpp

@@ -381,9 +381,11 @@ Expected<Pattern *> ModuleFile::readPattern(DeclContext *owningDC) {
       return subPattern;
     }
 
-    auto result = new (getContext()) TypedPattern(subPattern.get(), TypeLoc(),
+    auto type = getType(typeID);
+    auto result = new (getContext()) TypedPattern(subPattern.get(),
+                                                  /*typeRepr*/nullptr,
                                                   isImplicit);
-    recordPatternType(result, getType(typeID));
+    recordPatternType(result, type);
     restoreOffset.reset();
     return result;
   }

test/ModuleInterface/private-stored-members.swift

@@ -1,43 +1,40 @@
 // RUN: %empty-directory(%t)
 
 // RUN: %target-swift-frontend -emit-interface-path %t.swiftinterface -emit-module -o /dev/null %s
-// RUN: %FileCheck %s < %t.swiftinterface
+// RUN: %FileCheck %s < %t.swiftinterface --check-prefix CHECK --check-prefix COMMON
 
 // RUN: %target-swift-frontend -emit-interface-path %t-resilient.swiftinterface -enable-resilience -emit-module -o /dev/null %s
-// RUN: %FileCheck %s --check-prefix RESILIENT < %t-resilient.swiftinterface
+// RUN: %FileCheck %s --check-prefix RESILIENT --check-prefix COMMON < %t-resilient.swiftinterface
 
 // RUN: %target-swift-frontend -emit-module -o %t/Test.swiftmodule %t.swiftinterface -disable-objc-attr-requires-foundation-module
-// FIXME(rdar44144435): %target-swift-frontend -emit-module -o /dev/null -merge-modules %t/Test.swiftmodule -emit-interface-path - | %FileCheck %s
+// RUN: %target-swift-frontend -emit-module -o /dev/null -merge-modules %t/Test.swiftmodule -module-name Test -emit-interface-path - | %FileCheck %s --check-prefix CHECK --check-prefix COMMON
 
 // RUN: %target-swift-frontend -emit-module -o %t/TestResilient.swiftmodule -enable-resilience %t.swiftinterface -disable-objc-attr-requires-foundation-module
-// FIXME(rdar44144435): %target-swift-frontend -emit-module -o /dev/null -merge-modules %t/Test.swiftmodule -enable-resilience -emit-interface-path - | %FileCheck %s
+// RUN: %target-swift-frontend -emit-module -o /dev/null -merge-modules %t/TestResilient.swiftmodule -module-name TestResilient -enable-resilience -emit-interface-path - | %FileCheck %s --check-prefix RESILIENT --check-prefix COMMON
 
 
-// CHECK: struct MyStruct {{{$}}
-// RESILIENT: struct MyStruct {{{$}}
+// COMMON: struct MyStruct {{{$}}
 public struct MyStruct {
-// CHECK-NEXT: var publicVar: Int64{{$}}
-// RESILIENT-NEXT: var publicVar: Int64{{$}}
+// COMMON-NEXT: var publicVar: [[INT64:(Swift\.)?Int64]]{{$}}
   public var publicVar: Int64
 
-// CHECK-NEXT: let publicLet: Bool{{$}}
-// RESILIENT-NEXT: let publicLet: Bool{{$}}
+// COMMON-NEXT: let publicLet: [[BOOL:(Swift\.)?Bool]]{{$}}
   public let publicLet: Bool
 
-// CHECK-NEXT: internal var _: Int64{{$}}
-// RESILIENT-NOT: internal var _: Int64{{$}}
+// CHECK-NEXT: internal var _: [[INT64]]{{$}}
+// RESILIENT-NOT: internal var _: [[INT64]]{{$}}
   var internalVar: Int64
 
-// CHECK-NEXT: internal let _: Bool{{$}}
-// RESILIENT-NOT: internal let _: Bool{{$}}
+// CHECK-NEXT: internal let _: [[BOOL]]{{$}}
+// RESILIENT-NOT: internal let _: [[BOOL]]{{$}}
   let internalLet: Bool
 
-// CHECK-NEXT: private var _: Int64{{$}}
-// RESILIENT-NOT: private var _: Int64{{$}}
+// CHECK-NEXT: private var _: [[INT64]]{{$}}
+// RESILIENT-NOT: private var _: [[INT64]]{{$}}
   private var privateVar: Int64
 
-// CHECK-NEXT: private let _: Bool{{$}}
-// RESILIENT-NOT: private let _: Bool{{$}}
+// CHECK-NEXT: private let _: [[BOOL]]{{$}}
+// RESILIENT-NOT: private let _: [[BOOL]]{{$}}
   private let privateLet: Bool
 
 // CHECK-NOT: private var
@@ -50,39 +47,34 @@ public struct MyStruct {
 // RESILIENT-NOT: private static var
   private static var staticPrivateVar: Int64 = 0
 
-// CHECK-NEXT: var publicEndVar: Int64{{$}}
-// RESILIENT-NEXT: var publicEndVar: Int64{{$}}
+// COMMON: var publicEndVar: [[INT64]]{{$}}
   public var publicEndVar: Int64 = 0
 
-// CHECK: }{{$}}
-// RESILIENT: }{{$}}
+// COMMON: }{{$}}
 }
 
-// CHECK: class MyClass {{{$}}
-// RESILIENT: class MyClass {{{$}}
+// COMMON: class MyClass {{{$}}
 public class MyClass {
-// CHECK-NEXT: var publicVar: Int64{{$}}
-// RESILIENT-NEXT: var publicVar: Int64{{$}}
+// COMMON-NEXT: var publicVar: [[INT64]]{{$}}
   public var publicVar: Int64 = 0
 
-// CHECK-NEXT: let publicLet: Bool{{$}}
-// RESILIENT-NEXT: let publicLet: Bool{{$}}
+// COMMON-NEXT: let publicLet: [[BOOL]]{{$}}
   public let publicLet: Bool = true
 
-// CHECK-NEXT: internal var _: Int64{{$}}
-// RESILIENT-NOT: internal var _: Int64{{$}}
+// CHECK-NEXT: internal var _: [[INT64]]{{$}}
+// RESILIENT-NOT: internal var _: [[INT64]]{{$}}
   var internalVar: Int64 = 0
 
-// CHECK-NEXT: internal let _: Bool{{$}}
-// RESILIENT-NOT: internal let _: Bool{{$}}
+// CHECK-NEXT: internal let _: [[BOOL]]{{$}}
+// RESILIENT-NOT: internal let _: [[BOOL]]{{$}}
   let internalLet: Bool = true
 
-// CHECK-NEXT: private var _: Int64{{$}}
-// RESILIENT-NOT: private var _: Int64{{$}}
+// CHECK-NEXT: private var _: [[INT64]]{{$}}
+// RESILIENT-NOT: private var _: [[INT64]]{{$}}
   private var privateVar: Int64 = 0
 
-// CHECK-NEXT: private let _: Bool{{$}}
-// RESILIENT-NOT: private let _: Bool{{$}}
+// CHECK-NEXT: private let _: [[BOOL]]{{$}}
+// RESILIENT-NOT: private let _: [[BOOL]]{{$}}
   private let privateLet: Bool = true
 
 // CHECK-NOT: private var
@@ -95,12 +87,10 @@ public class MyClass {
 // RESILIENT-NOT: private static var
   private static var staticPrivateVar: Int64 = 0
 
-// CHECK-NEXT: var publicEndVar: Int64{{$}}
-// RESILIENT-NEXT: var publicEndVar: Int64{{$}}
+// COMMON: var publicEndVar: [[INT64]]{{$}}
   public var publicEndVar: Int64 = 0
 
   public init() {}
 
-// CHECK: }{{$}}
-// RESILIENT: }{{$}}
+// COMMON: }{{$}}
 }