Suppress the warning for inactive try/throw in do..catch

In the warning about having no try/throw within the body of a do..catch,
replace the walk of the inactive clauses of IfConfigDecl with a syntactic
check of inactive and unparsed regions to look for 'try' and 'throw'
keywords.

This both eliminates a dependency on IfConfigDecl and expands the
usefulness of this warning suppression to unparsed code.

Author: DougGregor

lib/ASTGen/Sources/ASTGen/CompilerBuildConfiguration.swift

@@ -341,6 +341,51 @@ private struct InactiveCodeContainsReferenceCache {
   let configuredRegions: ConfiguredRegions
 }
 
+/// Search in inactive/unparsed code to look for evidence that something that
+/// looks unused is actually used in some configurations.
+private enum InactiveCodeChecker {
+  case name(String)
+  case tryOrThrow
+
+  /// Search for the kind of entity in the given syntax node.
+  func search(syntax: SourceFileSyntax, configuredRegions: ConfiguredRegions) -> Bool {
+    // If there are no regions, everything is active. This is the common case.
+    if configuredRegions.isEmpty {
+      return false
+    }
+
+    for token in syntax.tokens(viewMode: .sourceAccurate) {
+      // Match what we're looking for, and continue iterating if it doesn't
+      // match.
+      switch self {
+      case .name(let name):
+        guard let identifier = token.identifier, identifier.name == name else {
+          continue
+        }
+
+        break
+
+      case .tryOrThrow:
+        guard let keywordKind = token.keywordKind,
+              keywordKind == .try || keywordKind == .throw else {
+          continue
+        }
+
+        break
+      }
+
+      // We matched what we were looking for, now check whether it is in an
+      // inactive or unparsed region.
+      if configuredRegions.isActive(token) != .active {
+        // Found it in a non-active region.
+        return true
+      }
+    }
+
+    return false
+  }
+}
+
 /// Determine whether the inactive code within the given search range
 /// contains a token referring to the given name.
 @_cdecl("swift_ASTGen_inactiveCodeContainsReference")
@@ -378,26 +423,31 @@ public func inactiveCodeContainsReference(
     untypedCachePtr.pointee = .init(cache)
   }
 
-  // If there are no regions, everything is active. This is the common case.
-  if configuredRegions.isEmpty {
-    return false
-  }
+  return InactiveCodeChecker.name(String(bridged: bridgedName))
+    .search(syntax: syntax, configuredRegions: configuredRegions)
+}
 
-  // Walk all of the tokens in the search range looking for one that references
-  // the given name.
-  let name = String(bridged: bridgedName)
-  for token in syntax.tokens(viewMode: .sourceAccurate) {
-    guard let identifier = token.identifier else {
-      continue
-    }
+@_cdecl("swift_ASTGen_inactiveCodeContainsTryOrThrow")
+public func inactiveCodeContainsTryOrThrow(
+  astContext: BridgedASTContext,
+  sourceFileBuffer: BridgedStringRef,
+  searchRange: BridgedStringRef
+) -> Bool {
+  // Parse the region.
 
-    if identifier.name == name && configuredRegions.isActive(token) != .active {
-      // Found it in a non-active region.
-      return true
-    }
-  }
+  // FIXME: Use 'ExportedSourceFile' when C++ parser is replaced.
+  let searchRangeBuffer = UnsafeBufferPointer<UInt8>(start: searchRange.data, count: searchRange.count)
+  let syntax = Parser.parse(source: searchRangeBuffer)
+
+  // Compute the configured regions within the search range.
+  let configuration = CompilerBuildConfiguration(
+    ctx: astContext,
+    sourceBuffer: searchRangeBuffer
+  )
+  let configuredRegions = syntax.configuredRegions(in: configuration)
 
-  return false
+  return InactiveCodeChecker.tryOrThrow
+    .search(syntax: syntax, configuredRegions: configuredRegions)
 }
 
 @_cdecl("swift_ASTGen_freeInactiveCodeContainsReferenceCache")

lib/Sema/TypeCheckEffects.cpp

@@ -18,6 +18,7 @@
 #include "TypeChecker.h"
 #include "TypeCheckConcurrency.h"
 #include "TypeCheckEffects.h"
+#include "swift/AST/ASTBridging.h"
 #include "swift/AST/ASTWalker.h"
 #include "swift/AST/DiagnosticsSema.h"
 #include "swift/AST/Effects.h"
