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: subsumption checking #53891

Merged
merged 72 commits into from
Sep 16, 2021
Merged
Changes from 1 commit
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
8ddbf28
WIP
alrz Jul 21, 2021
3a3f72a
Revert
alrz Jul 21, 2021
a24334d
Tweaks
alrz Jul 21, 2021
52e6be5
Move
alrz Jul 21, 2021
e3ce4fb
Remove unused usings
alrz Jul 21, 2021
3625d2f
Simplify
alrz Jul 21, 2021
5a359d0
Merge remote-tracking branch 'origin/features/list-patterns' into lis…
alrz Jul 26, 2021
de4f14a
WIP
alrz Jul 27, 2021
2beb033
WIP
alrz Jul 29, 2021
0dfd167
WIP
alrz Jul 29, 2021
ae7b003
Inline
alrz Jul 29, 2021
787b93d
Revert
alrz Jul 29, 2021
fa8a663
Extract method
alrz Jul 29, 2021
81d0e31
Add test
alrz Jul 30, 2021
91dbb3a
WIP
alrz Aug 1, 2021
f0adefa
Revert
alrz Aug 1, 2021
e671bd9
Cleanup
alrz Aug 3, 2021
0697233
Mark as unreachable
alrz Aug 4, 2021
0bb97b1
Renamings
alrz Aug 5, 2021
e5f1d86
Tweaks
alrz Aug 6, 2021
cdcd434
Add comments
alrz Aug 6, 2021
242690f
PR feedback
alrz Aug 7, 2021
ebd45ea
Add test
alrz Aug 7, 2021
8ed4ca8
Simplify
alrz Aug 7, 2021
33ca394
More tests
alrz Aug 7, 2021
eb44915
Test bad constants
alrz Aug 7, 2021
11ffabd
Merge values for matching indices
alrz Aug 8, 2021
d79cb4c
Revert
alrz Aug 12, 2021
7ca1ed7
WIP
alrz Aug 13, 2021
ee90c56
WIP
alrz Aug 13, 2021
dfa6b17
Tests
alrz Aug 13, 2021
389b9d0
Fixup
alrz Aug 13, 2021
ce9e8c4
Add skipped test
alrz Aug 13, 2021
c82209c
Do not drop the next node after a skipped evaluation
alrz Aug 14, 2021
b1d12cd
There cannot be any implication for an evaluation node
alrz Aug 14, 2021
635080d
Reorder
alrz Aug 15, 2021
67dc48b
PR feedback
alrz Aug 19, 2021
79c8fff
Swap
alrz Aug 19, 2021
50f4b1d
Run fuzz test
alrz Aug 19, 2021
9f38068
Typo
alrz Aug 19, 2021
840812a
Add PROTOTYPE comments
alrz Aug 19, 2021
565a972
Revert
alrz Aug 20, 2021
a20b47a
Add assertions
alrz Aug 21, 2021
18cdab3
Revert unrelated changes
alrz Sep 2, 2021
8d56fa8
Renamings
alrz Sep 2, 2021
a52aba1
Clarify comment
alrz Sep 2, 2021
24dcece
Add comments
alrz Sep 2, 2021
04d2de7
Reword
alrz Sep 2, 2021
5c2625b
New lines
alrz Sep 2, 2021
c86e44f
Add braces
alrz Sep 2, 2021
c5092df
Use statement-bodied methods across the file
alrz Sep 2, 2021
bebe7b0
New line
alrz Sep 2, 2021
8b1cead
Add comment on the test scenario
alrz Sep 2, 2021
2ca2b0c
Fixup and verify
alrz Sep 2, 2021
24f607d
Inheritdoc
alrz Sep 2, 2021
3307706
Whitespace
alrz Sep 2, 2021
cf73eab
Check for known result before shifting values
alrz Sep 2, 2021
7c63939
Whitespace
alrz Sep 2, 2021
6e109de
Revert new line
alrz Sep 2, 2021
ee27875
Reflect renamings in comment
alrz Sep 2, 2021
105f74f
Fixup
alrz Sep 2, 2021
ab253f7
Strengthen exhaustiveness tests
alrz Sep 2, 2021
aa99ba2
Revert
alrz Sep 2, 2021
bfeecd6
PR feedback
alrz Sep 5, 2021
418e644
Merge remote-tracking branch 'origin/features/list-patterns' into lis…
alrz Sep 5, 2021
f75e52a
PR feedback
alrz Sep 9, 2021
2df2a20
Use helpers
alrz Sep 9, 2021
18fcbdb
PR feedback
alrz Sep 10, 2021
7c13399
Extract debug-only helper
alrz Sep 10, 2021
b1d3a08
Extract method
alrz Sep 14, 2021
0e7737b
Rename
alrz Sep 14, 2021
68a9ab6
Revert code
alrz Sep 16, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fixup and verify
alrz committed Sep 2, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit 2ca2b0c6075b9659506df901329db506fa3976c1
7 changes: 3 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
Original file line number Diff line number Diff line change
@@ -1476,12 +1476,11 @@ private ImmutableArray<BoundPropertySubpattern> BindPropertyPatternClause(
memberType = member.Type;
// If we're dealing with the member that makes the type countable, and the type is also indexable, then it will be assumed to always return a non-negative value
isLengthOrCount = memberType.SpecialType == SpecialType.System_Int32 &&
alrz marked this conversation as resolved.
Show resolved Hide resolved
member.Symbol is { Name: WellKnownMemberNames.LengthPropertyName or WellKnownMemberNames.CountPropertyName, ContainingType: TypeSymbol containingType } memberSymbol &&
member.Symbol is { Name: WellKnownMemberNames.LengthPropertyName or WellKnownMemberNames.CountPropertyName } memberSymbol &&
(memberSymbol.Equals(((MethodSymbol?)GetWellKnownTypeMember(WellKnownMember.System_Array__get_Length, BindingDiagnosticBag.Discarded, syntax: node))?.AssociatedSymbol) ||
TryPerformPatternIndexerLookup(node, containingType, argIsIndex: true, indexerAccess: out _, patternSymbol: out _, out PropertySymbol? lengthProperty, BindingDiagnosticBag.Discarded) &&
memberSymbol.Equals(lengthProperty)); // If both Count and Length are present we want the one that makes this type countable.
TryPerformPatternIndexerLookup(node, inputType, argIsIndex: true, indexerAccess: out _, patternSymbol: out _, out PropertySymbol? lengthProperty, BindingDiagnosticBag.Discarded) &&
memberSymbol == (object)lengthProperty); // If both Count and Length are present we want the one that makes this type countable.
}

