13

Coverity has complained that various function calls in our codebase are not checking the return value.

Unchecked return value (CHECKED_RETURN)3. check_return: Calling Append without checking return value (as is done elsewhere 73 out of 78 times).

In the past, we would have simply resolved this issue (after double-checking that the return value really was not important) by casting the return to void (as discussed here):

(void)Foo.Append(bar);

However, we are moving towards enabling all warnings, and treating warnings as errors, so I'm a little concerned that the above code will generate an old-style-cast diagnostic. If that's the case, I will need to modify our code to the considerably uglier format:

static_cast<void>( Foo.Append(bar) );

However, both gcc and clang seem to be able to compile this code (the first form) without complaining. So I suppose the final form of my question is this: Is casting a function return to void considered an exception to the rule as far as C-style casts are concerned? Or do I need to double check my code and see if the lines in question aren't actually being included in those builds?

NathanOliver
  • 171,901
  • 28
  • 288
  • 402
Tim Randall
  • 4,040
  • 1
  • 17
  • 39
  • 1
    The MSVC compiler emits an analyzer warning for the `(void)` cast because it is a "C style cast". I don't get that warning when using `static_cast`. – Werner Henze Nov 26 '18 at 16:38
  • 2
    Tangential: You could `#define IGNORE_RET(func) static_cast(func)` and then have `IGNORE_RET(Foo.Append(bar));` to self document that you are intentionally ignoring the return. – NathanOliver Nov 26 '18 at 16:39
  • @NathanOliver I was considering something like that. We do have a macro called UNUSED but its primary use is unused function parameters, and I was concerned that if I used that, some future "optimization" would improve it so it didn't evaluate its parameter... so I shied away from it. A more clearly named macro, as you suggest, might be the best solution – Tim Randall Nov 26 '18 at 16:42
  • @WernerHenze Thank you for the information. Which version of MSVC are you using? I didn't see this warning – Tim Randall Nov 26 '18 at 16:44
  • Does anyone else have issues with the fact that you're ignoring a return value specifically marked to not be ignored? Either the marking is erronous (get rid of it then!) or you really should not ignore it. – rubenvb Nov 26 '18 at 16:59
  • @rubenvb usually when we append an item to a vector, we check the return value to see if we ran out of memory. Sometimes, though, we resize the vector to pre-allocate enough items to hold everything we're going to add, then iterate through without checking for failure in every individual case. – Tim Randall Nov 26 '18 at 17:04
  • 1
    @TimRandall I just tested again to confirm and saw the warning using VS15.9.2. As this is a code analysis warning it is not sufficient to just compile the code, you also have to enable code analysis and set the rule set to "Microsoft All Rules". – Werner Henze Nov 26 '18 at 18:51

1 Answers1

10

It's fine.

(void) f(x);

is always equivalent to a static_cast as per [expr.static.cast]/6:

Any expression can be explicitly converted to type cv void, in which case it becomes a discarded-value expression.

Converting the result of a function to void is the way to make an expression a discard-value-expression. Now, the C++ way should be static_cast<void>(...)but (void) ... is an idiom (and is shorter).

Since the latter is well-defined and really common in codebases, gcc1 and clang2 made it not trigger Wold-style-cast.

It's well-defined, recognized by major compilers. It's fine.


1) g++ documentation --- 3.5 Options Controlling C++ Dialect

-Wold-style-cast (C++ and Objective-C++ only)
Warn if an old-style (C-style) cast to a non-void type is used within a C++ program. The new-style casts (dynamic_cast, static_cast, reinterpret_cast, and const_cast) are less vulnerable to unintended effects and much easier to search for.

2) not documented

Tim Randall
  • 4,040
  • 1
  • 17
  • 39
YSC
  • 38,212
  • 9
  • 96
  • 149