Skip to content

Conversation

HT154
Copy link
Contributor

@HT154 HT154 commented Jun 23, 2025

No description provided.

@HT154 HT154 marked this pull request as draft June 23, 2025 22:56
@HT154 HT154 force-pushed the reflect-all-properties-methods branch 2 times, most recently from fae0f86 to 0d21d7d Compare July 9, 2025 00:57
Comment on lines +115 to 109
public VmTyped getMirror(VmClass clazz) {
return MirrorFactories.propertyFactory.create(new Mirror(this, clazz));
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how to derive allAnnotations/allModifiers without somehow "currying" the VmClass into the mirror for the ClassProperty. If you've got any better ideas here, I'm all ears!

@HT154 HT154 marked this pull request as ready for review July 9, 2025 15:49
@HT154 HT154 force-pushed the reflect-all-properties-methods branch 2 times, most recently from e1c24e7 to b4e49f1 Compare August 29, 2025 20:26
Copy link
Contributor

@stackoverflow stackoverflow left a comment

Choose a reason for hiding this comment

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

Looks good.


/// The annotations of this module, appended with any from the containing class's superclasses.
///
/// Annotations are ordered starting with the current class and walking up the class hierarchy.
Copy link
Contributor

Choose a reason for hiding this comment

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

You mentioned the order here, but allProperties/Methods/Modifiers also follow the same rule right?

Copy link
Contributor Author

@HT154 HT154 Sep 19, 2025

Choose a reason for hiding this comment

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

I'm a little concerned with defining behavior for allProperties/allMethods because those are implemented in terms of the existing VmClass.collectAll(Properties|Methods) methods and iteration order isn't well-defined for them.

This also isn't applicable for allModifiers because the implementation builds the bitset with bitwise OR and calls VmModifier.getMirrors, which has a stable order for elements based on bits set.

@HT154 HT154 force-pushed the reflect-all-properties-methods branch from b4e49f1 to 44693f7 Compare September 19, 2025 18:30
@HT154 HT154 force-pushed the reflect-all-properties-methods branch from 44693f7 to 53d5405 Compare September 19, 2025 19:05
@HT154 HT154 merged commit 63f89fb into apple:main Sep 19, 2025
4 checks passed
@HT154 HT154 deleted the reflect-all-properties-methods branch September 19, 2025 19:24
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