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

Custom types : MACRO expansion bug #2267

Closed
pfeatherstone opened this issue Jul 14, 2020 · 5 comments · Fixed by #2287
Closed

Custom types : MACRO expansion bug #2267

pfeatherstone opened this issue Jul 14, 2020 · 5 comments · Fixed by #2287
Assignees
Labels
confirmed kind: bug release item: ⚡ improvement solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Milestone

Comments

@pfeatherstone
Copy link
Contributor

In the following code:

#define NLOHMANN_JSON_EXPAND( x ) x
#define NLOHMANN_JSON_GET_MACRO(_1,_2,_3,_4,_5,_6, _7, _8, _9, _10, _11, NAME,...) NAME

#define NLOHMANN_JSON_PASTE(...) NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_GET_MACRO(__VA_ARGS__, NLOHMANN_JSON_PASTE11, \
        NLOHMANN_JSON_PASTE10, NLOHMANN_JSON_PASTE9, NLOHMANN_JSON_PASTE8, NLOHMANN_JSON_PASTE7, \
        NLOHMANN_JSON_PASTE6, NLOHMANN_JSON_PASTE5, NLOHMANN_JSON_PASTE4, NLOHMANN_JSON_PASTE3, \
        NLOHMANN_JSON_PASTE2, NLOHMANN_JSON_PASTE1)(__VA_ARGS__))
#define NLOHMANN_JSON_PASTE2(func,  v1)                                      func(v1)
#define NLOHMANN_JSON_PASTE3(func,  v1, v2)                                  NLOHMANN_JSON_PASTE2(func, v1) NLOHMANN_JSON_PASTE2(func, v2)
#define NLOHMANN_JSON_PASTE4(func,  v1, v2, v3)                              NLOHMANN_JSON_PASTE2(func, v1) NLOHMANN_JSON_PASTE3(func, v2, v3)
#define NLOHMANN_JSON_PASTE5(func,  v1, v2, v3, v4)                          NLOHMANN_JSON_PASTE2(func, v1) NLOHMANN_JSON_PASTE4(func, v2, v3, v4)
#define NLOHMANN_JSON_PASTE6(func,  v1, v2, v3, v4, v5)                      NLOHMANN_JSON_PASTE2(func, v1) NLOHMANN_JSON_PASTE5(func, v2, v3, v4, v5)
#define NLOHMANN_JSON_PASTE7(func,  v1, v2, v3, v4, v5, v6)                  NLOHMANN_JSON_PASTE2(func, v1) NLOHMANN_JSON_PASTE6(func, v2, v3, v4, v5, v6)
#define NLOHMANN_JSON_PASTE8(func,  v1, v2, v3, v4, v5, v6, v7)              NLOHMANN_JSON_PASTE2(func, v1) NLOHMANN_JSON_PASTE7(func, v2, v3, v4, v5, v6, v7)
#define NLOHMANN_JSON_PASTE9(func,  v1, v2, v3, v4, v5, v6, v7, v8)          NLOHMANN_JSON_PASTE2(func, v1) NLOHMANN_JSON_PASTE8(func, v2, v3, v4, v5, v6, v7, v8)
#define NLOHMANN_JSON_PASTE10(func, v1, v2, v3, v4, v5, v6, v7, v8, v9)      NLOHMANN_JSON_PASTE2(func, v1) NLOHMANN_JSON_PASTE8(func, v2, v3, v4, v5, v6, v7, v8, v9)
#define NLOHMANN_JSON_PASTE11(func, v1, v2, v3, v4, v5, v6, v7, v8, v9, v10) NLOHMANN_JSON_PASTE2(func, v1) NLOHMANN_JSON_PASTE8(func, v2, v3, v4, v5, v6, v7, v8, v9, v10)

NLOHMANN_JSON_PASTE10 should use NLOHMANN_JSON_PASTE9 not NLOHMANN_JSON_PASTE8.
Similar issue with NLOHMANN_JSON_PASTE11

Very quick fix.

On another note, unfortunately I think we should have unrolled the macro manually up to 32 or 64 arguments. i think 11 is too small (it is for me at least).
@nlohmann What do you think ?

@nlohmann
Copy link
Owner

Thanks for noting! Good catch!

PRs for both issues (the quick fix and an extension to more arguments) are welcome!

@pfeatherstone
Copy link
Contributor Author

Yep will do. Probs this weekend as work won't permit.

@nlohmann
Copy link
Owner

Cool!

@pfeatherstone
Copy link
Contributor Author

@nlohmann I noticed in the documentation you use https://github.com/edlund/amalgamate to create the single header. What is the exact command you use? Maybe the amalgamation should be done at 'build' time (somewhere as part of the CMake build if that's possible) and not commit the single header to git.

@nlohmann
Copy link
Owner

The amalgamation is created with make amalgamate. We decided to put the single header into the repository as people would be confused if they can only find that header in the release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed kind: bug release item: ⚡ improvement solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants