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.

Author: tshortli

lib/Sema/TypeCheckAvailability.cpp

@@ -44,6 +44,9 @@
 #include "llvm/Support/SaveAndRestore.h"
 using namespace swift;
 
+static const Decl *
+concreteSyntaxDeclForAvailableAttribute(const Decl *AbstractSyntaxDecl);
+
 ExportContext::ExportContext(
     DeclContext *DC, AvailabilityRange runningOSVersion,
     FragileFunctionKind kind, bool spi, bool exported, bool implicit,
@@ -451,6 +454,8 @@ class TypeRefinementContextBuilder : private ASTWalker {
   };
   std::vector<ContextInfo> ContextStack;
 
+  llvm::SmallVector<const Decl *, 4> ConcreteDeclStack;
+
   /// Represents an entry in a stack of pending decl body type refinement
   /// contexts. TRCs in this stack should be pushed onto \p ContextStack when
   /// \p BodyStmt is encountered.
@@ -540,12 +545,27 @@ class TypeRefinementContextBuilder : private ASTWalker {
     return MacroWalking::Arguments;
   }
 
+  bool shouldSkipDecl(Decl *D) const {
+    // Implicit decls don't have source locations so they cannot have a TRC.
+    if (D->isImplicit())
+      return true;
+
+    // Only visit a node that has a corresponding concrete syntax node if we are
+    // already walking that concrete syntax node.
+    auto *concreteDecl = concreteSyntaxDeclForAvailableAttribute(D);
+    if (concreteDecl != D) {
+      if (ConcreteDeclStack.empty() || ConcreteDeclStack.back() != concreteDecl)
+        return true;
+    }
+
+    return false;
+  }
+
   PreWalkAction walkToDeclPre(Decl *D) override {
     PrettyStackTraceDecl trace(stackTraceAction(), D);
 
-    // Implicit decls don't have source locations so they cannot have a TRC.
-    if (D->isImplicit())
-      return Action::Continue();
+    if (shouldSkipDecl(D))
+      return Action::SkipNode();
 
     // The AST of this decl may not be ready to traverse yet if it hasn't been
     // full typechecked. If that's the case, we leave a placeholder node in the
@@ -561,10 +581,21 @@ class TypeRefinementContextBuilder : private ASTWalker {
 
     // Create TRCs that cover only the body of the declaration.
     buildContextsForBodyOfDecl(D);
+
+    // If this decl is the concrete syntax decl for some abstract syntax decl,
+    // push it onto the stack so that the abstract syntax decls may be visited.
+    auto *abstractDecl = abstractSyntaxDeclForAvailableAttribute(D);
+    if (abstractDecl != D) {
+      ConcreteDeclStack.push_back(D);
+    }
     return Action::Continue();
   }
 
   PostWalkAction walkToDeclPost(Decl *D) override {
+    if (!ConcreteDeclStack.empty() && ConcreteDeclStack.back() == D) {
+      ConcreteDeclStack.pop_back();
+    }
+
     while (ContextStack.back().ScopeNode.getAsDecl() == D) {
       ContextStack.pop_back();
     }
@@ -652,16 +683,9 @@ class TypeRefinementContextBuilder : private ASTWalker {
     if (isa<AbstractStorageDecl>(D) && D->getDeclContext()->isLocalContext())
       return nullptr;
 
-    // Don't introduce for variable declarations that have a parent pattern
-    // binding; all of the relevant information is on the pattern binding.
-    if (auto var = dyn_cast<VarDecl>(D)) {
-      if (var->getParentPatternBinding())
-        return nullptr;
-    }
-
-    // Don't introduce for enum element declarations. All the relevant
-    // information is on the enum case declaration.
-    if (isa<EnumElementDecl>(D))
+    // Don't introduce for abstract syntax nodes that have separate concrete
+    // syntax nodes. The TRC will be introduced for the concrete node instead.
+    if (concreteSyntaxDeclForAvailableAttribute(D) != D)
       return nullptr;
 
     // Declarations with an explicit availability attribute always get a TRC.
@@ -1686,7 +1710,8 @@ concreteSyntaxDeclForAvailableAttribute(const Decl *AbstractSyntaxDecl) {
   // event, multiple variables can be introduced with a single 'var'),
   // so suggest adding an attribute to the PatterningBindingDecl instead.
   if (auto *VD = dyn_cast<VarDecl>(AbstractSyntaxDecl)) {
-    return VD->getParentPatternBinding();
+    if (auto *PBD = VD->getParentPatternBinding())
+      return PBD;
   }
 
   // Similarly suggest applying the Fix-It to the parent enum case rather than

test/Sema/availability_refinement_contexts.swift

@@ -286,3 +286,20 @@ enum SomeEnum {
   @available(OSX 53, *)
   case b, c
 }
+
+// CHECK-NEXT: {{^}}  (decl_implicit version=50 decl=someComputedGlobalVar
+// CHECK-NEXT: {{^}}    (decl version=51 decl=_
+// CHECK-NEXT: {{^}}    (decl version=52 decl=_
+
+var someComputedGlobalVar: Int {
+  @available(OSX 51, *)
+  get { 1 }
+
+  @available(OSX 52, *)
+  set { }
+}
+
+// CHECK-NEXT: {{^}}  (decl version=51 decl=FinalDecl
+
+@available(OSX 51, *)
+typealias FinalDecl = Int