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

Reply via email to