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

Fix for anonymous record execution order #6606

Merged
merged 5 commits into from
Apr 23, 2019
Merged

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented Apr 22, 2019

Fix for #6487. Untested as yet, pushing through CI.

@dsyme
Copy link
Contributor Author

dsyme commented Apr 22, 2019

Tests added, this is ready (once green)

@@ -6554,21 +6552,35 @@ let permuteExprList (sigma: int[]) (exprs: Expr list) (ty: TType list) (names: s
/// let sigma = Array.map #Index ()
/// However the presence of static fields means .Index may index into a non-compact set of instance field indexes.
/// We still need to sort by index.
let mkRecordExpr g (lnk, tcref, tinst, rfrefs: RecdFieldRef list, args, m) =
let mkRecordExpr g (lnk, tcref, tinst, unsortedRecdFields: RecdFieldRef list, unsortedFieldExprs, m) =
Copy link
Contributor

Choose a reason for hiding this comment

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

Those are better names for sure.

Copy link
Contributor

@TIHan TIHan left a comment

Choose a reason for hiding this comment

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

Looks fine to me. There could be potential to get rid of some duplication between mkRecordExpr and mkAnonRecd but it's probably fine we keep it the way it is.

@dsyme
Copy link
Contributor Author

dsyme commented Apr 22, 2019

Looks fine to me. There could be potential to get rid of some duplication between mkRecordExpr and mkAnonRecd but it's probably fine we keep it the way it is.

Yes, I wrote it so I could do a line-for-line comparison and eventually share, but figured it would obfuscate the code for this PR to actually share

@KevinRansom KevinRansom merged commit b3df14c into dotnet:master Apr 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants