-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Compare constraints lazily across partial type declarations #34850
Conversation
{ | ||
var typeParameters = TypeParameters; | ||
|
||
if (constraintClauses.Length > 0) |
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.
if (constraintClauses.Length > 0) [](start = 12, length = 33)
Is this check redundant? #Closed
private ImmutableArray<TypeParameterConstraintClause> MakeTypeParameterConstraintsLate( | ||
ImmutableArray<TypeParameterConstraintClause> constraintClauses, | ||
DiagnosticBag diagnostics) | ||
{ |
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.
{ [](start = 8, length = 1)
Consider adding an assert constraintClauses.IsEarly()
#Closed
Could you provide some details on what was the root cause of the stack overflow? #Closed |
Done with review pass (iteration 1) #Closed |
In reply to: 481044805 [](ancestors = 481044805) |
@dotnet/roslyn-compiler please review. |
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.
LGTM (iteration 2)
}"; | ||
var comp = CreateCompilation(new[] { source1, source2 }); | ||
comp.VerifyDiagnostics( | ||
// (3,15): error CS0265: Partial declarations of 'C<T>' have inconsistent constraints for type parameter 'T' |
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.
Probably unrelated, but this is a terrible error message. We should say what the inconsistencies are. #Resolved
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.
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.
LGTM (commit 2). I do wish that the error message there was better though.
Fixes #34841.