[NFC] Refactor Side-Effecting Requests to be Explicitly so

Introduce evaluator::SideEffect, the type of a request that performs
some operation solely to execute its side effects. Thankfully, there are
precious few requests that need to use this type in practice, but it's
good to call them out explicitly so we can get around to making them
behave much more functionally in the future.

Author: CodaFi

include/swift/AST/SILOptimizerRequests.h

@@ -50,7 +50,7 @@ llvm::hash_code hash_value(const SILPipelineExecutionDescriptor &desc);
 /// Executes a SIL pipeline plan on a SIL module.
 class ExecuteSILPipelineRequest
     : public SimpleRequest<ExecuteSILPipelineRequest,
-                           bool(SILPipelineExecutionDescriptor),
+                           evaluator::SideEffect(SILPipelineExecutionDescriptor),
                            CacheKind::Uncached> {
 public:
   using SimpleRequest::SimpleRequest;
@@ -59,8 +59,8 @@ class ExecuteSILPipelineRequest
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<bool> evaluate(Evaluator &evaluator,
-                                SILPipelineExecutionDescriptor desc) const;
+  llvm::Expected<evaluator::SideEffect>
+  evaluate(Evaluator &evaluator, SILPipelineExecutionDescriptor desc) const;
 };
 
 void simple_display(llvm::raw_ostream &out,

include/swift/AST/SILOptimizerTypeIDZone.def

@@ -15,4 +15,5 @@
 //===----------------------------------------------------------------------===//
 
 SWIFT_REQUEST(SILOptimizer, ExecuteSILPipelineRequest,
-              bool(SILPipelineExecutionDescriptor), Uncached, NoLocationInfo)
+              evaluator::SideEffect(SILPipelineExecutionDescriptor),
+              Uncached, NoLocationInfo)

include/swift/AST/TypeCheckRequests.h

@@ -1247,7 +1247,7 @@ class OperatorPrecedenceGroupRequest
 /// Computes the raw values for an enum type.
 class EnumRawValuesRequest :
     public SimpleRequest<EnumRawValuesRequest,
-                         bool (EnumDecl *, TypeResolutionStage),
+                         evaluator::SideEffect (EnumDecl *, TypeResolutionStage),
                          CacheKind::SeparatelyCached> {
 public:
   using SimpleRequest::SimpleRequest;
@@ -1256,7 +1256,7 @@ class EnumRawValuesRequest :
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<bool>
+  llvm::Expected<evaluator::SideEffect>
   evaluate(Evaluator &evaluator, EnumDecl *ED, TypeResolutionStage stage) const;
 
 public:
@@ -1266,8 +1266,8 @@ class EnumRawValuesRequest :
 
   // Separate caching.
   bool isCached() const;
-  Optional<bool> getCachedResult() const;
-  void cacheResult(bool value) const;
+  Optional<evaluator::SideEffect> getCachedResult() const;
+  void cacheResult(evaluator::SideEffect value) const;
 };
 
 /// Determines if an override is ABI compatible with its base method.
