Skip to content

Conversation

@cston
Copy link
Contributor

@cston cston commented Jan 25, 2018

No description provided.

@cston cston requested a review from a team as a code owner January 25, 2018 06:20
@cston
Copy link
Contributor Author

cston commented Jan 25, 2018

@dotnet/roslyn-compiler please review.

@AlekseyTs
Copy link
Contributor

Should this target features/compiler or some other branch targeting earlier release?

@cston cston changed the base branch from master to dev15.7.x January 25, 2018 18:30
Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (iteration 1)

{
return false;
case VarianceKind.Out:
if (!HasImplicitReferenceConversion(sourceTypeArgument, destinationTypeArgument, ref useSiteDiagnostics))
Copy link
Member

@jcouv jcouv Jan 25, 2018

Choose a reason for hiding this comment

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

Should this be return HasImplicitReferenceConversion(...);?
Same on In case. #Closed

Copy link
Contributor Author

@cston cston Jan 25, 2018

Choose a reason for hiding this comment

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

We can't return unconditionally because we need to check all type arguments. #Resolved

@jcouv jcouv added this to the 15.7 milestone Jan 25, 2018
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM with a suggestion

@cston
Copy link
Contributor Author

cston commented Jan 25, 2018

@dotnet-bot test windows_release_unit32_prtest

@cston cston merged commit 33561d0 into dotnet:dev15.7.x Jan 25, 2018
@cston cston deleted the cleanup branch January 25, 2018 21:10
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.

3 participants