src: fix compilation error in node_crypto_common #220

danbev · 2019-12-06T08:10:31Z

Currently, the following compilation error is generated:

In file included from ../src/node_crypto_common.cc:1:
In file included from ../src/env-inl.h:28:
In file included from ../src/env.h:35:
In file included from ../src/handle_wrap.h:27:
In file included from ../src/async_wrap.h:27:
../src/base_object.h:42:10: error:
inline function 'node::BaseObject::~BaseObject' is not defined
[-Werror,-Wundefined-inline]
  inline ~BaseObject() override;
         ^
../src/node_crypto.h:90:3: note: used here
  ~SecureContext() override {

This commit includes base_object-inl.h to avoid this error.

Checklist

make -j4 test-only (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

addaleax

LGTM as a workaround … I can’t reproduce the error here – does addaleax/node@7d0e9a5 also resolve this for you? Including one header should not mean having to include another header too

danbev · 2019-12-06T13:00:36Z

does addaleax/node@7d0e9a5 also resolve this for you?

Thanks, it does for node_crypto_common I'm getting the same issue for node_quic_socket.cc now:

compilation error

 ccache clang++ -Qunused-arguments -o /Users/danielbevenius/work/nodejs/quic/node_quic/out/Release/obj.target/libnode/src/node_quic_socket.o ../src/node_quic_socket.cc '-DV8_DEPRECATION_WARNINGS' '-DV8_IMMINENT_DEPRECATION_WARNINGS' '-D_DARWIN_USE_64_BIT_INODE=1' '-DOPENSSL_NO_PINSHARED' '-DOPENSSL_THREADS' '-DNODE_ARCH="x64"' '-DNODE_WANT_INTERNALS=1' '-DV8_DEPRECATION_WARNINGS=1' '-DNODE_OPENSSL_SYSTEM_CERT_PATH=""' '-DHAVE_INSPECTOR=1' '-DHAVE_DTRACE=1' '-DNODE_REPORT' '-DNODE_EXPERIMENTAL_QUIC=1' '-D__POSIX__' '-DNODE_USE_V8_PLATFORM=1' '-DNODE_HAVE_I18N_SUPPORT=1' '-DNODE_HAVE_SMALL_ICU=1' '-DNODE_PLATFORM="darwin"' '-DHAVE_OPENSSL=1' '-DUCONFIG_NO_SERVICE=1' '-DU_ENABLE_DYLOAD=0' '-DU_STATIC_IMPLEMENTATION=1' '-DU_HAVE_STD_STRING=1' '-DUCONFIG_NO_BREAK_ITERATION=0' '-D_LARGEFILE_SOURCE' '-D_FILE_OFFSET_BITS=64' '-DNGHTTP2_STATICLIB' '-DNGTCP2_STATICLIB' '-DNGHTTP3_STATICLIB' -I../src -I/Users/danielbevenius/work/nodejs/quic/node_quic/out/Release/obj/gen -I/Users/danielbevenius/work/nodejs/quic/node_quic/out/Release/obj/gen/include -I/Users/danielbevenius/work/nodejs/quic/node_quic/out/Release/obj/gen/src -I../deps/histogram/src -I../deps/v8/include -I../deps/icu-small/source/i18n -I../deps/icu-small/source/common -I../deps/zlib -I../deps/llhttp/include -I../deps/cares/include -I../deps/uv/include -I../deps/nghttp2/lib/includes -I../deps/ngtcp2/lib/includes -I../deps/ngtcp2/crypto/includes -I../deps/nghttp3/lib/includes -I../deps/brotli/c/include -I../deps/openssl/openssl/include  -Os -gdwarf-2 -mmacosx-version-min=10.10 -arch x86_64 -Wall -Wendif-labels -W -Wno-unused-parameter -Werror=undefined-inline -Wall -Wendif-labels -W -Wno-unused-parameter -std=gnu++1y -stdlib=libc++ -fno-rtti -fno-exceptions -fno-strict-aliasing -MMD -MF /Users/danielbevenius/work/nodejs/quic/node_quic/out/Release/.deps//Users/danielbevenius/work/nodejs/quic/node_quic/out/Release/obj.target/libnode/src/node_quic_socket.o.d.raw   -c
In file included from ../src/node_crypto_common.cc:1:
In file included from ../src/env-inl.h:28:
In file included from ../src/env.h:35:
In file included from ../src/handle_wrap.h:27:
In file included from ../src/async_wrap.h:27:
../src/base_object.h:41:10: error: inline function 'node::BaseObject::BaseObject' is not defined [-Werror,-Wundefined-inline]
  inline BaseObject(Environment* env, v8::Local<v8::Object> object);
         ^
../src/node_crypto.h:182:9: note: used here
      : BaseObject(env, wrap) {
        ^
In file included from ../src/node_crypto_common.cc:1:
In file included from ../src/env-inl.h:28:
In file included from ../src/env.h:35:
In file included from ../src/handle_wrap.h:27:
In file included from ../src/async_wrap.h:27:
../src/base_object.h:42:10: error: inline function 'node::BaseObject::~BaseObject' is not defined [-Werror,-Wundefined-inline]
  inline ~BaseObject() override;
         ^
../src/node_crypto.h:181:3: note: used here
  SecureContext(Environment* env, v8::Local<v8::Object> wrap)
  ^
In file included from ../src/node_crypto_common.cc:1:
In file included from ../src/env-inl.h:28:
In file included from ../src/env.h:35:
In file included from ../src/handle_wrap.h:27:
In file included from ../src/async_wrap.h:27:
../src/base_object.h:69:15: error: inline function 'node::BaseObject::MakeWeak' is not defined [-Werror,-Wundefined-inline]
  inline void MakeWeak();
              ^
../src/node_crypto.h:183:5: note: used here
    MakeWeak();
    ^
In file included from ../src/node_crypto_common.cc:1:
In file included from ../src/env-inl.h:28:
In file included from ../src/env.h:35:
In file included from ../src/handle_wrap.h:27:
In file included from ../src/async_wrap.h:27:
../src/base_object.h:56:23: error: inline function 'node::BaseObject::env' is not defined [-Werror,-Wundefined-inline]
  inline Environment* env() const;
                      ^
../src/node_crypto.h:189:7: note: used here
      env()->isolate()->AdjustAmountOfExternalAllocatedMemory(-kExternalSize);
      ^
In file included from ../src/node_crypto_common.cc:1:
In file included from ../src/env-inl.h:28:
In file included from ../src/env.h:35:
In file included from ../src/handle_wrap.h:27:
In file included from ../src/async_wrap.h:27:
../src/base_object.h:100:23: error: inline function 'node::BaseObject::OnGCCollect' is not defined [-Werror,-Wundefined-inline]
  virtual inline void OnGCCollect();
                      ^
../src/node_crypto.h:463:3: note: used here
  KeyObject(Environment* env,
  ^
5 errors generated.
make[1]: *** [/Users/danielbevenius/work/nodejs/quic/node_quic/out/Release/obj.target/libnode/src/node_crypto_common.o] Error 1
make[1]: *** Waiting for unfinished jobs....
../src/node_quic_socket.cc:1061:15: warning: unused variable 'socket' [-Wunused-variable]
  QuicSocket* socket =
              ^
1 warning generated.
rm 23b9307565d9fe2fcb314bdb403fb6b4b5600dbc.intermediate cca4bbec20be3370a730595aba68a1a175696f85.intermediate
make: *** [node] Error 2

addaleax · 2019-12-06T15:55:47Z

@danbev What if you also move the constructors to the .cc file? I could push another commit with that but since I can’t reproduce these problems locally it would be cool if you could look into it :)

benhalverson · 2019-12-10T07:12:27Z

I also ran into this same error when building using the instructions from #183

In file included from ../src/node_crypto_common.cc:1:
In file included from ../src/env-inl.h:28:
In file included from ../src/env.h:35:
In file included from ../src/handle_wrap.h:27:
In file included from ../src/async_wrap.h:27:
../src/base_object.h:42:10: error: inline function 'node::BaseObject::~BaseObject' is not defined [-Werror,-Wundefined-inline]
  inline ~BaseObject() override;
         ^
../src/node_crypto.h:90:3: note: used here
  ~SecureContext() override {
  ^
In file included from ../src/node_crypto_common.cc:1:
In file included from ../src/env-inl.h:28:
In file included from ../src/env.h:35:
In file included from ../src/handle_wrap.h:27:
In file included from ../src/async_wrap.h:27:
../src/base_object.h:41:10: error: inline function 'node::BaseObject::BaseObject' is not defined [-Werror,-Wundefined-inline]
  inline BaseObject(Environment* env, v8::Local<v8::Object> object);
         ^
../src/node_crypto.h:184:9: note: used here
      : BaseObject(env, wrap) {
        ^
In file included from ../src/node_crypto_common.cc:1:
In file included from ../src/env-inl.h:28:
In file included from ../src/env.h:35:
In file included from ../src/handle_wrap.h:27:
In file included from ../src/async_wrap.h:27:
../src/base_object.h:69:15: error: inline function 'node::BaseObject::MakeWeak' is not defined [-Werror,-Wundefined-inline]
  inline void MakeWeak();
              ^
../src/node_crypto.h:185:5: note: used here
    MakeWeak();
    ^
In file included from ../src/node_crypto_common.cc:1:
In file included from ../src/env-inl.h:28:
In file included from ../src/env.h:35:
In file included from ../src/handle_wrap.h:27:
In file included from ../src/async_wrap.h:27:
../src/base_object.h:56:23: error: inline function 'node::BaseObject::env' is not defined [-Werror,-Wundefined-inline]
  inline Environment* env() const;
                      ^
../src/node_crypto.h:191:7: note: used here
      env()->isolate()->AdjustAmountOfExternalAllocatedMemory(-kExternalSize);
      ^
In file included from ../src/node_crypto_common.cc:1:
In file included from ../src/env-inl.h:28:
In file included from ../src/env.h:35:
In file included from ../src/handle_wrap.h:27:
In file included from ../src/async_wrap.h:27:
../src/base_object.h:100:23: error: inline function 'node::BaseObject::OnGCCollect' is not defined [-Werror,-Wundefined-inline]
  virtual inline void OnGCCollect();
                      ^
../src/node_crypto.h:90:3: note: used here
  ~SecureContext() override {
  ^
5 errors generated.
make[1]: *** [/Users/benhalverson/projects/quic/out/Release/obj.target/libnode/src/node_crypto_common.o] Error 1
make[1]: *** Waiting for unfinished jobs....
rm 02f95cd8513699ec8a17ae7a735e824d698d34ae.intermediate d1a9d93465a74e8e9a51afbd8f57ab43bc278890.intermediate 06ff0a10a6f61c187fc74b16c487ee548479c3a7.intermediate c8ac677321316261231dd0d51277117b05f31410.intermediate
make: *** [node] Error 2

danbev · 2019-12-10T07:15:14Z

What if you also move the constructors to the .cc file? I could push another commit with that but since I can’t reproduce these problems locally it would be cool if you could look into it :)

Sorry about the late reply. I'll take a look at this now. Thanks

jasnell · 2019-12-13T18:42:38Z

Heads up, this will need to be rebased

Move destructors for subclasses of `BaseObject` from node_crypto.h to node_crypto.cc. This removes the need to include base_object-inl.h when using node_crypto.h in some cases.

Move constructor and destructors for subclasses of `BaseObject` from node_crypto.h to node_crypto.cc. This removes the need to include base_object-inl.h when using node_crypto.h in some cases. PR-URL: #220 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

danbev · 2019-12-16T05:42:54Z

Landed in 2a284ba.

Move constructor and destructors for subclasses of `BaseObject` from node_crypto.h to node_crypto.cc. This removes the need to include base_object-inl.h when using node_crypto.h in some cases. PR-URL: nodejs#220 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

Move constructor and destructors for subclasses of `BaseObject` from node_crypto.h to node_crypto.cc. This removes the need to include base_object-inl.h when using node_crypto.h in some cases. PR-URL: #220 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

Originally landed in the QUIC repo Move constructor and destructors for subclasses of `BaseObject` from node_crypto.h to node_crypto.cc. This removes the need to include base_object-inl.h when using node_crypto.h in some cases. Original review metadata: ``` PR-URL: nodejs/quic#220 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> ```

Originally landed in the QUIC repo Move constructor and destructors for subclasses of `BaseObject` from node_crypto.h to node_crypto.cc. This removes the need to include base_object-inl.h when using node_crypto.h in some cases. Original review metadata: ``` PR-URL: nodejs/quic#220 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> ``` PR-URL: #31872 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>

danbev mentioned this pull request Dec 6, 2019

doc,tools: fix make doc target for quic.md #221

Closed

2 tasks

addaleax approved these changes Dec 6, 2019

View reviewed changes

jasnell approved these changes Dec 6, 2019

View reviewed changes

addaleax force-pushed the master branch from b3b9c0e to 3bd75b5 Compare December 11, 2019 19:03

jasnell force-pushed the master branch from cf320b5 to 04c4af9 Compare December 13, 2019 18:23

addaleax mentioned this pull request Dec 14, 2019

quic: initial experimental quic implementation nodejs/node#30943

Closed

4 tasks

addaleax and others added 2 commits December 16, 2019 06:13

src: move BaseObject subclass dtors out of node_crypto.h

0b1a9f3

Move destructors for subclasses of `BaseObject` from node_crypto.h to node_crypto.cc. This removes the need to include base_object-inl.h when using node_crypto.h in some cases.

src: move BaseObject subclass ctors out of node_crypto.h

9353cb5

danbev closed this Dec 16, 2019

danbev deleted the crypto_common_compilation_error branch December 16, 2019 05:43

jasnell mentioned this pull request Feb 19, 2020

src: move BaseObject subclass dtors/ctors out of node_crypto.h nodejs/node#31872

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

src: fix compilation error in node_crypto_common #220

src: fix compilation error in node_crypto_common #220

danbev commented Dec 6, 2019

addaleax left a comment

danbev commented Dec 6, 2019

addaleax commented Dec 6, 2019

benhalverson commented Dec 10, 2019

danbev commented Dec 10, 2019

jasnell commented Dec 13, 2019

danbev commented Dec 16, 2019

src: fix compilation error in node_crypto_common #220

src: fix compilation error in node_crypto_common #220

Conversation

danbev commented Dec 6, 2019

Checklist

addaleax left a comment

Choose a reason for hiding this comment

danbev commented Dec 6, 2019

addaleax commented Dec 6, 2019

benhalverson commented Dec 10, 2019

danbev commented Dec 10, 2019

jasnell commented Dec 13, 2019

danbev commented Dec 16, 2019