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

Core: Natively convert enum/BitField with Variant #101003

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Repiteo
Copy link
Contributor

@Repiteo Repiteo commented Jan 1, 2025

A big reason the above issue happened in the first place is because SO MUCH of the enum binding logic happens after the fact, when the overwhelming majority could be determined via <type_traits> magic. The only exception seems to be the qualified name, which this PR keeps as an explicitly provided variable; absolutely everything else is now handled natively. While this could probably be implemented more cleanly via C++20 concepts, the entire bindsystem would benefit from them; so for now, SFINAE1 gets the job done.

Footnotes

  1. https://en.cppreference.com/w/cpp/language/sfinae

@Repiteo Repiteo added this to the 4.4 milestone Jan 1, 2025
@Repiteo Repiteo requested a review from a team as a code owner January 1, 2025 18:38
core/variant/type_info.h Outdated Show resolved Hide resolved
@Repiteo Repiteo force-pushed the core/variant-native-enum branch 2 times, most recently from 625458d to 7a5497f Compare January 5, 2025 00:25
@Repiteo Repiteo requested review from a team as code owners January 5, 2025 00:25
@Repiteo Repiteo removed request for a team January 5, 2025 00:27
@Repiteo Repiteo removed request for a team January 5, 2025 00:27
@Repiteo

This comment was marked as outdated.

@Repiteo Repiteo force-pushed the core/variant-native-enum branch from 7a5497f to 304aecb Compare January 5, 2025 00:34
Copy link
Contributor

@Ivorforce Ivorforce left a comment

Choose a reason for hiding this comment

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

This is looking great! I think the LOC change alone shows the reduction in complexity this PR brings. Looking forward to when this gets merged 😁

core/variant/binder_common.h Outdated Show resolved Hide resolved
core/variant/type_info.h Outdated Show resolved Hide resolved
core/variant/type_info.h Outdated Show resolved Hide resolved
core/variant/variant.h Show resolved Hide resolved
core/variant/variant.h Show resolved Hide resolved
core/variant/variant_internal.h Outdated Show resolved Hide resolved
core/variant/variant_internal.h Outdated Show resolved Hide resolved
@Repiteo Repiteo force-pushed the core/variant-native-enum branch from 304aecb to 99a936f Compare January 5, 2025 15:34
@Repiteo
Copy link
Contributor Author

Repiteo commented Jan 5, 2025

Gave it some more thought, and I'm gonna walk back on trying to handle qualified names automatically. While I'm still largely confident of that implementation, it wasn't air-tight; I'd rather not have the rest of the PR be potentially bogged down by it. I'll revisit that idea in the future, once I have an unambiguously universal solution.

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.

Cannot declare virtual method with enum from own class
2 participants