niyue commented on code in PR #39098:
URL: https://github.com/apache/arrow/pull/39098#discussion_r1417035796
##########
cpp/src/gandiva/engine.h:
##########
@@ -38,15 +43,20 @@ class GANDIVA_EXPORT Engine {
llvm::LLVMContext* context() { return context_.get(); }
llvm::IRBuilder<>* ir_builder() { return ir_builder_.get(); }
LLVMTypes* types() { return &types_; }
- llvm::Module* module() { return module_; }
+
+ /// Retrieve LLVM module in the engine.
+ /// This should only be called before `FinalizeModule` is called
+ llvm::Module* module();
/// Factory method to create and initialize the engine object.
///
/// \param[in] config the engine configuration
/// \param[in] cached flag to mark if the module is already compiled and
cached
- /// \param[out] engine the created engine
- static Status Make(const std::shared_ptr<Configuration>& config, bool cached,
- std::unique_ptr<Engine>* engine);
+ /// \param[in] object_cache an optional object_cache used for building the
module
+ /// \return arrow::Result containing the created engine
+ static Result<std::unique_ptr<Engine>> Make(
+ const std::shared_ptr<Configuration>& config, bool cached,
+ std::optional<std::reference_wrapper<GandivaObjectCache>> object_cache =
std::nullopt);
Review Comment:
Since this PR have to change the `Engine::Make` and `LLVMGenerator::Make`
signatures anyway, I change the factory method to return an `arrow::Result`
instead of `arrow::Status` so that it is simpler to use and the optional
parameter like `object_cache` can be used.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
