[PATCH][CMake] Fix PR14200, llvm-config output misses -fno-rtti

[Hans Wennborg via llvm-commits]

Merged this (r245064) to 3.7 in r245235.

Cheers,
Hans

On Fri, Aug 14, 2015 at 10:58 AM, Hans Wennborg <hans at chromium.org> wrote:
> Thanks!
>
> Let's have this bake for a few days. If nothing comes up, I'll merge
> it on Monday.
>
> Cheers,
> Hans
>
> On Fri, Aug 14, 2015 at 9:24 AM, Chris Bieneman <beanz at apple.com> wrote:
>> I committed this in r245064.
>>
>> Assuming there are no issues that arise on the bots this should be safe to
>> bring back to 3.7.
>>
>> David, thank you again for this patch.
>>
>> -Chris
>>
>> On Aug 14, 2015, at 8:45 AM, Chris Bieneman via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>>
>> David,
>>
>> Thank you for the patch. I’m more than happy to land the patch for you. I’ll
>> commit it within the next hour or so.
>>
>> Thanks,
>> -Chris
>>
>> On Aug 14, 2015, at 2:02 AM, David Wiberg <dwiberg at gmail.com> wrote:
>>
>> 2015-08-14 2:13 GMT+02:00 Tom Stellard <tom at stellard.net>:
>>
>> On Thu, Aug 13, 2015 at 03:22:54PM -0700, Chris Bieneman wrote:
>>
>> (+Tom Stellard)
>>
>> Looking at the patches, they’re probably the right way to go as a temporary
>> fix. Certainly this is the change that should go into 3.7, so if we want
>> this for 3.7 we should push it there. Any better fix will be bigger and
>> riskier.
>>
>> Tom has been working on improving llvm-config recently, and has sent out a
>> batch of patches (sorry for not getting to review them yet). In general I
>> think Tom’s solution is closer to what we actually want to end up with.
>>
>> The code path in these patches makes me feel a little dirty because it uses
>> get_property to read cxx flags of the target, but we set those flags based
>> on variables in the llvm_update_compile_flags function. That whole flow just
>> feels wrong to me, but I don’t think there is a simple solution. A better
>> solution will probably involve refactoring (and maybe eliminating)
>> llvm_update_compile_flags.
>>
>> TL;DR: These patches are fine to land, but we really should solve this in a
>> better way in the not-so-distant future.
>>
>> Thoughts?
>>
>>
>> I still don't know enough about CMake to comment on the implementation
>> details, but I think a quick fix like this is really the only way we can
>> get this fixed in the 3.7 branch, so I would be in favor of merging it.
>>
>> The correct solution requires some pretty invasive changes, like
>> s/NDEBUG/LLVM_NDEBUG/ everywhere, and it will probably need to bake for
>> a while in ToT to shake out all the issues.
>>
>> -Tom
>>
>>
>> Thanks for looking at the patch. I didn't push for landing it since I
>> noticed Tom's work which seems to be the correct long-term solution.
>> As I mentioned in my initial message I do not have commit rights so I
>> need some help with that part.
>>
>> Best regards
>> David
>>
>> -Chris
>>
>> On Aug 13, 2015, at 2:43 PM, Chandler Carruth via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>>
>> Once we have the right solution, this should definitely go to the 3.7
>> branch.
>>
>> I'm not sure this is the best way to get the compile flags though. CC-ing
>> Chris as he's probably the most immersed in this side of CMake at the
>> moment.
>>
>> On Thu, Aug 13, 2015 at 9:55 AM Hans Wennborg <hans at chromium.org
>> <mailto:hans at chromium.org>> wrote:
>> On Sun, Aug 2, 2015 at 9:27 AM, David Wiberg <dwiberg at gmail.com
>> <mailto:dwiberg at gmail.com>> wrote:
>>
>> Hi,
>>
>> I took a stab at solving PR14200. My CMake knowledge is low but I think the
>> solution is reasonable as it reuses the existing logic for setting flags.
>>
>>
>> Alexey: you're a cmake wizard. Can you take a look at this?
>>
>> Chandler: you're the cmake owner. I'd like to merge this to 3.7 when
>> it lands if that's OK.
>>
>> Things worth noting during review:
>> - The change means that both RTTI and exception handling flags will be
>> reported by llvm-config.
>> - I'm creating a variable with the same name as a target property. I don't
>> know if this breaks any convention.
>>
>>
>> I tried this locally, and the output of llvm-config --cxxflags changed from:
>>
>>  -I/work/llvm/include -I/work/llvm/build.release/include  -fPIC
>> -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter
>> -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic
>> -Wno-long-long -Wno-maybe-uninitialized -Wno-comment -std=c++11
>> -ffunction-sections -fdata-sections -O3   -D_GNU_SOURCE
>> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
>>
>> to:
>>
>>  -I/work/llvm/include -I/work/llvm/build.release/include  -fPIC
>> -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter
>> -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic
>> -Wno-long-long -Wno-maybe-uninitialized -Wno-comment -std=c++11
>> -ffunction-sections -fdata-sections -O3  -fno-exceptions -fno-rtti
>> -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS
>> -D__STDC_LIMIT_MACROS
>>
>> which seems correct.
>>
>> Thanks,
>> Hans
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>