[llvm] r178713 - Make it possible to include llvm-c without including C++ headers. Patch by Filip Pizlo.

[Eric Christopher]

On Wed, Apr 3, 2013 at 8:51 PM, Filip Pizlo <fpizlo at apple.com> wrote:
>
> On Apr 3, 2013, at 7:03 PM, Eric Christopher <echristo at gmail.com> wrote:
>
> This seems like a pretty horrific hack.
>
>
> I'm not sure I share your subjective assessment of this fix.
>

That's your prerogative.

> I don't think we want to rely
> on the preprocessor to do this sort of thing.
>
>
> I've got no objections to not naming unused parameters in headers, in
> fact, when I did this same sort of thing a while back I cleaned that
> up and it seems like a much better way to go.
>
>
> Unused parameters wasn't the only warning that came up.  -Wmissing-noreturn
> is another, and that one is trickier to fix.
>

It is, but only slightly and could be turned off by a client instead.
I don't see
any reason to burden the llvm code base with changes for a particular client's
set of warnings.

> My intuition is that it's unwise to expect WebKit's use of
> -Wmissing-noreturn and -Wunused-param to be the last case of build failures
> in LLVM clients due to unexpected uses of warning C++ flags.  This change
> ensures that downstream clients can benefit from LLVM's C API even if they
> use these, and other, flags - since the C API headers themselves follow
> really good hygiene (mostly by virtue of not having any function bodies, and
> sticking to simple pure C declarations).  This change also preserves
> existing behavior for clients who use the C++ API instead of the C one, or
> who have already side-stepped the build warnings/errors by other means.
>
> Do you disagree with this intuition, or do you believe that there exists a
> better long-term solution?
>

I do disagree with your intuition.

There's no need to have a preprocessor macro to deal with this and it
can be solved by either making llvm clean to those warnings or
requiring clients to turn off those warnings on those files.

Another possibility is for you to undef cplusplus around the includes.
Ok, this one is pretty lame, but would work :)

We've fixed up various different compile warnings in the past and no
doubt will in the future. We currently build clean with a lot of
warnings and with more than just a couple compilers for our own
project files. I don't see anything wrong with building clean for
those warnings either or anything else as it comes up.

-eric