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

warning C4002 on windows with msvs2022 #352

Closed
zhmt opened this issue Mar 6, 2022 · 6 comments · Fixed by microsoft/vcpkg#24820
Closed

warning C4002 on windows with msvs2022 #352

zhmt opened this issue Mar 6, 2022 · 6 comments · Fixed by microsoft/vcpkg#24820

Comments

@zhmt
Copy link
Contributor

zhmt commented Mar 6, 2022

Describe the bug
my code

JSONCONS_ALL_MEMBER_TRAITS(ezg::g4::gamedesign::GameSettingMap, EquipsMap, EquipsList, .... 50+)
  1. llvm refused to compile, it produced an error:
    too many arguments for function-like macro invocation

  2. msvs give a warning:
    warning C4002: too many arguments for function-like macro invocation : JSONCONS_EXPAND_CALL5.

I checked the definition of JSONCONS_EXPAND_CALL5 (in json_traits_macros.hpp ), and I got one line seems like an error:
JSONCONS_EXPAND_CALL5(Call, P1, P2, P3, P4, 47, )
I changed it to (delete the last ","):
JSONCONS_EXPAND_CALL5(Call, P1, P2, P3, P4, 47)

everything seems ok. Both msvc and llvm compiled silently and sucssesfully.

@zhmt zhmt added the Bug label Mar 6, 2022
@danielaparker
Copy link
Owner

You're right, I guess we haven't had a user with 47+ parameters before, nor a test case with that many parameters. Thanks for reporting this. Would you consider submitting a pull request with this change? (makes it easier to credit the contribution)

@zhmt
Copy link
Contributor Author

zhmt commented Mar 8, 2022

I have submmited a pull request. Happy to contribute.

@zhmt zhmt closed this as completed Mar 8, 2022
@danielaparker
Copy link
Owner

We should keep this open, until we have a release with the fix.

Your PR is merged with master. It also looks like you've reached the limit of the number of parameters supported (50) so I've increased that limit to 70. I've also added a test case that covers the case of 70 parameters.

@zhmt
Copy link
Contributor Author

zhmt commented Mar 8, 2022

Nice work!! I have extended the limit to 54 myself.
I should download the newest code from master now.
Thank you!

@zhmt
Copy link
Contributor Author

zhmt commented Mar 8, 2022

I have tested . Everything works well.

@danielaparker
Copy link
Owner

Fixed in 0.168.4

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

Successfully merging a pull request may close this issue.

2 participants