How sharp is your attention as a C++ programmer?

Sometimes, we share code snippets with tricly errors found by PVS-Studio in some open project in our internal chat. Like, who can quickly spot what's wrong with the code?

Yesterday, a colleague dropped this code fragment from the SereneDB project:

template <typename T>
struct NumericParameter : public Parameter {
  using ValueType = T;
  ....
  std::string name() const override {
    if constexpr (std::is_same_v<ValueType, int16_t>) {
      return "int16";
    } else if constexpr (std::is_same_v<ValueType, uint16_t>) {
      return "uint16";
    } else if constexpr (std::is_same_v<ValueType, int32_t>) {
      return "int32";
    } else if constexpr (std::is_same_v<ValueType, uint32_t>) {
      return "uint32";
    } else if constexpr (std::is_same_v<ValueType, int64_t>) {
      return "int64";
    } else if constexpr (std::is_same_v<ValueType, uint64_t>) {
      return "uint64";
    } else if constexpr (std::is_same_v<ValueType, size_t>) {
      return "size";
    } else if constexpr (std::is_same_v<ValueType, double>) {
      return "double";
    } else {
      static_assert("unsupported ValueType");
    }
  }
  ....
};

To be honest, I read this passage twice and still couldn't find the bug. And I gave up. So yeah, it's worthy posting it here; you can test your attentiveness too :)

Answer:

PVS-Studio analyzer issues the warning: V591 Non-void function should return a value. parameters.h 222

Yeah, the warning seems odd and looks like a false positive because it can't be that the function just stopped working without returning a value via the return operator. If the else branch is taken, there's static_assert, and the code simply shouldn't compile. In all other cases, we see return ‘something’;.

But there's a catch!

Take another look at this line:

static_assert(‘unsupported ValueType’);

The problem is static_assert is used incorrectly: bool-constexpr is missing. Actually, the string literal is implicitly converted to true, so static_assert will never interrupt compilation. As a result, the else branch of the function returns nothing. That means the function ends up with undefined behavior for all NumericParameter specializations, except the ones handled earlier in the if constexpr chain.

Look at the correct option:

static_assert(false, ‘unsupported ValueType’);