[Diag] QoI: ownership in protocol check should not be 'else' clause

[davezarzycki]

Hey @DougGregor – This might be worth cherry-picking into the 4.2 branch.

[davezarzycki]

@swift-ci please smoke test

[davezarzycki]

@swift-ci please smoke test linux

[xedin]

Nice! I guess the idea here was that if something already set attr->isInvalid() there is no reason to try and diagnose it again... You might want to add that check to new condition as well.

[davezarzycki]

@xedin – I'm not following your comment. The idea here is that sometimes multiple diagnostics need to be reported for a given AST node. There are two diagnostics here that should never have been joined via an else clause. The rest of the pull request is just code clean up.

@swift-ci please smoke test linux

[xedin]

I just wanted to point out that maybe it was else if because that places shouldn't have diagnosed for both these problems, but probably not...

[davezarzycki]

Digging through the git blame history, it looks like an honest mistake from the start. See #11744.

[xedin]

Sounds good!