Skip to content

feat(compiler): handle nested container ref pointer options in C++ compiler correctly#3735

Merged
chaokunyang merged 1 commit into
apache:mainfrom
BaldDemian:cpp-nested-container
Jun 9, 2026
Merged

feat(compiler): handle nested container ref pointer options in C++ compiler correctly#3735
chaokunyang merged 1 commit into
apache:mainfrom
BaldDemian:cpp-nested-container

Conversation

@BaldDemian

@BaldDemian BaldDemian commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Why?

The C++ compiler should correctly apply ref pointer options for nested container element/value references.

Additional note:

When submitting PR 3731, I did not notice that Fory currently only supports nested collection types in C++:

Nested collection support is target-capability based. The C++ generator accepts
nested collection specs such as `list<list<...>>`, `list<map<...>>`, and
`map<..., list<...>>`; targets that have not implemented nested field specs
continue to reject them. Use a message wrapper when you need portable schemas
across all targets.

However, since that PR merely added a piece of defensive programming logic, it may not need to be reverted.

Similar issues also exist in the Python and JavaScript compilers. However, since they do not support nested container types, I have not fixed them for now.

What does this PR do?

  • Handle nested container ref pointer options correctly.
  • Add corresponding testcase.

Related issues

N/A.

AI Contribution Checklist

  • Substantial AI assistance was used in this PR: no
  • If yes, I included a completed AI Contribution Checklist in this PR description and the required AI Usage Disclosure.
  • If yes, my PR description includes the required ai_review summary and screenshot evidence of the final clean AI review results from both fresh reviewers on the current PR diff or current HEAD after the latest code changes.

Does this PR introduce any user-facing change?

N/A.

Benchmark

N/A.

in cpp_output
)
assert (
"std::map<std::string, "

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

C++ should use unorderd_map, could you open a new pr for this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No problem, working on it

@BaldDemian BaldDemian force-pushed the cpp-nested-container branch from fc1ace7 to c1f2c96 Compare June 9, 2026 08:21
@chaokunyang chaokunyang merged commit 2ec5625 into apache:main Jun 9, 2026
46 checks passed
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.

2 participants