@@ -1718,7 +1718,7 @@ enum class ImplicitMemberAction : uint8_t {
 
 class ResolveImplicitMemberRequest
     : public SimpleRequest<ResolveImplicitMemberRequest,
-                           bool(NominalTypeDecl *, ImplicitMemberAction),
+                           evaluator::SideEffect(NominalTypeDecl *, ImplicitMemberAction),
                            CacheKind::Uncached> {
 public:
   using SimpleRequest::SimpleRequest;
@@ -1727,8 +1727,9 @@ class ResolveImplicitMemberRequest
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<bool> evaluate(Evaluator &evaluator, NominalTypeDecl *NTD,
-                                ImplicitMemberAction action) const;
+  llvm::Expected<evaluator::SideEffect>
+  evaluate(Evaluator &evaluator, NominalTypeDecl *NTD,
+           ImplicitMemberAction action) const;
 
 public:
   // Separate caching.
@@ -1984,21 +1985,23 @@ class DynamicallyReplacedDeclRequest
 
 class TypeCheckSourceFileRequest :
     public SimpleRequest<TypeCheckSourceFileRequest,
-                         bool (SourceFile *), CacheKind::SeparatelyCached> {
+                         evaluator::SideEffect (SourceFile *),
+                         CacheKind::SeparatelyCached> {
 public:
   using SimpleRequest::SimpleRequest;
 
 private:
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<bool> evaluate(Evaluator &evaluator, SourceFile *SF) const;
+  llvm::Expected<evaluator::SideEffect>
+  evaluate(Evaluator &evaluator, SourceFile *SF) const;
 
 public:
   // Separate caching.
   bool isCached() const { return true; }
-  Optional<bool> getCachedResult() const;
-  void cacheResult(bool result) const;
+  Optional<evaluator::SideEffect> getCachedResult() const;
+  void cacheResult(evaluator::SideEffect) const;
 };
 
 /// Computes whether the specified type or a super-class/super-protocol has the

include/swift/AST/TypeCheckerTypeIDZone.def

@@ -55,8 +55,8 @@ SWIFT_REQUEST(TypeChecker, DynamicallyReplacedDeclRequest,
 SWIFT_REQUEST(TypeChecker, EmittedMembersRequest, DeclRange(ClassDecl *),
               SeparatelyCached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, EnumRawValuesRequest,
-              bool (EnumDecl *, TypeResolutionStage), SeparatelyCached,
-              NoLocationInfo)
+              evaluator::SideEffect (EnumDecl *, TypeResolutionStage),
+              SeparatelyCached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, EnumRawTypeRequest,
               Type(EnumDecl *, TypeResolutionStage), SeparatelyCached,
               NoLocationInfo)
@@ -182,7 +182,8 @@ SWIFT_REQUEST(TypeChecker, SynthesizeAccessorRequest,
               AccessorDecl *(AbstractStorageDecl *, AccessorKind),
               SeparatelyCached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, TypeCheckFunctionBodyUntilRequest,
-              bool(AbstractFunctionDecl *, SourceLoc), Cached, NoLocationInfo)
+              bool(AbstractFunctionDecl *, SourceLoc),
+              Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, UnderlyingTypeRequest, Type(TypeAliasDecl *),
               SeparatelyCached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, USRGenerationRequest, std::string(const ValueDecl *),
@@ -207,8 +208,8 @@ SWIFT_REQUEST(TypeChecker, PreCheckFunctionBuilderRequest,
               FunctionBuilderClosurePreCheck(AnyFunctionRef),
               Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, ResolveImplicitMemberRequest,
-              bool(NominalTypeDecl *, ImplicitMemberAction), Uncached,
-              NoLocationInfo)
+              evaluator::SideEffect(NominalTypeDecl *, ImplicitMemberAction),
+              Uncached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, SPIGroupsRequest,
               llvm::ArrayRef<Identifier>(Decl *),
               Cached, NoLocationInfo)

include/swift/Basic/CTypeIDZone.def

@@ -35,6 +35,7 @@ SWIFT_TYPEID_NAMED(void, Void)
 SWIFT_TYPEID_NAMED(std::string, String)
 
 // C++ standard library types.
+SWIFT_TYPEID_NAMED(evaluator::SideEffect, SideEffect)
 SWIFT_TYPEID_TEMPLATE1_NAMED(std::vector, Vector, typename T, T)
 SWIFT_TYPEID_TEMPLATE1_NAMED(std::unique_ptr, UniquePtr, typename T, T)
 

include/swift/Basic/TypeID.h

@@ -66,6 +66,29 @@ constexpr uint64_t formTypeID(uint8_t zone, uint8_t type) {
   return (uint64_t(zone) << 8) | uint64_t(type);
 }
 
+namespace evaluator {
+/// The return type of requests that execute side effects.
+///
+/// In general, it is not appropriate to use the request evaluator framework to
+/// execute a request for the sake of its side effects. However, there are
+/// operations we would currently like to be requests because it makes modelling
+/// some aspect of their implementation particularly nice. For example, an
+/// operation that emits diagnostics to run some checking code in a primary
+/// file may be desirable to requestify because it should be run only once per
+/// declaration, but it has no coherent return value. Another category of
+/// side-effecting requests are those that adapt existing parts of the compiler that
+/// do not yet behave in a "functional" manner to have a functional interface. Consider
+/// the request to run the SIL Optimizer. In theory, it should be a request that takes in
+/// a SILModule and returns a SILModule. In practice, it is a request that executes
+/// effects against a SILModule.
+///
+/// To make these requests stand out - partially in the hope we can return and
+/// refactor them to behave in a more well-structured manner, partially because
+/// they cannot return \c void or we will get template substitution failures - we
+/// annotate them as computing an \c evaluator::SideEffect.
+using SideEffect = std::tuple<>;
+}
+
 // Define the C type zone (zone 0).
 #define SWIFT_TYPEID_ZONE C
 #define SWIFT_TYPEID_HEADER "swift/Basic/CTypeIDZone.def"

lib/AST/Decl.cpp

@@ -4125,7 +4125,7 @@ void NominalTypeDecl::synthesizeSemanticMembersIfNeeded(DeclName member) {
 
   if (auto actionToTake = action) {
     (void)evaluateOrDefault(Context.evaluator,
-        ResolveImplicitMemberRequest{this, actionToTake.getValue()}, false);
+        ResolveImplicitMemberRequest{this, actionToTake.getValue()}, {});
   }
 }
 
@@ -7486,7 +7486,7 @@ LiteralExpr *EnumElementDecl::getRawValueExpr() const {
   (void)evaluateOrDefault(
       getASTContext().evaluator,
       EnumRawValuesRequest{getParentEnum(), TypeResolutionStage::Interface},
-      true);
+      {});
   return RawValueExpr;
 }
 
