[PATCH] D29955: Allow externally dlopen-ed libraries to be registered as permanent libraries.

[Frederich Munch via Phabricator via llvm-commits]

marsupial added a comment.



> Yeah, I didn't mean to create more work for you in https://reviews.llvm.org/D30107! The 'with-lock' helper seemed like the right solution here, and I have still have some questions/comments about https://reviews.llvm.org/D30107, hence the lgtm.

It shouldn't be that much work integrating it...was just saying as the ** addPermanentLibraryWithLock** is a bit ugly and would go away anyway.

> 
> 
>> Otherwise, doesn't this make it impossible to get symbols from an already loaded library unless all libs are searched?
>>  That is, assuming **"libSomething"** is already loaded:
>>  DynamicLibrary  DL = DynamicLibrary::addPermanentLibrary(dlopen(**"libSomething"**));
>>  DL.getAddressOfSymbol("func"); // Fails
> 
> In that case, the client could check if DL is invalid, and then simply construct "DynamicLibrary(dlopen'd-value)". Perhaps the API could be cleaned up if we returned "Expected<DynamicLibrary>" from the static constructors, and got rid of the isValid method.

Why not just have ** addPermanentLibrary** take an optional **bool*** indicating whether it was already loaded or not?
Whether it was loaded or not seems orthogonal to the task of adding a permanent library/getting a platform independent way to do lookups on it?

Either way if https://reviews.llvm.org/D30107 is to be expanded per your comments this could also be discussed then rather than holding this up.


https://reviews.llvm.org/D29955