Skip to content

Commit 87308e3

Browse files
committed
AST: Avoid creating duplicate AvailabilityScopes under SequenceExprs.
Since availability scopes may be built at arbitrary times, the builder may encounter ASTs where SequenceExprs still exist and have not been folded, or it may encounter folded SequenceExprs that have not been removed from the AST. To avoid a double visit, track whether a SequenceExpr is folded and then customize how ASTVisitor handles folded sequences. Resolves rdar://142824799 and #78567.
1 parent 6b2fb2e commit 87308e3

File tree

6 files changed

+66
-2
lines changed

6 files changed

+66
-2
lines changed

include/swift/AST/ASTWalker.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,19 @@ enum class QualifiedIdentTypeReprWalkingScheme {
133133
ASTOrderRecursive
134134
};
135135

136+
/// Specifies the behavior for walking SequenceExprs.
137+
enum class SequenceWalking {
138+
/// Walk into every element of the sequence, regardless of what state the
139+
/// sequence is in.
140+
Default,
141+
142+
/// If the sequence has been folded by type checking, only walk into the
143+
/// elements that represent the operator nodes. This will ensure that the walk
144+
/// does not visit the same AST nodes twice when it encounters a sequence that
145+
/// has already been folded but hasn't been removed from the AST.
146+
OnlyWalkFirstOperatorWhenFolded
147+
};
148+
136149
/// An abstract class used to traverse an AST.
137150
class ASTWalker {
138151
public:
@@ -616,6 +629,13 @@ class ASTWalker {
616629
return MacroWalking::ArgumentsAndExpansion;
617630
}
618631

632+
/// This method configures how the walker should walk into SequenceExprs.
633+
/// Needing to customize this behavior should be rare, as sequence expressions
634+
/// are only encountered in un-typechecked ASTs.
635+
virtual SequenceWalking getSequenceWalkingBehavior() const {
636+
return SequenceWalking::Default;
637+
}
638+
619639
/// This method determines whether the given declaration should be
620640
/// considered to be in a macro expansion context. It can be configured
621641
/// by subclasses.

include/swift/AST/Expr.h

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -368,9 +368,10 @@ class alignas(8) Expr : public ASTAllocated<Expr> {
368368
IsObjC : 1
369369
);
370370

371-
SWIFT_INLINE_BITFIELD_FULL(SequenceExpr, Expr, 32,
371+
SWIFT_INLINE_BITFIELD_FULL(SequenceExpr, Expr, 32+1,
372372
: NumPadBits,
373-
NumElements : 32
373+
NumElements : 32,
374+
IsFolded: 1
374375
);
375376

376377
SWIFT_INLINE_BITFIELD(OpaqueValueExpr, Expr, 1,
@@ -4002,6 +4003,13 @@ class SequenceExpr final : public Expr,
40024003
getElements()[i] = e;
40034004
}
40044005

4006+
bool isFolded() const {
4007+
return static_cast<bool>(Bits.SequenceExpr.IsFolded);
4008+
}
4009+
void setFolded(bool folded) {
4010+
Bits.SequenceExpr.IsFolded = static_cast<unsigned>(folded);
4011+
}
4012+
40054013
// Implement isa/cast/dyncast/etc.
40064014
static bool classof(const Expr *E) {
40074015
return E->getKind() == ExprKind::Sequence;

lib/AST/ASTWalker.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -955,6 +955,16 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
955955
}
956956

957957
Expr *visitSequenceExpr(SequenceExpr *E) {
958+
if (Walker.getSequenceWalkingBehavior() ==
959+
SequenceWalking::OnlyWalkFirstOperatorWhenFolded &&
960+
E->isFolded()) {
961+
if (E->getNumElements() > 1) {
962+
if (Expr *Elt = doIt(E->getElement(1)))
963+
E->setElement(1, Elt);
964+
}
965+
return E;
966+
}
967+
958968
for (unsigned i = 0, e = E->getNumElements(); i != e; ++i)
959969
if (Expr *Elt = doIt(E->getElement(i)))
960970
E->setElement(i, Elt);

lib/Sema/PreCheckTarget.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1166,6 +1166,7 @@ class PreCheckTarget final : public ASTWalker {
11661166
if (!result)
11671167
return Action::Stop();
11681168
// Already walked.
1169+
seqExpr->setFolded(true);
11691170
return Action::SkipNode(result);
11701171
}
11711172

lib/Sema/TypeCheckAvailability.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,15 @@ class AvailabilityScopeBuilder : private ASTWalker {
538538
return MacroWalking::Arguments;
539539
}
540540

541+
SequenceWalking getSequenceWalkingBehavior() const override {
542+
// Since availability scopes may be built at arbitrary times, the builder
543+
// may encounter ASTs where SequenceExprs still exist and have not been
544+
// folded, or it may encounter folded SequenceExprs that have not been
545+
// removed from the AST. When folded exprs are encountered, its important
546+
// to avoid walking into the same AST nodes twice.
547+
return SequenceWalking::OnlyWalkFirstOperatorWhenFolded;
548+
}
549+
541550
/// Check whether this declaration is within a macro expansion buffer that
542551
/// will have its own availability scope that will be lazily expanded.
543552
bool isDeclInMacroExpansion(Decl *decl) const override {

test/Sema/availability_scopes.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,22 @@ func testStringInterpolation() {
321321
"""
322322
}
323323

324+
// CHECK-NEXT: {{^}} (decl_implicit version=50 decl=result
325+
// CHECK-NEXT: {{^}} (decl_implicit version=50 decl=unusedA
326+
// CHECK-NEXT: {{^}} (decl_implicit version=50 decl=unusedB
327+
328+
func testSequenceExprs(b: Bool, x: Int?) {
329+
let result = b
330+
? x.map {
331+
let unusedA: Int
332+
return $0
333+
}
334+
: x.map {
335+
let unusedB: Int
336+
return $0
337+
}
338+
}
339+
324340
// CHECK-NEXT: {{^}} (decl version=50 unavailable=macOS decl=unavailableOnMacOS()
325341
// CHECK-NEXT: {{^}} (decl_implicit version=50 unavailable=macOS decl=x
326342

0 commit comments

Comments
 (0)