@@ -7496,7 +7496,7 @@ LiteralExpr *EnumElementDecl::getStructuralRawValueExpr() const {
   (void)evaluateOrDefault(
       getASTContext().evaluator,
       EnumRawValuesRequest{getParentEnum(), TypeResolutionStage::Structural},
-      true);
+      {});
   return RawValueExpr;
 }
 

lib/AST/TypeCheckRequests.cpp

@@ -863,14 +863,14 @@ bool EnumRawValuesRequest::isCached() const {
   return std::get<1>(getStorage()) == TypeResolutionStage::Interface;
 }
 
-Optional<bool> EnumRawValuesRequest::getCachedResult() const {
+Optional<evaluator::SideEffect> EnumRawValuesRequest::getCachedResult() const {
   auto *ED = std::get<0>(getStorage());
   if (ED->LazySemanticInfo.hasCheckedRawValues())
-    return true;
+    return std::make_tuple<>();
   return None;
 }
 
-void EnumRawValuesRequest::cacheResult(bool) const {
+void EnumRawValuesRequest::cacheResult(evaluator::SideEffect) const {
   auto *ED = std::get<0>(getStorage());
   auto flags = ED->LazySemanticInfo.RawTypeAndFlags.getInt() |
       EnumDecl::HasFixedRawValues |
@@ -1268,15 +1268,16 @@ void DifferentiableAttributeTypeCheckRequest::cacheResult(
 // TypeCheckSourceFileRequest computation.
 //----------------------------------------------------------------------------//
 
-Optional<bool> TypeCheckSourceFileRequest::getCachedResult() const {
+Optional<evaluator::SideEffect>
+TypeCheckSourceFileRequest::getCachedResult() const {
   auto *SF = std::get<0>(getStorage());
   if (SF->ASTStage == SourceFile::TypeChecked)
-    return true;
+    return std::make_tuple<>();
 
   return None;
 }
 
-void TypeCheckSourceFileRequest::cacheResult(bool result) const {
+void TypeCheckSourceFileRequest::cacheResult(evaluator::SideEffect) const {
   auto *SF = std::get<0>(getStorage());
 
   // Verify that we've checked types correctly.

lib/SILOptimizer/PassManager/PassManager.cpp

@@ -276,11 +276,12 @@ class PassManagerDeserializationNotificationHandler final
 
 } // end anonymous namespace
 
-llvm::Expected<bool> ExecuteSILPipelineRequest::evaluate(
+llvm::Expected<evaluator::SideEffect>
+ExecuteSILPipelineRequest::evaluate(
     Evaluator &evaluator, SILPipelineExecutionDescriptor desc) const {
   SILPassManager PM(desc.SM, desc.IsMandatory, desc.IRMod);
   PM.executePassPipelinePlan(desc.Plan);
-  return false;
+  return std::make_tuple<>();
 }
 
 void swift::executePassPipelinePlan(SILModule *SM,

lib/Sema/CodeSynthesis.cpp

@@ -1085,7 +1085,7 @@ void TypeChecker::addImplicitConstructors(NominalTypeDecl *decl) {
   (void)decl->getDefaultInitializer();
 }
 
-llvm::Expected<bool>
+llvm::Expected<evaluator::SideEffect>
 ResolveImplicitMemberRequest::evaluate(Evaluator &evaluator,
                                        NominalTypeDecl *target,
                                        ImplicitMemberAction action) const {
@@ -1162,7 +1162,7 @@ ResolveImplicitMemberRequest::evaluate(Evaluator &evaluator,
   }
     break;
   }
-  return true;
+  return std::make_tuple<>();
 }
 
 llvm::Expected<bool>