Skip to content

fix(compiler): alias C++ union case types in metadata macros#3814

Open
BaldDemian wants to merge 1 commit into
apache:mainfrom
BaldDemian:cpp-macro
Open

fix(compiler): alias C++ union case types in metadata macros#3814
BaldDemian wants to merge 1 commit into
apache:mainfrom
BaldDemian:cpp-macro

Conversation

@BaldDemian

Copy link
Copy Markdown
Contributor

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(...) plus FORY_UNION_CASE(...) for larger unions.

Example IDL:

package probe.cpp_macro_fory_union;

union SmallChoice {
  map<string, any> by_name = 1;
  string name = 2;
}

Generated C++ code:

FORY_UNION(probe::cpp_macro_fory_union::SmallChoice,
  (by_name, std::unordered_map<std::string, std::any>, fory::F(1).map(fory::T::string(), fory::FieldNodeSpec{})),
  (name, std::string, fory::F(2))
);

Observed compile error:

/home/hpy/fory/cpp/fory/serialization/union_serializer.h:1203:18: error: 'FORY_UNION_CASE_META_fory' has not been declared
/tmp/fory_cpp_macro_pr_wql7zo9p/generated/probe_cpp_macro_fory_union.h:140:4: error: 'by_name' was not declared in this scope
/tmp/fory_cpp_macro_pr_wql7zo9p/generated/probe_cpp_macro_fory_union.h:140:54: error: expected primary-expression before ',' token
/home/hpy/fory/cpp/fory/serialization/union_serializer.h:1192:18: error: 'FORY_UNION_CASE_TYPE_fory' was not declared in this scope; did you mean 'FORY_UNION_CASE_TYPE_2'?

Root cause:

The generator passed raw C++ type names into FORY_UNION case tuples.
FORY_UNION rendered each case as (case_name, case_type, meta):

if len(union.fields) <= 16:
union_type = self.get_namespaced_type_name(union.name, parent_stack)
lines.append(f"FORY_UNION({union_type},")
for index, field in enumerate(union.fields):
case_type = self.generate_namespaced_type(
field.field_type,
False,
field.ref,
field.element_optional,
field.element_ref,
False,
False,
parent_stack,
)
case_ctor = self.to_snake_case(field.name)
meta = self.get_union_field_meta(field)
suffix = "," if index + 1 < len(union.fields) else ""
lines.append(f" ({case_ctor}, {case_type}, {meta}){suffix}")
lines.append(");")

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_UNION expects each case tuple to have either two entries (name, meta) or three entries (name, type, meta):

#define FORY_UNION_TUPLE_SIZE(tuple) FORY_UNION_TUPLE_SIZE_IMPL tuple
#define FORY_UNION_TUPLE_SIZE_IMPL(...) \
FORY_UNION_TUPLE_SIZE_SELECT(__VA_ARGS__, 3, 2, 1, 0)
#define FORY_UNION_TUPLE_SIZE_SELECT(_1, _2, _3, N, ...) N
#define FORY_UNION_CASE_NAME(tuple) \
FORY_PP_CONCAT(FORY_UNION_CASE_NAME_, FORY_UNION_TUPLE_SIZE(tuple))(tuple)
#define FORY_UNION_CASE_NAME_2(tuple) FORY_UNION_CASE_NAME_2_IMPL tuple
#define FORY_UNION_CASE_NAME_2_IMPL(name, meta) name
#define FORY_UNION_CASE_NAME_3(tuple) FORY_UNION_CASE_NAME_3_IMPL tuple
#define FORY_UNION_CASE_NAME_3_IMPL(name, type, meta) name
#define FORY_UNION_CASE_TYPE(Type, tuple) \
FORY_PP_CONCAT(FORY_UNION_CASE_TYPE_, FORY_UNION_TUPLE_SIZE(tuple)) \
(Type, tuple)
#define FORY_UNION_CASE_TYPE_2(Type, tuple) \
FORY_UNION_CASE_TYPE_2_IMPL(Type, tuple)
#define FORY_UNION_CASE_TYPE_2_IMPL(Type, tuple) \
typename ::fory::serialization::detail::UnionFactoryArg< \
decltype(&Type::FORY_UNION_CASE_NAME(tuple))>::type
#define FORY_UNION_CASE_TYPE_3(Type, tuple) FORY_UNION_CASE_TYPE_3_IMPL tuple
#define FORY_UNION_CASE_TYPE_3_IMPL(name, type, meta) type
#define FORY_UNION_CASE_META(tuple) \
FORY_PP_CONCAT(FORY_UNION_CASE_META_, FORY_UNION_TUPLE_SIZE(tuple))(tuple)
#define FORY_UNION_CASE_META_2(tuple) FORY_UNION_CASE_META_2_IMPL tuple
#define FORY_UNION_CASE_META_2_IMPL(name, meta) meta
#define FORY_UNION_CASE_META_3(tuple) FORY_UNION_CASE_META_3_IMPL tuple
#define FORY_UNION_CASE_META_3_IMPL(name, type, meta) meta

When the generated tuple contains:

(by_name, std::unordered_map<std::string, std::any>, meta)

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_fory or FORY_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_UNION and FORY_UNION_CASE metadata.

The generated alias name is derived from the union case name using the pattern:

ForyCase + PascalCase(case_name) + Type.

e.g.

class SmallChoice final {
 public:
  using ForyCaseByNameType = std::unordered_map<std::string, std::any>;
  ...
};

FORY_UNION(probe::cpp_macro_fory_union::SmallChoice,
  (by_name, probe::cpp_macro_fory_union::SmallChoice::ForyCaseByNameType, fory::F(1).map(...)),
  (name, std::string, fory::F(2))
);

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

  • Substantial AI assistance was used in this PR: no

Does this PR introduce any user-facing change?

N/A.

Benchmark

N/A.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant