git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/2] fix clang -Wtautological-compare with unsigned enum

From
John Keeping <john@keeping.me.uk>
Date
Jan 17, 2013, 17:02 UTC
Message-ID
<20130117170209.GF4574@serenity.lan>
In-Reply-To
<CA+55aFxYSX2iYPSafKdCDSfWSMfQxP3R3Hqh8GuiiR6EbWfk3w@mail.gmail.com>
On Thu, Jan 17, 2013 at 08:44:20AM -0800, Linus Torvalds wrote:
Show 28 quoted lines
> On Thu, Jan 17, 2013 at 3:00 AM, John Keeping <john@keeping.me.uk> wrote:
>>
>> There's also a warning that triggers with clang 3.2 but not clang trunk, which
>> I think is a legitimate warning - perhaps someone who understands integer type
>> promotion better than me can explain why the code is OK (patch->score is
>> declared as 'int'):
>>
>> builtin/apply.c:1044:47: warning: comparison of constant 18446744073709551615
>>     with expression of type 'int' is always false
>>     [-Wtautological-constant-out-of-range-compare]
>>         if ((patch->score = strtoul(line, NULL, 10)) == ULONG_MAX)
>>             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~
> 
> The warning seems to be very very wrong, and implies that clang has
> some nasty bug in it.
> 
> Since patch->score is 'int', and UNLONG_MAX is 'unsigned long', the
> conversion rules for the comparison is that the int result from the
> assignment is cast to unsigned long. And if you cast (int)-1 to
> unsigned long, you *do* get ULONG_MAX. That's true regardless of
> whether "long" has the same number of bits as "int" or is bigger. The
> implicit cast will be done as a sign-extension (unsigned long is not
> signed, but the source type of 'int' *is* signed, and that is what
> determines the sign extension on casting).
> 
> So the "is always false" is pure and utter crap. clang is wrong, and
> it is wrong in a way that implies that it actually generates incorrect
> code. It may well be worth making a clang bug report about this.

The warning doesn't occur with a build from their trunk so it looks like it's already fixed - it just won't make into into a release for about 5 months going by their timeline.

Thanks for the clear explanation.
Previous: Antoine PelisseNext: Phil Hord
Message 25 of 35 in “fix some clang warnings”
  1. fix some clang warningsMax Horn, Jan 16, 2013
  2. Jeff KingJan 16, 2013
  3. Junio C HamanoJan 16, 2013
  4. Antoine PelisseJan 16, 2013
  5. John KeepingJan 16, 2013
  6. Max HornJan 16, 2013
  7. Jeff KingJan 16, 2013
  8. Jeff KingJan 16, 2013
  9. Jeff KingJan 16, 2013
  10. John KeepingJan 16, 2013
  11. Jeff KingJan 16, 2013
  12. Antoine PelisseJan 16, 2013
  13. John KeepingJan 16, 2013
  14. Jeff KingJan 16, 2013
  15. John KeepingJan 16, 2013
  16. John KeepingJan 17, 2013
  17. 1/2 fix clang -Wconstant-conversion with bit fieldsAntoine Pelisse, Jan 16, 2013
  18. 2/2 fix clang -Wtautological-compare with unsigned enumAntoine Pelisse, Jan 16, 2013
  19. Antoine PelisseJan 16, 2013
  20. Antoine PelisseJan 17, 2013
  21. John KeepingJan 17, 2013
  22. combine-diff: suppress a clang warningJohn Keeping, Jan 17, 2013
  23. Linus TorvaldsJan 17, 2013
  24. Antoine PelisseJan 17, 2013
  25. John KeepingJan 17, 2013
  26. Phil HordJan 18, 2013
  27. Linus TorvaldsJan 18, 2013
  28. John KeepingJan 16, 2013
  29. Antoine PelisseJan 16, 2013
  30. Antoine PelisseJan 16, 2013
  31. Junio C HamanoJan 16, 2013
  32. Junio C HamanoJan 16, 2013
  33. Tomas CarneckyJan 16, 2013
  34. Matthieu MoyJan 16, 2013
  35. Miles BaderFeb 1, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.