Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DrgnParser: Handle enum values in template params #179

Merged
merged 1 commit into from
Jun 26, 2023

Conversation

ajor
Copy link
Contributor

@ajor ajor commented Jun 23, 2023

We want to use the fully qualified name for scoped enums to keep the C++ compiler happy. When a parameter expects an enum value, we must supply an enum value and not its underlying integer value.

Before:

isset_bitset<1, 0>

After:

isset_bitset<1, apache::thrift::detail::IssetBitsetOption::Unpacked>

@ajor ajor force-pushed the container-enums branch 2 times, most recently from dd7cbc5 to a504139 Compare June 26, 2023 12:38
@ajor ajor changed the title DrgnParser: Handle enum values in container template params DrgnParser: Handle enum values in template params Jun 26, 2023
@ajor ajor force-pushed the container-enums branch from a504139 to 57c907b Compare June 26, 2023 12:40
@ajor ajor marked this pull request as ready for review June 26, 2023 12:42
@ajor ajor force-pushed the container-enums branch from 57c907b to cae7287 Compare June 26, 2023 13:47
We want to use the fully qualified name for scoped enums to keep the C++
compiler happy. When a parameter expects an enum value, we must supply
an enum value and not its underlying integer value.

Before:
  isset_bitset<1, 0>

After:
  isset_bitset<1, apache::thrift::detail::IssetBitsetOption::Unpacked>
@ajor ajor force-pushed the container-enums branch from cae7287 to dd8cc11 Compare June 26, 2023 13:57
@ajor ajor merged commit 4bfa932 into facebookexperimental:main Jun 26, 2023
@ajor ajor deleted the container-enums branch June 26, 2023 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants