[NameLookup] Fix opaque decl astscope

include/swift/AST/IfConfigClause.h

@@ -39,7 +39,7 @@ struct IfConfigClause {
   ArrayRef<ASTNode> Elements;
 
   /// True if this is the active clause of the #if block.
-  bool isActive;
+  const bool isActive;
 
   IfConfigClause(SourceLoc Loc, Expr *Cond,
                  ArrayRef<ASTNode> Elements, bool isActive)

include/swift/AST/SourceFile.h

@@ -433,6 +433,9 @@ class SourceFile final : public FileUnit {
 
   OpaqueTypeDecl *lookupOpaqueResultType(StringRef MangledName) override;
 
+  /// Do not call when inside an inactive clause (\c
+  /// InInactiveClauseEnvironment)) because it will later on result in a lookup
+  /// to something that won't be in the ASTScope tree.
   void addUnvalidatedDeclWithOpaqueResultType(ValueDecl *vd) {
     UnvalidatedDeclsWithOpaqueReturnTypes.insert(vd);
   }

include/swift/Parse/Parser.h

@@ -158,6 +158,10 @@ class Parser {
 
   bool InPoundLineEnvironment = false;
   bool InPoundIfEnvironment = false;
+  /// Do not call \c addUnvalidatedDeclWithOpaqueResultType when in an inactive
+  /// clause because ASTScopes are not created in those contexts and lookups to
+  /// those decls will fail.
+  bool InInactiveClauseEnvironment = false;
   bool InSwiftKeyPath = false;
 
   LocalContext *CurLocalContext = nullptr;

lib/AST/ASTScopeLookup.cpp

@@ -36,6 +36,8 @@ using namespace swift;
 using namespace namelookup;
 using namespace ast_scope;
 
+static bool isLocWithinAnInactiveClause(const SourceLoc loc, SourceFile *SF);
+
 llvm::SmallVector<const ASTScopeImpl *, 0> ASTScopeImpl::unqualifiedLookup(
     SourceFile *sourceFile, const DeclName name, const SourceLoc loc,
     const DeclContext *const startingContext, DeclConsumer consumer) {
@@ -82,6 +84,12 @@ const ASTScopeImpl *ASTScopeImpl::findStartingScopeForLookup(
   if (!startingScope) {
     llvm::errs() << "ASTScopeImpl: resorting to startingScope hack, file: "
                  << sourceFile->getFilename() << "\n";
+    // The check is costly, and inactive lookups will end up here, so don't
+    // do the check unless we can't find the startingScope.
+    const bool isInInactiveClause =
+        isLocWithinAnInactiveClause(loc, sourceFile);
+    if (isInInactiveClause)
+      llvm::errs() << "  because location is within an inactive clause\n";
     llvm::errs() << "'";
     name.print(llvm::errs());
     llvm::errs() << "' ";
@@ -98,6 +106,11 @@ const ASTScopeImpl *ASTScopeImpl::findStartingScopeForLookup(
     // Might distort things
     //    if (fileScope->crossCheckWithAST())
     //      llvm::errs() << "Tree creation missed some DeclContexts.\n";
+
+    // Crash compilation even if NDEBUG
+    if (isInInactiveClause)
+      llvm::report_fatal_error(
+          "A lookup was attempted into an inactive clause");
   }
 
   ASTScopeAssert(startingScope, "ASTScopeImpl: could not find startingScope");
@@ -694,3 +707,36 @@ Optional<bool> PatternEntryInitializerScope::resolveIsCascadingUseForThisScope(
 
   return isCascadingUse;
 }
+
+bool isLocWithinAnInactiveClause(const SourceLoc loc, SourceFile *SF) {
+  class InactiveClauseTester : public ASTWalker {
+    const SourceLoc loc;
+    const SourceManager &SM;
+
+  public:
+    bool wasFoundWithinInactiveClause = false;
+
+    InactiveClauseTester(const SourceLoc loc, const SourceManager &SM)
+        : loc(loc), SM(SM) {}
+
+    bool walkToDeclPre(Decl *D) override {
+      if (const auto *ifc = dyn_cast<IfConfigDecl>(D)) {
+        for (const auto &clause : ifc->getClauses()) {
+          if (clause.isActive)
+            continue;
+          for (const auto n : clause.Elements) {
+            SourceRange sr = n.getSourceRange();
+            if (sr.isValid() && SM.rangeContainsTokenLoc(sr, loc)) {
+              wasFoundWithinInactiveClause = true;
+              return false;
+            }
+          }
+        }
+      }
+      return ASTWalker::walkToDeclPre(D);
+    }
+  };
+  InactiveClauseTester tester(loc, SF->getASTContext().SourceMgr);
+  SF->walk(tester);
+  return tester.wasFoundWithinInactiveClause;
+}

lib/Parse/ParseDecl.cpp

@@ -5163,7 +5163,7 @@ Parser::parseDeclVar(ParseDeclOptions Flags,
       VD->getAttrs() = Attributes;
       setLocalDiscriminator(VD);
       Decls.push_back(VD);
-      if (hasOpaqueReturnTy && sf) {
+      if (hasOpaqueReturnTy && sf && !InInactiveClauseEnvironment) {
         sf->addUnvalidatedDeclWithOpaqueResultType(VD);
       }
     });
@@ -5479,7 +5479,8 @@ ParserResult<FuncDecl> Parser::parseDeclFunc(SourceLoc StaticLoc,
                               CurDeclContext);
 
   // Let the source file track the opaque return type mapping, if any.
-  if (FuncRetTy && isa<OpaqueReturnTypeRepr>(FuncRetTy)) {
+  if (FuncRetTy && isa<OpaqueReturnTypeRepr>(FuncRetTy) &&
+      !InInactiveClauseEnvironment) {
     if (auto sf = CurDeclContext->getParentSourceFile()) {
       sf->addUnvalidatedDeclWithOpaqueResultType(FD);
     }
@@ -6354,7 +6355,8 @@ Parser::parseDeclSubscript(SourceLoc StaticLoc,
   Subscript->getAttrs() = Attributes;
 
   // Let the source file track the opaque return type mapping, if any.
-  if (ElementTy.get() && isa<OpaqueReturnTypeRepr>(ElementTy.get())) {
+  if (ElementTy.get() && isa<OpaqueReturnTypeRepr>(ElementTy.get()) &&
+      !InInactiveClauseEnvironment) {
     if (auto sf = CurDeclContext->getParentSourceFile()) {
       sf->addUnvalidatedDeclWithOpaqueResultType(Subscript);
     }

lib/Parse/ParseIfConfig.cpp

@@ -662,6 +662,8 @@ ParserResult<IfConfigDecl> Parser::parseIfConfig(
 
     // Parse elements
     SmallVector<ASTNode, 16> Elements;
+    llvm::SaveAndRestore<bool> S(InInactiveClauseEnvironment,
+                                 InInactiveClauseEnvironment || !isActive);
     if (isActive || !isVersionCondition) {
       parseElements(Elements, isActive);
     } else if (SyntaxContext->isEnabled()) {

test/NameBinding/disabled_opaque_decl.swift

@@ -0,0 +1,14 @@
+// Ensure scope construction does not crash
+// RUN: %target-swift-frontend -emit-module %s
+
+public protocol P { }
+
+extension Int: P { }
+
+#if false
+public struct Foo {
+  public func getP() -> some P {
+    return 17
+  }
+}
+#endif