Skip to content

Conversation

@jcouv
Copy link
Member

@jcouv jcouv commented Oct 3, 2025

Addresses ordering issue tracked by #78968
Relates to test plan #76130

@jcouv jcouv self-assigned this Oct 3, 2025
@jcouv jcouv mentioned this pull request Oct 3, 2025
16 tasks
@jcouv jcouv force-pushed the extensions-order branch from 49f0447 to 3026624 Compare October 3, 2025 12:03
@jcouv jcouv added the Feature - Extension Everything The extension everything feature label Oct 3, 2025
@jcouv jcouv marked this pull request as ready for review October 3, 2025 13:05
@jcouv jcouv requested a review from a team as a code owner October 3, 2025 13:05
copy[0] = copy[last];
copy[last] = temp;

return ImmutableCollectionsMarshal.AsImmutableArray(copy);
Copy link
Contributor

Choose a reason for hiding this comment

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

return ImmutableCollectionsMarshal.AsImmutableArray(copy);

Consider keeping the empty line above


// In DEBUG, swap the first and last elements of a read-only array, yielding a new read only array.
// This helps to avoid depending on accidentally sorted arrays.
// Use more aggressive reordering with Array.Reverse(copy) for spot-checking
Copy link
Contributor

Choose a reason for hiding this comment

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

// Use more aggressive reordering with Array.Reverse(copy) for spot-checking

It is not clear what this comment refers to

Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

I do not think we should be making these changes. We do not need to ensure this level of equivalency between DEBUG and RELEASE. For the purpose of semantic analysis, the order of members doesn't matter. Deterministic emit is the goal, and I think we are good there with respect to extensions feature. The goal behind the comment in the test plan is to confirm that whatever is causing the difference won't make problems for emit. And I think it doesn't.

@jcouv jcouv closed this Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Compilers Feature - Extension Everything The extension everything feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants