[llvm] r202295 - Silencing an MSVC signed comparison warning.

[David Blaikie]

On Wed, Feb 26, 2014 at 1:12 PM, Aaron Ballman <aaron at aaronballman.com>wrote:

> On Wed, Feb 26, 2014 at 4:06 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> >
> > On Wed, Feb 26, 2014 at 1:01 PM, Aaron Ballman <aaron at aaronballman.com>
> > wrote:
> >>
> >> On Wed, Feb 26, 2014 at 3:55 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >> >
> >> >
> >> >
> >> > On Wed, Feb 26, 2014 at 12:22 PM, Aaron Ballman <
> aaron at aaronballman.com>
> >> > wrote:
> >> >>
> >> >> Author: aaronballman
> >> >> Date: Wed Feb 26 14:22:20 2014
> >> >> New Revision: 202295
> >> >>
> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=202295&view=rev
> >> >> Log:
> >> >> Silencing an MSVC signed comparison warning.
> >> >
> >> >
> >> > Do we have a bug tracking Clang's implementation of this? (or turning
> it
> >> > on
> >> > for the LLVM build if it's already implemented)
> >>
> >> We have -Wsign-compare which I would imagine does the trick, but I
> >> believe we would not want it to fire on this code.
> >>
> >> > (either that, or can we turn off the MSVC warning if it's not
> considered
> >> > high value?)
> >>
> >> I think it's usually high value, except in this case where MSVC cannot
> >> figure out that 32/64 as a literal is "unsigned enough." That's why I
> >> would claim we wouldn't want Clang to fire a warning here anyway -- we
> >> can prove the comparison is fine.
> >
> >
> > So - both then: suppress the MSVC warning and enable the Clang one, if
> it's
> > not already enabled?
>
> I don't think we should suppress the MSVC warning,


This is mostly for your (& other MSVC) developers benefit so your builds
don't get broken by non-MSVC users... so I don't have any real basis on
which to object for myself. If that's the sort of thing you're willing to
accept...

But generally I think the project is of the opinion that "if the warning is
good enough to use, it's good enough to implement" so we should either
implement it or disable it.


> but I do think we
> should enable the Clang one. This comes up infrequently enough in MSVC
> that I'm not worried about occasionally making the literal type more
> explicit.
>
> I'm not really set up to run a clang bootstrap, so I don't feel
> comfortable turning the warning on for Clang (and LLVM). Would you
> mind giving it a whirl to see how chatty it is?


I'm going to assume far too chatty - but I'll try to remember to give it a
go at some point.

- David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140226/8783dd93/attachment.html>