Skip to content

Commit 630c367

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 0565209 commit 630c367

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,
@@ -3987,6 +3988,13 @@ class SequenceExpr final : public Expr,
39873988
getElements()[i] = e;
39883989
}
39893990

3991+
bool isFolded() const {
3992+
return static_cast<bool>(Bits.SequenceExpr.IsFolded);
3993+
}
3994+
void setFolded(bool folded) {
3995+
Bits.SequenceExpr.IsFolded = static_cast<unsigned>(folded);
3996+
}
3997+
39903998
// Implement isa/cast/dyncast/etc.
39913999
static bool classof(const Expr *E) {
39924000
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
@@ -1183,6 +1183,7 @@ class PreCheckTarget final : public ASTWalker {
11831183
if (!result)
11841184
return Action::Stop();
11851185
// Already walked.
1186+
seqExpr->setFolded(true);
11861187
return Action::SkipNode(result);
11871188
}
11881189

lib/Sema/TypeCheckAvailability.cpp

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

539+
SequenceWalking getSequenceWalkingBehavior() const override {
540+
// Since availability scopes may be built at arbitrary times, the builder
541+
// may encounter ASTs where SequenceExprs still exist and have not been
542+
// folded, or it may encounter folded SequenceExprs that have not been
543+
// removed from the AST. When folded exprs are encountered, its important
544+
// to avoid walking into the same AST nodes twice.
545+
return SequenceWalking::OnlyWalkFirstOperatorWhenFolded;
546+
}
547+
539548
/// Check whether this declaration is within a macro expansion buffer that
540549
/// will have its own availability scope that will be lazily expanded.
541550
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)