BoundPattern boundPattern = BindPattern(pattern, memberType, GetValEscape(memberType, inputValEscape), permitDesignations, hasErrors, diagnostics);
builder.Add(new BoundPropertySubpattern(p, member, isLengthOrCount, boundPattern));
}
Original file line number Diff line number Diff line change
@@ -1471,7 +1471,7 @@ public static void Main()
}

[Fact]
public void ImpossiblePattern()
public void ImpossiblePattern_01()
{
var source = @"
using System;
@@ -1538,6 +1538,60 @@ public void M(int[] a)
);
}

[Fact]
public void ImpossiblePattern_02()
{
var source = @"
interface IIndexable
{
int this[int i] { get; }
}
interface ICountableViaCount
{
int Count { get; }
}
interface ICountableViaLength
{
int Length { get; }
}
class X
{
public static void Test1<T>(T t) where T : ICountableViaCount
{
_ = t is { Count: -1 }; // ok
}
public static void Test2<T>(T t) where T : ICountableViaLength
{
_ = t is { Length: -1 }; // ok
}
public static void Test3<T>(T t) where T : IIndexable, ICountableViaCount
{
_ = t is { Count: -1 }; // 1
}
public static void Test4<T>(T t) where T : IIndexable, ICountableViaLength
{
_ = t is { Length: -1 }; // 2
}
public static void Test5<T>(T t) where T : IIndexable, ICountableViaLength, ICountableViaCount
{
_ = t is { Length: -1 }; // 3
_ = t is { Count: -1 }; // ok
Copy link
Member

@jcouv jcouv Sep 2, 2021

Choose a reason for hiding this comment

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

I find this behavior odd (although it matches the spec we agreed). I'll add an LDM open issue to propose that both be errors. What do you think?

Opened dotnet/csharplang#5137 #Pending

Copy link
Member Author

Choose a reason for hiding this comment

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

While there's no real-world scenario, I'd agree that doing this for both makes things less confusing in case the second property is added at a later point.

Copy link
Member

Choose a reason for hiding this comment

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

It looks like the spec already implies that both should be errors: "Length or Count properties are assumed to always return a non-negative value, if and only if the type is indexable."

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't mean it that way because "or" implies "either". Should be changed to "and" and we're on the same page.
Will go ahead and remove the symbol check. Best to be confirmed in LDM anyways though.

}
}
";
var compilation = CreateCompilation(source, parseOptions: TestOptions.RegularWithListPatterns);
compilation.VerifyEmitDiagnostics(
// (26,13): error CS8518: An expression of type 'T' can never match the provided pattern.
// _ = t is { Count: -1 }; // 1
Diagnostic(ErrorCode.ERR_IsPatternImpossible, "t is { Count: -1 }").WithArguments("T").WithLocation(26, 13),
// (30,13): error CS8518: An expression of type 'T' can never match the provided pattern.
// _ = t is { Length: -1 }; // 2
Diagnostic(ErrorCode.ERR_IsPatternImpossible, "t is { Length: -1 }").WithArguments("T").WithLocation(30, 13),
// (34,13): error CS8518: An expression of type 'T' can never match the provided pattern.
// _ = t is { Length: -1 }; // 3
Diagnostic(ErrorCode.ERR_IsPatternImpossible, "t is { Length: -1 }").WithArguments("T").WithLocation(34, 13));
}

[Fact]
public void BadConstant()
{