[haiku-commits] Re: haiku: hrev46660 - build/jam src/system/libroot/posix/glibc/stdio-common
- From: Jonathan Schleifer <js-haiku-commits@xxxxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Mon, 13 Jan 2014 15:03:38 +0100
Am 13.01.2014 um 14:42 schrieb Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>:
>> On January 13, 2014 at 2:14 PM Jonathan Schleifer
>> <js-haiku-commits@xxxxxxxxxxx> wrote:
>> You could define them per component of course. Just treating all warnings as
>> errors can often
>> break things through a simple compiler upgrade. While for GCC, we currently
>> enforce the GCC we
>> bundle, we should not do that for Clang, as it's always a cross-compiler that
>> supports Haiku
>> and takes quite long to build. Thus we should only check that the installed
>> Clang has a certain
>> minimum version (which would be 3.2 at the moment).
>
> That we cannot require a specific version of clang to use for building is
> indeed
> a very important limitation.
We could, but I don't think we should… Building Clang and LLVM takes quite a
while, as it's written in C++. On slower machines, you'd easily build several
hours, when you could just install a recent Clang from your distribution.
> I'm also not sure it's a good idea as compilers tend to introduce new
> optimizations that do not work correctly in all cases, or in other words, the
> same applies to -O2 vs. specifying all optimizations manually.
GCC tends to do that, yes. But I haven't seen that with Clang, at least on x86.
What I do have seen is that on other platforms, certain optimization levels are
broken. E.g. once on PPC, -O0 was broken, but -O2 worked. But I have never seen
that updating Clang broke code through optimizations, on the contrary. That's
why I feel quite confident in only requiring a minimum version and not an exact
version.
There was only one thing where an update of Clang broke the code: Using -Wall
-Werror ;).
> Both cases are problematic without a specific release to target, though, as
> the
> number and names of warnings and optimizations change between releases. If
> clang
> manages to maintain compatibility for those, we would only ever need a minimum
> release requirement, though.
The names of warnings in Clang usually do not change, so that it's possible to
have a list of warnings that we consider an error. What does change *very*
often though is what -Wall means: New warnings are added to -Wall all the time.
Also, we should add Clang warnings that are not in -Wall: -Wshorten-64-to-32
would help a lot with the 64-bit port, -Wdocumentation is quite nice to check
that documentation is correct, etc.
> In any case: useful warnings should always break the build, useless warnings
> should not be visible.
For another project (e.g. my ObjFW), I agree (and I do so there). For Haiku, I
don't. The number of warnings in Haiku is too high to do that right now. It
would take years to fix all warnings.
Consider this: We had several places where we severely violated the C++
standard, yet GCC did not complain. These errors were so severe that Clang
wouldn't even compile them, not even without -Werror. So GCC does not even emit
a warning for severe errors. Clang OTOH issues a warning for everything where
it thinks that the programmer should reconsider that code.
> Having useful warnings only visible never gets them fixed, or at most from
> time
> to time if someone thinks it's about time to go through that list again.
> Forcing
> each developer to fix their own problems works much better in practice. It
> also
> is just one other method to improve the overall code quality.
Well, it helps fixing them over time. Fixing them all at once is a task we
cannot accomplish for Haiku. Our general code quality is not good enough for
that (not meant as an insult, but sadly, a fact - and a lot of it is due having
imported 3rd party applications).
> It's okay if new compiler releases break the build if the warnings that
> trigger
> the issue improve the code quality. If not, the new warning should be turned
> off
> completely. If you go the other way around, as you suggest, you'll never
> (well,
> rarely, on manual intervention only) benefit from new warnings a new compiler
> release has to offer.
Well, we're not that far yet. If we could build Haiku without a single warning
right now, then yes, I agree. But we're too far from that right now. And even
if we get it to build without a single warning, the question is still: Should a
user who just wants to build Haiku get an error instead a warning, or should
only developers get an error instead of a warning?
> And that's why it also makes sense to require a specific compiler release to
> build Haiku, even for clang. However, for the latter, we might want to add an
> option to turn new warnings into actual warnings to be able to work with newer
> compiler releases without too much hassle (in general, I think updating the
> build files for each and every new compiler release sounds quite manageable).
Again, this is something to think about once we're warning-free ;).
> Turning off -Werror on a per component basis is not ideal either, but is kind
> of
> a reality clash we have to accept at least as a temporary work-around due to
> man
> power. An additional clang mode there would only make sense if clang offers a
> lot of useful warnings we do not want to hide in general, and that would
> cause a
> lot of work to fix for every component. Otherwise it would needlessly clutter
> the Jamfiles.
Exactly this is the case. Clang does not emit useless warnings. But the amount
of warnings is too damn high to fix them all right now.
So there are two solutions:
1.) Disable -Werror for Clang
2.) Disable all warnings which GCC does not emit
I chose 1, because 2 would require a list that would be longer than the list of
warnings that we really want to treat as an error.
> An alternative way would be a single Clang build file that does that per
> component, ideally turning off only the warnings that are problematic. The
> same
> could be done for GCC, so that we would have all problems listed in a single
> file, which would make it easier to follow the aim to keep those files empty.
Disabling all warnings would be more work then listing all warnings that we
actually consider an error. My list of warnings to disable grew to 5 lines,
then I stopped.
Really, you both need to build it with Clang once to see what I'm talking
about. The number of warnings almost beats you to death.
--
Jonathan
Other related posts:
