fix(cpp): support std::optional in Utf16_Utf8 string conversion#2981
Merged
Conversation
Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
With
--wstring use-wstring --no-boost(the C++17std::optionalcode path) and an optional string property, the generated C++ fails to compile:Root cause
The generated
Utf16_Utf8helper class (emitted inemitHelperFunctions,packages/quicktype-core/src/language/CPlusPlus/CPlusPlusRenderer.ts) only hadconvertoverloads forstd::shared_ptr,std::vector, andstd::map. There was no overload forstd::optional/boost::optional, so calls on optional string properties fell through to the generic fallback overload, which just returns the value unchanged — producing a type mismatch the compiler rejects.Fix
Added a recursive
convertoverload for the configured optional type (std::optionalorboost::optional, matching the--wstring/--no-boostrenderer options) toUtf16_Utf8, mirroring the existingshared_ptroverload. It's only emitted when the generated code actually has optional properties (haveOptionalProperties), matching how other optional-only helper code is conditionally emitted elsewhere in this renderer.Test coverage
Pinned the existing C++
wstring: use-wstringquicktest renderer option totest/inputs/json/priority/bug2521.json, which contains an optional string property (foopresent on only one array element). This exercises theUtf16_Utf8<std::optional<...>>::convertpath at compile time for theuse-wstringcode path, previously it ran against unpinned default quicktest inputs that didn't hit this combination.Verification
masterusing the repro from the issue/triage comment (schema2.json+--wstring use-wstring --no-boost ...): g++ 13.3 -std=c++17 fails with the exact error quoted above.g++ -std=c++17 -I. -c main2.cpp— compiles cleanly.bug2521.jsonwith--wstring use-wstring, built it against the realtest/fixtures/cplusplusdriver (main.cpp/json.hpp) withg++ -O0 -std=c++17— compiles and runs successfully (previously this exact combination is what the pinned quicktest now exercises in CI).npm run buildpasses.npm run test:unit— 163/163 tests pass.QUICKTEST=true FIXTURE=cplusplus script/testlocally; it hit an unrelated, pre-existing environment gap (this sandbox has nolibboost-dev, so the pre-existing["pokedex.json", { boost: "true" }]quicktest fails withboost/optional.hpp: No such file or directorybefore reaching later quicktests) — this is unrelated to this change and will run fully in CI, which has boost available.Fixes #2383
🤖 Generated with Claude Code