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

Test Plan for "List Patterns" #51289

Closed
78 of 91 tasks
333fred opened this issue Feb 17, 2021 · 9 comments
Closed
78 of 91 tasks

Test Plan for "List Patterns" #51289

333fred opened this issue Feb 17, 2021 · 9 comments
Assignees
Milestone

Comments

@333fred
Copy link
Member

333fred commented Feb 17, 2021

Championed issue: dotnet/csharplang#3435
Specification: https://github.com/dotnet/csharplang/blob/main/proposals/csharp-11.0/list-patterns.md

Compilers

Work items:

Scenarios:

  • Check of building VS/runtime repo
  • Need to test ref escape, such as indexer returning by ref instead of by value
  • Verify symbols/PDB
  • Update the test plan with new patterns
  • inputIsCountable is INotCountable and [..] (see ListPattern_NotCountableInterface)
  • inputHasExplicitIListImplAndIsNotListPatternable is IList and [..] (see ListPattern_ExplicitInterfaceImplementation)
  • LangVer
  • Spec checked in (PR)
  • Static indexer, inaccessible indexer or accessor, missing getter, void or string returning Length, ref/in parameters
    • list pattern on type with this[Index i] indexer
    • list pattern on type with this[Index i, int ignored = 0] indexer
    • list pattern on type with this[int i, int ignored = 0] indexer
    • slice pattern on type with this[Range r] indexer
    • slice pattern on type with this[Range r, int ignored = 0] indexer
    • slice pattern on type with this[int start, int end, int ignored = 0] indexer
    • real indexers preferred over implicit indexers
    • extensions ignored (see SlicePattern_ExtensionIgnored)
    • Length field doesn't count (see ListPattern_LengthFieldNotApplicable)
  • verify IL for list and slice patterns on string and array types (ranges spec calls out special cases for those, see ListPattern)
  • verify order of evaluation
  • verify that properties/indexers are only read once per DAG
  • verify NormalizeWhitespaces (see TestNormalizeListPattern_*)
  • this[params Range[] p] (see ListPattern_Range)
  • this[params Index[] p] (see ListPattern_Index)
  • this[params int[] p] (see ListPattern_Index)
  • this[long p] (see ListPattern_MemberLookup_Index_ErrorCases_4)
  • Slice(int start, int end, int ignored = 0) (invalid, see ListPattern_MemberLookup_Range_ErrorCases_3)
  • this[int start, int end, int ignored = 0] (invalid, see ListPattern_MemberLookup_Index_ErrorCases_3)
  • In short, we should cover anything that is valid with expr[N] or expr[^N] or expr[N..^M] (using either explicit or implicit indexer pattern) but not any other.
  • CallerInfo, optional parameters, params (see ListPattern_CallerInfo, ListPattern_Index)
  • Missing or obsolete types or members (property, accessor, ...) (see ListPattern_ObsoleteMembers, ListPattern_ObsoleteAccessors, ListPattern_IndexAndSliceReturnMissingTypes
    • Index and Range are required but optimized away (see ListPattern_IndexAndRangeAreNecessaryButOptimizedAway)
  • Accessor from base (see IndexerOverrideLacksAccessor)
  • Misplaced slice pattern (see SlicePattern_Misplaced)
  • Fallback from inaccessible this[Index] or this[Range] indexer (see ListPattern_MemberLookup_Fallback_InaccessibleIndexer)
  • ref returning indexers (see ListPattern_RefReturns)
  • list-pattern and Length property checks interact (see Pattern_Nullability_Exhaustiveness)
  • break with is { Length: -1 } or { Count: -1 } but not both (see ListPattern_LengthAndCountAreOrthogonal`)
  • effect of missing Index type on break (see LengthPattern_NegativeLengthTest_MissingIndex)
  • subsumption and exhaustiveness checking (see spec) (PR List patterns: subsumption checking #53891)
    • switch { [<10] => ..., [>10] => ...} (should warn)
    • [0, ..] => ..., [.., not 0] => ..., [var unreachable] => ..., [null, ..] => ..., [.., not null] => ..., [var unreachable] => ...
    • list-pattern nested in slice [0] => ..., [..[not 0]] => ..., [var unreachable] => ...
    • [1, 2, 3] => ..., [1, .., 3] => ...
    • no interaction with positional patterns, even on ITuple (see ListPattern_Tuples)
  • Verify that no length or slice calls are emitted for { .. } and { .._ }, and no slice call for { 1, 2, .. }, { 1, 2, ..var unused } (like for { Property: var unused }) (see SlicePattern_Subpattern)
  • Verify that we can efficiently use a linked list with { var head, ..var tail } (no)
  • Indexer override lacks a get accessor a la Compiler crash in pattern matching lowering - Error MSB6006: "csc.exe" exited with code -2146232797. (1, 1) #51801 (comment)
  • Test on tuples (fails) and ITuple (works, but no de-aliasing between positional and list patterns, see ListPattern_Tuples)
  • explainer (PR)
  • IOperation and ControlFlowGraph (PR)
  • val escape (PR)
  • list-pattern on interface, on nullable value type, on narrowed type (see ListPattern_NullableValueType, ListPattern_ExplicitInterfaceImplementation)
  • GetDeclaredSymbol and GetTypeInfo on designations (see ListPattern_Symbols_01)
  • SyntaxNormalizer (PR)
  • definite assignment (see SlicePattern_DefiniteAssignment)
  • nullability analysis (case [var x]: should give x the right annotation/state) (PR List patterns: add more tests and fix nullability analysis #53822)
    • maybe-null receiver (see ListPattern_Nullability_MaybeNullReceiver)
    • semantic model for var designations (see ListPattern_Nullability)
    • IOperation symbols reflect updated nullability (issue https://github.com/dotnet/roslyn/issues/57884)
  • subsumption checking for nullability (case [null, ..]: case [.., not null]: case [var unreachable]:)
  • Downgrade unreachable diagnostic for negative Length check to a warning (see notes, no longer applicable)
  • document negative Length break (PR Document break for negative Length tests #57149)
  • Adjust formatting for slice patterns (PR)
  • disallowed on dynamic receiver (see ListPattern_Dynamic)
  • disallowed in expression tree (see ListPattern_ExpressionTree)

Language

  • LDM: Should list-patterns interact with constant string patterns? (answer: no)
  • LDM: Should we use ... for slice pattern to work better with collection literals? (answer: no)
  • SPEC: tweak assumption around slicing and nullability to reflect LDM decision
  • clarify spec for evaluation of .. (no nested pattern) versus .. <nested pattern>
  • LDM: non-negative assumption should apply to both Count and Length (issue Open issue: non-negative assumption should apply to both Count and Length csharplang#5137)
  • LDM: should we allow indexers and Slice methods with uint, nint and other numeric types to make a type indexable/rangeable? (orthogonal and out-of-scope)
  • LDM: can we do something to simplify safe collection access scenario? if (index > 0 && index < xs.Count) { var x = xs[index]; DoSmth(x); } (out-of-scope)
  • LDM: confirm plan for IEnumerable (answer: C# 11 timeframe, ideally)
  • LDM: confirm plan for indexer/property pattern { [0]: ... } or { [var x]: ... } (answer: separate championed issue)
  • LDM: confirm on special length: ... pattern (answer: not a thing anymore)
  • LDM: confirm that [ .._ ] requires a sliceable type, whereas [ 1, .. ] doesn't (answer: correct)
  • LDM: do we really want a breaking change for { Length: -1 }? (answer: initially we thought we'd reduce the break to a warning, but we went back to taking the break) (Open issue: how to mitigate break for length patterns? csharplang#5226)
  • LDM: should list- and positional-patterns on ITuple interact? (question and comments below: "Should iTuple switch { (1,2,3) => 1, [1,2,3] => 2, _ => 0 } produce an error? ITuple is both countable and indexable.") (answer: no)
  • LDM: confirm formatting for slice pattern (answer: space)
  • LDM: various open questions: https://gist.github.com/jcouv/fa62923b3ee36de7f8553d2c1c58ec43
  • LDM: when a slice returns a type that is indexable/rangeable, do we assume that well-behaved types have certain consistency of behavior? (see ListPattern_Exhaustiveness_NestedSlice) (answer: yes)

IDE

  • completion for pattern (PR)
  • formatting for slice-pattern (..var or .. var?) (PR)
  • formatting for length-pattern on array type (is string[] [>0]) (not applicable)
  • QuickInfo on ..var x (PR)
  • FindAllReferences on Length/Count/indexers when they're used by list pattern (issue FindAllReferences on Length property or Slice method misses usages in list-patterns  #58190)
  • Add parens for clarity on slices with and or or
  • Switch common patterns to use list-pattern: if (expr[0] is pattern1&& expr[^1] is pattern2)
@333fred 333fred self-assigned this Feb 17, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Feb 17, 2021
@333fred 333fred changed the title Test Plan for "List Patterns Test Plan for "List Patterns" Feb 17, 2021
@jaredpar jaredpar added Feature Request Feature - List Patterns and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Feb 23, 2021
@jaredpar jaredpar modified the milestones: 16.10, C# 10 Feb 23, 2021
@jcouv jcouv self-assigned this Mar 23, 2021
@alrz
Copy link
Member

alrz commented May 12, 2021

A couple of more scenarios to test:
[jcouv: moved list to OP]

@Youssef1313
Copy link
Member

Note: Make sure obsoletion errors are correctly reported.

@alrz
Copy link
Member

alrz commented May 31, 2021

Do we want to report a subsumption error here?

case [1] { .., 1 }:
case [1] { 1, .. }:

If so, I'm gonna need to go get more coffee.

@jcouv
Copy link
Member

jcouv commented May 31, 2021

Do we want to report a subsumption error here?

What would be the error? "The slice is unnecessary"?

@alrz
Copy link
Member

alrz commented Jun 1, 2021

A better example:

case {1, .., 4}:
case {1, 2, 3, 4}: 
// ^ error CS8120: The switch case is unreachable. It has already been handled by a previous case or it is impossible to match.

@jcouv
Copy link
Member

jcouv commented Jun 1, 2021

That does seem useful, but I don't know how hard that would be to check.
Let's keep it on the plan and I'll get back to you to confirm. My feeling is that this could be added after we merge the feature (no IDE impact for this part).

@jcouv
Copy link
Member

jcouv commented Feb 23, 2022

LDM discussion today (on constant string patterns on Span<char>) brought up the question of whether list-patterns should interact with constant string patterns. Added an open question above.
For example: someString is { ['s', ..] => 0, "subsumed" => 1, ... }.

@jcouv jcouv modified the milestones: Compiler.Next, 17.1 Sep 26, 2022
@CyrusNajmabadi CyrusNajmabadi removed their assignment Nov 11, 2022
@jcouv
Copy link
Member

jcouv commented Nov 18, 2022

Closing test plan as completed. Issue with FAR isn't blocking.

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

No branches or pull requests

7 participants