diff --git a/include/swift/AST/ASTWalker.h b/include/swift/AST/ASTWalker.h index dbc76128fe9..e8f42599f1f 100644 --- a/include/swift/AST/ASTWalker.h +++ b/include/swift/AST/ASTWalker.h @@ -133,19 +133,6 @@ enum class QualifiedIdentTypeReprWalkingScheme { ASTOrderRecursive }; -/// Specifies the behavior for walking SequenceExprs. -enum class SequenceWalking { - /// Walk into every element of the sequence, regardless of what state the - /// sequence is in. - Default, - - /// If the sequence has been folded by type checking, only walk into the - /// elements that represent the operator nodes. This will ensure that the walk - /// does not visit the same AST nodes twice when it encounters a sequence that - /// has already been folded but hasn't been removed from the AST. - OnlyWalkFirstOperatorWhenFolded -}; - /// An abstract class used to traverse an AST. class ASTWalker { public: @@ -629,13 +616,6 @@ public: return MacroWalking::ArgumentsAndExpansion; } - /// This method configures how the walker should walk into SequenceExprs. - /// Needing to customize this behavior should be rare, as sequence expressions - /// are only encountered in un-typechecked ASTs. - virtual SequenceWalking getSequenceWalkingBehavior() const { - return SequenceWalking::Default; - } - /// This method determines whether the given declaration should be /// considered to be in a macro expansion context. It can be configured /// by subclasses. diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h index 3db4a881299..8026757fc0a 100644 --- a/include/swift/AST/Expr.h +++ b/include/swift/AST/Expr.h @@ -368,10 +368,9 @@ protected: IsObjC : 1 ); - SWIFT_INLINE_BITFIELD_FULL(SequenceExpr, Expr, 32+1, + SWIFT_INLINE_BITFIELD_FULL(SequenceExpr, Expr, 32, : NumPadBits, - NumElements : 32, - IsFolded: 1 + NumElements : 32 ); SWIFT_INLINE_BITFIELD(OpaqueValueExpr, Expr, 1, @@ -3970,6 +3969,9 @@ class SequenceExpr final : public Expr, private llvm::TrailingObjects { friend TrailingObjects; + /// The cached folded expression. + Expr *FoldedExpr = nullptr; + SequenceExpr(ArrayRef elements) : Expr(ExprKind::Sequence, /*Implicit=*/false) { Bits.SequenceExpr.NumElements = elements.size(); @@ -4005,11 +4007,14 @@ public: getElements()[i] = e; } - bool isFolded() const { - return static_cast(Bits.SequenceExpr.IsFolded); + /// Retrieve the folded expression, or \c nullptr if the SequencExpr has + /// not yet been folded. + Expr *getFoldedExpr() const { + return FoldedExpr; } - void setFolded(bool folded) { - Bits.SequenceExpr.IsFolded = static_cast(folded); + + void setFoldedExpr(Expr *foldedExpr) { + FoldedExpr = foldedExpr; } // Implement isa/cast/dyncast/etc. diff --git a/lib/AST/ASTWalker.cpp b/lib/AST/ASTWalker.cpp index 69db7cb5255..32c9fdf67aa 100644 --- a/lib/AST/ASTWalker.cpp +++ b/lib/AST/ASTWalker.cpp @@ -950,13 +950,18 @@ class Traversal : public ASTVisitorisFolded()) { - if (E->getNumElements() > 1) { - if (Expr *Elt = doIt(E->getElement(1))) - E->setElement(1, Elt); - } + // After folding, the SequenceExpr elements can contain a broken mess of + // partially folded and/or duplicate nodes (since folding can mutate nodes + // in-place). We remove the SequenceExpr from the AST after folding, but + // there's still a period of time during pre-checking when it's still in the + // AST. To avoid issues for e.g ASTScope and availability scope + // construction, only walk the folded expression if we have it. + if (auto *foldedExpr = E->getFoldedExpr()) { + auto *newExpr = doIt(foldedExpr); + if (!newExpr) + return nullptr; + + E->setFoldedExpr(newExpr); return E; } diff --git a/lib/AST/AvailabilityScopeBuilder.cpp b/lib/AST/AvailabilityScopeBuilder.cpp index 7748855f67e..69f0796e269 100644 --- a/lib/AST/AvailabilityScopeBuilder.cpp +++ b/lib/AST/AvailabilityScopeBuilder.cpp @@ -176,15 +176,6 @@ private: return MacroWalking::Arguments; } - SequenceWalking getSequenceWalkingBehavior() const override { - // 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. When folded exprs are encountered, its important - // to avoid walking into the same AST nodes twice. - return SequenceWalking::OnlyWalkFirstOperatorWhenFolded; - } - /// Check whether this declaration is within a macro expansion buffer that /// will have its own availability scope that will be lazily expanded. bool isDeclInMacroExpansion(Decl *decl) const override { diff --git a/lib/Sema/PreCheckTarget.cpp b/lib/Sema/PreCheckTarget.cpp index de3e3624a74..b459cf00320 100644 --- a/lib/Sema/PreCheckTarget.cpp +++ b/lib/Sema/PreCheckTarget.cpp @@ -1168,7 +1168,6 @@ public: if (!result) return Action::Stop(); // Already walked. - seqExpr->setFolded(true); return Action::SkipNode(result); } diff --git a/lib/Sema/TypeCheckExpr.cpp b/lib/Sema/TypeCheckExpr.cpp index 72360b08663..19c3e194b83 100644 --- a/lib/Sema/TypeCheckExpr.cpp +++ b/lib/Sema/TypeCheckExpr.cpp @@ -649,6 +649,7 @@ Expr *TypeChecker::foldSequence(SequenceExpr *expr, DeclContext *dc) { Expr *Result = ::foldSequence(dc, LHS, Elts, PrecedenceBound()); assert(Elts.empty()); + expr->setFoldedExpr(Result); return Result; } diff --git a/test/ClangImporter/objc_failable_inits.swift b/test/ClangImporter/objc_failable_inits.swift index b4d64beae44..8b361ed7b52 100644 --- a/test/ClangImporter/objc_failable_inits.swift +++ b/test/ClangImporter/objc_failable_inits.swift @@ -13,6 +13,10 @@ func testDictionary() { func testString() throws { // Optional let stringOpt = NSString(path: "blah", encoding: 0) + // expected-note@-1 {{short-circuit using 'guard' to exit this function early if the optional value contains 'nil'}} + // expected-note@-2 {{coalesce using '??' to provide a default when the optional value contains 'nil'}} + // expected-note@-3 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}} + _ = stringOpt as NSString // expected-error{{value of optional type 'NSString?' must be unwrapped to a value of type 'NSString'}} // expected-note@-1 {{coalesce using '??' to provide a default when the optional value contains 'nil'}} // expected-note@-2 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}} diff --git a/test/NameLookup/scope_map_rdar147751795.swift b/test/NameLookup/scope_map_rdar147751795.swift new file mode 100644 index 00000000000..d7bdfc736f8 --- /dev/null +++ b/test/NameLookup/scope_map_rdar147751795.swift @@ -0,0 +1,19 @@ +// Note: test of the scope map. All of these tests are line- and +// column-sensitive, so any additions should go at the end. + +struct X {} +var y: () -> X + +struct S { + let x: () = y = {() -> X in + return .init() + } +} + +// RUN: %target-swift-frontend -dump-scope-maps expanded %s 2>&1 | %FileCheck %s + +// rdar://147751795 - Make sure we don't end up with a duplicate closure scope. +// CHECK: PatternEntryInitializerScope {{.*}}, [8:15 - 10:3] entry 0 'x' +// CHECK-NEXT: `-ClosureParametersScope {{.*}}, [8:28 - 10:3] +// CHECK-NEXT: `-BraceStmtScope {{.*}}, [8:28 - 10:3] +// CHECK-EMPTY: