Fix well-known Equals method nullable analysis bug by RikkiGibson · Pull Request #41918 · dotnet/roslyn

RikkiGibson · 2020-02-25T01:45:40Z

I wrongly assumed that a method that implements e.g. IEquatable.Equals would be contained in a class that implements IEquatable.

edit: among other wrong assumptions! This PR has been a journey into how interfaces actually work/what's possible in .NET.

AlekseyTs · 2020-02-25T18:20:56Z

            foreach (var member in constructedType.GetMembers(WellKnownMemberNames.ObjectEquals))

We have a helper for this - AsMemberOfType #Closed

Refers to: src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs:3780 in d754534. [](commit_id = d754534, deletion_comment = False)

AlekseyTs · 2020-02-25T18:48:10Z

Done with review pass (iteration 1) #Closed

AlekseyTs · 2020-02-26T23:56:42Z

Done with review pass (iteration 5) #Closed

RikkiGibson · 2020-03-04T22:23:04Z

Temporarily marking for personal review so I can start working on the WIP on another machine.

AlekseyTs · 2020-03-10T17:31:29Z

Done with review pass (iteration 8) #Closed

…smethod-fix

AlekseyTs · 2020-03-10T22:08:16Z

Done with review pass (iteration 11) #Closed

AlekseyTs · 2020-03-12T00:37:20Z

Done with review pass (iteration 14) #Closed

AlekseyTs · 2020-03-12T23:56:43Z

@RikkiGibson It looks like CI jobs are timing out. Could that be related to the last commit? #Closed

AlekseyTs

LGTM (iteration 16)

RikkiGibson · 2020-03-13T18:50:36Z

@dotnet/roslyn-compiler Please review

Fix well-known Equals method bug

d754534

RikkiGibson added Area-Compilers Feature - Nullable Reference Types Nullable Reference Types labels Feb 25, 2020

RikkiGibson added this to the 16.6 milestone Feb 25, 2020

RikkiGibson requested a review from a team as a code owner February 25, 2020 01:45

RikkiGibson changed the title ~~Fix well-known Equals method bug~~ Fix well-known Equals method nullable analysis bug Feb 25, 2020

AlekseyTs reviewed Feb 25, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs

AlekseyTs reviewed Feb 25, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs

AlekseyTs reviewed Feb 25, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs

AlekseyTs reviewed Feb 25, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs

AlekseyTs reviewed Feb 25, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs

RikkiGibson added 3 commits February 25, 2020 11:33

Fix and address feedback

f4017e0

Address feedback

a25a48e

Fix Equals analysis of static methods

f40e90c

AlekseyTs reviewed Feb 26, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

AlekseyTs reviewed Feb 26, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs Outdated

Only check Equals method if type implements interface method

a2da4f0

AlekseyTs reviewed Feb 26, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs

Address feedback

741dea2

AlekseyTs reviewed Feb 27, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs Outdated

AlekseyTs reviewed Feb 27, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

AlekseyTs reviewed Feb 27, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

AlekseyTs reviewed Feb 27, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs Outdated

WIP

f2e1225

RikkiGibson added the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Mar 4, 2020

Optimize search for implementation method and add test

21fc7fa

AlekseyTs reviewed Mar 10, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

RikkiGibson added 2 commits March 10, 2020 12:38

Revert accidental change

b963824

Skip unnecessary base types and overridden methods

253f0bb

AlekseyTs reviewed Mar 10, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs Outdated

Merge branch 'master' of github.com:dotnet/roslyn into nullable-equal…

4a852b4

…smethod-fix

AlekseyTs reviewed Mar 10, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

AlekseyTs reviewed Mar 10, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

AlekseyTs reviewed Mar 10, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs

AlekseyTs reviewed Mar 10, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

RikkiGibson added 3 commits March 10, 2020 18:27

Fix bugs and add tests

24dcf9b

Test implementation method contained in interface

9cd3dfa

Fix expected diagnostics

d9b08cf

AlekseyTs reviewed Mar 12, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs Outdated

AlekseyTs reviewed Mar 12, 2020

View reviewed changes

Comment thread src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs Outdated

Address more feedback

052a438

Fix bug

742ceaf

AlekseyTs approved these changes Mar 13, 2020

View reviewed changes

RikkiGibson requested a review from a team March 13, 2020 18:26

jaredpar approved these changes Mar 16, 2020

View reviewed changes

RikkiGibson merged commit efc067e into dotnet:master Mar 16, 2020

ghost modified the milestones: 16.6, Next Mar 16, 2020

RikkiGibson deleted the nullable-equalsmethod-fix branch March 16, 2020 23:53

sharwell modified the milestones: Next, 16.6.P2 Mar 17, 2020

Conversation

RikkiGibson commented Feb 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlekseyTs commented Feb 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlekseyTs commented Feb 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlekseyTs commented Feb 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RikkiGibson commented Mar 4, 2020

Uh oh!

Uh oh!

AlekseyTs commented Mar 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlekseyTs commented Mar 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlekseyTs commented Mar 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlekseyTs commented Mar 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlekseyTs left a comment

Choose a reason for hiding this comment

Uh oh!

RikkiGibson commented Mar 13, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

RikkiGibson commented Feb 25, 2020 •

edited

Loading

AlekseyTs commented Feb 25, 2020 •

edited

Loading

AlekseyTs commented Feb 25, 2020 •

edited

Loading

AlekseyTs commented Feb 26, 2020 •

edited

Loading

AlekseyTs commented Mar 10, 2020 •

edited

Loading

AlekseyTs commented Mar 10, 2020 •

edited

Loading

AlekseyTs commented Mar 12, 2020 •

edited

Loading

AlekseyTs commented Mar 12, 2020 •

edited

Loading