[NFC] Drop llvm::Expected from Evaluation Points

A request is intended to be a pure function of its inputs. That function could, in theory, fail. In practice, there were basically no requests taking advantage of this ability - the few that were using it to explicitly detect cycles can just return reasonable defaults instead of forwarding the error on up the stack.

This is because cycles are checked by *the Evaluator*, and are unwound by the Evaluator.

Therefore, restore the idea that the evaluate functions are themselves pure, but keep the idea that *evaluation* of those requests may fail. This model enables the best of both worlds: we not only keep the evaluator flexible enough to handle future use cases like cancellation and diagnostic invalidation, but also request-based dependencies using the values computed at the evaluation points. These aforementioned use cases would use the llvm::Expected interface and the regular evaluation-point interface respectively.

Author: CodaFi

Diff for: include/swift/AST/AccessRequests.h

@@ -41,8 +41,7 @@ class AccessLevelRequest :
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<AccessLevel> evaluate(Evaluator &evaluator,
-                                       ValueDecl *decl) const;
+  AccessLevel evaluate(Evaluator &evaluator, ValueDecl *decl) const;
 
 public:
   // Separate caching.
@@ -65,8 +64,7 @@ class SetterAccessLevelRequest :
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<AccessLevel>
-  evaluate(Evaluator &evaluator, AbstractStorageDecl *decl) const;
+  AccessLevel evaluate(Evaluator &evaluator, AbstractStorageDecl *decl) const;
 
 public:
   // Separate caching.
@@ -88,8 +86,7 @@ class DefaultAndMaxAccessLevelRequest :
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<DefaultAndMax>
-  evaluate(Evaluator &evaluator, ExtensionDecl *decl) const;
+  DefaultAndMax evaluate(Evaluator &evaluator, ExtensionDecl *decl) const;
 
 public:
   // Separate caching.

Diff for: include/swift/AST/Evaluator.h

@@ -54,7 +54,7 @@ using AbstractRequestFunction = void(void);
 /// Form the specific request function for the given request type.
 template<typename Request>
 using RequestFunction =
-  llvm::Expected<typename Request::OutputType>(const Request &, Evaluator &);
+  typename Request::OutputType(const Request &, Evaluator &);
 
 /// Pretty stack trace handler for an arbitrary request.
 template<typename Request>

Diff for: include/swift/AST/IRGenRequests.h

@@ -124,7 +124,7 @@ class IRGenSourceFileRequest
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<std::unique_ptr<llvm::Module>>
+  std::unique_ptr<llvm::Module>
   evaluate(Evaluator &evaluator, IRGenDescriptor desc) const;
 
 public:
@@ -142,7 +142,7 @@ class IRGenWholeModuleRequest
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<std::unique_ptr<llvm::Module>>
+  std::unique_ptr<llvm::Module>
   evaluate(Evaluator &evaluator, IRGenDescriptor desc) const;
 
 public:

Diff for: include/swift/AST/NameLookupRequests.h

@@ -153,7 +153,7 @@ class SuperclassDeclRequest :
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<ClassDecl *>
+  ClassDecl *
   evaluate(Evaluator &evaluator, NominalTypeDecl *subject) const;
 
 public:
@@ -197,7 +197,7 @@ class HasMissingDesignatedInitializersRequest :
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<bool>
+  bool
   evaluate(Evaluator &evaluator, ClassDecl *subject) const;
 
 public:
@@ -219,7 +219,7 @@ class ExtendedNominalRequest :
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<NominalTypeDecl *>
+  NominalTypeDecl *
   evaluate(Evaluator &evaluator, ExtensionDecl *ext) const;
 
 public:
@@ -283,7 +283,7 @@ class CustomAttrNominalRequest :
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<NominalTypeDecl *>
+  NominalTypeDecl *
   evaluate(Evaluator &evaluator, CustomAttr *attr, DeclContext *dc) const;
 
 public:
