On Thu, Apr 7, 2016 at 7:15 PM, Barry Smith <[email protected]> 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 <[email protected]> wrote: > > > > Satish Balay <[email protected]> writes: > > > >> On Thu, 7 Apr 2016, Jed Brown wrote: > >> > >>> Barry Smith <[email protected]> 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
