On Thu, Apr 7, 2016 at 7:15 PM, Barry Smith <bsmith@mcs.anl.gov> wrote:

   This works.

 unsigned int joe = index%PETSC_BITS_PER_BYTE;
    if (joe > 7) joe = 7;
  return (BT_idx        = index/PETSC_BITS_PER_BYTE,
          BT_c          = array[BT_idx],
          BT_mask       = (char)(1 << joe),

I think it is because clang is not smart enough to know that (index%PETSC_BITS_PER_BYTE) is always between 0 and 7.

   Hence it thinks that you might be doing something like    1 << 9  which is as I understand it undefined http://stackoverflow.com/questions/21909412/why-would-the-outcome-of-this-shift-left-operation-be-deemed-undefined

  So do we pander to the clang analyzer or just leave the warning? I vote to leave the warning and not change the code.

I believe Bill also encouraged us toward clean code rather than 0 warnings. I like this as well.

   Matt
 
  Barry



> On Apr 7, 2016, at 5:56 PM, Jed Brown <jed@jedbrown.org> wrote:
>
> Satish Balay <balay@mcs.anl.gov> writes:
>
>> On Thu, 7 Apr 2016, Jed Brown wrote:
>>
>>> Barry Smith <bsmith@mcs.anl.gov> writes:
>>>>   Should we caste to an unsigned PetscInt first then?
>>>
>>> It should be unsigned, yes.  Does that fix the warning?
>>
>> Nope..
>>
>> Commenting out the following line - or changing the argument thus is
>> making a difference.
>>
>> diff --git a/src/mat/impls/baij/seq/baijfact.c b/src/mat/impls/baij/seq/baijfact.c
>> index fea37cb..e2c210c 100644
>> --- a/src/mat/impls/baij/seq/baijfact.c
>> +++ b/src/mat/impls/baij/seq/baijfact.c
>> @@ -1081,7 +1081,7 @@ PetscErrorCode MatICCFactorSymbolic_SeqBAIJ(Mat fact,Mat A,IS perm,const MatFact
>>         ncols_upper++;
>>       }
>>     }
>> -    ierr = PetscIncompleteLLAdd(ncols_upper,cols,levels,cols_lvl,am,nlnk,lnk,lnk_lvl,lnkbt);CHKERRQ(ierr);
>> +    ierr = PetscIncompleteLLAdd(ncols,cols,levels,cols_lvl,am,nlnk,lnk,lnk_lvl,lnkbt);CHKERRQ(ierr);
>
> That looks like it changes the semantics.
>
> In any case, that file wasn't even mentioned in the message that Barry
> shared.  If it is indeed the same issue, then it would appear that the
> static analyzer has determined that it is possible for index to be
> negative.




--
What most experimenters take for granted before they begin their experiments is infinitely more interesting than any results to which their experiments lead.
-- Norbert Wiener