JDevlieghere 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);
----------------
labath wrote:
> 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)
Okay, sounds like we're on the same page, I misunderstood your comment as "this
can't work for the current case" which led to the confusion on my part. I agree
that making the interface as safe as possible is desirable.
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
