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

List-patterns: mitigate break with Length property patterns #56721

Closed
wants to merge 2 commits into from

Conversation

jcouv
Copy link
Member

@jcouv jcouv commented Sep 24, 2021

Language design issue: dotnet/csharplang#5226
Relates to test plan #51289

TODO:

  • Confirm language design
  • Add LangVer check

@jcouv jcouv added this to the 17.1 milestone Sep 28, 2021
@@ -806,7 +806,7 @@ public static void Multiple(bool includeStackTrace, params Action[] assertions)
return;

var stringBuilder = new StringBuilder($"{exceptions.Count} out of {assertions.Length} assertions failed.");
foreach (var (index, ex) in exceptions)
foreach (var (index, ex) in exceptions.OrderBy(e => e.Index))
Copy link
Member

Choose a reason for hiding this comment

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

If we need to do this (why?) there's no need to run assertions in the reverse order above.

Comment on lines +350 to +352
case ErrorCode.WRN_SwitchArmSubsumedIfNonNegativeLength:
case ErrorCode.WRN_IsPatternAlwaysIfNonNegativeLength:
case ErrorCode.WRN_IsPatternImpossibleIfNonNegativeLength:
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't these be reported for level 7 only?

Copy link
Member Author

Choose a reason for hiding this comment

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

LDM discussed a regular warning (as a documented breaking change), not a warning wave warning.

@jcouv
Copy link
Member Author

jcouv commented Oct 13, 2021

From discussion with LDM today (10/13/2021) we're leaning towards taking the break after all. We'll revisit if some real-world types hit this issue in some serious way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants