many duplicateAssignExpression false positives in einspline

[Daniel Marjamäki]

I can see in daca@home that there are many duplicateAssignExpression false positives in einspline.

Here is an example code. Cppcheck warns about the doffset and coffset that is marked with comments below:

void
recompute_UBspline_2d_s (UBspline_2d_s* spline, float *data)
{
  int Mx = spline->x_grid.num;
  int My = spline->y_grid.num;
  int Nx, Ny;

  if (spline->xBC.lCode == PERIODIC || spline->xBC.lCode == ANTIPERIODIC)     Nx = Mx+3;
  else                           Nx = Mx+2;
  if (spline->yBC.lCode == PERIODIC || spline->yBC.lCode == ANTIPERIODIC)     Ny = My+3;
  else                           Ny = My+2;

  // First, solve in the X-direction 
  for (int iy=0; iy<My; iy++) {
    intptr_t doffset = iy;
    intptr_t coffset = iy;
    find_coefs_1d_s (spline->x_grid, spline->xBC, data+doffset, My,
             spline->coefs+coffset, Ny);
  }

  // Now, solve in the Y-direction
  for (int ix=0; ix<Nx; ix++) {
    intptr_t doffset = ix*Ny; // <------ doffset
    intptr_t coffset = ix*Ny; // <------ coffset
    find_coefs_1d_s (spline->y_grid, spline->yBC, spline->coefs+doffset, 1, 
             spline->coefs+coffset, 1);
  }
}

In einspline there are many warnings for such code. Because there are so many cases with such pattern I do believe that is intentional and therefore these warnings are false positives.

So what should we do to fix the false positives. One possible heuristic to avoid false positives is to match the variable names. Since there is no "doffset" variable in rhs do not show the warning.

As far as I know this check is meant to detect such mistake:

    int x = something.x;
    int y = something.x;

Any other ideas?

[Paul Fultz]

Because there are so many cases with such pattern I do believe that is intentional and therefore these warnings are false positives.

Well there is a performance issue as it will calculate the value twice, instead of once. I think writing it like this:

    intptr_t doffset = ix*Ny;
    intptr_t coffset = doffset;

Makes it more intentional and clearer that coffset should reuse the value of doffset.

Since there is no "doffset" variable in rhs do not show the warning.

I dont think thats great. It would miss mistakes like this:

    int x = something.GetX();
    int y = something.GetY();

As far as I know this check is meant to detect such mistake:

It could also find mistakes like this:

int xalpha = x*a;
int yalpha = x*a;

[Daniel Marjamäki]

Yes there would be such false negatives. However off the top of my head I can't see any other heuristic that could avoid the false positive in this case. With an "AI" we could see there is a common pattern and avoid the warnings. The motivation for the warnings are that there could be a mistake and if there is similar code in >10 places then it's more likely by intention.

Your fix is reasonable but I assume the compiler could do that optimisation.

Since this is 1 project I think it's ok if the conclusion is that they must suppress. However if we could have seen some possible heuristic that would be better. :-(

[Daniel Marjamäki]

When I looked at the daca@home results for duplicateAssignExpressions yesterday I did not feel that the noise ratio was very good.