[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
diff --git a/include/swift/AST/Pattern.h b/include/swift/AST/Pattern.h
index 2210c0c..5579d8a 100644
--- a/include/swift/AST/Pattern.h
+++ b/include/swift/AST/Pattern.h
@@ -383,14 +383,25 @@
/// 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 @@
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) {
diff --git a/lib/AST/ASTWalker.cpp b/lib/AST/ASTWalker.cpp
index e390cd2..062eef5 100644
--- a/lib/AST/ASTWalker.cpp
+++ b/lib/AST/ASTWalker.cpp
@@ -1571,8 +1571,9 @@
else
return nullptr;
if (!P->isImplicit())
- if (doIt(P->getTypeLoc()))
- return nullptr;
+ if (auto *TR = P->getTypeRepr())
+ if (doIt(TR))
+ return nullptr;
return P;
}
diff --git a/lib/AST/Pattern.cpp b/lib/AST/Pattern.cpp
index 587ed12..0d4cf765 100644
--- a/lib/AST/Pattern.cpp
+++ b/lib/AST/Pattern.cpp
@@ -400,6 +400,30 @@
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 @@
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.
diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp
index 666d925..fceaf9f 100644
--- a/lib/ClangImporter/ImportDecl.cpp
+++ b/lib/ClangImporter/ImportDecl.cpp
@@ -117,10 +117,7 @@
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
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
index 50aa2ee..6a05539 100644
--- a/lib/Parse/ParseDecl.cpp
+++ b/lib/Parse/ParseDecl.cpp
@@ -4502,8 +4502,8 @@
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 @@
}
TypedPattern *NewTP = new (Context) TypedPattern(PrevPat,
- TP->getTypeLoc());
+ TP->getTypeRepr());
NewTP->setPropagatedType();
PBDEntries[i-1].setPattern(NewTP);
}
diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp
index 4030b09..fa4a348 100644
--- a/lib/Sema/CodeSynthesis.cpp
+++ b/lib/Sema/CodeSynthesis.cpp
@@ -1271,6 +1271,7 @@
Ctx.getIdentifier("tmp2"),
Get);
Tmp2VD->setType(VD->getType());
+ Tmp2VD->setInterfaceType(VD->getInterfaceType());
Tmp2VD->setImplicit();
@@ -1297,9 +1298,8 @@
}
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 @@
// 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 @@
// 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());
diff --git a/lib/Sema/DerivedConformanceEquatableHashable.cpp b/lib/Sema/DerivedConformanceEquatableHashable.cpp
index 09e635f..1ab14a8 100644
--- a/lib/Sema/DerivedConformanceEquatableHashable.cpp
+++ b/lib/Sema/DerivedConformanceEquatableHashable.cpp
@@ -221,7 +221,7 @@
// 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 @@
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(
diff --git a/lib/Sema/DerivedConformances.cpp b/lib/Sema/DerivedConformances.cpp
index 8ff09fd..af92c8c 100644
--- a/lib/Sema/DerivedConformances.cpp
+++ b/lib/Sema/DerivedConformances.cpp
@@ -340,9 +340,7 @@
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(
diff --git a/lib/Sema/TypeCheckPattern.cpp b/lib/Sema/TypeCheckPattern.cpp
index f4abb52..d83e1eb 100644
--- a/lib/Sema/TypeCheckPattern.cpp
+++ b/lib/Sema/TypeCheckPattern.cpp
@@ -699,7 +699,7 @@
if (TP->hasType())
return TP->getType()->hasError();
- TypeLoc &TL = TP->getTypeLoc();
+ TypeLoc TL = TP->getTypeLoc();
bool hadError = TC.validateType(TL, resolution, options);
if (hadError) {
diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp
index 129b385..c887a7d 100644
--- a/lib/Serialization/Deserialization.cpp
+++ b/lib/Serialization/Deserialization.cpp
@@ -381,9 +381,11 @@
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;
}
diff --git a/test/ModuleInterface/private-stored-members.swift b/test/ModuleInterface/private-stored-members.swift
index 0512a42..c2f4936 100644
--- a/test/ModuleInterface/private-stored-members.swift
+++ b/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 @@
// 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 @@
// 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: }{{$}}
}
