Skip to content

Commit 07a987c

Browse files
committed
Sema: Build TypeRefinementContexts correctly for VarDecls.
The `TypeRefinementContextBuilder` could visit a given `VarDecl` in the AST multiple times when the `VarDecl` has a parent `PatternBindingDecl`, resulting in duplicate TRC nodes.
1 parent f658648 commit 07a987c

File tree

2 files changed

+56
-14
lines changed

2 files changed

+56
-14
lines changed

lib/Sema/TypeCheckAvailability.cpp

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@
4444
#include "llvm/Support/SaveAndRestore.h"
4545
using namespace swift;
4646

47+
static const Decl *
48+
concreteSyntaxDeclForAvailableAttribute(const Decl *AbstractSyntaxDecl);
49+
4750
ExportContext::ExportContext(
4851
DeclContext *DC, AvailabilityRange runningOSVersion,
4952
FragileFunctionKind kind, bool spi, bool exported, bool implicit,
@@ -451,6 +454,8 @@ class TypeRefinementContextBuilder : private ASTWalker {
451454
};
452455
std::vector<ContextInfo> ContextStack;
453456

457+
llvm::SmallVector<const Decl *, 4> ConcreteDeclStack;
458+
454459
/// Represents an entry in a stack of pending decl body type refinement
455460
/// contexts. TRCs in this stack should be pushed onto \p ContextStack when
456461
/// \p BodyStmt is encountered.
@@ -540,12 +545,27 @@ class TypeRefinementContextBuilder : private ASTWalker {
540545
return MacroWalking::Arguments;
541546
}
542547

548+
bool shouldSkipDecl(Decl *D) const {
549+
// Implicit decls don't have source locations so they cannot have a TRC.
550+
if (D->isImplicit())
551+
return true;
552+
553+
// Only visit a node that has a corresponding concrete syntax node if we are
554+
// already walking that concrete syntax node.
555+
auto *concreteDecl = concreteSyntaxDeclForAvailableAttribute(D);
556+
if (concreteDecl != D) {
557+
if (ConcreteDeclStack.empty() || ConcreteDeclStack.back() != concreteDecl)
558+
return true;
559+
}
560+
561+
return false;
562+
}
563+
543564
PreWalkAction walkToDeclPre(Decl *D) override {
544565
PrettyStackTraceDecl trace(stackTraceAction(), D);
545566

546-
// Implicit decls don't have source locations so they cannot have a TRC.
547-
if (D->isImplicit())
548-
return Action::Continue();
567+
if (shouldSkipDecl(D))
568+
return Action::SkipNode();
549569

550570
// The AST of this decl may not be ready to traverse yet if it hasn't been
551571
// full typechecked. If that's the case, we leave a placeholder node in the
@@ -561,10 +581,21 @@ class TypeRefinementContextBuilder : private ASTWalker {
561581

562582
// Create TRCs that cover only the body of the declaration.
563583
buildContextsForBodyOfDecl(D);
584+
585+
// If this decl is the concrete syntax decl for some abstract syntax decl,
586+
// push it onto the stack so that the abstract syntax decls may be visited.
587+
auto *abstractDecl = abstractSyntaxDeclForAvailableAttribute(D);
588+
if (abstractDecl != D) {
589+
ConcreteDeclStack.push_back(D);
590+
}
564591
return Action::Continue();
565592
}
566593

567594
PostWalkAction walkToDeclPost(Decl *D) override {
595+
if (!ConcreteDeclStack.empty() && ConcreteDeclStack.back() == D) {
596+
ConcreteDeclStack.pop_back();
597+
}
598+
568599
while (ContextStack.back().ScopeNode.getAsDecl() == D) {
569600
ContextStack.pop_back();
570601
}
@@ -652,16 +683,9 @@ class TypeRefinementContextBuilder : private ASTWalker {
652683
if (isa<AbstractStorageDecl>(D) && D->getDeclContext()->isLocalContext())
653684
return nullptr;
654685

655-
// Don't introduce for variable declarations that have a parent pattern
656-
// binding; all of the relevant information is on the pattern binding.
657-
if (auto var = dyn_cast<VarDecl>(D)) {
658-
if (var->getParentPatternBinding())
659-
return nullptr;
660-
}
661-
662-
// Don't introduce for enum element declarations. All the relevant
663-
// information is on the enum case declaration.
664-
if (isa<EnumElementDecl>(D))
686+
// Don't introduce for abstract syntax nodes that have separate concrete
687+
// syntax nodes. The TRC will be introduced for the concrete node instead.
688+
if (concreteSyntaxDeclForAvailableAttribute(D) != D)
665689
return nullptr;
666690

667691
// Declarations with an explicit availability attribute always get a TRC.
@@ -1686,7 +1710,8 @@ concreteSyntaxDeclForAvailableAttribute(const Decl *AbstractSyntaxDecl) {
16861710
// event, multiple variables can be introduced with a single 'var'),
16871711
// so suggest adding an attribute to the PatterningBindingDecl instead.
16881712
if (auto *VD = dyn_cast<VarDecl>(AbstractSyntaxDecl)) {
1689-
return VD->getParentPatternBinding();
1713+
if (auto *PBD = VD->getParentPatternBinding())
1714+
return PBD;
16901715
}
16911716

16921717
// Similarly suggest applying the Fix-It to the parent enum case rather than

test/Sema/availability_refinement_contexts.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,3 +286,20 @@ enum SomeEnum {
286286
@available(OSX 53, *)
287287
case b, c
288288
}
289+
290+
// CHECK-NEXT: {{^}} (decl_implicit version=50 decl=someComputedGlobalVar
291+
// CHECK-NEXT: {{^}} (decl version=51 decl=_
292+
// CHECK-NEXT: {{^}} (decl version=52 decl=_
293+
294+
var someComputedGlobalVar: Int {
295+
@available(OSX 51, *)
296+
get { 1 }
297+
298+
@available(OSX 52, *)
299+
set { }
300+
}
301+
302+
// CHECK-NEXT: {{^}} (decl version=51 decl=FinalDecl
303+
304+
@available(OSX 51, *)
305+
typealias FinalDecl = Int

0 commit comments

Comments
 (0)