nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.52k stars 29.56k forks source link

code in v8-function-callback.h fails to compile on windows #52895

Closed bmacnaughton closed 5 months ago

bmacnaughton commented 5 months ago

Version

v20.0.0

Platform

Microsoft Windows NT 10.0.19045.0 x64

Subsystem

v8 header files

What steps will reproduce the bug?

try to compile an addon on windows

How often does it reproduce? Is there a required condition?

100%

What is the expected behavior? Why is that the expected behavior?

the addon compiles

What do you see instead?

  addon.cc
C:\...\AppData\Local\Temp\prebuildify\node\22.0.0\include\no
de\v8-function-callback.h(408,62): warning C4003: not enough arguments for func
tion-like macro invocation 'min' [C:\Development\node-fn-inspect\build\fninspec
t.vcxproj]
  (compiling source file '../src/addon.cc')

C:\...\AppData\Local\Temp\prebuildify\node\22.0.0\include\no
de\v8-function-callback.h(409,62): warning C4003: not enough arguments for func
tion-like macro invocation 'max' [C:\Development\node-fn-inspect\build\fninspec
t.vcxproj]
  (compiling source file '../src/addon.cc')

C:\...\AppData\Local\Temp\prebuildify\node\22.0.0\include\no
de\v8-function-callback.h(408,62): error C2589: '(': illegal token on right sid
e of '::' [C:\Development\node-fn-inspect\build\fninspect.vcxproj]
  (compiling source file '../src/addon.cc')

C:\...\AppData\Local\Temp\prebuildify\node\22.0.0\include\no
de\v8-function-callback.h(408,62): error C2760: syntax error: ')' was unexpecte
d here; expected 'expression' [C:\Development\node-fn-inspect\build\fninspect.v
cxproj]
  (compiling source file '../src/addon.cc')

Additional information

The problem is that windef.h defines macros min and max. The v8 code in v8-function-callback.h can work around it by:

template <typename T>
void ReturnValue<T>::Set(uint16_t i) {
  static_assert(std::is_base_of<T, Integer>::value, "type check");
  using I = internal::Internals;
  // wrap std::numeric_limits<uint16_t>::min in parens to avoid interpretation as macro
  static_assert(I::IsValidSmi((std::numeric_limits<uint16_t>::min)()));
// ditto max
  static_assert(I::IsValidSmi((std::numeric_limits<uint16_t>::max)()));
  SetInternal(I::IntToSmi(i));
}

There may be other ways around this but I was able to compile our addon using this workaround.

targos commented 5 months ago

This was fixed in https://github.com/nodejs/node/pull/52794 and will be in the next release.