<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Apr 7, 2016 at 7:15 PM, Barry Smith <span dir="ltr"><<a href="mailto:bsmith@mcs.anl.gov" target="_blank">bsmith@mcs.anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
   This works.<br>
<br>
 unsigned int joe = index%PETSC_BITS_PER_BYTE;<br>
    if (joe > 7) joe = 7;<br>
  return (BT_idx        = index/PETSC_BITS_PER_BYTE,<br>
          BT_c          = array[BT_idx],<br>
          BT_mask       = (char)(1 << joe),<br>
<br>
I think it is because clang is not smart enough to know that (index%PETSC_BITS_PER_BYTE) is always between 0 and 7.<br>
<br>
   Hence it thinks that you might be doing something like    1 << 9  which is as I understand it undefined <a href="http://stackoverflow.com/questions/21909412/why-would-the-outcome-of-this-shift-left-operation-be-deemed-undefined" rel="noreferrer" target="_blank">http://stackoverflow.com/questions/21909412/why-would-the-outcome-of-this-shift-left-operation-be-deemed-undefined</a><br>
<br>
  So do we pander to the clang analyzer or just leave the warning? I vote to leave the warning and not change the code.<br></blockquote><div><br></div><div>I believe Bill also encouraged us toward clean code rather than 0 warnings. I like this as well.</div><div><br></div><div>   Matt</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  Barry<br>
<br>
<br>
<br>
> On Apr 7, 2016, at 5:56 PM, Jed Brown <<a href="mailto:jed@jedbrown.org">jed@jedbrown.org</a>> wrote:<br>
><br>
> Satish Balay <<a href="mailto:balay@mcs.anl.gov">balay@mcs.anl.gov</a>> writes:<br>
><br>
>> On Thu, 7 Apr 2016, Jed Brown wrote:<br>
>><br>
>>> Barry Smith <<a href="mailto:bsmith@mcs.anl.gov">bsmith@mcs.anl.gov</a>> writes:<br>
>>>>   Should we caste to an unsigned PetscInt first then?<br>
>>><br>
>>> It should be unsigned, yes.  Does that fix the warning?<br>
>><br>
>> Nope..<br>
>><br>
>> Commenting out the following line - or changing the argument thus is<br>
>> making a difference.<br>
>><br>
>> diff --git a/src/mat/impls/baij/seq/baijfact.c b/src/mat/impls/baij/seq/baijfact.c<br>
>> index fea37cb..e2c210c 100644<br>
>> --- a/src/mat/impls/baij/seq/baijfact.c<br>
>> +++ b/src/mat/impls/baij/seq/baijfact.c<br>
>> @@ -1081,7 +1081,7 @@ PetscErrorCode MatICCFactorSymbolic_SeqBAIJ(Mat fact,Mat A,IS perm,const MatFact<br>
>>         ncols_upper++;<br>
>>       }<br>
>>     }<br>
>> -    ierr = PetscIncompleteLLAdd(ncols_upper,cols,levels,cols_lvl,am,nlnk,lnk,lnk_lvl,lnkbt);CHKERRQ(ierr);<br>
>> +    ierr = PetscIncompleteLLAdd(ncols,cols,levels,cols_lvl,am,nlnk,lnk,lnk_lvl,lnkbt);CHKERRQ(ierr);<br>
><br>
> That looks like it changes the semantics.<br>
><br>
> In any case, that file wasn't even mentioned in the message that Barry<br>
> shared.  If it is indeed the same issue, then it would appear that the<br>
> static analyzer has determined that it is possible for index to be<br>
> negative.<br>
<br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature">What most experimenters take for granted before they begin their experiments is infinitely more interesting than any results to which their experiments lead.<br>-- Norbert Wiener</div>
</div></div>