Re: [clang] 6654719 - [CMake] Fix logic error: NOT LIBCLANG_BUILD_STATIC does not imply PIC

[David Zarzycki via cfe-commits]

Hans,

Non-Windows non-PIC setups were working just fine until four days ago when D75068 landed. I tried to narrowly unbreak non-Windows non-PIC systems but apparently that broke Windows. For that, I'm sorry. That being said, non-Windows systems are broken again and this breakable was foreseeable given my revert. What is your plan for non-Windows systems?

As a part of the post-revert discussion in D75068, cristian.adam suggested that the check for WIN32 should be added back, but nobody followed through on that. What is stopping us from adding Christian's proposed change? Specifically:

if(LLVM_ENABLE_PIC OR (WIN32 AND NOT LIBCLANG_BUILD_STATIC))

Dave

On Wed, Apr 29, 2020, at 9:19 AM, Hans Wennborg wrote:
> On Sun, Apr 26, 2020 at 1:17 PM David Zarzycki via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
> >
> >
> > Author: David Zarzycki
> > Date: 2020-04-26T07:16:42-04:00
> > New Revision: 665471907a5c072c6653a38c35f35e5d54cef220
> >
> > URL: https://github.com/llvm/llvm-project/commit/665471907a5c072c6653a38c35f35e5d54cef220
> > DIFF: https://github.com/llvm/llvm-project/commit/665471907a5c072c6653a38c35f35e5d54cef220.diff
> >
> > LOG: [CMake] Fix logic error: NOT LIBCLANG_BUILD_STATIC does not imply PIC
> 
> But it does imply building libclang.dll.
> 
> Your change broke building libclang.dll when configured with "cmake
> -GNinja -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS=clang
> -DLLVM_TARGETS_TO_BUILD=X86 -DLLVM_ENABLE_PIC=OFF" on Windows, see the
> discussion here: https://reviews.llvm.org/D74564#1884556
> 
> I've reverted in 209ab6d8835cd88320ceb814893759cfbda91d15.
> 
> >
> > Added:
> >
> >
> > Modified:
> >     clang/tools/libclang/CMakeLists.txt
> >
> > Removed:
> >
> >
> >
> > ################################################################################
> > diff  --git a/clang/tools/libclang/CMakeLists.txt b/clang/tools/libclang/CMakeLists.txt
> > index bb2b14cc8e27..9368501592a9 100644
> > --- a/clang/tools/libclang/CMakeLists.txt
> > +++ b/clang/tools/libclang/CMakeLists.txt
> > @@ -77,7 +77,7 @@ if(MSVC)
> >    set(LLVM_EXPORTED_SYMBOL_FILE)
> >  endif()
> >
> > -if(LLVM_ENABLE_PIC OR NOT LIBCLANG_BUILD_STATIC)
> > +if(LLVM_ENABLE_PIC)
> >    set(ENABLE_SHARED SHARED)
> >  endif()
> >
> >
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>