4

I am looking for the best way to avoid cppcoreguidelines-init-variables warnings with clang-tidy.

std::istringstream in(line);
float x, y, z, rotx, roty, rotz;
in >> x >> y >> z >> rotx >> roty >> rotz;

-> variable 'x' is not initialized, variable 'y' is not initialized, ...

double min_intensity, max_intensity;    
cv::Point point_min, point_max;
cv::minMaxLoc(input, &min_intensity, &max_intensity, &point_min, &point_max, elliptic_mask_around_center);

-> variable 'min_intensity' is not initialized, ...

I could assign a value for x, y, ... and min_intensity and max_intensity, but I do not see why I should do it.

What would be the best way to avoid this warning?

float x = 0.F; // initialize to a value, but 6 lines, and it makes no sense because x is never 0
float x = NAN; // works but 6 lines, and needs <cmath> inclusion
float x, y, z, rotx, roty, rotz; // NOLINT [not very practical, and means the warning is a false positive]

Thank you for your help.

PJ127
  • 986
  • 13
  • 23
  • 2
    when input from the stream fails then the variables will have an indeterminate value and using them later in your code will invoke undefined behavior. You better do initialize them – 463035818_is_not_an_ai Jul 26 '21 at 07:57
  • 3
    Actually checking the success of the IO operation rather than blindly assuming it worked and marching on as if everything is rosy wouldn't hurt either. – WhozCraig Jul 26 '21 at 08:05
  • @463035818_is_not_a_number A check to see if the read worked is essential here regardless because even if you initialize them with an arbitrary value, it's a bug waiting to happen if the read fails. – Galik Jul 26 '21 at 08:11
  • @463035818_is_not_a_number: That hasn’t been true since [at least C++11](https://en.cppreference.com/w/cpp/locale/num_get/get#Notes). – Davis Herring Jul 26 '21 at 08:22
  • @DavisHerring yes and no. If extraction of `x` fails then the stream is in an error state and subsequent extractions will not initialize the other variables – 463035818_is_not_an_ai Jul 26 '21 at 08:29

1 Answers1

2

In general you should initialize variables always because reading from an uninitialized variable is undefined behavior.

When extraction from the stream fails then since C++11 a 0 is assigned. However, this is only the case as long as the stream is not in a fail state. Hence, when reading x fails then x will have a value of 0 but the others will have an indetermindate value.

You should always check if extraction succeded:

if (in >> x >> y >> z >> rotx >> roty >> rotz) { 
     // use x,y,z,rotx,roty,rotz
}

And if you want to use part of the input even in case extraction fails at some point, you should read them seperately:

if (in >> x) {
     // use x
}
if (in >> y) {
     // use y
}
// ...

If you do that, the initialization is strictly speaking not necessary. However, reading from a stream is way more expensive than initializing a variable, so to be consistent (see eg here) I would suggest to initialize them anyhow. Don't worry about more lines of code, a common recommendation is to declare a single variable per line anyhow (see eg here):

std::istringstream in(line);
float x = 0.0;
float y = 0.0;
float rotx = 0.0;
float roty = 0.0;
float rotz = 0.0;
if (in >> x >> y >> z >> rotx >> roty >> rotz) {
     // extraction succeeded
} else {
     // extraction failed
}
463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185
  • 1
    Thank you very much for your answer. This might be heretical, but why do I care if this is undefined behavior, as long as the program does not crash? If extraction failed, I won't use x, y, z, so I don't mind what their value might be initialized with (0, NAN, -1, 325.824, ... - NAN would seem a more reasonable choice by the way). – PJ127 Jul 27 '21 at 08:15
  • 1
    @PJ127 because you don't know what your program does when it has undefined behavior unless you study the assembly. It does not necessarily crash, but it might do something completely unexpected without crashing. You have no guarantee that the program still does what it does when you compile it again with different compiler flags, with a different compiler, or the same compiler on the next day. The truth is that there are lots of programs out there that have undefined behavior and they do work, until they don't. Always initializing variables is an easy way to avoid nasty bugs, so why not do it? – 463035818_is_not_an_ai Jul 27 '21 at 08:21