Remove DependencyScope

Author: CodaFi

include/swift/AST/EvaluatorDependencies.h

@@ -21,6 +21,7 @@
 #include "swift/AST/AnyRequest.h"
 #include "swift/AST/AttrKind.h"
 #include "swift/AST/SourceFile.h"
+#include "swift/Basic/NullablePtr.h"
 #include "llvm/ADT/PointerIntPair.h"
 
 namespace swift {
@@ -32,48 +33,13 @@ namespace detail {
 template <typename...> using void_t = void;
 } // namespace detail
 
-/// The "scope" of a dependency edge tracked by the evaluator.
-///
-/// Dependency scopes come in two flavors: cascading and private. A private
-/// edge captures dependencies discovered in contexts that are not visible to
-/// to other files. For example, a conformance to a private protocol, or the use
-/// of any names inside of a function body. A cascading edge, by contrast,
-/// captures dependencies discovered in the remaining visible contexts. These
-/// are types with at least \c internal visibility, names defined or used
-/// outside of function bodies with at least \c internal visibility, etc. A
-/// dependency that has cascading scope is so-named because upon traversing the
-/// edge, a reader such as the driver should continue transitively evaluating
-/// further dependency edges.
-///
-/// A cascading edge is always conservatively correct to produce, but it comes
-/// at the cost of increased resources spent (and possibly even wasted!) during
-/// incremental compilation. A private edge, by contrast, is more efficient for
-/// incremental compilation but it is harder to safely use.
-///
-/// To ensure that these edges are registered consistently with the correct
-/// scopes, requests that act as the source of dependency edges are required
-/// to specify a \c DependencyScope under which all evaluated sub-requests will
-/// register their dependency edges. In this way, \c DependencyScope values
-/// form a stack-like structure and are pushed and popped by the evaluator
-/// during the course of request evaluation.
-///
-/// When determining the kind of scope a request should use, always err on the
-/// side of a cascading scope unless there is absolute proof any discovered
-/// dependencies will be private. Inner requests may also defensively choose to
-/// flip the dependency scope from private to cascading in the name of safety.
-enum class DependencyScope : bool {
-  Private = false,
-  Cascading = true,
-};
-
-// A \c DependencySource is currently defined to be a parent source file and
-// an associated dependency scope.
+// A \c DependencySource is currently defined to be a parent source file.
 //
 // The \c SourceFile instance is an artifact of the current dependency system,
 // and should be scrapped if possible. It currently encodes the idea that
 // edges in the incremental dependency graph invalidate entire files instead
 // of individual contexts.
-using DependencySource = llvm::PointerIntPair<SourceFile *, 1, DependencyScope>;
+using DependencySource = swift::NullablePtr<SourceFile>;
 
 struct DependencyRecorder;
 
@@ -306,31 +272,21 @@ struct DependencyRecorder {
                                  ReferenceEnumerator f) const ;
 
 public:
-  /// Returns the scope of the current active scope.
-  ///
-  /// If there is no active scope, the result always cascades.
-  evaluator::DependencyScope getActiveSourceScope() const {
-    if (dependencySources.empty()) {
-      return evaluator::DependencyScope::Cascading;
-    }
-    return dependencySources.back().getInt();
-  }
-
   /// Returns the active dependency's source file, or \c nullptr if no
   /// dependency source is active.
   ///
   /// The use of this accessor is strongly discouraged, as it implies that a
   /// dependency sink is seeking to filter out names based on the files they
   /// come from. Existing callers are being migrated to more reasonable ways
   /// of judging the relevancy of a dependency.
-  SourceFile *getActiveDependencySourceOrNull() const {
+  evaluator::DependencySource getActiveDependencySourceOrNull() const {
     if (dependencySources.empty())
       return nullptr;
     switch (mode) {
     case Mode::LegacyCascadingDependencies:
-      return dependencySources.back().getPointer();
+      return dependencySources.back();
     case Mode::DirectDependencies:
-      return dependencySources.front().getPointer();
+      return dependencySources.front();
     }
   }
 
@@ -350,7 +306,7 @@ struct DependencyRecorder {
       auto Source = Req.readDependencySource(coll);
       // If there is no source to introduce, bail. This can occur if
       // a request originates in the context of a module.
-      if (!Source.getPointer()) {
+      if (Source.isNull()) {
         return;
       }
       coll.dependencySources.emplace_back(Source);
@@ -370,7 +326,7 @@ struct DependencyRecorder {
   bool isActiveSourceCascading() const {
     switch (mode) {
     case Mode::LegacyCascadingDependencies:
-      return getActiveSourceScope() == evaluator::DependencyScope::Cascading;
+      return false;
     case Mode::DirectDependencies:
       return false;
     }

lib/AST/ASTScopeLookup.cpp

@@ -585,7 +585,7 @@ Optional<bool> PatternEntryInitializerScope::resolveIsCascadingUseForThisScope(
   // initializing stored property of a type
   auto *const patternDeclContext = decl->getDeclContext();
   if (patternDeclContext->isTypeContext())
-    return isCascadingUseAccordingTo(PBI->getParent());
+    return false;
 
   // initializing global or local
   if (PBI)

lib/AST/Evaluator.cpp

@@ -381,8 +381,7 @@ void Evaluator::dumpDependenciesGraphviz() const {
 
 void evaluator::DependencyRecorder::realize(
     const DependencyCollector::Reference &ref) {
-  auto *source = getActiveDependencySourceOrNull();
-  assert(source && "cannot realize dependency without associated file!");
+  auto *source = getActiveDependencySourceOrNull().get();
   if (!source->isPrimary()) {
     return;
   }
@@ -429,8 +428,8 @@ void evaluator::DependencyRecorder::record(
     const llvm::SetVector<swift::ActiveRequest> &stack,
     llvm::function_ref<void(DependencyCollector &)> rec) {
   assert(!isRecording && "Probably not a good idea to allow nested recording");
-  auto *source = getActiveDependencySourceOrNull();
-  if (!source || !source->isPrimary()) {
+  auto source = getActiveDependencySourceOrNull();
+  if (source.isNull() || !source.get()->isPrimary()) {
     return;
   }
 
@@ -450,12 +449,12 @@ void evaluator::DependencyRecorder::replay(
     const swift::ActiveRequest &req) {
   assert(!isRecording && "Probably not a good idea to allow nested recording");
 
-  auto *source = getActiveDependencySourceOrNull();
+  auto source = getActiveDependencySourceOrNull();
   if (mode == Mode::LegacyCascadingDependencies) {
     return;
   }
 
-  if (!source || !source->isPrimary()) {
+  if (source.isNull() || !source.get()->isPrimary()) {
     return;
   }
 

lib/AST/NameLookupRequests.cpp

@@ -96,11 +96,8 @@ evaluator::DependencySource InheritedProtocolsRequest::readDependencySource(
   // type conforms to Hashable which itself looks up Equatable during
   // qualified lookup.
   if (!PD->getParentSourceFile())
-    return { nullptr, e.getActiveSourceScope() };
-  return {
-    e.getActiveDependencySourceOrNull(),
-    DependencyScope::Private,
-  };
+    return nullptr;
+  return e.getActiveDependencySourceOrNull();
 }
 
 void InheritedProtocolsRequest::writeDependencySink(
@@ -184,7 +181,7 @@ void ExtendedNominalRequest::writeDependencySink(
   auto *SF = std::get<0>(getStorage())->getParentSourceFile();
   if (!SF)
     return;
-  if (SF != tracker.getRecorder().getActiveDependencySourceOrNull())
+  if (SF != tracker.getRecorder().getActiveDependencySourceOrNull().getPtrOrNull())
     return;
   tracker.addPotentialMember(value);
 }
@@ -209,14 +206,7 @@ void GetDestructorRequest::cacheResult(DestructorDecl *value) const {
 
 evaluator::DependencySource GetDestructorRequest::readDependencySource(
     const evaluator::DependencyRecorder &eval) const {
-  // Looking up the deinitializer currently always occurs in a private
-  // scope because it is impossible to reference 'deinit' in user code, and a
-  // valid 'deinit' declaration cannot occur outside of the
-  // definition of a type.
-  return {
-    eval.getActiveDependencySourceOrNull(),
-    evaluator::DependencyScope::Private
-  };
+  return eval.getActiveDependencySourceOrNull();
 }
 
 //----------------------------------------------------------------------------//
@@ -371,7 +361,7 @@ swift::extractNearestSourceLoc(const LookupConformanceDescriptor &desc) {
 evaluator::DependencySource ModuleQualifiedLookupRequest::readDependencySource(
     const evaluator::DependencyRecorder &eval) const {
   auto *DC = std::get<0>(getStorage());
-  return { DC->getParentSourceFile(), evaluator::DependencyScope::Private };
+  return DC->getParentSourceFile();
 }
 
 void ModuleQualifiedLookupRequest::writeDependencySink(
@@ -403,13 +393,13 @@ void LookupConformanceInModuleRequest::writeDependencySink(
   if (!Adoptee)
     return;
 
-  auto *source = reqTracker.getRecorder().getActiveDependencySourceOrNull();
-  if (!source)
+  auto source = reqTracker.getRecorder().getActiveDependencySourceOrNull();
+  if (source.isNull())
     return;
 
   // Decline to record conformances defined outside of the active module.
   auto *conformance = lookupResult.getConcrete();
-  if (source->getParentModule() !=
+  if (source.get()->getParentModule() !=
       conformance->getDeclContext()->getParentModule())
     return;
   reqTracker.addPotentialMember(Adoptee);
@@ -421,12 +411,7 @@ void LookupConformanceInModuleRequest::writeDependencySink(
 
 evaluator::DependencySource UnqualifiedLookupRequest::readDependencySource(
     const evaluator::DependencyRecorder &) const {
-  auto &desc = std::get<0>(getStorage());
-  // FIXME(Evaluator Incremental Dependencies): This maintains compatibility
-  // with the existing scheme, but the existing scheme is totally ad-hoc. We
-  // should remove this flag and ensure that non-cascading qualified lookups
-  // occur in the right contexts instead.
-  return {desc.DC->getParentSourceFile(), evaluator::DependencyScope::Cascading};
+  return std::get<0>(getStorage()).DC->getParentSourceFile();
 }
 
 void UnqualifiedLookupRequest::writeDependencySink(
@@ -441,11 +426,8 @@ void UnqualifiedLookupRequest::writeDependencySink(
 
 evaluator::DependencySource QualifiedLookupRequest::readDependencySource(
     const evaluator::DependencyRecorder &) const {
-  auto *dc = std::get<0>(getStorage());
-  return {
-    dyn_cast<SourceFile>(dc->getModuleScopeContext()),
-    evaluator::DependencyScope::Private
-  };
+  auto *DC = std::get<0>(getStorage())->getModuleScopeContext();
+  return dyn_cast<SourceFile>(DC);
 }
 
 // Define request evaluation functions for each of the name lookup requests.

lib/AST/TypeCheckRequests.cpp

@@ -159,11 +159,7 @@ void SuperclassTypeRequest::cacheResult(Type value) const {
 
 evaluator::DependencySource SuperclassTypeRequest::readDependencySource(
     const evaluator::DependencyRecorder &e) const {
-  const auto access = std::get<0>(getStorage())->getFormalAccess();
-  return {
-    e.getActiveDependencySourceOrNull(),
-    evaluator::DependencyScope::Private,
-  };
+  return e.getActiveDependencySourceOrNull();
 }
 
 void SuperclassTypeRequest::writeDependencySink(
@@ -1285,12 +1281,8 @@ void CheckRedeclarationRequest::cacheResult(evaluator::SideEffect) const {
 
 evaluator::DependencySource CheckRedeclarationRequest::readDependencySource(
     const evaluator::DependencyRecorder &eval) const {
-  auto *current = std::get<0>(getStorage());
-  auto *currentDC = current->getDeclContext();
-  return {
-    currentDC->getParentSourceFile(),
-    evaluator::DependencyScope::Private,
-  };
+  auto *currentDC = std::get<0>(getStorage())->getDeclContext();
+  return currentDC->getParentSourceFile();
 }
 
 void CheckRedeclarationRequest::writeDependencySink(
@@ -1320,16 +1312,7 @@ void CheckRedeclarationRequest::writeDependencySink(
 evaluator::DependencySource
 LookupAllConformancesInContextRequest::readDependencySource(
     const evaluator::DependencyRecorder &collector) const {
-  const auto *nominal = std::get<0>(getStorage())
-                            ->getAsGenericContext()
-                            ->getSelfNominalTypeDecl();
-  if (!nominal) {
-    return {collector.getActiveDependencySourceOrNull(),
-            evaluator::DependencyScope::Cascading};
-  }
-
-  return {collector.getActiveDependencySourceOrNull(),
-          evaluator::DependencyScope::Private};
+  return collector.getActiveDependencySourceOrNull();
 }
 
 void LookupAllConformancesInContextRequest::writeDependencySink(
@@ -1369,7 +1352,7 @@ void ResolveTypeEraserTypeRequest::cacheResult(Type value) const {
 
 evaluator::DependencySource TypeCheckSourceFileRequest::readDependencySource(
     const evaluator::DependencyRecorder &e) const {
-  return {std::get<0>(getStorage()), evaluator::DependencyScope::Cascading};
+  return std::get<0>(getStorage());
 }
 
 Optional<evaluator::SideEffect>
@@ -1429,12 +1412,7 @@ void TypeCheckFunctionBodyRequest::cacheResult(BraceStmt *body) const {
 evaluator::DependencySource
 TypeCheckFunctionBodyRequest::readDependencySource(
     const evaluator::DependencyRecorder &e) const {
-  // We're going under a function body scope, unconditionally flip the scope
-  // to private.
-  return {
-    std::get<0>(getStorage())->getParentSourceFile(),
-    evaluator::DependencyScope::Private
-  };
+  return std::get<0>(getStorage())->getParentSourceFile();
 }
 
 //----------------------------------------------------------------------------//

lib/IRGen/IRGenRequests.cpp

@@ -102,11 +102,11 @@ evaluator::DependencySource IRGenRequest::readDependencySource(
 
   // We don't track dependencies in whole-module mode.
   if (auto *mod = desc.Ctx.dyn_cast<ModuleDecl *>()) {
-    return {nullptr, e.getActiveSourceScope()};
+    return nullptr;
   }
 
   auto *primary = desc.Ctx.get<FileUnit *>();
-  return {dyn_cast<SourceFile>(primary), evaluator::DependencyScope::Cascading};
+  return dyn_cast<SourceFile>(primary);
 }
 
 // Define request evaluation functions for each of the IRGen requests.

lib/Parse/ParseRequests.cpp

@@ -186,7 +186,7 @@ SourceFileParsingResult ParseSourceFileRequest::evaluate(Evaluator &evaluator,
 
 evaluator::DependencySource ParseSourceFileRequest::readDependencySource(
     const evaluator::DependencyRecorder &e) const {
-  return {std::get<0>(getStorage()), evaluator::DependencyScope::Cascading};
+  return std::get<0>(getStorage());
 }
 
 Optional<SourceFileParsingResult>
@@ -229,7 +229,7 @@ void swift::simple_display(llvm::raw_ostream &out,
 evaluator::DependencySource
 CodeCompletionSecondPassRequest::readDependencySource(
     const evaluator::DependencyRecorder &e) const {
-  return {std::get<0>(getStorage()), e.getActiveSourceScope()};
+  return std::get<0>(getStorage());
 }
 
 // Define request evaluation functions for each of the type checker requests.

lib/SILGen/SILGenRequests.cpp

@@ -52,12 +52,11 @@ evaluator::DependencySource ASTLoweringRequest::readDependencySource(
 
   // We don't track dependencies in whole-module mode.
   if (auto *mod = desc.context.dyn_cast<ModuleDecl *>()) {
-    return {nullptr, e.getActiveSourceScope()};
+    return nullptr;
   }
 
   // If we have a single source file, it's the source of dependencies.
-  auto *unit = desc.context.get<FileUnit *>();
-  return {dyn_cast<SourceFile>(unit), evaluator::DependencyScope::Cascading};
+  return dyn_cast<SourceFile>(desc.context.get<FileUnit *>());
 }
 
 ArrayRef<FileUnit *> ASTLoweringDescriptor::getFilesToEmit() const {