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

VB has non-deterministic output when anonymous types differ in case #48417

Closed
jaredpar opened this issue Oct 7, 2020 · 2 comments · Fixed by #49370
Closed

VB has non-deterministic output when anonymous types differ in case #48417

jaredpar opened this issue Oct 7, 2020 · 2 comments · Fixed by #49370
Assignees
Milestone

Comments

@jaredpar
Copy link
Member

jaredpar commented Oct 7, 2020

Consider the following VB code

' Module1.vb
Module Module1
  Sub Method()
    Console.WriteLine(New With { .X = 42 })
  End Sub
End Module
' Module2.vb
Module Module2
  Sub Method()
    Console.WriteLine(New With { .x = 42 })
  End Sub
End Module

The output of this program is non-deterministic. Because VB is case insensitive under the hood here only a single anonymous type object is created. The casing of the field representing X here will be determined by the first use of the anonymous type. Given that our files are bound in parallel if we have an anonymous type that differs by only case of the properties in different files then which we emit as the field name is non-deterministic.

This commit exposed the issue in our code base:

b15b40c

This changed the casing of the second property in the anonymous type to be span where it used to be Span. That conflicts with the anonymous type in AbstractReferenceHighligtingTests.vb which uses Span. Since that commit we've seen several failures of the determinism leg and when diffing the string table it appears that the primary difference is the casing of the span field in the string table.

@jaredpar
Copy link
Member Author

jaredpar commented Oct 7, 2020

@AlekseyTs i'm not quite sure what the best fix is going to be here. Think we need to find some way to pick the "best" definition here but I don't have any intuition as to what that would be. Also would like to ensure you agree with the analysis here.

@AlekseyTs
Copy link
Contributor

I think we are ordering anonymous type templates to emit them in deterministic order. If I remember correctly, they are ordered based on lexical order of the first usage. If we are not picking casing of property names in the same order, I think we should. I will have to take a closer look at the current implementation to provide more details, but I think we can solve this problem.

jasonmalinowski added a commit to jasonmalinowski/roslyn that referenced this issue Oct 8, 2020
This adds Microsoft.CodeAnalysis.EditorFeatures2.UnitTests.dll since
right now that binary is impacted by
dotnet#48417.
AlekseyTs added a commit to AlekseyTs/roslyn that referenced this issue Nov 14, 2020
AlekseyTs added a commit to AlekseyTs/roslyn that referenced this issue Nov 18, 2020
AlekseyTs added a commit that referenced this issue Nov 19, 2020
lukas-lansky added a commit to lukas-lansky/roslyn that referenced this issue Mar 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants