gamesh411 added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/DoNotReferAtomicTwiceCheck.cpp:21
+ Finder->addMatcher(
+ declRefExpr(hasType(hasUnqualifiedDesugaredType(atomicType())),
+ to(varDecl().bind("atomic")),
----------------
I have run the tests and I found that the last bad test case is not detected. I
have played with clang-query to investigate and I have found that the main
reason is the other `declRefExpr` that matches is not on the RHS but also on
LHS, just one level deeper in the AST.
You could patch it further, but IMO that would surely become way too complex to
understand and maintain in the long run. What I propose is to not try to solve
everything with ASTMatchers. This problem is inherently symmetric, and there is
no way match based on the complexity of an expression, which could be used a
way to deduplicate results.
I propose the following:
use a matcher like this:
```
expr(
hasDescendant(
declRefExpr(
to(
varDecl(
hasType(
hasUnqualifiedDesugaredType(
atomicType()
)
)
).bind("varDecl")
)
).bind("one")
),
hasDescendant(
declRefExpr(
to(
varDecl(
equalsBoundNode("varDecl")
)
),
unless(equalsBoundNode("one"))
).bind("other")
)
);
```
This will match conflicting DeclRefs with every pair-combinations. Then you
could use bound nodes `one` and `other` and a set data structure to deduplicate
the reports as needed. Note you should also watch out for expressions where 3
references exist to the same VarDecl. That case would emit 6 diagnostics, 4
references 12, etc. (Also 4 references could result in the first 2 then the
second two being processed, and you would still have duplication. Handling this
is probably not really worth it, as I would think this case is unlikely in
real-world code.)
I would consider this approach more customizable because you decide how to
deduplicate the results. It is also more understandable, as deduplicating with
Matchers is not really idiomatic in this symmetric case IMHO. I would not go as
far as to say anything about performance, but I would be surprised to find a
good, straightforward filtering solution using Matchers.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77493/new/
https://reviews.llvm.org/D77493
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
