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

Serialisation macros: increase upper bound on number of member variables #2287

Merged
merged 28 commits into from
Jul 20, 2020
Merged

Serialisation macros: increase upper bound on number of member variables #2287

merged 28 commits into from
Jul 20, 2020

Conversation

pfeatherstone
Copy link
Contributor

This pull request contains a bit of rough and ready code to generate the serialisation/deserialisation macros. The upper bound on number of member variables is increased to 64. You can increase the upper bound arbitrarily using the aforementioned code.

nlohmann and others added 9 commits July 17, 2020 20:41
…ation/deserialisation macros for custom types

		- netbeans project configurations
…ation/deserialisation macros for custom types

		- updated .gitignore file to ignore "dist" and "build" directories in netbeans project
…ation/deserialisation macros for custom types

		- used code to increase the upper bound on number of member variables to 64
…ation/deserialisation macros for custom types

		- used code to increase the upper bound on number of member variables to 64
…ation/deserialisation macros for custom types

		- updated .gitignore file to ignore private netbeans project configurations
@pfeatherstone
Copy link
Contributor Author

This solves #2267

@coveralls
Copy link

coveralls commented Jul 18, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 35b899e on pfeatherstone:develop into 43ab8a2 on nlohmann:develop.

@pfeatherstone
Copy link
Contributor Author

@nlohmann There is one test that is failing in continuous-integration, specifically with configuration "Xcode: xcode10 C++ AMD64". All other tests are fine. It's failing on 'download_test_data'. Any ideas?

@nlohmann
Copy link
Owner

I restarted the job.

@pfeatherstone
Copy link
Contributor Author

I restarted the job.

Thanks. Ready for review

add inline specifier for detail::combine
Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think having the tools/NlohmannMacroBuilder/main.cpp file is sufficient, and it should be moved into third_party/macro_builder/main.cpp to avoid creating a new directory. The Makefiles etc. are overhead as the tool will (if ever) be called by a maintainer if we ever find 64 arguments to be insufficient.

tools/NlohmannMacroBuilder/main.cpp Outdated Show resolved Hide resolved
nlohmann and others added 11 commits July 19, 2020 18:09
Add static assertion for missing binary function in SAX interface
…ation/deserialisation macros for custom types

		- netbeans project configurations
…ation/deserialisation macros for custom types

		- updated .gitignore file to ignore "dist" and "build" directories in netbeans project
…ation/deserialisation macros for custom types

		- used code to increase the upper bound on number of member variables to 64
…ation/deserialisation macros for custom types

		- used code to increase the upper bound on number of member variables to 64
…ation/deserialisation macros for custom types

		- updated .gitignore file to ignore private netbeans project configurations
…Current upper bound on number of member variables is set to 64
…ation macro building code, can revert the changes made to the .gitignore file
test/src/unit-udt_macro.cpp Outdated Show resolved Hide resolved
@pfeatherstone
Copy link
Contributor Author

Ok, the combination of valgrind with some of the older compilers doesn't like using std::tie and std::tuple to implement the operator== overload in the unit test. It complains with error Conditional jump or move depends on uninitialised value(s)

@pfeatherstone
Copy link
Contributor Author

Rewrote operator==. Doesn't use std::tie. Hopefully, valgrind and clang will no longer complain about uninitialised variables

@pfeatherstone
Copy link
Contributor Author

This is still not ready as I have to rebase the fork. This will require a battle with git which i don't have time for today.

@nlohmann
Copy link
Owner

No need to rebase as long as there are no merge conflicts.

… up. std::tie probably wasn't the problem initially
@pfeatherstone
Copy link
Contributor Author

@nlohmann Seems to be ready

Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@nlohmann nlohmann merged commit 8344857 into nlohmann:develop Jul 20, 2020
@nlohmann nlohmann self-assigned this Jul 20, 2020
@nlohmann nlohmann added this to the Release 3.8.1 milestone Jul 20, 2020
@nlohmann
Copy link
Owner

Thanks!

@nlohmann
Copy link
Owner


🔖 Release item

This issue/PR will be part of the next release of the library. This template helps preparing the release notes.

Type

  • ✨ New Feature
  • 🐛 Bug Fix
  • ⚡️ Improvement
  • 🔨 Further Change
  • 🔥 Deprecated function

Description


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