On 01/12/2014 05:24 PM, Jonathan Schleifer wrote:
Am 12.01.2014 um 14:34 schrieb Ingo Weinhold <ingo_weinhold@xxxxxx>:I'd like such a fine grained approach for clang as well. What I have in mind is adding a third, optional, parameter to EnableWerror, which can be a build feature specification, so that one can write, e.g. EnableWerror src add-ons accelerants 3dfx : : !clang ; to enable -Werror, except for clang. Obviously "clang" and "gcc" would need to be declared as build features for that to work. I haven't checked whether at this point the build feature code has already been initialized, but it should definitely possible to set things up this way.That's not the right approach.
Is that so?
Just enable -Werror for those type of warnings which already are fixed.
If you want to enable errors per warning type and component, then I think this is way too fine grained and would make the EnableWerror section in in ArchitectureRules quite messy. If you want to enable errors for a warning type for the whole code base, then I think this not fine grained enough. The code base is big enough that there may be hundreds of occurrences and it would be impractical or at least very much not fun for anyone to fix them this way (e.g. ask Alex about the printf() format warnings for 64 bit).
I think the current approach to enable errors per component is very reasonable. It also fits with the that developers usually work on a specific component at a time.
Clang and GCC use the same names for error types, so that works. There is nothing wrong with displaying warnings. They should always all be displayed. But only certain types should cause an error.
Well, I disagree. Either a warning type is useful, in which case the code should be fixed. Or the warning isn't useful and it should be disabled. This can be (and is already) done on a per-component basis (in the respective Jamfiles) or globally. E.g. if we didn't disable the multi-char warning we'd see thousands of those warnings for a regular build. I see no point in cluttering the build output with useless warnings.
As mentioned above, this doesn't work generally due to CCFLAGS not being architecture specific. ATM we don't have any configuration variable that could be used for this purpose. We'd either have to add one (or consequently a whole set for C, C++, and linker flags) -- though for performance reasons I'm a bit reluctant to do that lightly -- or choose a more explicit approach, i.e. define an architecture specific variable (e.g. HAIKU_GLIBC_CCFLAGS_<arch>) with the flags in question and add them in each concerned Jamfile (CCFLAGS += HAIKU_GLIBC_CCFLAGS_$(TARGET_PLATFORM_ARCH)). This seems less elegant, but I prefer it since at least it is more obvious where those weird flags come from.Hm, that isn't really better than the old patch before which added it to each file. I wanted to get rid of the need to add it to every single Jamfile. Code duplication always seems awfully wrong to me. I think we really need to have C, C++ and linker flags for each target platform arch. I'm sure this won't be the last time something like that is needed.
There are per architecture variables for various flags (cf. the ArchitectureSetup rule), but they are not configurable per subdirectory. That could be changed, but it isn't entirely for free. And I don't think this possible use case rectifies it.
A possible alternative to setting the flags in each of glibc's Jamfiles would be to adjust the global TARGET_CCFLAGS_<arch> variables in the root Jamfile for glibc and reset them after the last SubInclude. If it turns out that something like this is needed elsewhere as well, we could add rules to make it more convenient.
CU, Ingo
