[PATCH] D112334: [clang-tidy] Suppress readability-static-access-through-instance for CUDA built-in variables

Tue Oct 26 06:12:27 PDT 2021

whisperity added a comment.

In D112334#3084533 <https://reviews.llvm.org/D112334#3084533>, @aaron.ballman wrote:

> In D112334#3081213 <https://reviews.llvm.org/D112334#3081213>, @carlosgalvezp wrote:
>
>> Btw, regarding this `CHECK-MESSAGES-NOT`, how does it work? I can't find it in `check_clang_tidy.py`. Wouldn't the test fail anyway if unexpected warnings are printed?
>
> FileCheck matches input CHECK lines with output lines, so if there's no CHECK line for a particular output, FileCheck won't report any problems with it. (`-verify` is used by the frontend to verify diagnostics, but that's not something that can be used within clang-tidy currently.) That said, I'm not entirely certain of the mechanisms behind `CHECK-MESSAGES-NOT` in the python scripts.

Just a clarification for the record, as I came across this too. There is no business logic per se associated with `CHECK-MESSAGES-NOT`. I think it's some sort of a bad example that we observed and taken from each other and sometimes overusing. `CHECK-MESSAGES-NOT` is useful in the context where there is a warning that currently isn't emitted (because the check isn't fully finished, or depends on a //FIXME// or something) but you **want** the message to appear later in the code. It indicates what should be there, but currently isn't there and we are not expecting it. The moment the check is further developed, it's easy to cut back the `-NOT` part (`2t-ld2w` if you're using Vim) and turn it into a proper check that //expects// the warning to be there. (And of course, hopefully also emits it!)

Semantically, it is equivalent to saying `// NO-WARN: Clang is awesome!` or `// MY-LITTLE-PONY` or whatever in the text. It will be ignored by the script. 😉  The script only triggers for `CHECK-MESSAGES:`, `CHECK-FIXES:` and `CHECK-NOTES:` and the prefixes you may or may not be able to specify at invocation explicitly.

IMHO if we never want that warning to pop it is misleading and noise to add the would-be text of a warning there. If you want to indicate that there is no need for a warning (any warning emitted by the check!), I believe it is better to use `// NO-WARN`. That indicates both the intent that the code in that line or nearby //SHOULD PASS// but also clearly documents that it wasn't a developer oversight that the test appears as a "negative case" (i.e. you did not //forget// to add the `CHECK-` lines in).

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112334/new/

https://reviews.llvm.org/D112334