-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Relax conversion requirements for pattern-matching involving type parameters. #18784
Changes from all commits
9d0ab48
47a9d7f
8e19970
0570548
98287a8
1ad87bf
8615a76
3257f88
e2a1eb1
089227f
1eb6954
fd2109e
d9bff8f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -130,7 +130,7 @@ private DecisionTree AddByValue(DecisionTree.Guarded guarded, BoundConstantPatte | |
|
||
private DecisionTree AddByValue(DecisionTree.ByValue byValue, BoundConstantPattern value, DecisionMaker makeDecision) | ||
{ | ||
Debug.Assert(value.Value.Type == byValue.Type); | ||
Debug.Assert(value.Value.Type.Equals(byValue.Type, TypeCompareKind.IgnoreDynamicAndTupleNames)); | ||
if (byValue.Default != null) | ||
{ | ||
return AddByValue(byValue.Default, value, makeDecision); | ||
|
@@ -221,7 +221,7 @@ private DecisionTree AddByValue(DecisionTree.ByType byType, BoundConstantPattern | |
var kvp = byType.TypeAndDecision[i]; | ||
var matchedType = kvp.Key; | ||
var decision = kvp.Value; | ||
if (matchedType.TupleUnderlyingTypeOrSelf() == value.Value.Type.TupleUnderlyingTypeOrSelf()) | ||
if (matchedType.Equals(value.Value.Type, TypeCompareKind.IgnoreDynamicAndTupleNames)) | ||
{ | ||
forType = decision; | ||
break; | ||
|
@@ -260,21 +260,30 @@ private DecisionTree AddByType(DecisionTree decision, TypeSymbol type, DecisionM | |
case DecisionTree.DecisionKind.ByValue: | ||
{ | ||
var byValue = (DecisionTree.ByValue)decision; | ||
DecisionTree result; | ||
if (byValue.Default == null) | ||
{ | ||
byValue.Default = makeDecision(byValue.Expression, byValue.Type); | ||
if (byValue.Default.MatchIsComplete) | ||
if (byValue.Type.Equals(type, TypeCompareKind.IgnoreDynamicAndTupleNames)) | ||
{ | ||
byValue.MatchIsComplete = true; | ||
result = byValue.Default = makeDecision(byValue.Expression, byValue.Type); | ||
} | ||
else | ||
{ | ||
byValue.Default = new DecisionTree.ByType(byValue.Expression, byValue.Type, null); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bug fix to insert a needed conversion. |
||
result = AddByType(byValue.Default, type, makeDecision); | ||
} | ||
|
||
return byValue.Default; | ||
} | ||
else | ||
{ | ||
Debug.Assert(byValue.Default.Type == type); | ||
return Add(byValue.Default, makeDecision); | ||
result = AddByType(byValue.Default, type, makeDecision); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bug fix to insert a needed conversion. |
||
} | ||
|
||
if (byValue.Default.MatchIsComplete) | ||
{ | ||
byValue.MatchIsComplete = true; | ||
} | ||
|
||
return result; | ||
} | ||
case DecisionTree.DecisionKind.Guarded: | ||
return AddByType((DecisionTree.Guarded)decision, type, makeDecision); | ||
|
@@ -321,7 +330,7 @@ private DecisionTree AddByType(DecisionTree.ByType byType, TypeSymbol type, Deci | |
if (byType.TypeAndDecision.Count != 0) | ||
{ | ||
var lastTypeAndDecision = byType.TypeAndDecision.Last(); | ||
if (lastTypeAndDecision.Key.TupleUnderlyingTypeOrSelf() == type.TupleUnderlyingTypeOrSelf()) | ||
if (lastTypeAndDecision.Key.Equals(type, TypeCompareKind.IgnoreDynamicAndTupleNames)) | ||
{ | ||
result = Add(lastTypeAndDecision.Value, makeDecision); | ||
} | ||
|
@@ -533,7 +542,7 @@ private DecisionTree Add(DecisionTree.ByType byType, DecisionMaker makeDecision) | |
TypeSymbol patternType, | ||
ref HashSet<DiagnosticInfo> useSiteDiagnostics) | ||
{ | ||
if (expressionType == patternType) | ||
if ((object)expressionType == (object)patternType) | ||
{ | ||
return true; | ||
} | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add tests that ignore
dynamic
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of a way to make that happen. The is no constant pattern whose type is generic. This could probably be simplified to
(matchedType == value.Value.Type)
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I know a way to make it happen. An enum type nested in a generic class.