[Evaluator] Remove hasActiveResolveMacroRequest().

Author: hborla

include/swift/AST/Evaluator.h

@@ -208,18 +208,6 @@ class Evaluator {
   /// is treated as a stack and is used to detect cycles.
   llvm::SetVector<ActiveRequest> activeRequests;
 
-  /// How many `ResolveMacroRequest` requests are active.
-  ///
-  /// This allows us to quickly determine whether there is any
-  /// `ResolveMacroRequest` active in the active request stack.
-  /// It saves us from a linear scan through `activeRequests` when
-  /// we need to determine this information.
-  ///
-  /// Why on earth would we need to determine this information?
-  /// Please see the extended comment that goes with the constructor
-  /// of `UnqualifiedLookupRequest`.
-  unsigned numActiveResolveMacroRequests = 0;
-
   /// A cache that stores the results of requests.
   evaluator::RequestCache cache;
 
@@ -342,16 +330,6 @@ class Evaluator {
     return activeRequests.count(ActiveRequest(request));
   }
 
-  /// Determine whether there is any active "resolve macro" request
-  /// on the request stack.
-  ///
-  /// Why on earth would we need to determine this information?
-  /// Please see the extended comment that goes with the constructor
-  /// of `UnqualifiedLookupRequest`.
-  bool hasActiveResolveMacroRequest() const {
-    return numActiveResolveMacroRequests > 0;
-  }
-
 private:
   /// Diagnose a cycle detected in the evaluation of the given
   /// request.

lib/AST/Evaluator.cpp

@@ -63,8 +63,6 @@ Evaluator::Evaluator(DiagnosticEngine &diags, const LangOptions &opts)
 bool Evaluator::checkDependency(const ActiveRequest &request) {
   // Record this as an active request.
   if (activeRequests.insert(request)) {
-    if (request.getAs<ResolveMacroRequest>())
-      ++numActiveResolveMacroRequests;
     return false;
   }
 
@@ -74,9 +72,6 @@ bool Evaluator::checkDependency(const ActiveRequest &request) {
 }
 
 void Evaluator::finishedRequest(const ActiveRequest &request) {
-  if (request.getAs<ResolveMacroRequest>())
-    --numActiveResolveMacroRequests;
-
   assert(activeRequests.back() == request);
   activeRequests.pop_back();
 }

lib/AST/NameLookupRequests.cpp

@@ -519,24 +519,14 @@ swift::extractNearestSourceLoc(CustomRefCountingOperationDescriptor desc) {
 // macros, because doing so trivially creates a cyclic dependency amongst the
 // macro expansions that will be detected by the request-evaluator.
 //
-// Our lookup requests don't always have enough information to answer the
-// question "is this part of an argument to a macro?", so we do a much simpler,
-// more efficient, and not-entirely-sound hack based on the request-evaluator.
-// Specifically, if we are in the process of resolving a macro (which is
-// determined by checking for the presence of a `ResolveMacroRequest` in the
-// request-evaluator stack), then we adjust the options used for the name
-// lookup request we are forming to exclude macro expansions. The evaluation
-// of that request will then avoid expanding any macros, and not produce any
-// results that involve entries in already-expanded macros. By adjusting the
-// request itself, we still distinguish between requests that can and cannot
-// look into macro expansions, so it doesn't break caching for those immediate
-// requests.
-//
-// Over time, we should seek to replace this heuristic with a location-based
-// check, where we use ASTScope to determine whether we are inside a macro
-// argument. This existing check might still be useful because it's going to
-// be faster than a location-based query, but the location-based query can be
-// fully correct.
+// We use source locations to answer the question "is this part of an argument
+// to a macro?" through `namelookup::isInMacroArgument`. If the answer is yes,
+// then we adjust the options used for the name lookup request we are forming
+// to exclude macro expansions. The evaluation of that request will then avoid
+// expanding any macros, and not produce any results that involve entries in
+// already-expanded macros. By adjusting the request itself, we still
+// distinguish between requests that can and cannot look into macro expansions,
+// so it doesn't break caching for those immediate requests.
 
 /// Exclude macros in the unqualified lookup descriptor if we need to.
 static UnqualifiedLookupDescriptor excludeMacrosIfNeeded(