Avoiding the warning: operation on ‘count’ may be undefined [-Wsequence-point]

Question

I'm working with the Node.js N-API and I'm making a little wrapper that will make exporting C++ functions easy.

template<class T, class... Targs> napi_value Api::create(const char* name, T (* const cb)(Targs...))
{
    // creates JavaScript function that will call cbProxy<> when called
    return create(name, cbProxy<T, Targs...>, cb);
}

template<class T, class... Targs> napi_value Api::cbProxy(const napi_env env, const napi_callback_info info)
{
    // number of arguments
    size_t count = sizeof...(Targs);
    ApiValue args[count];
    T (* cb)(Targs...);
    // retrieve arguments and callback
    if (!Api::getParams(env, info, args, count, &cb))
        return nullptr;

    T ret = cb(Api::getValue<Targs>(&args[--count])...);    
    return Api(env).create(ret);
}

template<> bool Api::getValue(ApiValue* value)
{
    return value->toBool();
}

template<> double Api::getValue(ApiValue* value)
{
    return value->toDouble();
}

template<> int32_t Api::getValue(ApiValue* value)
{
    return value->toInt32();
}

The idea is to call api.create("TestFunction", testFn); which will return a JS function.
When it's called, it will call Api::cbProxy<> which converts the JS parameters to the equivalent C++ types with Api::getValue<T>() and calls testFn (cb).

Lets say testFn has the following signature: int testFn(bool bVal, double dVal)
The line T ret = cb(Api::getValue<Targs>(&args[--count])...); will expand to

int ret = cb(Api::getValue<bool>(&args[--count]), 
             Api::getValue<double>(&args[--count]));

Which works 100% fine, but it triggers a compiler warning. I'm a bit of a novice regarding C++, so I'm looking for a better way to write this.

Basically I want to traverse the array args and the list of type parameters Targs at once.

Just to clarify: The --count in the expanded function call triggers the following warning:
warning: operation on ‘count’ may be undefined [-Wsequence-point]

@max66 As the title states, "warning: operation on ‘count’ may be undefined [-Wsequence-point]" — Katja, May 25 '18 at 02:26
Possible duplicate of [Undefined behavior and sequence points](https://stackoverflow.com/questions/4176328/undefined-behavior-and-sequence-points) — , May 25 '18 at 02:36
Next time, whenever you find yourself saying something "works 100% fine, but...", I hope your second thought is "Nope. Something definitely wrong." But it is good you don't ignore warning. — , May 25 '18 at 02:40
@NickyC It relies on undefined behavior so it's 100% wrong, but I was unable to figure out another way. — Katja, May 25 '18 at 02:45
The double `--count` here is at least well-defined in C++17, but still not a great story. — chris, May 25 '18 at 02:48
The `args` definition is a VLA non-standard extension; to fix this `count` should be `const` — M.M, May 25 '18 at 02:57

max66 · Accepted Answer · 2018-05-25T03:01:06.857

I suppose that the problem is that in

int ret = cb(Api::getValue<bool>(&args[--count]), 
             Api::getValue<double>(&args[--count]));

the order of evaluation of argument argument is ~~implementation dependent~~ undefined bahaviour (M.M correction) so, given count starting from 2, can be

int ret = cb(Api::getValue<bool>(&args[1]), 
             Api::getValue<double>(&args[0]));

or

int ret = cb(Api::getValue<bool>(&args[0]), 
             Api::getValue<double>(&args[1]));

To be sure that the first index of args (the bool one) is 1 and that the second one (the double one) is 0, a possible way is use variadic indexes.

If you can use C++14, using a cbProxyHelper() method, you can try something as follows (caution: code not tested)

template <typename T, typename ... Targs, std::size_t ... Is>
napi_value Api::cbProxyHelper (const napi_env env,
                               const napi_callback_info info,
                               std::index_sequence<Is...> const &)
 {
   // number of arguments
   constexpr std::size_t count = sizeof...(Targs);
   ApiValue args[count];
   T (* cb)(Targs...);
   // retrieve arguments and callback
   if (!Api::getParams(env, info, args, count, &cb))
      return nullptr;

   T ret = cb(Api::getValue<Targs>(&args[count-1U-Is])...);    
   return Api(env).create(ret);
 }

template <typename T, typename ... Targs>
napi_value Api::cbProxy (const napi_env env, const napi_callback_info info)
 { return cbProxyHelper(env, info, std::index_sequence_for<Targs...>{}); }

If you're using C++11, simulate std::index_sequence and std::make_index_sequence isn't really difficult.

I figured the order of evaluation would give problems. Thanks for the really neat solution! — Katja, May 25 '18 at 02:49
The original code was *undefined behaviour* prior to C++17 ; in C++17 it was changed to *unspecified* which behaves as you describe — M.M, May 25 '18 at 02:56
@Smokey - You're welcome. Answer a little improved (aesthetically improved): `std::index_sequence_for` instead `std::make_index_sequence`. — max66, May 25 '18 at 02:57
@M.M - you're right: undefined behavior; I was convinced was implementation dependent. Corrected; thanks. — max66, May 25 '18 at 03:02

Avoiding the warning: operation on ‘count’ may be undefined [-Wsequence-point]

1 Answers1