Skip to content

De-duplicate a lot of variant_internal.h #105246

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

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

Conversation

Ivorforce
Copy link
Member

@Ivorforce Ivorforce commented Apr 10, 2025

variant_internal.h (and similar headers) are currently a big old mess of duplicated logic.

This PR is a first step towards de-duplication.
As a codestyle PR, it should not affect performance (and probably won't have noticeable impact on compile time either).

Explanation

In practice, I see the following types / use-cases:

  • VariantGetInternalPtr: For types that are stored in Variant, get a pointer to their value.
    • Some are stored behind additional logic, e.g. pooling or reference counts, indicated by is_local.
  • VariantImplicitConvert (new): For types that are not stored directly in Variant, but act as if they are, using another type as storage surrogate (e.g. float or enums).
  • VariantInternalAccessor: Offers an API to get or set any type that can be stored in Variant (both direct types and implicit conversion types).

Using this logic, structs can be simplified:

  • VariantGetInternalPtr has only 3 logical implementations (ignoring Object): Those that are stored in the Variant, those that use a pointer to a heap value, and those that use PackedArrayRef. This can be abstracted and de-duplicated.
  • VariantInitializer and VariantDefaultInitializer for is_local types can just assign to the payload.
  • VariantInternalAccessor for stored types can access the stored type by reference, and for conversion types can return and accept copies.

Other changes

Tighter requirements on the declared types allowed me to spot a few accessor mistakes in the codebase, which I addressed.

Addendum

In follow up PRs, more duplicated logic should be removed from these headers, and duplicated functions and structs should be phased out. This is merely a first step establishing a way forward.

…mplicitly converted to and from `Variant`.

De-duplicate a lot of `VariantGetInternalPtr`, `VariantInternalAccessor`, `VariantInitializer` and `VariantDefaultInitializer`.
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.

1 participant