llvm dev - Mar 2016 - Suspicious code in WholeProgramDevirt.cpp?

In DevirtModule::tryUniqueRetValOpt, there's a lambda that starts like this:

// IsOne controls whether we look for a 0 or a 1.
  auto tryUniqueRetValOptFor = [&](bool IsOne) {
    const BitSetInfo *UniqueBitSet = 0;
    for (const VirtualCallTarget &Target : TargetsForSlot) {
      if (Target.RetVal == IsOne ? 1 : 0) {
        if (UniqueBitSet)
          return false;
        UniqueBitSet = Target.BS;
      }
    }

I'm working on a patch to turn up the warning levels that LLVM compiles at,
and I got a new warning:

C4805 '==': unsafe mix of type 'const uint64_t' and type
'bool' in operation


...pointing at this line:

if (Target.RetVal == IsOne ? 1 : 0) {


It looks to me like that, instead of comparing Target.RetVal *to* 1 or 0,
it's comparing Target.RetVal *to* IsOne, which I doubt is the intended
outcome!

What's up with that? Is that a bug?

Sincerely,
Alexander Riccio
--
"Change the world or go home."
about.me/ariccio

<http://about.me/ariccio>
If left to my own devices, I will build more.
⁂
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160307/8d1d73ce/attachment.html>

Yes, this should have read:

if (Target.RetVal == (IsOne ? 1 : 0)) {

As it turns out, this evaluates to the same thing.

I've committed a fix in r262907.

Peter


On Mon, Mar 7, 2016 at 7:18 PM, <Alexander G. Riccio> via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> In DevirtModule::tryUniqueRetValOpt, there's a lambda that starts like
> this:
>
> // IsOne controls whether we look for a 0 or a 1.
>   auto tryUniqueRetValOptFor = [&](bool IsOne) {
>     const BitSetInfo *UniqueBitSet = 0;
>     for (const VirtualCallTarget &Target : TargetsForSlot) {
>       if (Target.RetVal == IsOne ? 1 : 0) {
>         if (UniqueBitSet)
>           return false;
>         UniqueBitSet = Target.BS;
>       }
>     }
>
> I'm working on a patch to turn up the warning levels that LLVM compiles
> at, and I got a new warning:
>
> C4805 '==': unsafe mix of type 'const uint64_t' and type
'bool' in
>> operation
>
>
> ...pointing at this line:
>
> if (Target.RetVal == IsOne ? 1 : 0) {
>
>
> It looks to me like that, instead of comparing Target.RetVal *to* 1 or 0,
> it's comparing Target.RetVal *to* IsOne, which I doubt is the intended
> outcome!
>
> What's up with that? Is that a bug?
>
> Sincerely,
> Alexander Riccio
> --
> "Change the world or go home."
> about.me/ariccio
>
> <http://about.me/ariccio>
> If left to my own devices, I will build more.
> ⁂
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>

-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160307/6a7e15ef/attachment.html>