Skip to content

Conversation

@cartermp
Copy link
Contributor

Fixes #5495

In F#, the order of declarations matters, including with `open` statements. This is unlike C#, where the effect of `using` and `using static` is independent of the ordering of those statements in a file.

In F#, because elements opened into a scope can shadow others already present. This means that reordering `open` statements can alter the meaning of code. As a result, sorting alphanumerically (or pseudorandomly) is generally not recommended, lest you generate different behavior that you might expect.
In F#, elements opened into a scope can shadow others already present. This means that reordering `open` statements could alter the meaning of code. As a result, sorting all `open` statements alphanumerically (or pseudorandomly) is generally not recommended, lest you generate different behavior that you might expect.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does it make sense to mention pseudorandom sorting of open statements here? Is that something someone would actually consider?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's fair, the issue here is any sorting which isn't topological is likely to result in unwanted shadowing.

Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

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

This LGTM @cartermp

You can :shipit: when ready.

@cartermp cartermp merged commit 3e23c99 into master May 21, 2018
@cartermp cartermp deleted the cartermp-patch-1 branch May 21, 2018 19:44
@cmeeren
Copy link
Contributor

cmeeren commented May 21, 2018

Shouldn't alphanumericall be spelled with a single l?

@cmeeren
Copy link
Contributor

cmeeren commented May 21, 2018

Or rather, alphanumerically is probably the correct word in that context.

@mairaw
Copy link
Contributor

mairaw commented May 21, 2018

@cmeeren I've opened PR #5526 to fix this.

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.

6 participants