@@ -303,7 +303,7 @@ class GetDestructorRequest :
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<DestructorDecl *>
+  DestructorDecl *
   evaluate(Evaluator &evaluator, ClassDecl *classDecl) const;
 
 public:
@@ -324,7 +324,7 @@ class GenericParamListRequest :
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<GenericParamList *>
+  GenericParamList *
   evaluate(Evaluator &evaluator, GenericContext *value) const;
 
 public:
@@ -347,7 +347,7 @@ class ExpandASTScopeRequest
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<ast_scope::ASTScopeImpl *>
+  ast_scope::ASTScopeImpl *
   evaluate(Evaluator &evaluator, ast_scope::ASTScopeImpl *,
            ast_scope::ScopeCreator *) const;
 
@@ -407,8 +407,8 @@ class UnqualifiedLookupRequest
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<LookupResult> evaluate(Evaluator &evaluator,
-                                        UnqualifiedLookupDescriptor desc) const;
+  LookupResult evaluate(Evaluator &evaluator,
+                        UnqualifiedLookupDescriptor desc) const;
 };
 
 using QualifiedLookupResult = SmallVector<ValueDecl *, 4>;
@@ -427,7 +427,7 @@ class LookupInModuleRequest
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<QualifiedLookupResult>
+  QualifiedLookupResult
   evaluate(Evaluator &evaluator, const DeclContext *moduleOrFile, DeclName name,
            NLKind lookupKind, namelookup::ResolutionKind resolutionKind,
            const DeclContext *moduleScopeContext) const;
@@ -445,10 +445,10 @@ class AnyObjectLookupRequest
 private:
   friend SimpleRequest;
 
-  llvm::Expected<QualifiedLookupResult> evaluate(Evaluator &evaluator,
-                                                 const DeclContext *dc,
-                                                 DeclNameRef name,
-                                                 NLOptions options) const;
+  QualifiedLookupResult evaluate(Evaluator &evaluator,
+                                 const DeclContext *dc,
+                                 DeclNameRef name,
+                                 NLOptions options) const;
 };
 
 class ModuleQualifiedLookupRequest
@@ -464,10 +464,10 @@ class ModuleQualifiedLookupRequest
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<QualifiedLookupResult> evaluate(Evaluator &evaluator,
-                                                 const DeclContext *DC,
-                                                 ModuleDecl *mod, DeclNameRef name,
-                                                 NLOptions opts) const;
+  QualifiedLookupResult evaluate(Evaluator &evaluator,
+                                 const DeclContext *DC,
+                                 ModuleDecl *mod, DeclNameRef name,
+                                 NLOptions opts) const;
 };
 
 class QualifiedLookupRequest
@@ -483,7 +483,7 @@ class QualifiedLookupRequest
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<QualifiedLookupResult>
+  QualifiedLookupResult
   evaluate(Evaluator &evaluator, const DeclContext *DC,
            SmallVector<NominalTypeDecl *, 4> decls,
            DeclNameRef name,
@@ -534,7 +534,7 @@ class DirectLookupRequest
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<TinyPtrVector<ValueDecl *>>
+  TinyPtrVector<ValueDecl *>
   evaluate(Evaluator &evaluator, DirectLookupDescriptor desc) const;
 };
 
@@ -609,8 +609,8 @@ class LookupOperatorRequest
                        CacheKind::Uncached>;
 
   // Evaluation.
-  llvm::Expected<OperatorType *> evaluate(Evaluator &evaluator,
-                                          OperatorLookupDescriptor desc) const;
+  OperatorType *
+  evaluate(Evaluator &evaluator, OperatorLookupDescriptor desc) const;
 };
 
 using LookupPrefixOperatorRequest = LookupOperatorRequest<PrefixOperatorDecl>;
@@ -631,7 +631,7 @@ class DirectOperatorLookupRequest
 private:
   friend SimpleRequest;
 
