Skip to content

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

Closed
jcouv wants to merge 2 commits intodotnet:features/list-patternsfrom
jcouv:length-break
Closed

List-patterns: mitigate break with Length property patterns #56721
jcouv wants to merge 2 commits intodotnet:features/list-patternsfrom
jcouv:length-break

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

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