Visual Studio 2013 build warnings

[d3x0r]

Recently updated to node 8.6.0, and when I build my plugin, I get these warnings from node.h.
(when building with cmake-js which does not specify warnings to ignore)
I notice when building with node-gyp that I don't get this warning; but node-gyp adds these ignores to the build. 4267;4351;4355;4800;4251 Although I'm not sure where those come from. I did find (unrelated) this in node-gyp's addon.gypi file. and that hasn't changed since 10/20/2016 so I guess this sort of warning has been an issue on-and-off for a while; although I've had no such warnings around ObjectWrap....

        'msvs_disabled_warnings': [
          # warning C4251: 'node::ObjectWrap::handle_' : class 'v8::Persistent<T>'
          # needs to have dll-interface to be used by
          # clients of class 'node::ObjectWrap'
          4251
        ],

This is a quote of an instance of the warning.

<user>\.cmake-js\node-x64\v8.6.0\include\node\node.h(625): 
   warning C4251: 'node::CallbackScope::try_catch_': class 'v8::TryCatch' needs to have dll-interface to be used by clients of class 'node::CallbackScope' (compiling source file ...]
  <user>\.cmake-js\node-x64\v8.6.0\include\node\v8.h(8150): note: see declaration of 'v8::TryCatch' (compiling source file ...)

Trying to capture what it actually means in order to fix it...

line 8150 of v8.h

class V8_EXPORT TryCatch {

while the class that's using trycatch

class NODE_EXTERN CallbackScope {
 public:
  CallbackScope(v8::Isolate* isolate,
                v8::Local<v8::Object> resource,
                async_context asyncContext);
  ~CallbackScope();

 private:
  InternalCallbackScope* private_;
  v8::TryCatch try_catch_;  // line 625

...

The warning does go away if NODE_EXTERN is removed from the class definition. ObjectWrap has no sort of link specification on it (now). And no other class has NODE_EXTERN applied to it.

In 8.4.0 there was no such thing as 'CallbackScope' so this has been a recent addition, and the extra qualifier has been hidden by node-gyp.

---

Edit:
I could make this a pull request which is the one line change to node.h; but I think I'd fail because I can run no tests, and can't really get 'affect subsystems'

https://github.com/d3x0r/node/tree/patch-1

[seishun]

It was previously discussed in #15570. Basically, this warning is overzealous. It should hopefully be fixed in VS2017 15.6.

[bnoordhuis]

Looks like this has been answered. Closing.

[d3x0r]

Ya know, this isn't really about node building; this is about node.h causing warnings that node-gyp hides and I would have to hide using cmake-js or other tools.
the node header previously generated the same C4251 warning with wrapped class, but it was fixed.... this too can be fixed.