[Lldb-commits] [lldb] r210035 - Fix most of the remaining Windows build warnings.

[Todd Fiala]

I've now tested that final cmake patch on Ubuntu Linux 12.04/gcc-4.8.2,
MacOSX 10.9.3/clang, FreeBSD 10/clang and they all pass.

I'll go check it in.  Ping me with your failure case if you end up with one
after the change.


On Tue, Jun 17, 2014 at 1:14 PM, Todd Fiala <tfiala at google.com> wrote:

> Thanks, Zachary.
>
> Ed - I'm testing the patch on my FreeBSD VM.  Finally firing it up again...
>
>
> On Tue, Jun 17, 2014 at 1:13 PM, Zachary Turner <zturner at google.com>
> wrote:
>
>> Yes, this was for building with MSVC, which does not support those linker
>> options.  I guess either the patch you sent out earlier, or explicitly
>> excluding those compilers which don't support this flag works.
>>
>>
>> On Tue, Jun 17, 2014 at 1:11 PM, Todd Fiala <tfiala at google.com> wrote:
>>
>>> Zachary - were you trying to build on Windows without a gcc-compatible
>>> compiler, and getting linker errors/warnings in that case?
>>>
>>> If so, a correct check might really be the final patch I sent out
>>> earlier:
>>>
>>> diff --git a/CMakeLists.txt b/CMakeLists.txt
>>> index bad83e8..4cfa1d9 100644
>>> --- a/CMakeLists.txt
>>> +++ b/CMakeLists.txt
>>> @@ -218,7 +218,8 @@ macro(add_lldb_library name)
>>>     endif ()
>>>
>>>    if(LLDB_USED_LIBS)
>>> -    if (LLVM_COMPILER_IS_GCC_COMPATIBLE)
>>> +    # The Darwin linker doesn't understand --start-group/--end-group.
>>> +    if (LLVM_COMPILER_IS_GCC_COMPATIBLE AND NOT "${CMAKE_SYSTEM_NAME}"
>>> MATCHES "Darwin")
>>>
>>>
>>>
>>> On Tue, Jun 17, 2014 at 1:02 PM, Todd Fiala <tfiala at google.com> wrote:
>>>
>>>> This latest patch works on Ubuntu12.04/gcc4.8.2 and MacOSX10.9.3/clang.
>>>>  But thinking more about it, the predicate as a whole (changing based on
>>>> compiler rather than Linker) is just incorrect.  So the platform checks
>>>> there before, which isn't a bad proxy for the system linker requirements,
>>>> is a better check than the compiler test.
>>>>
>>>> Zachary - what were you originally intending to address with the
>>>> change?  Which platform(s) were affected by the patch?  We can possibly
>>>> handle that via another mechanism.  In hindsight I'm not a big fan of
>>>> testing the compiler since it isn't enough to get this right on all
>>>> platofmrs
>>>>
>>>> -Todd
>>>>
>>>>
>>>> On Tue, Jun 17, 2014 at 12:43 PM, Todd Fiala <tfiala at google.com> wrote:
>>>>
>>>>> Keying off that link executable variable won't work.  Here's what it's
>>>>> set to on MacOSX:
>>>>>
>>>>> CMAKE_CXX_LINK_EXECUTABLE=<CMAKE_CXX_COMPILER>  <FLAGS>
>>>>> <CMAKE_CXX_LINK_FLAGS> <LINK_FLAGS> <OBJECTS>  -o <TARGET> <LINK_LIBRARIES>
>>>>>
>>>>> Ok - so maybe what we need is this patch (which I'm testing over here
>>>>> now):
>>>>> diff --git a/CMakeLists.txt b/CMakeLists.txt
>>>>> index bad83e8..4cfa1d9 100644
>>>>> --- a/CMakeLists.txt
>>>>> +++ b/CMakeLists.txt
>>>>> @@ -218,7 +218,8 @@ macro(add_lldb_library name)
>>>>>    endif ()
>>>>>
>>>>>    if(LLDB_USED_LIBS)
>>>>> -    if (LLVM_COMPILER_IS_GCC_COMPATIBLE)
>>>>> +    # The Darwin linker doesn't understand --start-group/--end-group.
>>>>> +    if (LLVM_COMPILER_IS_GCC_COMPATIBLE AND NOT
>>>>> "${CMAKE_SYSTEM_NAME}" MATCHES "Darwin")
>>>>>        target_link_libraries(${name} ${cmake_2_8_12_PUBLIC}
>>>>>                              -Wl,--start-group ${LLDB_USED_LIBS}
>>>>> -Wl,--end-group)
>>>>>      else()
>>>>>
>>>>>
>>>>>
>>>>> On Tue, Jun 17, 2014 at 12:31 PM, Todd Fiala <tfiala at google.com>
>>>>> wrote:
>>>>>
>>>>>> It looks like we might be able to test ${CMAKE_CXX_LINK_EXECUTABLE}
>>>>>> to exclude those that don't need --start-group/--end-group linker flags.
>>>>>>
>>>>>> However, this is getting pretty far past the original contributor
>>>>>> patch.  I'm more inclined to roll it back at this point until the whole
>>>>>> thing gets more testing.
>>>>>>
>>>>>>
>>>>>> On Tue, Jun 17, 2014 at 12:25 PM, Todd Fiala <tfiala at google.com>
>>>>>> wrote:
>>>>>>
>>>>>>> Ok - so given it's a linker related flags issue, what is the correct
>>>>>>> cmake way to test out the linker being used?
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Jun 17, 2014 at 11:49 AM, Ed Maste <emaste at freebsd.org>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> On 17 June 2014 14:33, Todd Fiala <tfiala at google.com> wrote:
>>>>>>>> >
>>>>>>>> > Hey Ed - can you test this out on FreeBSD?  This is a patch that
>>>>>>>> fixes cmake on MacOSX.  I verified it works on MacOSX and Linux with
>>>>>>>> cmake/ninja.
>>>>>>>> >
>>>>>>>> > The original code looks like it was opting in for the
>>>>>>>> --start-group/--end-group linker options on FreeBSD, which this patch would
>>>>>>>> change but might be a no-op on your end.
>>>>>>>>
>>>>>>>> It's odd that this is a compiler-dependent test, I'd expect it to
>>>>>>>> be a
>>>>>>>> linker issue.  In any case, we definitely needed --start-group and
>>>>>>>> --end-group in the past.  If this test works with Linux though it
>>>>>>>> will
>>>>>>>> probably work with FreeBSD too.  I'll test it.
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Todd Fiala | Software Engineer |  tfiala at google.com |  650-943-3180
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Todd Fiala | Software Engineer |  tfiala at google.com |  650-943-3180
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Todd Fiala | Software Engineer |  tfiala at google.com |  650-943-3180
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Todd Fiala | Software Engineer |  tfiala at google.com |  650-943-3180
>>>>
>>>
>>>
>>>
>>> --
>>> Todd Fiala | Software Engineer |  tfiala at google.com |  650-943-3180
>>>
>>
>>
>
>
> --
> Todd Fiala | Software Engineer |  tfiala at google.com |  650-943-3180
>



-- 
Todd Fiala | Software Engineer | tfiala at google.com | 650-943-3180
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140617/86919fed/attachment.html>