I had a talk with Lang about the ExecutionEngine library structuring, and it 
sounds like there are some problems there that need to be worked out.

Luckily for this specific case, I think the solution is actually quite simple:

```
diff --git a/include/llvm/ExecutionEngine/ExecutionEngine.h 
b/include/llvm/ExecutionEngine/ExecutionEngine.h
index f68337c..cc99f94 100644
--- a/include/llvm/ExecutionEngine/ExecutionEngine.h
+++ b/include/llvm/ExecutionEngine/ExecutionEngine.h
@@ -15,7 +15,6 @@
 #ifndef LLVM_EXECUTIONENGINE_EXECUTIONENGINE_H
 #define LLVM_EXECUTIONENGINE_EXECUTIONENGINE_H
 
-#include "RuntimeDyld.h"
 #include "llvm-c/ExecutionEngine.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
@@ -49,6 +48,7 @@ class ObjectCache;
 class RTDyldMemoryManager;
 class Triple;
 class Type;
+class JITSymbolResolver;
 
 namespace object {
   class Archive;
```

It seems to me that there is no reason why ExecutionEngine.h needs to include 
RuntimeDyld.h. a forward declaration of the JITSymbolResolver class will 
suffice.

-Chris

> On Mar 30, 2017, at 11:41 AM, Michał Górny via Phabricator 
> <revi...@reviews.llvm.org> wrote:
> 
> mgorny added a comment.
> 
> In https://reviews.llvm.org/D31367#714305, @beanz wrote:
> 
>> Please revert your patch. It is *not* the right solution and is masking 
>> underlying problems.
> 
> 
> Reverted in r299095. Now I'd really appreciate some help in figuring out how 
> to fix it properly.
> 
>> ExecutionEngine headers directly reference symbols from RuntimeDyld, so we 
>> should enforce a requirement that anyone using ExeuctionEngine also link 
>> RuntimeDyld. This works today as expected for static archive builds. It is 
>> only broken with `BUILD_SHARED_LIBS`. I suspect the problem is that when 
>> built as a DSO ExecutionEngine has no unresolved symbols against 
>> RuntimeDyld, and the linker probably isn't including the reference to 
>> RuntimeDyld in the produced ExecutionEngine DSO.
> 
> The DSO is linking to RuntimeDyld. However, obviously the `PRIVATE` linkage 
> forced in `llvm_add_library` is not the correct solution when the symbols are 
> explicitly used in the headers. It should be `PUBLIC` for that particular 
> component. Any suggestion on how to fix that API? Should I add an additional 
> `PUBLIC_LINK_COMPONENTS` (and `PUBLIC_LINK_LIBS`)?
> 
> 
> Repository:
>  rL LLVM
> 
> https://reviews.llvm.org/D31367
> 
> 
> 

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to