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

[Hans Wennborg via llvm-commits]

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
>
>