beanz added a comment.

I question the general utility of this patch, and I don't really think this 
does anything we want the build system doing.

If you specify `STATIC` and `SHARED` to `llvm_add_library` it creates the 
shared library target as the one that everything links against, which is 
exactly what you don't want. There are two primary reasons why 
`BUILD_SHARED_LIBS` is not recommended for non-development builds both have to 
do with how shared object linking works:

(1) Is that clang when linked against shared libraries is *way* slower (I think 
last I measured was over 10%). This is caused by the need for resolving linkage 
at process launch every time a process is launched.

(2) Is that LLVM relies on a strange mess of static initializers used for 
static registration. The most problematic one is cl::opt. When you link a 
static archive only the object files that contain unresolved symbols get pulled 
in, when you link against a shared object, all of it gets pulled in. Because 
static archives selectively pull in object files (and their static 
initializers) we have had issues in the past where the static build of LLVM 
works, but the shared build produces runtime errors from cl::opt.

I'm curious to understand the problem you are trying to solve. From other posts 
on the list it really sounds to me like you want a variant of libClang that 
exports the entire Clang API surface rather than just the C API. I think there 
are better ways to accomplish that.

Exposing Clang's libraries for piecemeal consumption is a much bigger problem 
than just the build system not building libClang*.so. For example, Clang has no 
equivalent of llvm-config, so there is no external representation of the clang 
dependency graph that users can rely on.

This all aside, I also have issues with this implementation described inline 
below.



================
Comment at: clang/CMakeLists.txt:451
+option(CLANG_ENABLE_SHARED_LIBRARIES "Build libclang* as shared libraries." ON)
+option(CLANG_ENABLE_STATIC_LIBRARIES "Build libclang* as static libraries." ON)
+
----------------
These shouldn't both default to `On`, that is a change in behavior that would 
be a build-time regression for anyone not interested in building shared 
libraries. `STATIC` should default `On`, and `SHARED` default `Off`.

Also you need to check that one of the two options is enabled. If both are 
`Off` confusing things will happen.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61804/new/

https://reviews.llvm.org/D61804



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

Reply via email to