@@ -522,9 +523,7 @@ class EffectsHandlingWalker : public ASTWalker {
     ShouldRecurse_t recurse = ShouldRecurse;
     // Skip the implementations of all local declarations... except
     // PBD.  We should really just have a PatternBindingStmt.
-    if (auto ic = dyn_cast<IfConfigDecl>(D)) {
-      recurse = asImpl().checkIfConfig(ic);
-    } else if (auto patternBinding = dyn_cast<PatternBindingDecl>(D)) {
+    if (auto patternBinding = dyn_cast<PatternBindingDecl>(D)) {
       if (patternBinding->isAsyncLet())
         recurse = asImpl().checkAsyncLet(patternBinding);
     } else if (auto macroExpansionDecl = dyn_cast<MacroExpansionDecl>(D)) {
@@ -1717,10 +1716,6 @@ class ApplyClassifier {
       return ShouldRecurse;
     }
 
-    ShouldRecurse_t checkIfConfig(IfConfigDecl *D) {
-      return ShouldRecurse;
-    }
-
     ShouldRecurse_t checkForEach(ForEachStmt *S) {
       classification.merge(Self.classifyForEach(S));
       return ShouldRecurse;
@@ -1832,10 +1827,6 @@ class ApplyClassifier {
       return ShouldRecurse;
     }
 
-    ShouldRecurse_t checkIfConfig(IfConfigDecl *D) {
-      return ShouldRecurse;
-    }
-
     ShouldRecurse_t checkDoCatch(DoCatchStmt *S) {
       return ShouldRecurse;
     }
@@ -2719,6 +2710,14 @@ class Context {
 
 };
 
+/// ASTGen helper function to look for a "try" or "throw" in inactive code
+/// within the given source file.
+extern "C" bool swift_ASTGen_inactiveCodeContainsTryOrThrow(
+    BridgedASTContext ctx,
+    BridgedStringRef sourceFileBuffer,
+    BridgedStringRef searchRange
+);
+
 /// A class to walk over a local context and validate the correctness
 /// of its error coverage.
 class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage> {
@@ -3165,7 +3164,7 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
 
     S->getBody()->walk(*this);
 
-    diagnoseNoThrowInDo(S, scope);
+    diagnoseNoThrowInDo(S, scope, S->getBody()->getSourceRange());
 
     return MaxThrowingKind;
   }
@@ -3189,17 +3188,40 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
 
     S->getBody()->walk(*this);
 
-    diagnoseNoThrowInDo(S, scope);
+    diagnoseNoThrowInDo(S, scope, S->getBody()->getSourceRange());
 
     scope.preserveCoverageFromNonExhaustiveCatch();
     return MaxThrowingKind;
   }
 
-  void diagnoseNoThrowInDo(DoCatchStmt *S, ContextScope &scope) {
+  /// Determine whether the inactive code within the given body range
+  /// contains a "try" or a "throw".
+  bool inactiveCodeContainsTryOrThrow(SourceRange bodyRange) {
+#if SWIFT_BUILD_SWIFT_SYNTAX
+    SourceManager &sourceMgr = Ctx.SourceMgr;
+    auto bufferID = sourceMgr.findBufferContainingLoc(bodyRange.Start);
+    StringRef sourceFileText = sourceMgr.getEntireTextForBuffer(bufferID);
+
+    // Extract the search text from that buffer.
+    auto searchTextCharRange = Lexer::getCharSourceRangeFromSourceRange(
+        sourceMgr, bodyRange);
+    StringRef searchText = sourceMgr.extractText(searchTextCharRange, bufferID);
+
+    return swift_ASTGen_inactiveCodeContainsTryOrThrow(
+        Ctx, sourceFileText, searchText);
+#else
+    return false;
+#endif
+  }
+
+  void diagnoseNoThrowInDo(
+      DoCatchStmt *S, ContextScope &scope, SourceRange bodyRange
+  ) {
     // Warn if nothing threw within the body, unless this is the
     // implicit do/catch in a debugger function.
     if (!Flags.has(ContextFlags::HasAnyThrowSite) &&
-        !scope.wasTopLevelDebuggerFunction()) {
+        !scope.wasTopLevelDebuggerFunction() &&
+        !inactiveCodeContainsTryOrThrow(bodyRange)) {
       Ctx.Diags.diagnose(S->getCatches().front()->getStartLoc(),
                          diag::no_throw_in_do_with_catch);
     }
@@ -3318,41 +3340,6 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
     return ShouldNotRecurse;
   }
 
-  ShouldRecurse_t checkIfConfig(IfConfigDecl *ICD) {
-    // Check the inactive regions of a #if block to disable warnings that may
-    // be due to platform specific code.
-    struct ConservativeThrowChecker : public ASTWalker {
-      CheckEffectsCoverage &CEC;
-      ConservativeThrowChecker(CheckEffectsCoverage &CEC) : CEC(CEC) {}
-
-      MacroWalking getMacroWalkingBehavior() const override {
-        return MacroWalking::Arguments;
-      }
-
-      PostWalkResult<Expr *> walkToExprPost(Expr *E) override {
-        if (isa<TryExpr>(E))
-          CEC.Flags.set(ContextFlags::HasAnyThrowSite);
-        return Action::Continue(E);
-      }
-
-      PostWalkResult<Stmt *> walkToStmtPost(Stmt *S) override {
-        if (isa<ThrowStmt>(S))
-          CEC.Flags.set(ContextFlags::HasAnyThrowSite);
-
-        return Action::Continue(S);
-      }
-    };
-
-    for (auto &clause : ICD->getClauses()) {
-      // Active clauses are handled by the normal AST walk.
-      if (clause.isActive) continue;
-
-      for (auto elt : clause.Elements)
-        elt.walk(ConservativeThrowChecker(*this));
-    }
-    return ShouldRecurse;
-  }
-
   ShouldRecurse_t checkThrow(ThrowStmt *S) {
     if (auto classification = getApplyClassifier().classifyThrow(S)) {
       Flags.set(ContextFlags::HasAnyThrowSite);

test/Parse/errors.swift

@@ -49,6 +49,13 @@ func one() {
   } catch {    // don't warn, #if code should be scanned.
   }
 
+  do {
+#if compiler(>=10)
+    throw opaque_error()
+#endif
+  } catch {    // don't warn, #if code should be scanned.
+  }
+
   do {
 #if false
     throw opaque_error()