Repository navigation
CallbackScope should not have NODE_EXTERN? #20758
Copy link
Copy link
Closed
Labels
addonsIssues and PRs related to native addons.Issues and PRs related to native addons.invalidIssues and PRs that are invalid.Issues and PRs that are invalid.questionIssues asking questions about Node.js.Issues asking questions about Node.js.wrong repoIssues that should be opened in another repository.Issues that should be opened in another repository.
Description
Version: Since dec, 2017...
This wasn't an issue previously.
(Sorry for vagueness)
At the bottom is the result of a build of an addon.
node-x64\v10.1.0\include\node\node.h(637): warning C4251: 'node::CallbackScope::try_catch_': class 'v8::TryCatch' needs to have dll-interface to be us
ed by clients of class 'node::CallbackScope'
https://github.com/nodejs/node/blob/master/src/node.h#L663
Deleting NODE_EXTERN on that line of code fixes the warning.
(rather than modifying the V8 headers to have NODE_EXTERN on TryCatch)
Also #15570
#15570 (comment)
This is hidden by node-gyp which, by default, enables disable warning C4251 in windows.
cmake-js does not auto-disable this warning... so it shows up for those builds.
"H:\dev2\kcore\kcore\node_modules\sack.vfs\build\ALL_BUILD.vcxproj" (default target) (1) ->
"H:\dev2\kcore\kcore\node_modules\sack.vfs\build\sack_vfs.vcxproj" (default target) (3) ->
(ClCompile target) ->
C:\Users\Panther\.cmake-js\node-x64\v10.1.0\include\node\node.h(637): warning C4251: 'node::CallbackScope::try_catch_': class 'v8::TryCatch' needs to have dll-interface to be us
ed by clients of class 'node::CallbackScope' (compiling source file H:\dev2\kcore\kcore\node_modules\sack.vfs\src\com_interface.cc)C:\Users\Panther\.cmake-js\node-x64\v10.1.0\incl
ude\node\node.h(637): warning C4251: 'node::CallbackScope::try_catch_': class 'v8::TryCatch' needs to have dll-interface to be used by clients of class 'node::CallbackScope' (comp
iling source file H:\dev2\kcore\kcore\node_modules\sack.vfs\src\sql_module.cc) [H:\dev2\kcore\kcore\node_modules\sack.vfs\build\sack_vfs.vcxproj]
C:\Users\Panther\.cmake-js\node-x64\v10.1.0\include\node\node.h(637): warning C4251: 'node::CallbackScope::try_catch_': class 'v8::TryCatch' needs to have dll-interface to be us
ed by clients of class 'node::CallbackScope' (compiling source file H:\dev2\kcore\kcore\node_modules\sack.vfs\src\tls_interface.cc) [H:\dev2\kcore\kcore\node_modules\sack.vfs\buil
d\sack_vfs.vcxproj]
C:\Users\Panther\.cmake-js\node-x64\v10.1.0\include\node\node.h(637): warning C4251: 'node::CallbackScope::try_catch_': class 'v8::TryCatch' needs to have dll-interface to be us
ed by clients of class 'node::CallbackScope' (compiling source file H:\dev2\kcore\kcore\node_modules\sack.vfs\src\jsonParse.cc) [H:\dev2\kcore\kcore\node_modules\sack.vfs\build\sa
ck_vfs.vcxproj]
C:\Users\Panther\.cmake-js\node-x64\v10.1.0\include\node\node.h(637): warning C4251: 'node::CallbackScope::try_catch_': class 'v8::TryCatch' needs to have dll-interface to be us
ed by clients of class 'node::CallbackScope' (compiling source file H:\dev2\kcore\kcore\node_modules\sack.vfs\src\websocket_module.cc) [H:\dev2\kcore\kcore\node_modules\sack.vfs\b
uild\sack_vfs.vcxproj]
C:\Users\Panther\.cmake-js\node-x64\v10.1.0\include\node\node.h(637): warning C4251: 'node::CallbackScope::try_catch_': class 'v8::TryCatch' needs to have dll-interface to be us
ed by clients of class 'node::CallbackScope' (compiling source file H:\dev2\kcore\kcore\node_modules\sack.vfs\src\thread_module.cc)C:\Users\Panther\.cmake-js\node-x64\v10.1.0\incl
ude\node\node.h(637): warning C4251: 'node::CallbackScope::try_catch_': class 'v8::TryCatch' needs to have dll-interface to be used by clients of class 'node::CallbackScope' (comp
iling source file H:\dev2\kcore\kcore\node_modules\sack.vfs\src\vfs_module.cc) [H:\dev2\kcore\kcore\node_modules\sack.vfs\build\sack_vfs.vcxproj]
C:\Users\Panther\.cmake-js\node-x64\v10.1.0\include\node\node.h(637): warning C4251: 'node::CallbackScope::try_catch_': class 'v8::TryCatch' needs to have dll-interface to be us
ed by clients of class 'node::CallbackScope' (compiling source file H:\dev2\kcore\kcore\node_modules\sack.vfs\src\srg_module.cc) [H:\dev2\kcore\kcore\node_modules\sack.vfs\build\s
ack_vfs.vcxproj]
C:\Users\Panther\.cmake-js\node-x64\v10.1.0\include\node\node.h(637): warning C4251: 'node::CallbackScope::try_catch_': class 'v8::TryCatch' needs to have dll-interface to be us
ed by clients of class 'node::CallbackScope' (compiling source file H:\dev2\kcore\kcore\node_modules\sack.vfs\src\network_module.cc) [H:\dev2\kcore\kcore\node_modules\sack.vfs\bui
ld\sack_vfs.vcxproj]
C:\Users\Panther\.cmake-js\node-x64\v10.1.0\include\node\node.h(637): warning C4251: 'node::CallbackScope::try_catch_': class 'v8::TryCatch' needs to have dll-interface to be us
ed by clients of class 'node::CallbackScope' (compiling source file H:\dev2\kcore\kcore\node_modules\sack.vfs\src\task_module.cc) [H:\dev2\kcore\kcore\node_modules\sack.vfs\build\
sack_vfs.vcxproj]
C:\Users\Panther\.cmake-js\node-x64\v10.1.0\include\node\node.h(637): warning C4251: 'node::CallbackScope::try_catch_': class 'v8::TryCatch' needs to have dll-interface to be us
ed by clients of class 'node::CallbackScope' (compiling source file H:\dev2\kcore\kcore\node_modules\sack.vfs\src\reg_access.cc) [H:\dev2\kcore\kcore\node_modules\sack.vfs\build\s
ack_vfs.vcxproj]
C:\Users\Panther\.cmake-js\node-x64\v10.1.0\include\node\node.h(637): warning C4251: 'node::CallbackScope::try_catch_': class 'v8::TryCatch' needs to have dll-interface to be us
ed by clients of class 'node::CallbackScope' (compiling source file H:\dev2\kcore\kcore\node_modules\sack.vfs\src\config_module.cc) [H:\dev2\kcore\kcore\node_modules\sack.vfs\buil
d\sack_vfs.vcxproj]
H:\dev2\kcore\kcore\node_modules\sack.vfs\src\sack\sack.cc(66053): warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss of data [H:\dev2\kcore\kcore\node_
modules\sack.vfs\build\sack_vfs.vcxproj]
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
addonsIssues and PRs related to native addons.Issues and PRs related to native addons.invalidIssues and PRs that are invalid.Issues and PRs that are invalid.questionIssues asking questions about Node.js.Issues asking questions about Node.js.wrong repoIssues that should be opened in another repository.Issues that should be opened in another repository.
Activity
[-]CallbackScope should not have NODE_EXTERN [/-][+]CallbackScope should not have NODE_EXTERN?[/+]bnoordhuis commentedon May 16, 2018
That's a cmake-js bug. I had a quick look at its sources and I don't see it defining
BUILDING_NODE_EXTENSION, except on macos. Define it and you'll get proper linkage.d3x0r commentedon May 16, 2018
I get a lot of different warnings that are also 4251 with BUILDING_NODE_EXTENSION enabled....
So far it was easier to just remove the one NODE_EXTERN that isn't specified for ANY other class anyway.
This is the ONLY line
class NODE_EXTERN CallbackScope {that has 'class' and NODE_EXTERN. or... that's the only class that has NODE_EXTERN; none of the other classes do...