Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

not enough actual parameters for macro 'min' #1180

Closed
VikingExplorer opened this issue Feb 19, 2018 · 8 comments
Closed

not enough actual parameters for macro 'min' #1180

VikingExplorer opened this issue Feb 19, 2018 · 8 comments

Comments

@VikingExplorer
Copy link

Similar to 984 except that I'm defining NOMINMAX:

#undef max
#define NOMINMAX										// needed for: https://stackoverflow.com/questions/22744262/cant-call-stdmax-because-minwindef-h-defines-max
#define RAPIDJSON_HAS_STDSTRING 1
#include "rapidjson/document.h"

My code includes document.h in only two places, and both are like the above. I turned on Show Includes:

1>Note: including file:       c:\dev\couloir\generic\afx\mvc\couloirafx\include\ModelData.hpp
1>Note: including file:        C:\Dev\Couloir\3pLibs\vcpkg\installed\Win32\include\rapidjson/document.h
1>Note: including file:         c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\reader.h
1>Note: including file:          c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\allocators.h
1>Note: including file:           c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\rapidjson.h
1>Note: including file:            C:\Program Files (x86)\Windows Kits\10\Include\10.0.10240.0\ucrt\inttypes.h
1>Note: including file:            C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\VC\Tools\MSVC\14.12.25827\include\cassert
1>Note: including file:             C:\Program Files (x86)\Windows Kits\10\Include\10.0.10240.0\ucrt\assert.h
1>Note: including file:          c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\stream.h
1>Note: including file:           c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\encodings.h
1>Note: including file:          c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\encodedstream.h
1>Note: including file:           c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\stream.h
1>Note: including file:           c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\memorystream.h
1>Note: including file:            c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\stream.h
1>Note: including file:          c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\internal/meta.h
1>Note: including file:          c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\internal/stack.h
1>Note: including file:           c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\internal\swap.h
1>Note: including file:          c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\internal/strtod.h
1>Note: including file:           c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\internal\ieee754.h
1>Note: including file:           c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\internal\biginteger.h
1>Note: including file:           c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\internal\diyfp.h
1>Note: including file:           c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\internal\pow10.h
1>Note: including file:          c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\error/error.h
1>Note: including file:         c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\internal/strfunc.h
1>Note: including file:          c:\dev\couloir\3plibs\vcpkg\installed\win32\include\rapidjson\internal\../stream.h
1>C:\Dev\Couloir\3pLibs\vcpkg\installed\Win32\include\rapidjson/document.h(964): warning C4003: not enough actual parameters for macro 'min'

@chwarr
Copy link
Contributor

chwarr commented Feb 20, 2018

I fixed this in commit 6e38649. If you can't switch to that or newer, try defining NOMINMAX on the command line (e.g., passing /DNOMINMAX to cl.exe or whatever you need to do for this environment you're in) so that nothing gets included without it defined.

@VikingExplorer
Copy link
Author

I finally got a chance to check which version I'm using. I'm getting this from vcpkg. From rapidjson.h:

#define RAPIDJSON_MAJOR_VERSION 1
#define RAPIDJSON_MINOR_VERSION 1
#define RAPIDJSON_PATCH_VERSION 0

which according to https://repology.org/metapackage/rapidjson/versions and this GitHub repo, is the latest released version.

I tried using /DNOMINMAX on the CL command line, but got:

1>c:\program files (x86)\microsoft sdks\windows\v7.1a\include\gdiplustypes.h(470): error C3861: 'min': identifier not found

The 1.1 version was released on Aug 25, 2016. Seems like it's been a while. Any idea of when the next version will be released?

@ras0219-msft
Copy link

(Also, if you'd like to temporarily use the latest master until there's an update, you can use vcpkg remove rapidjson:win32; vcpkg install rapidjson:win32 --head)

@VikingExplorer
Copy link
Author

When I tried switching to the 64 bit configuration, I started getting this error:

C2039 'GetObjectW': is not a member of 'rapidjson::GenericValue<rapidjson::UTF8,rapidjson::MemoryPoolAllocatorrapidjson::CrtAllocator>' CouloirAFX-WinW c:\dev\couloir\generic\afx\mvc\couloirafx\modeldata.cpp 112

