Skip to content

Conversation

Repiteo
Copy link
Contributor

@Repiteo Repiteo commented Oct 15, 2025

The recent merge of #111561 enabled me to revisit this idea of simplifying typed container include logic, as that turned out to be the missing link in making everything else work! Instead of having to jump through hoops of dependency chains, type_info.h already has all the information it needs to declare relevant typing metadata for TypedArray and TypedDictionary.

The primary difference in this implementation compared to before is how the implementation is all handled in type_info.h now, instead of variant.h. This makes more sense logistically, as the typing metadata already exists in that context & could be safely reused. Because the information can be safely parsed ahead of time, the need for dedicated macros is completely gone: all typed containers can rely on a single implementation that covers all use-cases!

This entirely purges any attempted "bonus" codestyle changes/fixes from the original PR attempt: this attempts as atomic of a conversion as possible. As such, instead of removing the typed container headers entirely, they're now simple stub files. Miraculously, there was exactly one file where an additional include was required; everything else happens to utilize type_info.h transitively, which is a silver-lining of our current #include issues. A future PR focused entirely on stylistic changes can remove those headers outright & any references to them.

@Repiteo Repiteo added this to the 4.x milestone Oct 15, 2025
@Repiteo Repiteo requested a review from a team as a code owner October 15, 2025 00:55
@Repiteo Repiteo requested a review from a team as a code owner October 15, 2025 00:55
@Repiteo Repiteo removed the request for review from a team October 15, 2025 00:55
Copy link
Member

@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.

Overall, this looks like a nice consolidation of logic. But I have two concerns:

  • Why declare TypedDictionary when another file has to be included to get its definitions? Why not just declare it there? If we do it like this, trying to use TypedDictionary when not including type_info will error with difficult to understand messages.
  • Why declare in type_info.h? Why not declare in typed_dictionary.hand includetype_info.h? Having the "internal" stuff shared across TypedArrayandTypedDictionarymakes sense to me, but not having theTypedArrayandTypedDictionary` definitions themselves there.

@Repiteo Repiteo force-pushed the core/typed-container-migration-v2 branch from 28c25ae to fd59570 Compare October 15, 2025 12:54
@Repiteo Repiteo changed the title Core: Migrate typed container logic to type_info.h Core: Consolidate typed container logic with type_info.h Oct 15, 2025
@Repiteo Repiteo force-pushed the core/typed-container-migration-v2 branch from fd59570 to 751bf1b Compare October 15, 2025 13:01
@Repiteo
Copy link
Contributor Author

Repiteo commented Oct 15, 2025

Thinking about it more, those two concerns were from when this was originally implemented in variant.h, which would've naturally overlapped with any base Array/Dictionary invokations. That's no longer the case, so keeping them in dedicated files should be fine after all.

@Repiteo Repiteo force-pushed the core/typed-container-migration-v2 branch from 751bf1b to 7390c35 Compare October 15, 2025 13:05
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.

2 participants