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

An assert in CustomModifierUtils should be adjusted to check Nullable modifiers as well #30740

Closed
AlekseyTs opened this issue Oct 24, 2018 · 1 comment · Fixed by #33327
Closed

Comments

@AlekseyTs
Copy link
Contributor

            Debug.Assert(resultType.Equals(destinationType,
                TypeCompareKind.IgnoreCustomModifiersAndArraySizesAndLowerBounds)); // Same object/dynamic and tuple names as destination type.
@jaredpar jaredpar added this to the 16.0 milestone Oct 29, 2018
@jaredpar jaredpar added the Bug label Oct 29, 2018
@AlekseyTs
Copy link
Contributor Author

Right now the code looks like this:

            Debug.Assert(resultType.Equals(destinationType,
                TypeCompareKind.IgnoreCustomModifiersAndArraySizesAndLowerBounds | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes)); // Same object/dynamic and tuple names as destination type.

It should look like the code above instead, i.e. TypeCompareKind.IgnoreNullableModifiersForReferenceTypes shouldn't be used for the purpose of comparison. The reason is that we are supposed to transfer custom modifiers, but other aspects (tuple names, dynamic, and nullable modifiers) should remain intact.

jaredpar added a commit to jaredpar/roslyn that referenced this issue Feb 7, 2019
The assert for custom modifier checking should include the nullable
validation

closes dotnet#30740
jaredpar added a commit to jaredpar/roslyn that referenced this issue Feb 12, 2019
The assert for custom modifier checking should include the nullable
validation

closes dotnet#30740
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