labath added inline comments.
================ Comment at: lldb/include/lldb/Host/HostInfoBase.h:44-46 + using SharedLibraryDirectoryHelper = void(FileSpec &this_file); + + static void Initialize(SharedLibraryDirectoryHelper *helper = nullptr); ---------------- JDevlieghere wrote: > labath wrote: > > JDevlieghere wrote: > > > Why a function pointer and no `function_ref`? > > function_ref can only be used for objects which do not escape the current > > function (just like StringRef), while this value is persisted. It could be > > a std::function, but that seems a bit overkill. > Forgive my ignorance, but how does that apply to > `SharedLibraryDirectoryHelper` which is a static method? It doesn't -- persisting a function_ref pointing to a static method would actually work in this case. However, that's the _only_ thing that would work, so there's no benefit to doing it. And there's the downside that if someone tries to something more fancy through that function_ref, it would blow up. E.g., something like this would be impossible ``` HostInfo::Initialize([important_variable](FileSpec &this_file) { do_stuff(this_file, important_variable); }); ``` Because by the time that the function_ref gets called, the storage for `important_variable` (along with the entire lambda object) would be destroyed. It's exactly the same situation as with a StringRef. Imagine code like: ``` StringRef g_foo; HostInfo::Initialize(StringRef foo) { g_foo = foo; } HostInfo::Stuff() { /* do something with g_foo */ } ``` This would work just fine if you always pass a string literal for the StringRef, but would blow up as soon as one tries to pass a temporary string. And there are two ways to make it safer: - use std::string (like std::function) - use const char * (like a raw function pointer, except function pointers are even safer, as dynamically allocating functions is not common) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96779/new/ https://reviews.llvm.org/D96779 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits