fix(compiler): alias C++ union case types in metadata macros#3814
Open
BaldDemian wants to merge 1 commit into
Open
fix(compiler): alias C++ union case types in metadata macros#3814BaldDemian wants to merge 1 commit into
BaldDemian wants to merge 1 commit into
Conversation
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.
Why?
The C++ compiler can emit union metadata macros that fail to compile when a union case payload type contains a C++ template comma, e.g.
std::unordered_map<K, V>.There are two affected paths:
FORY_UNION(...)for unions with at most 16 cases.FORY_UNION_IDS(...)plusFORY_UNION_CASE(...)for larger unions.Example IDL:
Generated C++ code:
Observed compile error:
Root cause:
The generator passed raw C++ type names into
FORY_UNIONcase tuples.FORY_UNIONrendered each case as(case_name, case_type, meta):fory/compiler/fory_compiler/generators/cpp.py
Lines 1203 to 1221 in 23aa3f9
This is unsafe for template types such as
std::unordered_map<std::string, std::any>.C++ macro preprocessing happens before the compiler understands that the comma inside
<K, V>belongs to a template argument list. The preprocessor treats that comma as a macro argument separator.FORY_UNIONexpects each case tuple to have either two entries(name, meta)or three entries(name, type, meta):fory/cpp/fory/serialization/union_serializer.h
Lines 1179 to 1207 in 23aa3f9
When the generated tuple contains:
the preprocessor sees more entries than intended because of the comma in
std::unordered_map<std::string, std::any>.That breaks tuple-size selection and causes invalid macro names such as
FORY_UNION_CASE_META_foryorFORY_UNION_CASE_TYPE_fory.What does this PR do?
Fix this by generating a union-scoped type alias for case payload types whose rendered C++ type
contains a comma, and use that alias in
FORY_UNIONandFORY_UNION_CASEmetadata.The generated alias name is derived from the union case name using the pattern:
ForyCase+ PascalCase(case_name) +Type.e.g.
The macro now receives
SmallChoice::ForyCaseByNameType, which contains no template comma and can be parsed safely by the preprocessor.Note: I will handle naming collisions universally in a new PR.
Related issues
N/A.
AI Contribution Checklist
noDoes this PR introduce any user-facing change?
N/A.
Benchmark
N/A.