llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Victor Chernyakin (localspook)
<details>
<summary>Changes</summary>
PR #<!-- -->122265 taught `IgnoreUnlessSpelledInSource` to ignore certain
`CallExpr`s. Because that PR was only concerned with decompositions, it was
very conservative with which `CallExpr`s it ignored. Issue #<!-- -->150874,
however, shows that we need to be more aggressive about ignoring `CallExpr`s.
This snippet:
```cpp
struct S {
operator int(this const S&);
};
void f() {
int(S {});
}
```
Generates an AST like:
```cpp
CXXFunctionalCastExpr <col:3, col:11> 'int' functional cast to int
<NoOp>
`-ImplicitCastExpr <col:7, col:10> 'int' <UserDefinedConversion>
part_of_explicit_cast
`-CallExpr <col:7, col:10> 'int'
|-ImplicitCastExpr <col:7> 'int (*)(const S &)'
<FunctionToPointerDecay>
| `-DeclRefExpr <col:7> 'int (const S &)' lvalue CXXConversion
0x3641e4b0 'operator int' 'int (const S &)'
`-MaterializeTemporaryExpr <col:7, col:10> 'const S' lvalue
`-ImplicitCastExpr <col:7, col:10> 'const S' <NoOp>
`-CXXFunctionalCastExpr <col:7, col:10> 'S' functional cast to S
<NoOp>
`-InitListExpr <col:9, col:10> 'S'
```
The `CallExpr` there should be ignored, but isn't. This change fixes that.
---
Full diff: https://github.com/llvm/llvm-project/pull/183882.diff
3 Files Affected:
- (modified)
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-casting.cpp
(+19)
- (modified) clang/lib/AST/Expr.cpp (+4-20)
- (modified) clang/unittests/AST/ASTTraverserTest.cpp (+36)
``````````diff
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-casting.cpp
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-casting.cpp
index 3e723b8b61d1d..67b65d2e7f799 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-casting.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-casting.cpp
@@ -13,6 +13,8 @@
// RUN: %check_clang_tidy -std=c++20 -check-suffix=,ALIASES %s
readability-redundant-casting %t -- \
// RUN: -config='{CheckOptions: {
readability-redundant-casting.IgnoreTypeAliases: true }}' \
// RUN: -- -fno-delayed-template-parsing -D CXX_20=1
+// RUN: %check_clang_tidy -std=c++23-or-later -check-suffix=,CXX23 %s
readability-redundant-casting %t -- \
+// RUN: -- -fno-delayed-template-parsing
struct A {};
struct B : A {};
@@ -245,3 +247,20 @@ int g1() {
return fp();
}
} // namespace gh170476
+
+
+#if __cplusplus >= 202302L
+
+struct S {
+ operator int(this const S&);
+};
+
+void testCastingWithExplicitObjectParameterConversionOperator() {
+ int i = int(S {});
+
+ int j = int(S {}.operator int());
+ // CHECK-MESSAGES-CXX23: :[[@LINE-1]]:11: warning: redundant explicit
casting to the same type 'int' as the sub-expression, remove this casting
[readability-redundant-casting]
+ // CHECK-FIXES-CXX23: int j = S {}.operator int();
+}
+
+#endif // __cplusplus >= 202302L
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 9632d88fae4e4..45daed276547d 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -2550,18 +2550,6 @@ Stmt *BlockExpr::getBody() {
// Generic Expression Routines
//===----------------------------------------------------------------------===//
-/// Helper to determine wether \c E is a CXXConstructExpr constructing
-/// a DecompositionDecl. Used to skip Clang-generated calls to std::get
-/// for structured bindings.
-static bool IsDecompositionDeclRefExpr(const Expr *E) {
- const auto *Unwrapped = E->IgnoreUnlessSpelledInSource();
- const auto *Ref = dyn_cast<DeclRefExpr>(Unwrapped);
- if (!Ref)
- return false;
-
- return isa_and_nonnull<DecompositionDecl>(Ref->getDecl());
-}
-
bool Expr::isReadIfDiscardedInCPlusPlus11() const {
// In C++11, discarded-value expressions of a certain form are special,
// according to [expr]p10:
@@ -3178,10 +3166,11 @@ Expr *Expr::IgnoreUnlessSpelledInSource() {
};
// Used when Clang generates calls to std::get for decomposing
- // structured bindings.
+ // structured bindings or for implicit calls to member functions
+ // with explicit object parameters.
auto IgnoreImplicitCallSingleStep = [](Expr *E) {
auto *C = dyn_cast<CallExpr>(E);
- if (!C)
+ if (!C || isa<UserDefinedLiteral>(C))
return E;
// Looking for calls to a std::get, which usually just takes
@@ -3197,12 +3186,7 @@ Expr *Expr::IgnoreUnlessSpelledInSource() {
if (A->getSourceRange() != E->getSourceRange())
return E;
- // If the argument refers to a DecompositionDecl construction,
- // ignore it.
- if (IsDecompositionDeclRefExpr(A))
- return A;
-
- return E;
+ return A;
};
return IgnoreExprNodes(
diff --git a/clang/unittests/AST/ASTTraverserTest.cpp
b/clang/unittests/AST/ASTTraverserTest.cpp
index bcbf01b6b0385..53b6096edb5ce 100644
--- a/clang/unittests/AST/ASTTraverserTest.cpp
+++ b/clang/unittests/AST/ASTTraverserTest.cpp
@@ -1659,6 +1659,42 @@ CallExpr
FN[0].getNodeAs<CallExpr>("decomp_call_with_default")),
R"cpp(
DeclRefExpr ''
+)cpp");
+ }
+
+ auto AST3 = buildASTFromCodeWithArgs(R"cpp(
+
+struct S {
+ operator int(this const S&);
+};
+
+void f() {
+ S s;
+ int i = int(s);
+}
+
+)cpp",
+ {"-std=c++23"});
+ {
+ auto FN = ast_matchers::match(
+ explicitCastExpr(hasSourceExpression(expr().bind("sourceExpr"))),
+ AST3->getASTContext());
+ EXPECT_EQ(FN.size(), 1u);
+
+ EXPECT_EQ(dumpASTString(TK_AsIs, FN[0].getNodeAs<Expr>("sourceExpr")),
+ R"cpp(
+ImplicitCastExpr
+`-CallExpr
+ |-ImplicitCastExpr
+ | `-DeclRefExpr 'operator int'
+ `-ImplicitCastExpr
+ `-DeclRefExpr 's'
+)cpp");
+
+ EXPECT_EQ(dumpASTString(TK_IgnoreUnlessSpelledInSource,
+ FN[0].getNodeAs<Expr>("sourceExpr")),
+ R"cpp(
+DeclRefExpr 's'
)cpp");
}
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/183882
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
