ABataev added inline comments.

================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:51
+  /// at the time, and location, the callback is invoked.
+  using FinalizeCallbackTy = std::function<void(InsertPointTy /* CodeGenIP 
*/)>;
+
----------------
jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > ABataev wrote:
> > > > `llvm::function_ref`?
> > > The lambda that is passed in here might go out of scope so we need to own 
> > > it. This is intentional.
> > Maybe better to save the intermediate data in CGOpenMPRuntime class rather 
> > than rely on owning lambda ref here? Clang does not use escaping decls and 
> > instead stores intermediate data explicitly. It really complicates analysis 
> > and potential source of resource leakage.
> I don't follow. Clang does use `std::function` to store callbacks that have 
> to life for a while. Why is this different? What would be the benefit of 
> having a function_ref here and a `std::function` in CGOpenMPRuntime? Note 
> that the FinaliztionInfo object is created in the front-end and the 
> std::function is assigned there already.
Clang uses it only in some rare cases, when it is really required, like typo 
correction or something like this. In other cases it is not recommended to use 
it.

I'm not saying that you need to store `std::function` in CGOpenMPRuntime class, 
I'm saying about necessary data.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70258



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to