[PATCH] D51381: [clang-tidy] fix check_clang_tidy to properly mix CHECK-NOTES and CHECK-MESSAGES

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Yes, you are write.

I would ensure in the script that `CHECK-MESSAGES XOR CHECK-NOTES` is
used, to avoid the mistake i made. I can do it in this diff as well.

Am 29.08.2018 um 20:29 schrieb Roman Lebedev via Phabricator:

> lebedev.ri added a comment.
> 
> In https://reviews.llvm.org/D51381#1217047, @JonasToth wrote:
> 
>> @lebedev.ri lets do it in the the other patch, to not split discussions.
> 
> Let's do it here instead, since that differential requires some changes to this script.
> 
> In https://reviews.llvm.org/D51381#1217047, @JonasToth wrote:
> 
>> But what do you mean by `You would still have to duplicate the check-lines for error: though.`?
> 
> In it's current state, `CHECK-MESSAGES` implies `-implicit-check-not={{warning|error}}`,
>  and `CHECK-NOTES` will imply `-implicit-check-not={{note|error}}`.
>  I.e. they both imply `-implicit-check-not=error`.
>  So **if** the check produces any `error:` output, **and** you want to use **both** the `CHECK-NOTES` and `CHECK-MESSAGES`,
>  you need to write the check-line for every `error: ` with both the `CHECK-NOTES` and `CHECK-MESSAGES` prefixes. 
>  This seems sub-optimal.
> 
> In https://reviews.llvm.org/D48714#1217044, @JonasToth wrote:
> 
>> In https://reviews.llvm.org/D48714#1216989, @lebedev.ri wrote:
>> 
>>> In https://reviews.llvm.org/D48714#1216537, @JonasToth wrote:
>>> 
>>>> I had to revert the `CHECK-NOTES` change that @lebedev.ri introduced with his revision. It fails the test, i think there is an inconsistency or so in the check-clang-tidy script. I will try to figure out whats the issue.
>>> 
>>> So what was the issue? Do you get the same results if you undo the https://reviews.llvm.org/D51381 and `s/CHECK-MESSAGES/CHECK-NOTES/`?
>> 
>> You are right that replacing all of it with `CHECK-NOTES` works as well.
>> 
>>   `FileCheck` is run twice if you have `FIXMES` as well. Having another run for the notes is consistent with how it worked before.
>>   If we go for the catch-all-with-one approach it might be a good idea to ensure that only one of `CHECK-MESSAGES` or `CHECK-NOTES` is present in the file and adjust the check_clang_tidy.py script a little.
> 
> I don't have a //strong// preference, but if that works i would **prefer** to go that route, since that is what the initial intended semantics of the `CHECK-NOTES`.
>  **If** that works for you?
> 
> Repository:
> 
>   rCTE Clang Tools Extra
> 
> https://reviews.llvm.org/D51381


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51381