jdoerfert marked an inline comment as done. jdoerfert 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 */)>; + ---------------- ABataev wrote: > 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. What necessary data? Can you please explain how you want it to look like and why? This version seems to work fine. 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
