================
@@ -71,20 +71,20 @@ void
ContainerDataPointerCheck::registerMatchers(MatchFinder *Finder) {
const auto Zero = integerLiteral(equals(0));
- const auto SubscriptOperator = callee(cxxMethodDecl(hasName("operator[]")));
-
- Finder->addMatcher(
+ const auto AddressOfMatcher =
unaryOperator(
unless(isExpansionInSystemHeader()), hasOperatorName("&"),
- hasUnaryOperand(expr(
- anyOf(cxxOperatorCallExpr(SubscriptOperator, argumentCountIs(2),
- hasArgument(0, ContainerExpr),
- hasArgument(1, Zero)),
- cxxMemberCallExpr(SubscriptOperator, on(ContainerExpr),
- argumentCountIs(1), hasArgument(0,
Zero)),
- arraySubscriptExpr(hasLHS(ContainerExpr), hasRHS(Zero))))))
- .bind(AddressOfName),
- this);
+ hasUnaryOperand(ignoringParenImpCasts(expr(anyOf(
+ cxxOperatorCallExpr(
+ hasOverloadedOperatorName("[]"), argumentCountIs(2),
+ hasArgument(0, ContainerExpr), hasArgument(1, Zero)),
+ cxxMemberCallExpr(callee(cxxMethodDecl(hasName("operator[]"))),
+ on(ContainerExpr), argumentCountIs(1),
+ hasArgument(0, Zero)),
+ arraySubscriptExpr(hasLHS(ContainerExpr), hasRHS(Zero)))))))
+ .bind(AddressOfName);
+
+ Finder->addMatcher(traverse(TK_AsIs, AddressOfMatcher), this);
----------------
zeyi2 wrote:
Hi, I tried to implement the new check without `TK_AsIs` and I encountered some
difficulties:
The former template case `m` is a simple "dependent type + subscript" inside a
template body. The old matcher can recognize containers by inspecting the
primary template declaration. The new test case `u` is an "overloaded operator*
followed by operator[]", it requires knowing that the dependent result type `T`
actually has a `data()` method to safely emit `(*p).data()`.
With `TK_IgnoreUnlessSpelledInSource`, many nodes remain dependent in
uninstantiated template bodies, which makes predicates fail to check whether
the type really has `.data()`.
To fix this issue, I switched to a syntax‑driven approach that doesn’t rely on
type constraints in the matcher (only matches address-of subscript zero) and do
all container detection in `check()`. But still, resolving "type has a public
data()" on a dependent specialization is not reliably possible, so now the fix
for `m (return v.data();)` cannot be emitted conservatively.
I think it will be hard to proceed without `TK_AsIs`, I may need to reimplement
a dependency-aware walk from a dependent specialization back to its primary
template in `check()`, which may introduce more problems.
Would you be open to a compromise where we enable `TK_AsIs` only for this
check? IMHO it is a more practical and low-risk option.
https://github.com/llvm/llvm-project/pull/165636
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
