GCC warning with -Wconversion and undefined behavior for unknown_array_type

[zbjornson]

This line: https://github.com/nodejs/node-addon-api/blob/da2e754a021e418a3ca36f3c10ea134c9541959f/napi.h#L1075 causes a warning with -Wconversion enabled:

 warning: the result of the conversion is unspecified because ‘-1’ is outside the range of type ‘napi_typedarray_type’ [-Wconversion]
  911 |     static const napi_typedarray_type unknown_array_type = static_cast<napi_typedarray_type>(-1);
      |

I don't think this is really an issue, but it would be nice to get rid of the warning.

(https://wiki.sei.cmu.edu/confluence/display/cplusplus/INT50-CPP.+Do+not+cast+to+an+out-of-range+enumeration+value)

[mhdawson]

@zbjornson would you like to submit a suggested PR? I'd have to think more about whether changing the value would be a problem or not but I assume there are other ways to suppress the warking as well.

[zbjornson]

Sure. I'll take a look at better solutions, but #pragma GCC diagnostics ignored "-Wconversion" would minimally work. The C++ spec defines this enum to have an implicit size of int, which has at least 16 bits, and the enum has 11 members currently, which is why I said it's probably not an issue. However, since C++17 this line is actually undefined behavior, not just an unspecified value.

node-addon-api could maybe have its own enum that has the same values as napi_typedarray_type plus an unknown type. I think that would be unobtrusive.

[github-actions[bot]]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

[github-actions[bot]]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

[mhdawson]

@zbjornson were you still planning to submit a PR on this?

[github-actions[bot]]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

[NickNaso]

I'm closing the issue because it has been solved with PR https://github.com/nodejs/node-addon-api/pull/1209