I have to admit I appreciate the effort the Visual Studio static code analyzer does. It has found some issues in my C/C++ code that could, under certain circumstances, lead to some unusual bugs.
However, there are some other places where it finds issues that I have already prepared for.
Take for instance something like this: I have a C array, with a static size somewhere:
static const int DataSize = 100;
int Data[DataSize];
...and then I have a method or function that accesses this data:
void DoSomething(size_t index)
{
if (index >= DataSize)
Panic("Out of bounds");
Data[index] = DoSomethingElse();
}
This Panic() function is meant to notify me when the precondition is not met, and points to an error that can't be recovered from. In a debug build, it breaks into the debugger so I can analyze what happened, while in a release build, it will intentionally crash the program, leaving some useful debug information.
However, if I compile this, Visual Studio 2019 gives me the following warning:
Warning C6386: Buffer overrun while writing to 'Data': the writable size is '400' bytes, but 'index' bytes might be written.
The warning is indeed valid, if I were not performing the precondition check just before.
One way to suppress this warning is by changing the code to something like:
void DoSomething(size_t index)
{
if (index >= DataSize)
{
Panic("Out of bounds");
return; // This is never reached...
}
Data[index] = DoSomethingElse();
}
But, not only this is unnecessarily verbose, it also may confuse the static code analyzer even more, because it can make it think that returning is a valid code path, further confusing it.
Another example, this time unrelated to arrays, and applicable to C and C++. Consider a function or method like this:
int DoSomeOperation(int operationType, int value)
{
int foo;
switch (operationType)
{
case 0:
foo = 10;
break;
case 1:
foo = value + 1;
break;
case 2:
foo = value - 1;
break;
// ... many more cases
default:
Panic("Invalid operationType");
// We never reach this line...
}
DoSomeOtherOperation(foo);
}
This raises the following warning in the DoSomeOtherOperation() line:
Warning C6001: Using uninitialized memory 'foo'.
Once again, I can either explicitly initialize foo, initialize it to something in the default case, or return from the function after Panic(), but technically, neither is necessary, since for all semantically valid values of operationType, foo is indeed initialized.
In fact, I might prefer to leave foo uninitialized at the beginning, so the static analyzer -does- catch a case where I did indeed missed the initialization.
Now, is there a way to annotate the Panic function, so the static code analyzer knows that this function will validate the precondition, or alternatively, that it terminates the program in all cases, so it doesn't have to consider the case where the function returns and the program continues?