void ModelData::process( XmlNode parent, GenericValue<UTF8<>>* json )
{
   String Value;

   if( json->IsObject() )
   {
      process( parent, json->GetObjectW() );       // line 112
   }

I tried getting the latest, as Robert suggested, but it was the same. Not sure how I ended up with this crazy looking code, but it builds and runs correctly in 32 bit mode.

Thoughts?

@chwarr
Copy link
Contributor

chwarr commented Mar 29, 2018

I looked through all the revisions of document.h in the repository from 8022a5f all the way back to 8f8e905 (the initial import from the SVN repository). None of these revisions to document.h have a GetObjectW member function.

This looks like a collision between the Win32 GDI function GetObject and its Unicode and ANSI names, which are implemented as macros based on, IIRC, the UNICODE macro.

Explicit use of GetObjectW appears to be working around a name conflict here.

My guess is that the 32- and 64-bit configurations have differing values for UNICODE. Though, there could be many reasons for this, like include file ordering.

I bet you can work around this issue by temporarily undef'ing the GetObject macro. Inspiration for this is from a StackOverflow answer:

void ModelData::process( XmlNode parent, GenericValue<UTF8<>>* json )
{
   String Value;

   if( json->IsObject() )
   {
      #pragma push_macro("GetObject")
      #undef GetObject
      process( parent, json->GetObject() );
      #pragma pop_macro("GetObject")
   }
   ...

Another option: if the compilation unit using GetObject doesn't need anything from the Windows SDK, omit including windows.h.

For the /DNOMINMAX breaking things, it looks like a GDI+ member function implementation uses min/max. If you don't need GDI+ in the same compilation unit that's using RapidJSON, consider not including its headers.

Failing all of that, you should be able to work around this by using the same push_macro trick that the old implementation had around your include of rapidjson.h. Though, omit the NOMINMAX check. Something like this:

#pragma push_macro("min")
#pragma push_macro("max")
#undef min
#undef max
#include <rapidjson/rapidjson.h> // or whatever headers
#pragma pop_macro("min")
#pragma pop_macro("max")

Of course, with all of these approaches, you'll need to make sure you define the relevant macros & such before the header gets included for the first time. Watch out for precompiled headers and similar patterns. cl.exe /showIncludes may help here.

@VikingExplorer
Copy link
Author

I truly appreciate your help. My Saga:

  1. Change UNICODE to be consistent
    From: WEB;SPDLOG_FMT_EXTERNAL;_NO_ASYNCRTIMP;_NO_PPLXIMP;PUGIXML_WCHAR_MODE;PUGIXML_NO_EXCEPTIONS;SPDLOG_WCHAR_FILENAMES;BOOST_LIB_DIAGNOSTIC;_WINDOWS;_USRDLL;_DEBUG;

To: WEB;SPDLOG_FMT_EXTERNAL;_NO_ASYNCRTIMP;_NO_PPLXIMP;PUGIXML_WCHAR_MODE;PUGIXML_NO_EXCEPTIONS;SPDLOG_WCHAR_FILENAMES;BOOST_LIB_DIAGNOSTIC;_WINDOWS;_UNICODE;UNICODE;_USRDLL;_DEBUG;

44>c:\dev\couloir\3plibs\vcpkg\installed\x64\include\rapidjson\document.h(964): error C2589: '(': illegal token on right side of '::'
return (d >= static_cast(std::numeric_limits<int64_t>::min()))

  1. Add push_macro around GetObject call
void ModelData::process( XmlNode parent, GenericValue<UTF8<>>* json )
{
   String Value;

   if( json->IsObject() )
   {
#pragma push_macro("GetObject")
#undef GetObject
      process( parent, json->GetObject() );        // <-- line 114  process( parent, json->GetObjectW() );
#pragma pop_macro("GetObject")
   }
}

1>c:\dev\couloir\generic\afx\mvc\couloirafx\modeldata.cpp(114): error C2039: 'GetObject': is not a member of 'rapidjson::GenericValue<rapidjson::UTF8,rapidjson::MemoryPoolAllocatorrapidjson::CrtAllocator>'
1>c:\dev\couloir\3plibs\vcpkg\installed\x64\include\rapidjson\document.h(2010): note: see declaration of 'rapidjson::GenericValue<rapidjson::UTF8,rapidjson::MemoryPoolAllocatorrapidjson::CrtAllocator>'
1>c:\dev\couloir\generic\afx\mvc\couloirafx\modeldata.cpp(114): error C2661: 'Couloir::ModelData::process': no overloaded function takes 1 arguments

  1. surround header file include with pragma
#pragma push_macro("min")
#pragma push_macro("max")
#undef min
#undef max
#define RAPIDJSON_HAS_STDSTRING 1
#include "rapidjson/document.h"
#pragma pop_macro("min")
#pragma pop_macro("max")

Same error on line 114

  1. rapidjson is included in only 2 files. Replaced the other one like UTF-8/16 string validation #3

Same error on line 114

  1. commented out #define RAPIDJSON_HAS_STDSTRING 1

Same error on line 114

  1. Replaced GetObject with get_object in rapidjson\document.h (only 4 teeny tiny places)

Peace broke out. Lions laid down with the lamb. Hatfields & McCoys became friends. Sun rose on a beautiful day, birds chirping, children played in complete safety.

No one knew that the people responsible for including c preprocessor MACROS in C++, but insisting with an evil smirk that they should not obey NameSpace rules, were taken out and shot. No trial, no defense attorneys to argue about the intricacies of C++ history or cpp standardese. They were just shot & their bodies sunk to the bottom of the Marianas trench.

And the world lived happily ever after...

@VikingExplorer
Copy link
Author

Deliberately choosing to name functions the same as ubiquitous MACRO names is not helpful.

Similarly, when std::committee decided on std::max, MS should have done a global search & replace from MAX to MS_MAXIMUM...

@VikingExplorer
Copy link
Author

Updated to latest in vcpkg & again changed:

FROM: static ObjectType Get(ValueType& v) { return v.GetObject(); }
TO: static ObjectType Get(ValueType& v) { return v.get_object(); }

Because life is too short to be stubborn about using a name that causes other people lots and lots of problems.

I'm a rude person, but I don't go out of my way to cause complete strangers pain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants