Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build fails with gcc 11 / C++20 #1007

Closed
urp opened this issue Jun 3, 2021 · 4 comments
Closed

Build fails with gcc 11 / C++20 #1007

urp opened this issue Jun 3, 2021 · 4 comments
Assignees

Comments

@urp
Copy link

urp commented Jun 3, 2021

Problem

Compiler error when including node.h with NAPI_VERSION=8:

napi.h:2477:60: error: expected unqualified-id before ‘)’ token
 2477 |     TypedThreadSafeFunction<ContextType, DataType, CallJs>();
      |                                                            ^
/home/adiletkutmanov/Development/lauda_measurement_napi/node_modules/node-addon-api/napi.h:2479:33: error: expected ‘)’ before ‘tsFunctionValue’
 2479 |         napi_threadsafe_function tsFunctionValue);
      |                                 ^~~~~~~~~~~~~~~~
      |                                 )
/home/adiletkutmanov/Development/lauda_measurement_napi/node_modules/node-addon-api/napi.h:2478:59: note: to match this ‘(’
 2478 |     TypedThreadSafeFunction<ContextType, DataType, CallJs>(
      |                                                           ^
In file included from /home/adiletkutmanov/Development/lauda_measurement_napi/node_modules/node-addon-api/napi.h:2725,
                 from /home/adiletkutmanov/Development/lauda_measurement_napi/src/lauda_measurement.cpp:1:
/home/adiletkutmanov/Development/lauda_measurement_napi/node_modules/node-addon-api/napi-inl.h:4764:8: error: no declaration matches ‘Napi::TypedThreadSafeFunction<ContextType, DataType, CallJs>::TypedThreadSafeFunction()’
 4764 | inline TypedThreadSafeFunction<ContextType, DataType, CallJs>::
      |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/adiletkutmanov/Development/lauda_measurement_napi/node_modules/node-addon-api/napi-inl.h:4764:8: note: no functions named ‘Napi::TypedThreadSafeFunction<ContextType, DataType, CallJs>::TypedThreadSafeFunction()’
In file included from /home/adiletkutmanov/Development/lauda_measurement_napi/src/lauda_measurement.cpp:1:
/home/adiletkutmanov/Development/lauda_measurement_napi/node_modules/node-addon-api/napi.h:2347:9: note: ‘class Napi::TypedThreadSafeFunction<ContextType, DataType, CallJs>’ defined here
 2347 |   class TypedThreadSafeFunction {​​​​​​​
      |         ^~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/adiletkutmanov/Development/lauda_measurement_napi/node_modules/node-addon-api/napi.h:2725,
                 from /home/adiletkutmanov/Development/lauda_measurement_napi/src/lauda_measurement.cpp:1:
/home/adiletkutmanov/Development/lauda_measurement_napi/node_modules/node-addon-api/napi-inl.h:4771:8: error: no declaration matches ‘Napi::TypedThreadSafeFunction<ContextType, DataType, CallJs>::TypedThreadSafeFunction(napi_threadsafe_function)’
 4771 | inline TypedThreadSafeFunction<ContextType, DataType, CallJs>::
      |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/adiletkutmanov/Development/lauda_measurement_napi/node_modules/node-addon-api/napi-inl.h:4771:8: note: no functions named ‘Napi::TypedThreadSafeFunction<ContextType, DataType, CallJs>::TypedThreadSafeFunction(napi_threadsafe_function)’
In file included from /home/adiletkutmanov/Development/lauda_measurement_napi/src/lauda_measurement.cpp:1:
/home/adiletkutmanov/Development/lauda_measurement_napi/node_modules/node-addon-api/napi.h:2347:9: note: ‘class Napi::TypedThreadSafeFunction<ContextType, DataType, CallJs>’ defined here
 2347 |   class TypedThreadSafeFunction {​​​​​​​
      |         ^~~~~~~~~~~~~~~~~~~~~~~

Workarounds:

  • Use gcc 11 with c++17
  • Use gcc 10 with c++20
@KevinEady
Copy link
Contributor

Hi @urp ,

I am able to replicate this issue the example in the readme.

Start container on gcc-11.1:

docker run -it --rm -v $(pwd):/app gcc:11.1 /bin/bash

And run these commands:

apt-get update && apt-get install -y cmake
curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.38.0/install.sh | bash

export NVM_DIR="$HOME/.nvm"
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"  # This loads nvm
[ -s "$NVM_DIR/bash_completion" ] && \. "$NVM_DIR/bash_completion"  # This loads nvm bash_completion
nvm install node

cd /tmp
git clone --branch add-typed-tsfn-example https://github.com/nodejs/node-addon-examples.git
cd node-addon-examples/typed_threadsafe_function/node-addon-api
npm i

I have noticed that using c++20 on my Mac (Apple clang version 12.0.0 (clang-1200.0.32.29)) does work. Not sure exactly how to proceed. We can discuss in the Node-API weekly meeting

cc: @mhdawson

@richardlau
Copy link
Member

FWIW C++20 support in gcc 11 is marked experimental: https://gcc.gnu.org/onlinedocs/gcc-11.1.0/gcc/C-Dialect-Options.html#C-Dialect-Options

@KevinEady
Copy link
Contributor

Thanks @richardlau for the insight! I am in the process of reporting the bug to GCC compiler team, currently waiting account creation.

@urp, unfortunately I think we will have to go with "known incompatibility" until GCC updates their compiler; since other c++20 compilers work, it is strictly an issue with GCC.

I will keep this issue open until I can link the GCC bug report in this ticket and our setup documentation page.

Thanks, Kevin

@KevinEady KevinEady self-assigned this Jun 10, 2021
KevinEady added a commit to KevinEady/node-addon-api that referenced this issue Jun 11, 2021
KevinEady added a commit to KevinEady/node-addon-api that referenced this issue Jun 11, 2021
@KevinEady
Copy link
Contributor

KevinEady commented Jun 11, 2021

This issue will be fixed when the associated PR is merged.

Thanks for reporting!

Edit: Please see the not-a-bug bug report comment for more information.

deepakrkris pushed a commit to deepakrkris/node-addon-api that referenced this issue Sep 23, 2021
Fixes: nodejs#1007

PR-URL: nodejs#1009
Reviewed-By: Michael Dawson <midawson@redhat.com>
deepakrkris pushed a commit to deepakrkris/node-addon-api that referenced this issue Oct 15, 2021
Fixes: nodejs#1007

PR-URL: nodejs#1009
Reviewed-By: Michael Dawson <midawson@redhat.com>
kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this issue Aug 24, 2022
Fixes: nodejs/node-addon-api#1007

PR-URL: nodejs/node-addon-api#1009
Reviewed-By: Michael Dawson <midawson@redhat.com>
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this issue Aug 26, 2022
Fixes: nodejs/node-addon-api#1007

PR-URL: nodejs/node-addon-api#1009
Reviewed-By: Michael Dawson <midawson@redhat.com>
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this issue Sep 19, 2022
Fixes: nodejs/node-addon-api#1007

PR-URL: nodejs/node-addon-api#1009
Reviewed-By: Michael Dawson <midawson@redhat.com>
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this issue Aug 11, 2023
Fixes: nodejs/node-addon-api#1007

PR-URL: nodejs/node-addon-api#1009
Reviewed-By: Michael Dawson <midawson@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants