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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to