Skip to content

ComputeImplementationForInterfaceMember shouldn't perform any validation of the result and shouldn't report any diagnostics for it if the type it is called for doesn't implement the interface, unless the mentioned operations affect resulting symbol.#42405

Merged
AlekseyTs merged 2 commits intodotnet:masterfrom
AlekseyTs:Issue42340
Mar 16, 2020

Conversation

@AlekseyTs
Copy link
Copy Markdown
Contributor

Fixes #42340.

…ion of the result and shouldn't report any diagnostics for it if the type it is called for doesn't implement the interface, unless the mentioned operations affect resulting symbol.

Fixes dotnet#42340.
@AlekseyTs AlekseyTs marked this pull request as ready for review March 14, 2020 04:31
@AlekseyTs AlekseyTs requested a review from a team as a code owner March 14, 2020 04:31
@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@gafter, @dotnet/roslyn-compiler Please review.

1 similar comment
@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@gafter, @dotnet/roslyn-compiler Please review.

@gafter gafter self-requested a review March 16, 2020 17:09
Copy link
Copy Markdown
Member

@gafter gafter left a comment

Choose a reason for hiding this comment

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

:shipit:

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-compiler Please review a small change, need a second sign-off.

#if !DEBUG
// Don't optimize in DEBUG for better coverage for the GetInterfaceLocation function.
if (useSiteDiagnostics != null)
if (useSiteDiagnostics != null && implementingTypeImplementsInterface)
Copy link
Copy Markdown
Member

@RikkiGibson RikkiGibson Mar 16, 2020

Choose a reason for hiding this comment

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

Does this make it so we add the useSiteDiagnostics in a release debug build even if implementingTypeImplementsInterface is false?

edit: Looks like I misread the condition above, however it still seems like there might be a behavior inconsistency between debug and release builds. #Resolved

Copy link
Copy Markdown
Contributor Author

@AlekseyTs AlekseyTs Mar 16, 2020

Choose a reason for hiding this comment

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

however it still seems like there might be a behavior inconsistency between debug and release builds.

That behavior difference is not observable, unless there is a bug in the methods called within the if statement. We intentionally stress them in DEBUG. There are other examples where we do extra work in DEBUG for the sake of extra validation.


In reply to: 393223483 [](ancestors = 393223483)

@AlekseyTs AlekseyTs merged commit d626cc5 into dotnet:master Mar 16, 2020
@ghost ghost added this to the Next milestone Mar 16, 2020
@sharwell sharwell modified the milestones: Next, 16.6.P2 Mar 17, 2020
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.

master fails bootstrap build

4 participants