-  llvm::Expected<TinyPtrVector<OperatorDecl *>>
+  TinyPtrVector<OperatorDecl *>
   evaluate(Evaluator &evaluator, OperatorLookupDescriptor descriptor,
            OperatorFixity fixity) const;
 };
@@ -649,7 +649,7 @@ class DirectPrecedenceGroupLookupRequest
 private:
   friend SimpleRequest;
 
-  llvm::Expected<TinyPtrVector<PrecedenceGroupDecl *>>
+  TinyPtrVector<PrecedenceGroupDecl *>
   evaluate(Evaluator &evaluator, OperatorLookupDescriptor descriptor) const;
 };
 
@@ -694,7 +694,7 @@ class LookupConformanceInModuleRequest
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<ProtocolConformanceRef> evaluate(
+  ProtocolConformanceRef evaluate(
       Evaluator &evaluator, LookupConformanceDescriptor desc) const;
 };
 

Diff for: include/swift/AST/SILGenRequests.h

@@ -89,7 +89,7 @@ class SILGenSourceFileRequest :
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<std::unique_ptr<SILModule>>
+  std::unique_ptr<SILModule>
   evaluate(Evaluator &evaluator, SILGenDescriptor desc) const;
 
 public:
@@ -107,7 +107,7 @@ class SILGenWholeModuleRequest :
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<std::unique_ptr<SILModule>>
+  std::unique_ptr<SILModule>
   evaluate(Evaluator &evaluator, SILGenDescriptor desc) const;
 
 public:

Diff for: include/swift/AST/SILOptimizerRequests.h

@@ -59,7 +59,7 @@ class ExecuteSILPipelineRequest
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<evaluator::SideEffect>
+  evaluator::SideEffect
   evaluate(Evaluator &evaluator, SILPipelineExecutionDescriptor desc) const;
 };
 

Diff for: include/swift/AST/SimpleRequest.h

@@ -148,9 +148,9 @@ SourceLoc extractNearestSourceLoc(const std::tuple<First, Rest...> &value) {
 ///
 /// The \c Derived class needs to implement several operations. The most
 /// important one takes an evaluator and the input values, then computes the
-/// final result, optionally bubbling up errors from recursive evaulations:
+/// final result:
 /// \code
-///   llvm::Expected<Output> evaluate(Evaluator &evaluator, Inputs...) const;
+///   Output evaluate(Evaluator &evaluator, Inputs...) const;
 /// \endcode
 ///
 /// Cycle diagnostics can be handled in one of two ways. Either the \c Derived
@@ -194,7 +194,7 @@ class SimpleRequest<Derived, Output(Inputs...), Caching> {
   }
 
   template<size_t ...Indices>
-  llvm::Expected<Output>
+  Output
   callDerived(Evaluator &evaluator, std::index_sequence<Indices...>) const {
     static_assert(sizeof...(Indices) > 0, "Subclass must define evaluate()");
     return asDerived().evaluate(evaluator, std::get<Indices>(storage)...);
@@ -214,7 +214,7 @@ class SimpleRequest<Derived, Output(Inputs...), Caching> {
     : storage(inputs...) { }
 
   /// Request evaluation function that will be registered with the evaluator.
-  static llvm::Expected<OutputType>
+  static OutputType
   evaluateRequest(const Derived &request, Evaluator &evaluator) {
     return request.callDerived(evaluator,
                                std::index_sequence_for<Inputs...>());

Diff for: include/swift/AST/TBDGenRequests.h

@@ -92,8 +92,7 @@ class GenerateTBDRequest
   friend SimpleRequest;
 
   // Evaluation.
-  llvm::Expected<TBDFileAndSymbols> evaluate(Evaluator &evaluator,
-                                             TBDGenDescriptor desc) const;
+  TBDFileAndSymbols evaluate(Evaluator &evaluator, TBDGenDescriptor desc) const;
 };
 
 /// Report that a request of the given kind is being evaluated, so it