What does “warning: not all control paths return a value” mean? (C++)

Question

The exact warning I get is

warning C4715: "spv::Builder::makeFpConstant": not all control paths return a value

and spv::Builder::makeFpConstant is

Id Builder::makeFpConstant(Id type, double d, bool specConstant)
{
        assert(isFloatType(type));

        switch (getScalarTypeWidth(type)) {
        case 16:
                return makeFloat16Constant(d, specConstant);
        case 32:
                return makeFloatConstant(d, specConstant);
        case 64:
                return makeDoubleConstant(d, specConstant);
        }

        assert(false);
}

Can anyone help me?

This means that your function expects a return, but under certain circumstances your program may take route that end up in no return — Killzone Kid, Mar 18 '18 at 14:30
An assert is a "this should not happen". It is a kind of low effort contract, which is checked at runtime, and (typically) compiled out of the production code. You still need to handle that "never happen" control path to return or throw. Even if the only possible values that getScalarTypeWidth can return are 16, 32, and 64 -- the compiler doesn't know that. — Eljay, Mar 18 '18 at 15:03
@KillzoneKid: Why are you providing technical explanations in the comments section? Use comments to request clarification and critique the question, please. The answer section is where technical responses go. — Lightness Races in Orbit, Mar 18 '18 at 15:23
@LightnessRacesinOrbit unfortunately answer section is not mobile friendly — Killzone Kid, Mar 18 '18 at 15:28
@KillzoneKid: Then wait until you are at a normal computer before posting your answer please. Comments cannot be peer reviewed properly and that is _absolutely crucial_ on this site. That technical responses can be peer reviewed is literally this website's entire purpose for being. Thanks — Lightness Races in Orbit, Mar 18 '18 at 15:33
@KillzoneKid I use the mobile app and post answers all the time. It is certainly "answer-friendly" just fine. What it is not is "code-friendly", though. But the answer to this question does not require writing much code. — Remy Lebeau, Mar 18 '18 at 19:10
@RemyLebeau True, code is nightmare, but short answers are usually subpar. I'm ok with waiting to get to PC though. — Killzone Kid, Mar 18 '18 at 22:26

Lightness Races in Orbit · Answer 1 · 2018-03-18T15:16:41.273

An assert does not count as a piece of control flow logic. It's just a "documenting" contract check. In release builds it doesn't even happen! It's a debug tool, only.

So, you're left with a function that omits a return statement if the scalar type width is not 16, 32 or 64. This means your program has undefined behaviour. The warning is telling you that you need to return something in all cases, even if you think the other cases won't happen at runtime.

In this case I'd probably throw an exception if you get past the switch without returning — this may then be handled like any other exceptional case. An alternative if you don't want exceptions is to call std::terminate yourself (which is what the assertion will ultimately do in a debug build).

However, terminating is a bit of a nuclear option and you don't really want your program terminating in production; you want it kicking out a proper diagnostic message via your existing exception handling channels, so that when your customer reports a bug, they can say "apparently makeFpConstant got a value it didn't expect" and you know what to do. And if you don't want to leak function names to customers then you could at least pick some "secret" failure code that only your team/business knows (and document it internally!).

Terminating in a debug build is often fine, though, so leave that assert in too! Or just rely on the exception that you now have, which will result in a termination anyway if you don't catch it.

Here's how I'd probably write the function:

Id Builder::makeFpConstant(
   const Id type,
   const double d,
   const bool specConstant
)
{
   assert(isFloatType(type));

   const auto width = getScalarTypeWidth(type);
   switch (width) {
      case 16: return makeFloat16Constant(d, specConstant);
      case 32: return makeFloatConstant(d, specConstant);
      case 64: return makeDoubleConstant(d, specConstant);
   }

   // Shouldn't get here!
   throw std::logic_error(
      "Unexpected scalar type width "
      + std::to_string(width)
      + " in Builder::makeFpConstant"
   );
}

In a tight loop in performance-critical code, the call to `std::terminate` may be superior. — Lightness Races in Orbit, Mar 18 '18 at 15:25

score 1 · Answer 2 · answered Mar 18 '18 at 14:31

1

Well, the warning message says exactly what's wrong.

If NDEBUG is defined (i.e. asserts are disabled) and neither of the cases is taken, then the control reaches the end of function before any of the return statements (which results in undefined behaviour).

answered Mar 18 '18 at 14:31

HolyBlackCat

78,603
9
131
207

3

A solution is to insert a call to terminate. – Yakk - Adam Nevraumont Mar 18 '18 at 14:36
But his code will produce warning if `-Wall` is enabled as well. – Mar 18 '18 at 15:17
@GRC It probably will. What are you trying to say? – HolyBlackCat Mar 18 '18 at 15:21
oh actually what I am trying to say if `NDEBUG` is not defined it will produce warning. My comment was more like clarification than comment on your post. – Mar 18 '18 at 15:53

What does “warning: not all control paths return a value” mean? (C++)

2 Answers2

Linked

Related