[Openmp-commits] [PATCH] D141227: [OpenMP][libomptarget] Implement memory lock/unlock API in NextGen plugins

[Thorsten via Phabricator via Openmp-commits]

tschuett added inline comments.


================
Comment at: openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h:288
+  /// Find an allocation that intersects with a specific buffer pointer.
+  const EntryTy *findIntersecting(void *Buffer) const {
+    if (Allocs.empty())
----------------
jdoerfert wrote:
> tschuett wrote:
> > I would return `std::optional<const EntryTy*>` to avoid using `nullptr` to model failure.
> Why would we want that? It seems to me null is a fine, didn't find the entry response. We don't need to box things just to have boxed them. 
Because these are C idoms in C++ code. LLVM is inconsistent whether returning true or false denotes success. The boxing overhead is negligible. The intent of your APIs is much clearer. `tryFindIntersecting` is a fallible API. òptional` is a great tool to for fallible APIs. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141227