[PATCH] D77493: [clang-tidy] Add do-not-refer-atomic-twice check

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 10 08:08:23 PST 2020


aaron.ballman added a comment.

In D77493#1962465 <https://reviews.llvm.org/D77493#1962465>, @njames93 wrote:

> In my mind this check is definitely in the realm of the static analyser, clang-tidy just isn't designed for this.

+1, I think this probably should be handled in the static analyzer if we want to catch the truly problematic cases.



================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-do-not-refer-atomic-twice.cpp:77
+  n3++;
+}
----------------
jfb wrote:
> Can you check that non-atomic-accesses to the variable also work, for example taking the atomic's address, using it in unevaluated `sizeof` / `offsetof`, etc.
Additionally, this may be one of the rare cases where we do want to warn even if the problem happens due to macro expansion, so there should be some tests involving atomic uses within macros. A degenerate case we should be careful of is the `?:` operator:
```
atomic ? atomic : non_atomic; // bad, two reads
whatever ? atomic : atomic; // fine, only one read
```


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D77493



More information about the cfe-commits mailing list