NanNew<v8::String>(char *) compiler warning C4344 help

[mathiask88]

Hi,

I need some help understanding the warning AppVeyor/MSVC is complaining about:

..\src\node_snap7_client.cpp(909): warning C4344: behavior change: use of explicit template arguments results in call to 'v8::Local<T> NanNew<v8::String,char*>(A0)' [C:\projects\node-snap7\build\node_snap7.vcxproj]
          with
          [
              T=v8::String,
              A0=char *
          ]
          but the regular function 'v8::Local<T> NanNew(const char *)' is a better match
          with
          [
              T=v8::String
          ]
          if you expect 'v8::Local<T> NanNew(const char *)' to be called then you need to make it an explicit specialization
          with
          [
              T=v8::String
          ]

Line 909 is NanNew<v8::String>(BlockInfo->CodeDate)); where BlockInfo->CodeDate is a char[11] *.

[kkoopa]

Seems like a bug... Probably not in msvc, but in the NanNew rewrite. Are the convenience functions creating problems?

Summoning @agnat.

[mathiask88]

It is a bit strange. If I compile it on my Desktop with VS13 this warning is not thrown.

[kkoopa]

It also seems that similar warnings from msvc have been around since long before the rewrite. Could it be a compiler bug after all?

[mathiask88]

Due to this twitter post https://twitter.com/appveyor/status/534724949022224384 they are using VS13 Update 4 for their build workers, same version as on my PC. Where is the difference?

[kkoopa]

I get the warning with my trusty old VS2010, the last sane version. It seems only to apply to non-const char *.

[mathiask88]

Yes, BlockInfo is a Pointer of a struct and in the struct CodeDate is a non-const char[11]. But why is AppVeyor throwing the warnings and my VS not?? Strange....

[mathiask88]

I am a C++ beginner, but I think I can ignore the warning. The compiler chooses the template function because of the explicit type, but advises that the convenience function would be a better match(Because the Factory only got functions with 2 arguments??). But the result is the same, because the convenience function calls the explicit one. Did I get this right?

[agnat]

I need some help understanding the warning [...] MSVC is complaining about

Hehe... you're not alone. I need that on a t-shirt.

I'd say the compiler does what the programmer intended and what any sane compiler would do. Not sure why it warns about it. The warning would make sense (kind of) if v8::String was an argument type. But it isn't. So in our case the warning doesn't make sense, is misleading and can safely be ignored.

We should probably just go ahead and disable it: smallest possible scope, push, pop... that sort of thing.

But why is AppVeyor throwing the warnings and my VS not?

Good question. I have no idea. Are you using the exact same commands on your desktop? Like: Opening the project file in VS might trigger different behavior than calling node-gyp on the command line. On the other hand this is a level 1 warning. So, no... no clue... :-/

[agnat]

The compiler chooses the template function because of the explicit type,

Right.

but advises that the convenience function would be a better match

I think it advises that without the explicit type it would have called the convenience function. In our case this is not surprising. It is so non-surprising that it is misleading. There simply is no way to call the ... uh... inconvenience function without an explicit type argument.

(Because the Factory only got functions with 2 arguments??).

Um, no. Let's contrive an example that makes sense of the warning:

struct foo {};

template <typename T> void bar(T t) {}  // [a]
void bar(foo f) {}                      // [b]

foo f;
bar(f);      // calls b
bar<foo>(f); // calls a!

In this case the presence of the explicit type argument changes the called function and the VS guys think it is worth a warning.

But the result is the same, because the convenience function calls the explicit one. Did I get this right?

Exactly.

[mathiask88]

I need some help understanding the warning [...] MSVC is complaining about

Hehe... you're not alone. I need that on a t-shirt.

:grin:

But why is AppVeyor throwing the warnings and my VS not?

Good question. I have no idea. Are you using the exact same commands on your desktop? Like: Opening the project file in VS might trigger different behavior than calling node-gyp on the command line. On the other hand this is a level 1 warning. So, no... no clue... :-/

Yes I triggered the build with node-gyp configure build in the command promt. True, this warning is level 1 and the compiler is set to /W3. So no idea where the difference comes from. Maybe the MsBuild version differs after all.

Thanks for your explanation!

[kkoopa]

I don't know if it will be possible to limit the scope of disabled warning from within nan.h. I think it will have to be all or nothing. Could probably go with all here, if _MSC_VER matches VS2010. I don't think this shows up by default in later versions.

[agnat]

Hm, yeah. It's a call site warning. So, the "smallest possible scope" is probably the whole CU. ;)

[agnat]

Here is an old webkit ticket: https://bugs.webkit.org/show_bug.cgi?id=22325

Similar issue, similar explanation, similar conclusion...

[mathiask88]

I don't mind if we don't bloat nan with global pragmas just to prevent older MSVC from printing senseless warnings.

[kkoopa]

It's not a question of bloat as much as that it leaks out to the entire compilation unit and can mask legitimate errors.

On January 19, 2015 4:04:22 PM EET, Mathias notifications@github.com wrote:

I don't mind if we don't bloat nan with global pragmas just to prevent older MSVC from printing senseless warnings.

---

Reply to this email directly or view it on GitHub: https://github.com/rvagg/nan/issues/246#issuecomment-70496240

[mathiask88]

It's not a question of bloat as much as that it leaks out to the entire compilation unit and can mask legitimate errors.

Yes, therefore I think the best is to do nothing and if someone will ever get this warning he/she can read this issue and disable the warning on call site. Hopefully very unlikely, because it is already fixed in newer MSVC.

[kkoopa]

Workaround is to use the convenience function.

[mathiask88]

I figured out why appveyor and my VS13 printed different compiler warnings. AppVeyor has VS10/12/13 installed and the default toolset is VS10. I added "msbuild_toolset": "v120" in my binding.gyp under the windows condition and then appveyor selected VS13. I just wanted to share because this still was an open question.