Merge pull request swiftlang#22460 from jrose-apple/i-cannot-contain-my-excitement

Extend transitive availability checking to initial value expressions (better version)

Author: jrose-apple

lib/AST/ASTVerifier.cpp

@@ -540,11 +540,19 @@ class Verifier : public ASTWalker {
     void verifyParsed(Stmt *S) {}
     void verifyParsed(Pattern *P) {}
     void verifyParsed(Decl *D) {
+      PrettyStackTraceDecl debugStack("verifying ", D);
       if (!D->getDeclContext()) {
-        Out << "every Decl should have a DeclContext";
-        PrettyStackTraceDecl debugStack("verifying DeclContext", D);
+        Out << "every Decl should have a DeclContext\n";
         abort();
       }
+      if (auto *DC = dyn_cast<DeclContext>(D)) {
+        if (D->getDeclContext() != DC->getParent()) {
+          Out << "Decl's DeclContext not in sync with DeclContext's parent\n";
+          D->getDeclContext()->dumpContext();
+          DC->getParent()->dumpContext();
+          abort();
+        }
+      }
     }
     template<typename T>
     void verifyParsedBase(T ASTNode) {

lib/Sema/TypeCheckAvailability.cpp

@@ -18,6 +18,7 @@
 #include "TypeChecker.h"
 #include "MiscDiagnostics.h"
 #include "swift/AST/ASTWalker.h"
+#include "swift/AST/Initializer.h"
 #include "swift/AST/NameLookup.h"
 #include "swift/AST/Pattern.h"
 #include "swift/AST/TypeRefinementContext.h"
@@ -856,27 +857,41 @@ static Optional<ASTNode> findInnermostAncestor(
 static const Decl *findContainingDeclaration(SourceRange ReferenceRange,
                                              const DeclContext *ReferenceDC,
                                              const SourceManager &SM) {
-  if (const Decl *D = ReferenceDC->getInnermostDeclarationDeclContext())
+  auto ContainsReferenceRange = [&](const Decl *D) -> bool {
+    if (ReferenceRange.isInvalid())
+      return false;
+    return SM.rangeContains(D->getSourceRange(), ReferenceRange);
+  };
+
+  if (const Decl *D = ReferenceDC->getInnermostDeclarationDeclContext()) {
+    // If we have an inner declaration context, see if we can narrow the search
+    // down to one of its members. This is important for properties, which don't
+    // count as DeclContexts of their own but which can still introduce
+    // availability.
+    if (auto *IDC = dyn_cast<IterableDeclContext>(D)) {
+      auto BestMember = llvm::find_if(IDC->getMembers(),
+                                      ContainsReferenceRange);
+      if (BestMember != IDC->getMembers().end())
+        return *BestMember;
+    }
     return D;
+  }
 
-  // We couldn't find a suitable node by climbing the DeclContext
-  // hierarchy, so fall back to looking for a top-level declaration
-  // that contains the reference range. We will hit this case for
-  // top-level elements that do not themselves introduce DeclContexts,
-  // such as extensions and global variables. If we don't have a reference
-  // range, there is nothing we can do, so return null.
+  // We couldn't find a suitable node by climbing the DeclContext hierarchy, so
+  // fall back to looking for a top-level declaration that contains the
+  // reference range. We will hit this case for top-level elements that do not
+  // themselves introduce DeclContexts, such as global variables. If we don't
+  // have a reference range, there is nothing we can do, so return null.
   if (ReferenceRange.isInvalid())
     return nullptr;
 
   SourceFile *SF = ReferenceDC->getParentSourceFile();
   if (!SF)
     return nullptr;
 
-  for (Decl *D : SF->Decls) {
-    if (SM.rangeContains(D->getSourceRange(), ReferenceRange)) {
-      return D;
-    }
-  }
+  auto BestTopLevelDecl = llvm::find_if(SF->Decls, ContainsReferenceRange);
+  if (BestTopLevelDecl != SF->Decls.end())
+    return *BestTopLevelDecl;
 
   return nullptr;
 }
@@ -929,9 +944,13 @@ abstractSyntaxDeclForAvailableAttribute(const Decl *ConcreteSyntaxDecl) {
     // binding.
     ArrayRef<PatternBindingEntry> Entries = PBD->getPatternList();
     if (!Entries.empty()) {
-      VarDecl *VD = Entries.front().getPattern()->getSingleVar();
-      if (VD)
-        return VD;
+      const VarDecl *AnyVD = nullptr;
+      // FIXME: This is wasteful; we only need the first variable.
+      Entries.front().getPattern()->forEachVariable([&](const VarDecl *VD) {
+        AnyVD = VD;
+      });
+      if (AnyVD)
+        return AnyVD;
     }
   } else if (auto *ECD = dyn_cast<EnumCaseDecl>(ConcreteSyntaxDecl)) {
     // Similar to the PatternBindingDecl case above, we return the
@@ -1091,35 +1110,17 @@ static void findAvailabilityFixItNodes(SourceRange ReferenceRange,
   FoundVersionCheckNode =
       findInnermostAncestor(ReferenceRange, SM, SearchRoot, IsGuardable);
 
-  // Find some Decl that contains the reference range. We use this declaration
-  // as a starting place to climb the DeclContext hierarchy to find
-  // places to suggest adding @available() annotations.
-  InnermostAncestorFinder::MatchPredicate IsDeclaration = [](
-      ASTNode Node, ASTWalker::ParentTy Parent) { return Node.is<Decl *>(); };
-
-  Optional<ASTNode> FoundDeclarationNode =
-      findInnermostAncestor(ReferenceRange, SM, SearchRoot, IsDeclaration);
-
-  const Decl *ContainingDecl = nullptr;
-  if (FoundDeclarationNode.hasValue()) {
-    ContainingDecl = FoundDeclarationNode.getValue().get<Decl *>();
-  }
-
-  if (!ContainingDecl) {
-    ContainingDecl = ReferenceDC->getInnermostMethodContext();
-  }
-
   // Try to find declarations on which @available attributes can be added.
   // The heuristics for finding these declarations are biased towards deeper
   // nodes in the AST to limit the scope of suggested availability regions
   // and provide a better IDE experience (it can get jumpy if Fix-It locations
   // are far away from the error needing the Fix-It).
-  if (ContainingDecl) {
+  if (DeclarationToSearch) {
     FoundMemberLevelDecl =
-        ancestorMemberLevelDeclForAvailabilityFixit(ContainingDecl);
+        ancestorMemberLevelDeclForAvailabilityFixit(DeclarationToSearch);
 
     FoundTypeLevelDecl =
-        ancestorTypeLevelDeclForAvailabilityFixit(ContainingDecl);
+        ancestorTypeLevelDeclForAvailabilityFixit(DeclarationToSearch);
   }
 }
 
@@ -1408,7 +1409,7 @@ someEnclosingDeclMatches(SourceRange ReferenceRange,
   // Climb the DeclContext hierarchy to see if any of the containing
   // declarations matches the predicate.
   const DeclContext *DC = ReferenceDC;
-  do {
+  while (true) {
     auto *D = DC->getInnermostDeclarationDeclContext();
     if (!D)
       break;
@@ -1424,20 +1425,15 @@ someEnclosingDeclMatches(SourceRange ReferenceRange,
         return true;
     }
 
-    DC = DC->getParent();
-  } while (DC);
+    DC = D->getDeclContext();
+  }
 
   // Search the AST starting from our innermost declaration context to see if
   // if the reference is inside a property declaration but not inside an
   // accessor (this can happen for the TypeRepr for the declared type of a
   // property, for example).
   // We can't rely on the DeclContext hierarchy climb above because properties
-  // do not introduce a new DeclContext. This search is potentially slow, so we
-  // do it last and only if the reference declaration context is a
-  // type or global context.
-
-  if (!ReferenceDC->isTypeContext() && !ReferenceDC->isModuleScopeContext())
-    return false;
+  // do not introduce a new DeclContext.
 
   // Don't search for a containing declaration if we don't have a source range.
   if (ReferenceRange.isInvalid())
@@ -1448,28 +1444,11 @@ someEnclosingDeclMatches(SourceRange ReferenceRange,
       findContainingDeclaration(ReferenceRange, ReferenceDC, Ctx.SourceMgr);
 
   // We may not be able to find a declaration to search if the ReferenceRange
-  // is invalid (i.e., we are in synthesized code).
+  // isn't useful (i.e., we are in synthesized code).
   if (!DeclToSearch)
     return false;
 
-  InnermostAncestorFinder::MatchPredicate IsDeclaration =
-      [](ASTNode Node, ASTWalker::ParentTy Parent) {
-        return Node.is<Decl *>();
-  };
-
-  Optional<ASTNode> FoundDeclarationNode =
-      findInnermostAncestor(ReferenceRange, Ctx.SourceMgr,
-                            const_cast<Decl *>(DeclToSearch), IsDeclaration);
-
-  if (FoundDeclarationNode.hasValue()) {
-    const Decl *D = FoundDeclarationNode.getValue().get<Decl *>();
-    D = abstractSyntaxDeclForAvailableAttribute(D);
-    if (Pred(D)) {
-      return true;
-    }
-  }
-
-  return false;
+  return Pred(abstractSyntaxDeclForAvailableAttribute(DeclToSearch));
 }
 
 /// Returns true if the reference or any of its parents is an

test/attr/attr_availability_transitive_osx.swift

@@ -1,14 +1,18 @@
-// RUN: %target-typecheck-verify-swift
+// RUN: %target-typecheck-verify-swift -parse-as-library
 // REQUIRES: OS=macosx
 
 // Allow referencing unavailable API in situations where the caller is marked unavailable in the same circumstances.
 
 @available(OSX, unavailable)
-func osx() {} // expected-note 2{{'osx()' has been explicitly marked unavailable here}}
+@discardableResult
+func osx() -> Int { return 0 } // expected-note * {{'osx()' has been explicitly marked unavailable here}}
 
 @available(OSXApplicationExtension, unavailable)
 func osx_extension() {}
 
+@available(OSX, unavailable)
+func osx_pair() -> (Int, Int) { return (0, 0) } // expected-note * {{'osx_pair()' has been explicitly marked unavailable here}}
+
 func call_osx_extension() {
     osx_extension() // OK; osx_extension is only unavailable if -application-extension is passed.
 }
@@ -35,3 +39,84 @@ func osx_extension_call_osx_extension() {
 func osx_extension_call_osx() {
     osx() // expected-error {{'osx()' is unavailable}}
 }
+
+@available(OSX, unavailable)
+var osx_init_osx = osx() // OK
+
+@available(OSXApplicationExtension, unavailable)
+var osx_extension_init_osx = osx() // expected-error {{'osx()' is unavailable}}
+
+@available(OSX, unavailable)
+var osx_inner_init_osx = { let inner_var = osx() } // OK
+
+// FIXME: I'm not sure why this produces two errors instead of just one.
+@available(OSXApplicationExtension, unavailable)
+var osx_extension_inner_init_osx = { let inner_var = osx() } // expected-error 2 {{'osx()' is unavailable}}
+
+struct Outer {
+  @available(OSX, unavailable)
+  var osx_init_osx = osx() // OK
+
+  @available(OSX, unavailable)
+  lazy var osx_lazy_osx = osx() // OK
+
+  @available(OSXApplicationExtension, unavailable)
+  var osx_extension_init_osx = osx() // expected-error {{'osx()' is unavailable}}
+
+  @available(OSXApplicationExtension, unavailable)
+  var osx_extension_lazy_osx = osx() // expected-error {{'osx()' is unavailable}}
+
+  @available(OSX, unavailable)
+  var osx_init_multi1_osx = osx(), osx_init_multi2_osx = osx() // OK
+
+  @available(OSXApplicationExtension, unavailable)
+  var osx_extension_init_multi1_osx = osx(), osx_extension_init_multi2_osx = osx() // expected-error 2 {{'osx()' is unavailable}}
+
+  @available(OSX, unavailable)
+  var (osx_init_deconstruct1_osx, osx_init_deconstruct2_osx) = osx_pair() // OK
+
+  @available(OSXApplicationExtension, unavailable)
+  var (osx_extension_init_deconstruct1_osx, osx_extension_init_deconstruct2_osx) = osx_pair() // expected-error {{'osx_pair()' is unavailable}}
+
+  @available(OSX, unavailable)
+  var (_, osx_init_deconstruct2_only_osx) = osx_pair() // OK
+  
+  @available(OSXApplicationExtension, unavailable)
+  var (_, osx_extension_init_deconstruct2_only_osx) = osx_pair() // expected-error {{'osx_pair()' is unavailable}}
+
+  @available(OSX, unavailable)
+  var (osx_init_deconstruct1_only_osx, _) = osx_pair() // OK
+  
+  @available(OSXApplicationExtension, unavailable)
+  var (osx_extension_init_deconstruct1_only_osx, _) = osx_pair() // expected-error {{'osx_pair()' is unavailable}}
+
+  @available(OSX, unavailable)
+  var osx_inner_init_osx = { let inner_var = osx() } // OK
+  
+  // FIXME: I'm not sure why this produces two errors instead of just one.
+  @available(OSXApplicationExtension, unavailable)
+  var osx_extension_inner_init_osx = { let inner_var = osx() } // expected-error 2 {{'osx()' is unavailable}}
+}
+
+extension Outer {
+  @available(OSX, unavailable)
+  static var also_osx_init_osx = osx() // OK
+  
+  @available(OSXApplicationExtension, unavailable)
+  static var also_osx_extension_init_osx = osx() // expected-error {{'osx()' is unavailable}}
+}
+
+@available(OSX, unavailable)
+extension Outer {
+  static var outer_osx_init_osx = osx() // OK
+}
+
+@available(OSX, unavailable)
+struct NotOnOSX {
+  var osx_init_osx = osx() // OK
+  lazy var osx_lazy_osx = osx() // OK
+  var osx_init_multi1_osx = osx(), osx_init_multi2_osx = osx() // OK
+  var (osx_init_deconstruct1_osx, osx_init_deconstruct2_osx) = osx_pair() // OK
+  var (_, osx_init_deconstruct2_only_osx) = osx_pair() // OK
+  var (osx_init_deconstruct1_only_osx, _) = osx_pair() // OK
+}