Re: [PATCH] unpack-objects: fix compilation warning/error due to missing braces

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 12, 2022 at 11:16:10AM +0200, Ævar Arnfjörð Bjarmason wrote:

> >> I'm in favor of this. It would, of course, require extra
> >> special-casing for Apple's clang for which the version number bears no
> >> resemblance to reality since Apple invents their own version numbers.
> 
> FWIW I was imagining just providing that -Wno-* on clang versions <= 11,
> not special-casing Apple's in particular.

It's not about special-casing Apple in particular. It's that our
detect-compiler script does not understand which version of clang is
used for Apple's compiler. Their version numbers are totally mismatched.

So you either have to say "turn off this warning for clang totally", or
do the wrong thing when Apple's compiler is in use.

> I have a local patches that carry forward the idea I had in that thread,
> i.e. to drop all this version detection insanity and just compile a C
> program to detect the compiler.

Hmm. I prefer your earlier suggestion to use "$(CC) -E". This tool has
to run on every invocation of "make", so the lighter-weight it is, the
better. You say later...

> The compilation is then triggered by the include in config.mak.dev,
> which has a corresponding rule that creates the C program, then the
> generated *.mak, so once we do it once we're only ever including an
> already generated text file.

but that implies we don't have a dependency on the compiler itself.
You'd want to at least depend on the name of the compiler. But that
would also miss if running "clang" changes which version of clang you're
running. I know upgrading your compiler is rare-ish, but this is exactly
the kind of thing I expect to bite at the most annoying time (when
you're switching around versions to try to figure out how they behave).

> 	#ifdef __clang__
> 	#if __clang_major__ >= 7
> 		fn(util, "NEEDS_std-eq-gnu99", "1");
> 	#endif

This is still gross version detection, but I don't think we can avoid
it. However...

> 	#if __has_warning("-Wextra")
> 		fn(util, "HAS_Wextra", "1");
> 	#endif

...this is much nicer. It could still be implemented purely via "-E", as
far as I can see, like:

  #if __has_warning("-Wextra")
  HAS_Wextra = 1
  #endif

But then we end up having to do version comparisons for gcc anyway:

> 	#if __GNUC__ >= 6
> 		fn(util, "NEEDS_std-eq-gnu99", "1");
> 		fn(util, "HAS_Wextra", "1");

so it feels like we're back where we started. You've just encoded the
version checks in a different spot.

I dunno. I don't find this significantly less gross than the status quo.
I don't mind getting the version via "-E" rather than "-v", but whether
the policy logic is in cpp, or in shell, or in the Makefile, it still
needs to exist. Putting it in cpp allows using has_warning(), but since
that isn't available everywhere, I'm not sure it buys us much.

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux