3

I'm experiencing an issue where a static member function uses an UNUSED macro to silence compiler warnings. When the macro is in effect, it causes GCC and Clang to reject the function as a constexpr. Here's the test case:

$ cat test.cxx
#include <iostream>
#include <stdint.h>

#define UNUSED(x) ((void)x)

template <unsigned int N>
class Foo
{
public:
    enum {MIN_N=N}; enum {MAX_N=N}; enum {DEF_N=N};
    constexpr static size_t GetValidN(size_t n)
    {
        UNUSED(n); return DEF_N;
    }
};

class Bar : public Foo<16>
{
public:
    Bar(size_t n) : m_n(GetValidN(n)) {}
    size_t m_n;
};

int main(int argc, char* argv[])
{
    Bar b(10);
    return 0;
}

Here's the GCC error message:

$ g++ -std=c++11 test.cxx -o test.exe
test.cxx: In instantiation of ‘static constexpr size_t Foo<N>::GetValidN(size_t) [with unsigned int N = 16u; size_t = long unsigned int]’:
test.cxx:22:25:   required from here
test.cxx:16:5: error: body of constexpr function ‘static constexpr size_t Foo<N>::GetValidN(size_t) [with unsigned int N = 16u; size_t = long unsigned int]’ not a return-statement
     }
     ^

If I remove the use of UNUSED, then the source file compiles as expected:

constexpr static size_t GetValidN(size_t n)
{
    return DEF_N;
}

As far as I know, the #define UNUSED(x) ((void)x) is the only portable way to suppress unused variable warnings. I dread removing UNUSED because the macro suppresses thousands of warnings in a non-trivial C++ project with lots of interfaces. I'm not even sure I can remove UNUSED because of governance issues related to auditing and C&A.

How can I make the UNUSED macro work and play well with constexpr?


Clang produces a more helpful error message:

$ clang++ -std=c++11 test.cxx -o test.exe
test.cxx:15:2: warning: use of this statement in a constexpr function is a C++14
      extension [-Wc++14-extensions]
        UNUSED(n); return DEF_N;
        ^
test.cxx:4:19: note: expanded from macro 'UNUSED'
#define UNUSED(x) ((void)x)
                  ^
1 warning generated.

Another twist when moving from the clean room to production: Doxygen. This is closer to what happens in practice, so we can't omit the variable name.

//! \brief Returns a valid N
//! \param n a value to determine a valid N
//! \returns a valid N
constexpr static size_t GetValidN(size_t n)
{
    return DEF_N;
}
jww
  • 97,681
  • 90
  • 411
  • 885
  • Maybe `return UNUSED(n), DEF_N;`? Why does this function take `n`, anyway? – user2357112 Sep 05 '16 at 06:02
  • @user2357112 - Its an [MCVE](https://stackoverflow.com/help/mcve) of an interface. The real code is a little more interesting – jww Sep 05 '16 at 06:12

3 Answers3

3

You could simply avoid giving the argument a name, or comment it out:

constexpr size_t DEF_N = 42;

constexpr static size_t GetValidN(size_t /*n*/)
{
    return DEF_N;
}

live demo

kfsone
  • 23,617
  • 2
  • 42
  • 74
  • Thanks @kfsone. That kind of breaks Doxygen. – jww Sep 05 '16 at 06:24
  • You could write a macro conditioned on doxygen's preprocessor symbol, defined as `#define DOCNAME(x) x` when doxygen is active, and `#define DOCNAME(x)` when it isn't. – Sebastian Redl Sep 05 '16 at 07:45
2

In C++11 you have some limitations on the body of a constexpr function.

In your specific case you can use the comma operator to overcome them:

constexpr static size_t GetValidN(size_t n)
{
    return UNUSED(n), DEF_N;
}

In C++14, your function is ok as it is.

skypjack
  • 49,335
  • 19
  • 95
  • 187
  • The initial smoke test for this change went well; we committed it at [Add constexpr-ness to seckey.h classes](http://github.com/weidai11/cryptopp/commit/cf81d8a09998a879969e6096c68c068f971e9784). I'll return with feedback as the changes are more thoroughly tested under compilers (Clang, Comeau, GCC, ICC, MSVC, SunCC) and platforms (Android, iOS, Linux, Solaris, Windows {Desktop|Server|Phone|Store}, etc). – jww Sep 05 '16 at 07:31
  • Would you happen to know if an `assert` will trigger similar issues? Asserts are another tool I adore, and I hope a debugging and diagnostic aide would not preclude the C++11 feature during development. – jww Sep 05 '16 at 11:37
  • @jww What do you expect the assert will trigger exactly? I don't understand what you want, sorry. If you can give me more details, I'll try to give you an answer. Thanks. – skypjack Sep 05 '16 at 12:04
  • I'd like to know if an `assert` will cause a failed compile. – jww Sep 05 '16 at 12:14
  • @jww If you mean `static_assert`, it _evaluates_ a constant expression. As long as you can provide it (as an example by using `constexpr` functions) you should not have problem. – skypjack Sep 05 '16 at 12:19
  • Be careful of using the comma operator on down level versions of GCC in Debug builds. It broke us on OS X 10.5 with GCC 4.0; CentOS 5 with GCC 4.1 and Ubuntu 12 with GCC 4.6. The break was missing symbols and resulted in link errors. Also see [Remove comma operator from return values for StaticGetDefaultRounds and StaticGetValidKeyLength in non-constexpr builds (Issue 255)](https://github.com/weidai11/cryptopp/commit/f0e7b45bcbcc5b979d9d8df9d151f5951996e6e9). – jww Sep 07 '16 at 13:41
  • @jww Interesting, never heard about that before. Thank you. – skypjack Sep 07 '16 at 13:43
0

The simplest solution would be to just comment out the n as kfsone mentioned.

In C++17 you could even do it like this:

constexpr static size_t GetValidN([[maybe_unused]] size_t n)
{
    return DEF_N;
}

For more information see http://en.cppreference.com/w/cpp/language/attributes

I'm not sure if this is a stylistically sane solution, hopefully IDEs will find a way to make it look less ugly.

  • Commenting out `n` breaks the Doxgen documentation. I'm OK with the C++17 stuff, but we support pre-C++03 (GCC 3 and VC++ 2002) through anything you might encounter today (we actively est for breaks under C++14 and C++17). I'm not sure how easy it would be to adapt `[[maybe_unused]]` to 20 years of compilers. – jww Sep 05 '16 at 06:26
  • @jww You could do the other thing I mentioned if you can live with the way it looks. You will need a modern compiler though (MSVC should support this already IIRC, not sure about clang/gcc). –  Sep 05 '16 at 06:27