[lld] r254105 - Reapply r254098.

[Richard Trieu via llvm-commits]

On Mon, Nov 30, 2015 at 8:59 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> 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
>
>
It has been committed to Clang.  It is not turned on yet, but I did a
partial clean up during the testing of the warning so it shouldn't be too
hard to turn on.


>
>>> 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/20151201/892f382d/attachment.html>