[PATCH] D121630: [IRLinker] make IRLinker::AddLazyFor Optional. NFC

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 14 13:26:07 PDT 2022


dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

LGTM. I have a slight preference for `llvm::unique_function<>` instead of `Optional<std::function<>>`, but if you have a reason to prefer what's in the patch that's fine with me.



================
Comment at: llvm/include/llvm/Linker/IRMover.h:78-79
   ///   function importing from Src.
   Error move(std::unique_ptr<Module> Src, ArrayRef<GlobalValue *> ValuesToLink,
-             std::function<void(GlobalValue &GV, ValueAdder Add)> AddLazyFor,
-             bool IsPerformingImport);
+             LazyCallback AddLazyFor, bool IsPerformingImport);
   Module &getModule() { return Composite; }
----------------
What about using `llvm::unique_function` instead? You can pass `nullptr` to this parameter. You'd want document here in the API that `nullptr` was an acceptable input.

(Not sure on tradeoffs between `unique_function<>` and `Optional<function<>>`... I have the general impression that `unique_function` is a bit nicer since it can take move-only function objects.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121630/new/

https://reviews.llvm.org/D121630



More information about the llvm-commits mailing list