[lld] r254105 - Reapply r254098.

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 30 20:59:36 PST 2015


On Mon, Nov 30, 2015 at 6:23 PM, Richard Trieu <rtrieu at google.com> wrote:

> On Thu, Nov 26, 2015 at 11:20 AM, David Blaikie <dblaikie at gmail.com>
> wrote:
>
>>
>>
>> On Thu, Nov 26, 2015 at 11:14 AM, George Rimar <grimar at accesssoftek.com>
>> wrote:
>>
>>> >If it helps, I'll make it a post-commit review request: please change
>>> this to auto because of the subtleties here.
>>> >
>>> >Was this caught by a buildbot with Clang's warning, or due to a test
>>> failing on a machine where size_t != unsigned?
>>> >
>>> >- Dave
>>>
>>>
>>> If failed due to build bot`s error where size_t != unsigned I guess. My
>>> msvs/32x config compiled fine that place.
>>>
>>
>> OK - I think we have a warning for this too, but maybe it hasn't been
>> rolled out. Not sure. (cc'd Richard Trieu who implemented/worked on the
>> warning, to see what the status is)
>>
>
> Which warning are you talking about?  The warning assign to different
> integer types or one about range loops not having the correct type?
>
> -Wshorten-64-to-32 will sometimes catch assignments between size_t and
> unsigned, but this is a noisy warning and it may not detect it because it
> is inside templates.
>
> -Wrange-loop-analysis performs checks on the type, but it deals with
> constness and references, so it wouldn't catch this either.
>

This ^ is the warning I was referring to. It wasn't strictly applicable
here because the range-for's variable was non-const ref, not const ref. So
in this case, if the types mismatched (as they did on some build
configurations, where size_t wasn't really unsigned int), it was a hard
error anyway, but had const crept in there at all this might've been a
silent change in behavior if it weren't for range-loop-analysis.

Anyway, my question is: is -Wrange-loop-analysis committed to clang, and is
it enabled for LLVM builds (but I can test this myself, if that suits, of
course)

- Dave


>
>> But yeah, especially if this actually introduced a bug (not just a
>> compiler warning), I'd really prefer auto here. These sort of mistakes are
>> pretty easy.
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151130/01b51fcc/attachment.html>


More information about the llvm-commits mailing list