Simplify condition with DeMorgan's Law #81771
Conversation
The existing pattern is 3 `not`s and an (implicit) `and`. Can be much simpler as just an `or`.
|
@dotnet/roslyn-compiler for a simple review |
| else if (parameter.IsExtensionParameter() && | ||
| (InParameterDefaultValue || InAttributeArgument || | ||
| this.ContainingMember() is not { Kind: not SymbolKind.NamedType, IsStatic: false } || // We are not in an instance member | ||
| this.ContainingMember() is { Kind: SymbolKind.NamedType } or { IsStatic: true } || // We are not in an instance member |
There was a problem hiding this comment.
Strictly speaking this rewrite is not semantically equivalent because the recursive pattern includes an implicit not null test. Therefore, the process of pushing the not down the pattern should result in an explicit null test, combined with or with the rest. It is quite possible that it was intentional to have the implicit not null test to protect the following line from a possible null dereference. I suggest to use semantically equivalent rewrite and to protect the recently added this.ContainingMember().IsStatic access in the body of the if #Closed
|
Done with review pass (commit 1) |
AlekseyTs
left a comment
AlekseyTs
left a comment
There was a problem hiding this comment.
LGTM (commit 3). Since some other code was changed to make it more robust, please, adjust the title so that it no longer sounds like a pure refactoring change.
The commit message and title will be adjusted, I'll leave the GitHub title itself alone to avoid breaking inboxes. |
4 participants
The existing pattern is 3
nots and an (implicit)and. Can be much simpler as just anor. I found the existing condition highly complex to read.