Skip to content

fix: nlohmann::adl_serializer for std::optional#9147

Merged
Ericson2314 merged 9 commits intoNixOS:masterfrom
alex-ameen:patch-1
Nov 30, 2023
Merged

fix: nlohmann::adl_serializer for std::optional#9147
Ericson2314 merged 9 commits intoNixOS:masterfrom
alex-ameen:patch-1

Conversation

@alex-ameen
Copy link
Contributor

Motivation

This allows templates such as NLOHMANN_DEFINE_TYPE_* templates and other generators with things like std::vector<std::optional<T>>.

Context

This template definition conflicts with my own definition used in a nix plugin.
I can use y'alls copy but since I nest std::optional inside of other STL containers.

Priorities

Add 👍 to pull requests you find important.

This allows templates such as `NLOHMANN_DEFINE_TYPE_*` templates and other generators with things like `std::vector<std::optional<T>>`.
@alex-ameen alex-ameen requested a review from edolstra as a code owner October 12, 2023 16:28
@Ericson2314
Copy link
Member

@aakropotkin Can you test that your way works for deserializing types that do not have a default constructor? It looks like it would, but nlohmann/json is very finicky about this (it has some bad old stuff for backwards compat that instead mutates a default-constructed value).

@alex-ameen
Copy link
Contributor Author

@aakropotkin Can you test that your way works for deserializing types that do not have a default constructor? It looks like it would, but nlohmann/json is very finicky about this (it has some bad old stuff for backwards compat that instead mutates a default-constructed value).

Yeah I have a test case in my repo I can migrate over here.

@Ericson2314 Ericson2314 marked this pull request as draft October 12, 2023 18:27
@Ericson2314
Copy link
Member

Great, feel free to undraft once you have the tests.

@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Oct 21, 2023
@alex-ameen alex-ameen marked this pull request as ready for review October 21, 2023 14:45
@alex-ameen
Copy link
Contributor Author

Aight - we ready

@tomberek tomberek assigned Ericson2314 and unassigned Ericson2314 Nov 29, 2023
@tomberek tomberek requested a review from Ericson2314 November 29, 2023 18:04
@Ericson2314 Ericson2314 changed the title fix: adl_serializer for std::optional fix: nlohmann::adl_serializer for std::optional Nov 30, 2023
@Ericson2314 Ericson2314 enabled auto-merge (squash) November 30, 2023 00:54
@Ericson2314 Ericson2314 disabled auto-merge November 30, 2023 00:56
@Ericson2314 Ericson2314 disabled auto-merge November 30, 2023 00:56
@Ericson2314 Ericson2314 enabled auto-merge (squash) November 30, 2023 00:56
@Ericson2314 Ericson2314 merged commit 02bd821 into NixOS:master Nov 30, 2023
@alex-ameen alex-ameen deleted the patch-1 branch November 30, 2023 04:02
tebowy pushed a commit to tebowy/nix that referenced this pull request Jul 11, 2024
This allows templates such as `NLOHMANN_DEFINE_TYPE_*` templates and other generators with things like `std::vector<std::optional<T>>`.

Co-authored-by: John Ericson <John.Ericson@Obsidian.Systems>
(cherry picked from commit 02bd821)
Change-Id: I8b0ebcf2af4226610dadd565962f2d2327415a03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

with-tests Issues related to testing. PRs with tests have some priority

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants