[clang-tools-extra] [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses (PR #72068)
https://github.com/felix642 created https://github.com/llvm/llvm-project/pull/72068 Fixes #71852 From 54743ceec9845cdc450d5c163a293e73884a8df9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 12 Nov 2023 16:07:13 -0500 Subject: [PATCH] [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses Fixes #71852 --- .../ImplicitBoolConversionCheck.cpp | 4 +-- clang-tools-extra/docs/ReleaseNotes.rst | 4 +++ .../readability/implicit-bool-conversion.cpp | 30 +++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp index 69e6d73c4fcd7bb..9e627d281e4b0f9 100644 --- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp @@ -152,7 +152,7 @@ StringRef getEquivalentBoolLiteralForExpr(const Expr *Expression, return "false"; } - if (const auto *IntLit = dyn_cast(Expression)) { + if (const auto *IntLit = dyn_cast(Expression->IgnoreParens())) { return (IntLit->getValue() == 0) ? "false" : "true"; } @@ -385,7 +385,7 @@ void ImplicitBoolConversionCheck::handleCastFromBool( << DestType; if (const auto *BoolLiteral = - dyn_cast(Cast->getSubExpr())) { + dyn_cast(Cast->getSubExpr()->IgnoreParens())) { Diag << tooling::fixit::createReplacement( *Cast, getEquivalentForBoolLiteral(BoolLiteral, DestType, Context)); } else { diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index f49c412118e7d98..ab9ef8cdfb37a00 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -412,6 +412,10 @@ Changes in existing checks do-while loops into account for the `AllowIntegerConditions` and `AllowPointerConditions` options. +- Improved :doc:`readability-implicit-bool-conversion + ` check to provide + consistent suggestions when parentheses are added to the return value. + - Improved :doc:`readability-non-const-parameter ` check to ignore false-positives in initializer list of record. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp index 323cf813c047000..f7f5d506a9ce0e0 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp @@ -472,6 +472,36 @@ bool f(S& s) { } // namespace ignore_1bit_bitfields +int implicitConversionReturnInt() +{ +return true; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion bool -> 'int' +// CHECK-FIXES: return 1 +} + +int implicitConversionReturnIntWithParens() +{ +return (true); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion bool -> 'int' +// CHECK-FIXES: return 1 +} + + +bool implicitConversionReturnBool() +{ +return 1; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> bool +// CHECK-FIXES: return true +} + +bool implicitConversionReturnBoolWithParens() +{ +return (1); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> bool +// CHECK-FIXES: return true +} + + namespace PR47000 { int to_int(bool x) { return int{x}; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses (PR #72068)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/72068 From 649c7b8b936d848100b58e733dd358fa1d5914fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 12 Nov 2023 16:07:13 -0500 Subject: [PATCH] [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses Fixes #71852 --- .../ImplicitBoolConversionCheck.cpp | 5 ++-- clang-tools-extra/docs/ReleaseNotes.rst | 4 +++ .../readability/implicit-bool-conversion.cpp | 30 +++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp index 69e6d73c4fcd7bb..f0fca30de3b3c4d 100644 --- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp @@ -152,7 +152,8 @@ StringRef getEquivalentBoolLiteralForExpr(const Expr *Expression, return "false"; } - if (const auto *IntLit = dyn_cast(Expression)) { + if (const auto *IntLit = + dyn_cast(Expression->IgnoreParens())) { return (IntLit->getValue() == 0) ? "false" : "true"; } @@ -385,7 +386,7 @@ void ImplicitBoolConversionCheck::handleCastFromBool( << DestType; if (const auto *BoolLiteral = - dyn_cast(Cast->getSubExpr())) { + dyn_cast(Cast->getSubExpr()->IgnoreParens())) { Diag << tooling::fixit::createReplacement( *Cast, getEquivalentForBoolLiteral(BoolLiteral, DestType, Context)); } else { diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index f49c412118e7d98..ab9ef8cdfb37a00 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -412,6 +412,10 @@ Changes in existing checks do-while loops into account for the `AllowIntegerConditions` and `AllowPointerConditions` options. +- Improved :doc:`readability-implicit-bool-conversion + ` check to provide + consistent suggestions when parentheses are added to the return value. + - Improved :doc:`readability-non-const-parameter ` check to ignore false-positives in initializer list of record. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp index 323cf813c047000..f7f5d506a9ce0e0 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp @@ -472,6 +472,36 @@ bool f(S& s) { } // namespace ignore_1bit_bitfields +int implicitConversionReturnInt() +{ +return true; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion bool -> 'int' +// CHECK-FIXES: return 1 +} + +int implicitConversionReturnIntWithParens() +{ +return (true); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion bool -> 'int' +// CHECK-FIXES: return 1 +} + + +bool implicitConversionReturnBool() +{ +return 1; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> bool +// CHECK-FIXES: return true +} + +bool implicitConversionReturnBoolWithParens() +{ +return (1); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> bool +// CHECK-FIXES: return true +} + + namespace PR47000 { int to_int(bool x) { return int{x}; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses (PR #72068)
felix642 wrote: Hi @firewave, I think you are referencing a different issue. If I test #71852 with PR #72050 I do not get the expected behavior. https://github.com/llvm/llvm-project/pull/72068 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses (PR #72068)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/72068 From 649c7b8b936d848100b58e733dd358fa1d5914fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 12 Nov 2023 16:07:13 -0500 Subject: [PATCH 1/2] [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses Fixes #71852 --- .../ImplicitBoolConversionCheck.cpp | 5 ++-- clang-tools-extra/docs/ReleaseNotes.rst | 4 +++ .../readability/implicit-bool-conversion.cpp | 30 +++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp index 69e6d73c4fcd7bb..f0fca30de3b3c4d 100644 --- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp @@ -152,7 +152,8 @@ StringRef getEquivalentBoolLiteralForExpr(const Expr *Expression, return "false"; } - if (const auto *IntLit = dyn_cast(Expression)) { + if (const auto *IntLit = + dyn_cast(Expression->IgnoreParens())) { return (IntLit->getValue() == 0) ? "false" : "true"; } @@ -385,7 +386,7 @@ void ImplicitBoolConversionCheck::handleCastFromBool( << DestType; if (const auto *BoolLiteral = - dyn_cast(Cast->getSubExpr())) { + dyn_cast(Cast->getSubExpr()->IgnoreParens())) { Diag << tooling::fixit::createReplacement( *Cast, getEquivalentForBoolLiteral(BoolLiteral, DestType, Context)); } else { diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index f49c412118e7d98..ab9ef8cdfb37a00 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -412,6 +412,10 @@ Changes in existing checks do-while loops into account for the `AllowIntegerConditions` and `AllowPointerConditions` options. +- Improved :doc:`readability-implicit-bool-conversion + ` check to provide + consistent suggestions when parentheses are added to the return value. + - Improved :doc:`readability-non-const-parameter ` check to ignore false-positives in initializer list of record. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp index 323cf813c047000..f7f5d506a9ce0e0 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp @@ -472,6 +472,36 @@ bool f(S& s) { } // namespace ignore_1bit_bitfields +int implicitConversionReturnInt() +{ +return true; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion bool -> 'int' +// CHECK-FIXES: return 1 +} + +int implicitConversionReturnIntWithParens() +{ +return (true); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion bool -> 'int' +// CHECK-FIXES: return 1 +} + + +bool implicitConversionReturnBool() +{ +return 1; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> bool +// CHECK-FIXES: return true +} + +bool implicitConversionReturnBoolWithParens() +{ +return (1); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> bool +// CHECK-FIXES: return true +} + + namespace PR47000 { int to_int(bool x) { return int{x}; } From fc87991090bc83d72c51b7e825dcb726085c983c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Mon, 13 Nov 2023 20:41:43 -0500 Subject: [PATCH 2/2] fixup! [RISCV][GISel] Sink more code into getOperandsForBranch. NFC Updated ReleaseNotes.rst --- clang-tools-extra/docs/ReleaseNotes.rst | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ab9ef8cdfb37a00..2d1c222b5eaa114 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -410,11 +410,8 @@ Changes in existing checks - Improved :doc:`readability-implicit-bool-conversion ` check to take do-while loops into account for the `AllowIntegerConditions` and - `AllowPointerConditions` options. - -- Improved :doc:`readability-implicit-bool-conversion - ` check to provide - consistent suggestions when parentheses are added to the return value. + `AllowPointerConditions` options. It also now provides more consistent + suggestions when parentheses are added to the return value. - Improved :doc:`readability-non-const-parameter ` check to ignore ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi
[clang-tools-extra] [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses (PR #72068)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/72068 From 649c7b8b936d848100b58e733dd358fa1d5914fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 12 Nov 2023 16:07:13 -0500 Subject: [PATCH 1/2] [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses Fixes #71852 --- .../ImplicitBoolConversionCheck.cpp | 5 ++-- clang-tools-extra/docs/ReleaseNotes.rst | 4 +++ .../readability/implicit-bool-conversion.cpp | 30 +++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp index 69e6d73c4fcd7bb..f0fca30de3b3c4d 100644 --- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp @@ -152,7 +152,8 @@ StringRef getEquivalentBoolLiteralForExpr(const Expr *Expression, return "false"; } - if (const auto *IntLit = dyn_cast(Expression)) { + if (const auto *IntLit = + dyn_cast(Expression->IgnoreParens())) { return (IntLit->getValue() == 0) ? "false" : "true"; } @@ -385,7 +386,7 @@ void ImplicitBoolConversionCheck::handleCastFromBool( << DestType; if (const auto *BoolLiteral = - dyn_cast(Cast->getSubExpr())) { + dyn_cast(Cast->getSubExpr()->IgnoreParens())) { Diag << tooling::fixit::createReplacement( *Cast, getEquivalentForBoolLiteral(BoolLiteral, DestType, Context)); } else { diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index f49c412118e7d98..ab9ef8cdfb37a00 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -412,6 +412,10 @@ Changes in existing checks do-while loops into account for the `AllowIntegerConditions` and `AllowPointerConditions` options. +- Improved :doc:`readability-implicit-bool-conversion + ` check to provide + consistent suggestions when parentheses are added to the return value. + - Improved :doc:`readability-non-const-parameter ` check to ignore false-positives in initializer list of record. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp index 323cf813c047000..f7f5d506a9ce0e0 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp @@ -472,6 +472,36 @@ bool f(S& s) { } // namespace ignore_1bit_bitfields +int implicitConversionReturnInt() +{ +return true; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion bool -> 'int' +// CHECK-FIXES: return 1 +} + +int implicitConversionReturnIntWithParens() +{ +return (true); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion bool -> 'int' +// CHECK-FIXES: return 1 +} + + +bool implicitConversionReturnBool() +{ +return 1; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> bool +// CHECK-FIXES: return true +} + +bool implicitConversionReturnBoolWithParens() +{ +return (1); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> bool +// CHECK-FIXES: return true +} + + namespace PR47000 { int to_int(bool x) { return int{x}; } From 2b54c907b631a2bbb9036a4b0d4ee6188a54093c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Mon, 13 Nov 2023 20:42:43 -0500 Subject: [PATCH 2/2] fixup! [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses Updated ReleaseNotes.rst --- clang-tools-extra/docs/ReleaseNotes.rst | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ab9ef8cdfb37a00..2d1c222b5eaa114 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -410,11 +410,8 @@ Changes in existing checks - Improved :doc:`readability-implicit-bool-conversion ` check to take do-while loops into account for the `AllowIntegerConditions` and - `AllowPointerConditions` options. - -- Improved :doc:`readability-implicit-bool-conversion - ` check to provide - consistent suggestions when parentheses are added to the return value. + `AllowPointerConditions` options. It also now provides more consistent + suggestions when parentheses are added to the return value. - Improved :doc:`readability-non-const-parameter ` check to ignore ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/
[clang-tools-extra] [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses (PR #72068)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/72068 From 649c7b8b936d848100b58e733dd358fa1d5914fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 12 Nov 2023 16:07:13 -0500 Subject: [PATCH 1/3] [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses Fixes #71852 --- .../ImplicitBoolConversionCheck.cpp | 5 ++-- clang-tools-extra/docs/ReleaseNotes.rst | 4 +++ .../readability/implicit-bool-conversion.cpp | 30 +++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp index 69e6d73c4fcd7bb..f0fca30de3b3c4d 100644 --- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp @@ -152,7 +152,8 @@ StringRef getEquivalentBoolLiteralForExpr(const Expr *Expression, return "false"; } - if (const auto *IntLit = dyn_cast(Expression)) { + if (const auto *IntLit = + dyn_cast(Expression->IgnoreParens())) { return (IntLit->getValue() == 0) ? "false" : "true"; } @@ -385,7 +386,7 @@ void ImplicitBoolConversionCheck::handleCastFromBool( << DestType; if (const auto *BoolLiteral = - dyn_cast(Cast->getSubExpr())) { + dyn_cast(Cast->getSubExpr()->IgnoreParens())) { Diag << tooling::fixit::createReplacement( *Cast, getEquivalentForBoolLiteral(BoolLiteral, DestType, Context)); } else { diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index f49c412118e7d98..ab9ef8cdfb37a00 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -412,6 +412,10 @@ Changes in existing checks do-while loops into account for the `AllowIntegerConditions` and `AllowPointerConditions` options. +- Improved :doc:`readability-implicit-bool-conversion + ` check to provide + consistent suggestions when parentheses are added to the return value. + - Improved :doc:`readability-non-const-parameter ` check to ignore false-positives in initializer list of record. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp index 323cf813c047000..f7f5d506a9ce0e0 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp @@ -472,6 +472,36 @@ bool f(S& s) { } // namespace ignore_1bit_bitfields +int implicitConversionReturnInt() +{ +return true; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion bool -> 'int' +// CHECK-FIXES: return 1 +} + +int implicitConversionReturnIntWithParens() +{ +return (true); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion bool -> 'int' +// CHECK-FIXES: return 1 +} + + +bool implicitConversionReturnBool() +{ +return 1; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> bool +// CHECK-FIXES: return true +} + +bool implicitConversionReturnBoolWithParens() +{ +return (1); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> bool +// CHECK-FIXES: return true +} + + namespace PR47000 { int to_int(bool x) { return int{x}; } From 2b54c907b631a2bbb9036a4b0d4ee6188a54093c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Mon, 13 Nov 2023 20:42:43 -0500 Subject: [PATCH 2/3] fixup! [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses Updated ReleaseNotes.rst --- clang-tools-extra/docs/ReleaseNotes.rst | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ab9ef8cdfb37a00..2d1c222b5eaa114 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -410,11 +410,8 @@ Changes in existing checks - Improved :doc:`readability-implicit-bool-conversion ` check to take do-while loops into account for the `AllowIntegerConditions` and - `AllowPointerConditions` options. - -- Improved :doc:`readability-implicit-bool-conversion - ` check to provide - consistent suggestions when parentheses are added to the return value. + `AllowPointerConditions` options. It also now provides more consistent + suggestions when parentheses are added to the return value. - Improved :doc:`readability-non-const-parameter ` check to ignore ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/
[clang-tools-extra] [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses (PR #72068)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/72068 From 65eaa9e01bcd32dccf1e0c9bd3f2bdd3e53ccb2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 12 Nov 2023 16:07:13 -0500 Subject: [PATCH 1/3] [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses Fixes #71852 --- .../ImplicitBoolConversionCheck.cpp | 5 ++-- clang-tools-extra/docs/ReleaseNotes.rst | 4 +++ .../readability/implicit-bool-conversion.cpp | 30 +++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp index 69e6d73c4fcd7bb..f0fca30de3b3c4d 100644 --- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp @@ -152,7 +152,8 @@ StringRef getEquivalentBoolLiteralForExpr(const Expr *Expression, return "false"; } - if (const auto *IntLit = dyn_cast(Expression)) { + if (const auto *IntLit = + dyn_cast(Expression->IgnoreParens())) { return (IntLit->getValue() == 0) ? "false" : "true"; } @@ -385,7 +386,7 @@ void ImplicitBoolConversionCheck::handleCastFromBool( << DestType; if (const auto *BoolLiteral = - dyn_cast(Cast->getSubExpr())) { + dyn_cast(Cast->getSubExpr()->IgnoreParens())) { Diag << tooling::fixit::createReplacement( *Cast, getEquivalentForBoolLiteral(BoolLiteral, DestType, Context)); } else { diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index f49c412118e7d98..ab9ef8cdfb37a00 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -412,6 +412,10 @@ Changes in existing checks do-while loops into account for the `AllowIntegerConditions` and `AllowPointerConditions` options. +- Improved :doc:`readability-implicit-bool-conversion + ` check to provide + consistent suggestions when parentheses are added to the return value. + - Improved :doc:`readability-non-const-parameter ` check to ignore false-positives in initializer list of record. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp index 323cf813c047000..f7f5d506a9ce0e0 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp @@ -472,6 +472,36 @@ bool f(S& s) { } // namespace ignore_1bit_bitfields +int implicitConversionReturnInt() +{ +return true; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion bool -> 'int' +// CHECK-FIXES: return 1 +} + +int implicitConversionReturnIntWithParens() +{ +return (true); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion bool -> 'int' +// CHECK-FIXES: return 1 +} + + +bool implicitConversionReturnBool() +{ +return 1; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> bool +// CHECK-FIXES: return true +} + +bool implicitConversionReturnBoolWithParens() +{ +return (1); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> bool +// CHECK-FIXES: return true +} + + namespace PR47000 { int to_int(bool x) { return int{x}; } From f45ae6f72eb4585620bc567632c0d5b8019db79c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Mon, 13 Nov 2023 20:42:43 -0500 Subject: [PATCH 2/3] fixup! [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses Updated ReleaseNotes.rst --- clang-tools-extra/docs/ReleaseNotes.rst | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ab9ef8cdfb37a00..2d1c222b5eaa114 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -410,11 +410,8 @@ Changes in existing checks - Improved :doc:`readability-implicit-bool-conversion ` check to take do-while loops into account for the `AllowIntegerConditions` and - `AllowPointerConditions` options. - -- Improved :doc:`readability-implicit-bool-conversion - ` check to provide - consistent suggestions when parentheses are added to the return value. + `AllowPointerConditions` options. It also now provides more consistent + suggestions when parentheses are added to the return value. - Improved :doc:`readability-non-const-parameter ` check to ignore ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/73069 From 89281ccb5354e3d6349d10e6f9446194d2d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Thu, 16 Nov 2023 22:03:15 -0500 Subject: [PATCH 01/11] =?UTF-8?q?[clang-tidy]=C2=A0Added=20check=20to=20de?= =?UTF-8?q?tect=20redundant=20inline=20keyword?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This checks find usages of the inline keywork where it is already implicitly defined by the compiler and suggests it's removal. Fixes #72397 --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../RedundantInlineSpecifierCheck.cpp | 99 .../RedundantInlineSpecifierCheck.h | 36 ++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../redundant-inline-specifier.rst| 34 ++ .../redundant-inline-specifier.cpp| 110 ++ clang/include/clang/ASTMatchers/ASTMatchers.h | 2 +- 9 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5452c2d48a4617..811310db8c721a 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -20,6 +20,7 @@ add_clang_library(clangTidyReadabilityModule IdentifierLengthCheck.cpp IdentifierNamingCheck.cpp ImplicitBoolConversionCheck.cpp + RedundantInlineSpecifierCheck.cpp InconsistentDeclarationParameterNameCheck.cpp IsolateDeclarationCheck.cpp MagicNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index b8e6e641432060..3ce7bfecaecba6 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -39,6 +39,7 @@ #include "RedundantControlFlowCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" +#include "RedundantInlineSpecifierCheck.h" #include "RedundantMemberInitCheck.h" #include "RedundantPreprocessorCheck.h" #include "RedundantSmartptrGetCheck.h" @@ -93,6 +94,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-identifier-naming"); CheckFactories.registerCheck( "readability-implicit-bool-conversion"); +CheckFactories.registerCheck( +"readability-redundant-inline-specifier"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp new file mode 100644 index 00..e73b570df75915 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -0,0 +1,99 @@ +//===--- RedundantInlineSpecifierCheck.cpp - +// clang-tidy--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation CurrentLocation = RangeLocation.getBegin(); + Token CurrentToken; + while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts, + true) && + CurrentLocation < RangeLocation.getEnd() && + CurrentToken.isNot(tok::eof)) { +if (CurrentToken.is(tok::raw_identifier)) { + if (C
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/73069 From 89281ccb5354e3d6349d10e6f9446194d2d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Thu, 16 Nov 2023 22:03:15 -0500 Subject: [PATCH 01/12] =?UTF-8?q?[clang-tidy]=C2=A0Added=20check=20to=20de?= =?UTF-8?q?tect=20redundant=20inline=20keyword?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This checks find usages of the inline keywork where it is already implicitly defined by the compiler and suggests it's removal. Fixes #72397 --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../RedundantInlineSpecifierCheck.cpp | 99 .../RedundantInlineSpecifierCheck.h | 36 ++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../redundant-inline-specifier.rst| 34 ++ .../redundant-inline-specifier.cpp| 110 ++ clang/include/clang/ASTMatchers/ASTMatchers.h | 2 +- 9 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5452c2d48a461..811310db8c721 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -20,6 +20,7 @@ add_clang_library(clangTidyReadabilityModule IdentifierLengthCheck.cpp IdentifierNamingCheck.cpp ImplicitBoolConversionCheck.cpp + RedundantInlineSpecifierCheck.cpp InconsistentDeclarationParameterNameCheck.cpp IsolateDeclarationCheck.cpp MagicNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index b8e6e64143206..3ce7bfecaecba 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -39,6 +39,7 @@ #include "RedundantControlFlowCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" +#include "RedundantInlineSpecifierCheck.h" #include "RedundantMemberInitCheck.h" #include "RedundantPreprocessorCheck.h" #include "RedundantSmartptrGetCheck.h" @@ -93,6 +94,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-identifier-naming"); CheckFactories.registerCheck( "readability-implicit-bool-conversion"); +CheckFactories.registerCheck( +"readability-redundant-inline-specifier"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp new file mode 100644 index 0..e73b570df7591 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -0,0 +1,99 @@ +//===--- RedundantInlineSpecifierCheck.cpp - +// clang-tidy--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation CurrentLocation = RangeLocation.getBegin(); + Token CurrentToken; + while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts, + true) && + CurrentLocation < RangeLocation.getEnd() && + CurrentToken.isNot(tok::eof)) { +if (CurrentToken.is(tok::raw_identifier)) { + if (Current
[clang] [clang-tools-extra] [clang-tidy] Added check to detect redundant inline keyword (PR #73069)
https://github.com/felix642 created https://github.com/llvm/llvm-project/pull/73069 This checks find usages of the inline keywork where it is already implicitly defined by the compiler and suggests it's removal. Fixes #72397 From 894c3c837725c75f8ad19a185139d07b10fb2e0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Thu, 16 Nov 2023 22:03:15 -0500 Subject: [PATCH] =?UTF-8?q?[clang-tidy]=C2=A0Added=20check=20to=20detect?= =?UTF-8?q?=20redundant=20inline=20keyword?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This checks find usages of the inline keywork where it is already implicitly defined by the compiler and suggests it's removal. Fixes #72397 --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../RedundantInlineSpecifierCheck.cpp | 94 +++ .../RedundantInlineSpecifierCheck.h | 36 ++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../redundant-inline-specifier.rst| 34 ++ .../redundant-inline-specifier.cpp| 110 ++ clang/include/clang/ASTMatchers/ASTMatchers.h | 2 +- 9 files changed, 285 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5452c2d48a46173..811310db8c721a6 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -20,6 +20,7 @@ add_clang_library(clangTidyReadabilityModule IdentifierLengthCheck.cpp IdentifierNamingCheck.cpp ImplicitBoolConversionCheck.cpp + RedundantInlineSpecifierCheck.cpp InconsistentDeclarationParameterNameCheck.cpp IsolateDeclarationCheck.cpp MagicNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index b8e6e6414320600..3ce7bfecaecba64 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -39,6 +39,7 @@ #include "RedundantControlFlowCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" +#include "RedundantInlineSpecifierCheck.h" #include "RedundantMemberInitCheck.h" #include "RedundantPreprocessorCheck.h" #include "RedundantSmartptrGetCheck.h" @@ -93,6 +94,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-identifier-naming"); CheckFactories.registerCheck( "readability-implicit-bool-conversion"); +CheckFactories.registerCheck( +"readability-redundant-inline-specifier"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp new file mode 100644 index 000..7b67a7419c708b7 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -0,0 +1,94 @@ +//===--- RedundantInlineSpecifierCheck.cpp - clang-tidy--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, + const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation CurrentLocation = RangeLocation.getBegin(); + Token CurrentToken; + while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts, + tr
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
https://github.com/felix642 edited https://github.com/llvm/llvm-project/pull/73069 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/73069 From 89281ccb5354e3d6349d10e6f9446194d2d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Thu, 16 Nov 2023 22:03:15 -0500 Subject: [PATCH] =?UTF-8?q?[clang-tidy]=C2=A0Added=20check=20to=20detect?= =?UTF-8?q?=20redundant=20inline=20keyword?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This checks find usages of the inline keywork where it is already implicitly defined by the compiler and suggests it's removal. Fixes #72397 --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../RedundantInlineSpecifierCheck.cpp | 99 .../RedundantInlineSpecifierCheck.h | 36 ++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../redundant-inline-specifier.rst| 34 ++ .../redundant-inline-specifier.cpp| 110 ++ clang/include/clang/ASTMatchers/ASTMatchers.h | 2 +- 9 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5452c2d48a46173..811310db8c721a6 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -20,6 +20,7 @@ add_clang_library(clangTidyReadabilityModule IdentifierLengthCheck.cpp IdentifierNamingCheck.cpp ImplicitBoolConversionCheck.cpp + RedundantInlineSpecifierCheck.cpp InconsistentDeclarationParameterNameCheck.cpp IsolateDeclarationCheck.cpp MagicNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index b8e6e6414320600..3ce7bfecaecba64 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -39,6 +39,7 @@ #include "RedundantControlFlowCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" +#include "RedundantInlineSpecifierCheck.h" #include "RedundantMemberInitCheck.h" #include "RedundantPreprocessorCheck.h" #include "RedundantSmartptrGetCheck.h" @@ -93,6 +94,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-identifier-naming"); CheckFactories.registerCheck( "readability-implicit-bool-conversion"); +CheckFactories.registerCheck( +"readability-redundant-inline-specifier"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp new file mode 100644 index 000..e73b570df759153 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -0,0 +1,99 @@ +//===--- RedundantInlineSpecifierCheck.cpp - +// clang-tidy--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation CurrentLocation = RangeLocation.getBegin(); + Token CurrentToken; + while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts, + true) && + CurrentLocation < RangeLocation.getEnd() && + CurrentToken.isNot(tok::eof)) { +if (CurrentToken.is(tok::raw_identifier)) { + if (C
[clang-tools-extra] [clang] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/73069 From 89281ccb5354e3d6349d10e6f9446194d2d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Thu, 16 Nov 2023 22:03:15 -0500 Subject: [PATCH 1/2] =?UTF-8?q?[clang-tidy]=C2=A0Added=20check=20to=20dete?= =?UTF-8?q?ct=20redundant=20inline=20keyword?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This checks find usages of the inline keywork where it is already implicitly defined by the compiler and suggests it's removal. Fixes #72397 --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../RedundantInlineSpecifierCheck.cpp | 99 .../RedundantInlineSpecifierCheck.h | 36 ++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../redundant-inline-specifier.rst| 34 ++ .../redundant-inline-specifier.cpp| 110 ++ clang/include/clang/ASTMatchers/ASTMatchers.h | 2 +- 9 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5452c2d48a46173..811310db8c721a6 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -20,6 +20,7 @@ add_clang_library(clangTidyReadabilityModule IdentifierLengthCheck.cpp IdentifierNamingCheck.cpp ImplicitBoolConversionCheck.cpp + RedundantInlineSpecifierCheck.cpp InconsistentDeclarationParameterNameCheck.cpp IsolateDeclarationCheck.cpp MagicNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index b8e6e6414320600..3ce7bfecaecba64 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -39,6 +39,7 @@ #include "RedundantControlFlowCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" +#include "RedundantInlineSpecifierCheck.h" #include "RedundantMemberInitCheck.h" #include "RedundantPreprocessorCheck.h" #include "RedundantSmartptrGetCheck.h" @@ -93,6 +94,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-identifier-naming"); CheckFactories.registerCheck( "readability-implicit-bool-conversion"); +CheckFactories.registerCheck( +"readability-redundant-inline-specifier"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp new file mode 100644 index 000..e73b570df759153 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -0,0 +1,99 @@ +//===--- RedundantInlineSpecifierCheck.cpp - +// clang-tidy--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation CurrentLocation = RangeLocation.getBegin(); + Token CurrentToken; + while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts, + true) && + CurrentLocation < RangeLocation.getEnd() && + CurrentToken.isNot(tok::eof)) { +if (CurrentToken.is(tok::raw_identifier)) { + i
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
@@ -0,0 +1,113 @@ +//===--- RedundantInlineSpecifierCheck.cpp - clang-tidy===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Lex/Token.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +AST_POLYMORPHIC_MATCHER(isInlineSpecified, +AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl, +VarDecl)) { + if (const auto *FD = dyn_cast(&Node)) +return FD->isInlineSpecified(); + if (const auto *VD = dyn_cast(&Node)) +return VD->isInlineSpecified(); + llvm_unreachable("Not a valid polymorphic type"); +} + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation Loc = RangeLocation.getBegin(); + if (Loc.isMacroID()) +return std::nullopt; + + Token FirstToken; + Lexer::getRawToken(Loc, FirstToken, Sources, LangOpts, true); + std::optional CurrentToken = FirstToken; + while (CurrentToken && CurrentToken->getLocation() < RangeLocation.getEnd() && + CurrentToken->isNot(tok::eof)) { +if (CurrentToken->is(tok::raw_identifier) && +CurrentToken->getRawIdentifier() == "inline") + return CurrentToken->getLocation(); + +CurrentToken = +Lexer::findNextToken(CurrentToken->getLocation(), Sources, LangOpts); + } + return std::nullopt; +} + +void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + functionDecl(isInlineSpecified(), + anyOf(isConstexpr(), isDeleted(), isDefaulted(), + isInAnonymousNamespace(), felix642 wrote: Adding a parameter to differentiate "implicit" from "redundant" inline declarations seems like a good idea. Some users might be using it to hint the optimizer and might want to keep it if it's not implicit. I will add the parameter "StrictMode" suggested by @PiotrZSL which will be an extra condition to flag the following cases if they are not constexpr: - templates - functions declared in an anonymous namespace - functions marked as static https://github.com/llvm/llvm-project/pull/73069 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/73069 From 89281ccb5354e3d6349d10e6f9446194d2d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Thu, 16 Nov 2023 22:03:15 -0500 Subject: [PATCH 01/13] =?UTF-8?q?[clang-tidy]=C2=A0Added=20check=20to=20de?= =?UTF-8?q?tect=20redundant=20inline=20keyword?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This checks find usages of the inline keywork where it is already implicitly defined by the compiler and suggests it's removal. Fixes #72397 --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../RedundantInlineSpecifierCheck.cpp | 99 .../RedundantInlineSpecifierCheck.h | 36 ++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../redundant-inline-specifier.rst| 34 ++ .../redundant-inline-specifier.cpp| 110 ++ clang/include/clang/ASTMatchers/ASTMatchers.h | 2 +- 9 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5452c2d48a4617..811310db8c721a 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -20,6 +20,7 @@ add_clang_library(clangTidyReadabilityModule IdentifierLengthCheck.cpp IdentifierNamingCheck.cpp ImplicitBoolConversionCheck.cpp + RedundantInlineSpecifierCheck.cpp InconsistentDeclarationParameterNameCheck.cpp IsolateDeclarationCheck.cpp MagicNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index b8e6e641432060..3ce7bfecaecba6 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -39,6 +39,7 @@ #include "RedundantControlFlowCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" +#include "RedundantInlineSpecifierCheck.h" #include "RedundantMemberInitCheck.h" #include "RedundantPreprocessorCheck.h" #include "RedundantSmartptrGetCheck.h" @@ -93,6 +94,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-identifier-naming"); CheckFactories.registerCheck( "readability-implicit-bool-conversion"); +CheckFactories.registerCheck( +"readability-redundant-inline-specifier"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp new file mode 100644 index 00..e73b570df75915 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -0,0 +1,99 @@ +//===--- RedundantInlineSpecifierCheck.cpp - +// clang-tidy--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation CurrentLocation = RangeLocation.getBegin(); + Token CurrentToken; + while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts, + true) && + CurrentLocation < RangeLocation.getEnd() && + CurrentToken.isNot(tok::eof)) { +if (CurrentToken.is(tok::raw_identifier)) { + if (C
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
@@ -0,0 +1,99 @@ +//===--- RedundantInlineSpecifierCheck.cpp - +// clang-tidy--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation CurrentLocation = RangeLocation.getBegin(); + Token CurrentToken; + while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts, + true) && + CurrentLocation < RangeLocation.getEnd() && + CurrentToken.isNot(tok::eof)) { +if (CurrentToken.is(tok::raw_identifier)) { + if (CurrentToken.getRawIdentifier() == "inline") { +return CurrentToken.getLocation(); + } +} +CurrentLocation = CurrentToken.getEndLoc(); + } + return std::nullopt; +} + +void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + functionDecl(unless(isExpansionInSystemHeader()), unless(isImplicit()), + unless(hasAncestor(lambdaExpr())), isInline(), + anyOf(isConstexpr(), isDeleted(), + allOf(isDefinition(), hasAncestor(recordDecl()), + unless(hasAncestor(cxxConstructorDecl()) + .bind("fun_decl"), + this); + + Finder->addMatcher( + varDecl(isInline(), unless(isImplicit()), + anyOf(allOf(isConstexpr(), unless(isStaticStorageClass())), felix642 wrote: The description of this matcher says : >Matches variable/function declarations that have "static" storage >class specifier ("static" keyword) written in the source. From what I understand, it needs to have the "static" keyword written in the source. Is there a better matcher that I could use in that case? https://github.com/llvm/llvm-project/pull/73069 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/73069 From 89281ccb5354e3d6349d10e6f9446194d2d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Thu, 16 Nov 2023 22:03:15 -0500 Subject: [PATCH 1/3] =?UTF-8?q?[clang-tidy]=C2=A0Added=20check=20to=20dete?= =?UTF-8?q?ct=20redundant=20inline=20keyword?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This checks find usages of the inline keywork where it is already implicitly defined by the compiler and suggests it's removal. Fixes #72397 --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../RedundantInlineSpecifierCheck.cpp | 99 .../RedundantInlineSpecifierCheck.h | 36 ++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../redundant-inline-specifier.rst| 34 ++ .../redundant-inline-specifier.cpp| 110 ++ clang/include/clang/ASTMatchers/ASTMatchers.h | 2 +- 9 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5452c2d48a46173..811310db8c721a6 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -20,6 +20,7 @@ add_clang_library(clangTidyReadabilityModule IdentifierLengthCheck.cpp IdentifierNamingCheck.cpp ImplicitBoolConversionCheck.cpp + RedundantInlineSpecifierCheck.cpp InconsistentDeclarationParameterNameCheck.cpp IsolateDeclarationCheck.cpp MagicNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index b8e6e6414320600..3ce7bfecaecba64 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -39,6 +39,7 @@ #include "RedundantControlFlowCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" +#include "RedundantInlineSpecifierCheck.h" #include "RedundantMemberInitCheck.h" #include "RedundantPreprocessorCheck.h" #include "RedundantSmartptrGetCheck.h" @@ -93,6 +94,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-identifier-naming"); CheckFactories.registerCheck( "readability-implicit-bool-conversion"); +CheckFactories.registerCheck( +"readability-redundant-inline-specifier"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp new file mode 100644 index 000..e73b570df759153 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -0,0 +1,99 @@ +//===--- RedundantInlineSpecifierCheck.cpp - +// clang-tidy--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation CurrentLocation = RangeLocation.getBegin(); + Token CurrentToken; + while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts, + true) && + CurrentLocation < RangeLocation.getEnd() && + CurrentToken.isNot(tok::eof)) { +if (CurrentToken.is(tok::raw_identifier)) { + i
[clang-tools-extra] [clang] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/73069 From 89281ccb5354e3d6349d10e6f9446194d2d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Thu, 16 Nov 2023 22:03:15 -0500 Subject: [PATCH 1/4] =?UTF-8?q?[clang-tidy]=C2=A0Added=20check=20to=20dete?= =?UTF-8?q?ct=20redundant=20inline=20keyword?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This checks find usages of the inline keywork where it is already implicitly defined by the compiler and suggests it's removal. Fixes #72397 --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../RedundantInlineSpecifierCheck.cpp | 99 .../RedundantInlineSpecifierCheck.h | 36 ++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../redundant-inline-specifier.rst| 34 ++ .../redundant-inline-specifier.cpp| 110 ++ clang/include/clang/ASTMatchers/ASTMatchers.h | 2 +- 9 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5452c2d48a46173..811310db8c721a6 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -20,6 +20,7 @@ add_clang_library(clangTidyReadabilityModule IdentifierLengthCheck.cpp IdentifierNamingCheck.cpp ImplicitBoolConversionCheck.cpp + RedundantInlineSpecifierCheck.cpp InconsistentDeclarationParameterNameCheck.cpp IsolateDeclarationCheck.cpp MagicNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index b8e6e6414320600..3ce7bfecaecba64 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -39,6 +39,7 @@ #include "RedundantControlFlowCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" +#include "RedundantInlineSpecifierCheck.h" #include "RedundantMemberInitCheck.h" #include "RedundantPreprocessorCheck.h" #include "RedundantSmartptrGetCheck.h" @@ -93,6 +94,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-identifier-naming"); CheckFactories.registerCheck( "readability-implicit-bool-conversion"); +CheckFactories.registerCheck( +"readability-redundant-inline-specifier"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp new file mode 100644 index 000..e73b570df759153 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -0,0 +1,99 @@ +//===--- RedundantInlineSpecifierCheck.cpp - +// clang-tidy--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation CurrentLocation = RangeLocation.getBegin(); + Token CurrentToken; + while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts, + true) && + CurrentLocation < RangeLocation.getEnd() && + CurrentToken.isNot(tok::eof)) { +if (CurrentToken.is(tok::raw_identifier)) { + i
[clang-tools-extra] [clang] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
@@ -0,0 +1,99 @@ +//===--- RedundantInlineSpecifierCheck.cpp - +// clang-tidy--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation CurrentLocation = RangeLocation.getBegin(); + Token CurrentToken; + while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts, felix642 wrote: Hi @PiotrZSL, thank you for the feedback. I've made some changes to the method, let me know if that's what you meant. https://github.com/llvm/llvm-project/pull/73069 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/73069 From 89281ccb5354e3d6349d10e6f9446194d2d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Thu, 16 Nov 2023 22:03:15 -0500 Subject: [PATCH 1/5] =?UTF-8?q?[clang-tidy]=C2=A0Added=20check=20to=20dete?= =?UTF-8?q?ct=20redundant=20inline=20keyword?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This checks find usages of the inline keywork where it is already implicitly defined by the compiler and suggests it's removal. Fixes #72397 --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../RedundantInlineSpecifierCheck.cpp | 99 .../RedundantInlineSpecifierCheck.h | 36 ++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../redundant-inline-specifier.rst| 34 ++ .../redundant-inline-specifier.cpp| 110 ++ clang/include/clang/ASTMatchers/ASTMatchers.h | 2 +- 9 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5452c2d48a46173..811310db8c721a6 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -20,6 +20,7 @@ add_clang_library(clangTidyReadabilityModule IdentifierLengthCheck.cpp IdentifierNamingCheck.cpp ImplicitBoolConversionCheck.cpp + RedundantInlineSpecifierCheck.cpp InconsistentDeclarationParameterNameCheck.cpp IsolateDeclarationCheck.cpp MagicNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index b8e6e6414320600..3ce7bfecaecba64 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -39,6 +39,7 @@ #include "RedundantControlFlowCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" +#include "RedundantInlineSpecifierCheck.h" #include "RedundantMemberInitCheck.h" #include "RedundantPreprocessorCheck.h" #include "RedundantSmartptrGetCheck.h" @@ -93,6 +94,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-identifier-naming"); CheckFactories.registerCheck( "readability-implicit-bool-conversion"); +CheckFactories.registerCheck( +"readability-redundant-inline-specifier"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp new file mode 100644 index 000..e73b570df759153 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -0,0 +1,99 @@ +//===--- RedundantInlineSpecifierCheck.cpp - +// clang-tidy--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation CurrentLocation = RangeLocation.getBegin(); + Token CurrentToken; + while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts, + true) && + CurrentLocation < RangeLocation.getEnd() && + CurrentToken.isNot(tok::eof)) { +if (CurrentToken.is(tok::raw_identifier)) { + i
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
@@ -0,0 +1,99 @@ +//===--- RedundantInlineSpecifierCheck.cpp - +// clang-tidy--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation CurrentLocation = RangeLocation.getBegin(); + Token CurrentToken; + while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts, + true) && + CurrentLocation < RangeLocation.getEnd() && + CurrentToken.isNot(tok::eof)) { +if (CurrentToken.is(tok::raw_identifier)) { + if (CurrentToken.getRawIdentifier() == "inline") { +return CurrentToken.getLocation(); + } +} +CurrentLocation = CurrentToken.getEndLoc(); + } + return std::nullopt; +} + +void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + functionDecl(unless(isExpansionInSystemHeader()), unless(isImplicit()), + unless(hasAncestor(lambdaExpr())), isInline(), felix642 wrote: Using `TK_IgnoreUnlessSpelledInSource` instead of `isImplicit()` seems to have to solved this issue. https://github.com/llvm/llvm-project/pull/73069 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
@@ -0,0 +1,25 @@ +.. title:: clang-tidy - readability-redundant-inline-specifier + +readability-redundant-inline-specifier +== + +Checks for instances of the ``inline`` keyword in code where it is redundant felix642 wrote: I apologies, but I'm not quite sure to understand what you're asking me to do here? Would you be able to give me more info? https://github.com/llvm/llvm-project/pull/73069 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
@@ -0,0 +1,109 @@ +//===--- RedundantInlineSpecifierCheck.cpp - clang-tidy===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Lex/Token.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +AST_POLYMORPHIC_MATCHER(isInlineSpecified, +AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl, +VarDecl)) { + if (const auto *FD = dyn_cast(&Node)) +return FD->isInlineSpecified(); + if (const auto *VD = dyn_cast(&Node)) +return VD->isInlineSpecified(); + llvm_unreachable("Not a valid polymorphic type"); +} + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + Token FirstToken; + Lexer::getRawToken(RangeLocation.getBegin(), FirstToken, Sources, LangOpts, + true); + std::optional CurrentToken = FirstToken; + while (CurrentToken && CurrentToken->getLocation() < RangeLocation.getEnd() && + CurrentToken->isNot(tok::eof)) { +if (CurrentToken->is(tok::raw_identifier) && +CurrentToken->getRawIdentifier() == "inline") + return CurrentToken->getLocation(); + +CurrentToken = +Lexer::findNextToken(CurrentToken->getLocation(), Sources, LangOpts); + } + return std::nullopt; +} + +void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + functionDecl(unless(isExpansionInSystemHeader()), isInlineSpecified(), + anyOf(isConstexpr(), isDeleted(), + allOf(isDefinition(), hasAncestor(recordDecl() + .bind("fun_decl"), + this); + + Finder->addMatcher( + varDecl(isInlineSpecified(), + anyOf(allOf(isConstexpr(), unless(isStaticStorageClass())), +hasAncestor(recordDecl( + .bind("var_decl"), + this); + + Finder->addMatcher( + functionTemplateDecl(has(functionDecl(isInlineSpecified( felix642 wrote: Well, I think that it's fair to emit a warning if either a forward declaration of a function definition has a redundant inline specifier. In both case we want to suggest to remove it don't we ? https://github.com/llvm/llvm-project/pull/73069 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
@@ -0,0 +1,109 @@ +//===--- RedundantInlineSpecifierCheck.cpp - clang-tidy===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Lex/Token.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +AST_POLYMORPHIC_MATCHER(isInlineSpecified, +AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl, +VarDecl)) { + if (const auto *FD = dyn_cast(&Node)) +return FD->isInlineSpecified(); + if (const auto *VD = dyn_cast(&Node)) +return VD->isInlineSpecified(); + llvm_unreachable("Not a valid polymorphic type"); +} + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + Token FirstToken; + Lexer::getRawToken(RangeLocation.getBegin(), FirstToken, Sources, LangOpts, + true); + std::optional CurrentToken = FirstToken; + while (CurrentToken && CurrentToken->getLocation() < RangeLocation.getEnd() && + CurrentToken->isNot(tok::eof)) { +if (CurrentToken->is(tok::raw_identifier) && +CurrentToken->getRawIdentifier() == "inline") + return CurrentToken->getLocation(); + +CurrentToken = +Lexer::findNextToken(CurrentToken->getLocation(), Sources, LangOpts); + } + return std::nullopt; +} + +void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + functionDecl(unless(isExpansionInSystemHeader()), isInlineSpecified(), + anyOf(isConstexpr(), isDeleted(), felix642 wrote: From what I can see default methods are implicitly inlined : https://cppinsights.io/s/fb9b0f8e. I've adjusted my matcher to take them into consideration. Let me know if it's ok with you. https://github.com/llvm/llvm-project/pull/73069 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/73069 From 89281ccb5354e3d6349d10e6f9446194d2d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Thu, 16 Nov 2023 22:03:15 -0500 Subject: [PATCH 1/6] =?UTF-8?q?[clang-tidy]=C2=A0Added=20check=20to=20dete?= =?UTF-8?q?ct=20redundant=20inline=20keyword?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This checks find usages of the inline keywork where it is already implicitly defined by the compiler and suggests it's removal. Fixes #72397 --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../RedundantInlineSpecifierCheck.cpp | 99 .../RedundantInlineSpecifierCheck.h | 36 ++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../redundant-inline-specifier.rst| 34 ++ .../redundant-inline-specifier.cpp| 110 ++ clang/include/clang/ASTMatchers/ASTMatchers.h | 2 +- 9 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5452c2d48a46173..811310db8c721a6 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -20,6 +20,7 @@ add_clang_library(clangTidyReadabilityModule IdentifierLengthCheck.cpp IdentifierNamingCheck.cpp ImplicitBoolConversionCheck.cpp + RedundantInlineSpecifierCheck.cpp InconsistentDeclarationParameterNameCheck.cpp IsolateDeclarationCheck.cpp MagicNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index b8e6e6414320600..3ce7bfecaecba64 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -39,6 +39,7 @@ #include "RedundantControlFlowCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" +#include "RedundantInlineSpecifierCheck.h" #include "RedundantMemberInitCheck.h" #include "RedundantPreprocessorCheck.h" #include "RedundantSmartptrGetCheck.h" @@ -93,6 +94,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-identifier-naming"); CheckFactories.registerCheck( "readability-implicit-bool-conversion"); +CheckFactories.registerCheck( +"readability-redundant-inline-specifier"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp new file mode 100644 index 000..e73b570df759153 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -0,0 +1,99 @@ +//===--- RedundantInlineSpecifierCheck.cpp - +// clang-tidy--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation CurrentLocation = RangeLocation.getBegin(); + Token CurrentToken; + while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts, + true) && + CurrentLocation < RangeLocation.getEnd() && + CurrentToken.isNot(tok::eof)) { +if (CurrentToken.is(tok::raw_identifier)) { + i
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
@@ -0,0 +1,109 @@ +//===--- RedundantInlineSpecifierCheck.cpp - clang-tidy===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Lex/Token.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +AST_POLYMORPHIC_MATCHER(isInlineSpecified, +AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl, +VarDecl)) { + if (const auto *FD = dyn_cast(&Node)) +return FD->isInlineSpecified(); + if (const auto *VD = dyn_cast(&Node)) +return VD->isInlineSpecified(); + llvm_unreachable("Not a valid polymorphic type"); +} + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + Token FirstToken; + Lexer::getRawToken(RangeLocation.getBegin(), FirstToken, Sources, LangOpts, + true); + std::optional CurrentToken = FirstToken; + while (CurrentToken && CurrentToken->getLocation() < RangeLocation.getEnd() && + CurrentToken->isNot(tok::eof)) { +if (CurrentToken->is(tok::raw_identifier) && +CurrentToken->getRawIdentifier() == "inline") + return CurrentToken->getLocation(); + +CurrentToken = +Lexer::findNextToken(CurrentToken->getLocation(), Sources, LangOpts); + } + return std::nullopt; +} + +void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + functionDecl(unless(isExpansionInSystemHeader()), isInlineSpecified(), + anyOf(isConstexpr(), isDeleted(), + allOf(isDefinition(), hasAncestor(recordDecl() + .bind("fun_decl"), + this); + + Finder->addMatcher( + varDecl(isInlineSpecified(), + anyOf(allOf(isConstexpr(), unless(isStaticStorageClass())), felix642 wrote: I've adjusted the matcher to match on variables in anonymous namespace. https://github.com/llvm/llvm-project/pull/73069 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
@@ -0,0 +1,109 @@ +//===--- RedundantInlineSpecifierCheck.cpp - clang-tidy===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Lex/Token.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +AST_POLYMORPHIC_MATCHER(isInlineSpecified, +AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl, +VarDecl)) { + if (const auto *FD = dyn_cast(&Node)) +return FD->isInlineSpecified(); + if (const auto *VD = dyn_cast(&Node)) +return VD->isInlineSpecified(); + llvm_unreachable("Not a valid polymorphic type"); +} + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + Token FirstToken; + Lexer::getRawToken(RangeLocation.getBegin(), FirstToken, Sources, LangOpts, + true); + std::optional CurrentToken = FirstToken; + while (CurrentToken && CurrentToken->getLocation() < RangeLocation.getEnd() && + CurrentToken->isNot(tok::eof)) { +if (CurrentToken->is(tok::raw_identifier) && +CurrentToken->getRawIdentifier() == "inline") + return CurrentToken->getLocation(); + +CurrentToken = +Lexer::findNextToken(CurrentToken->getLocation(), Sources, LangOpts); + } + return std::nullopt; +} + +void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + functionDecl(unless(isExpansionInSystemHeader()), isInlineSpecified(), felix642 wrote: Same here, it should now match on functions in anonymous namespace. https://github.com/llvm/llvm-project/pull/73069 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/73069 From 89281ccb5354e3d6349d10e6f9446194d2d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Thu, 16 Nov 2023 22:03:15 -0500 Subject: [PATCH 1/7] =?UTF-8?q?[clang-tidy]=C2=A0Added=20check=20to=20dete?= =?UTF-8?q?ct=20redundant=20inline=20keyword?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This checks find usages of the inline keywork where it is already implicitly defined by the compiler and suggests it's removal. Fixes #72397 --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../RedundantInlineSpecifierCheck.cpp | 99 .../RedundantInlineSpecifierCheck.h | 36 ++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../redundant-inline-specifier.rst| 34 ++ .../redundant-inline-specifier.cpp| 110 ++ clang/include/clang/ASTMatchers/ASTMatchers.h | 2 +- 9 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5452c2d48a46173..811310db8c721a6 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -20,6 +20,7 @@ add_clang_library(clangTidyReadabilityModule IdentifierLengthCheck.cpp IdentifierNamingCheck.cpp ImplicitBoolConversionCheck.cpp + RedundantInlineSpecifierCheck.cpp InconsistentDeclarationParameterNameCheck.cpp IsolateDeclarationCheck.cpp MagicNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index b8e6e6414320600..3ce7bfecaecba64 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -39,6 +39,7 @@ #include "RedundantControlFlowCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" +#include "RedundantInlineSpecifierCheck.h" #include "RedundantMemberInitCheck.h" #include "RedundantPreprocessorCheck.h" #include "RedundantSmartptrGetCheck.h" @@ -93,6 +94,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-identifier-naming"); CheckFactories.registerCheck( "readability-implicit-bool-conversion"); +CheckFactories.registerCheck( +"readability-redundant-inline-specifier"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp new file mode 100644 index 000..e73b570df759153 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -0,0 +1,99 @@ +//===--- RedundantInlineSpecifierCheck.cpp - +// clang-tidy--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation CurrentLocation = RangeLocation.getBegin(); + Token CurrentToken; + while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts, + true) && + CurrentLocation < RangeLocation.getEnd() && + CurrentToken.isNot(tok::eof)) { +if (CurrentToken.is(tok::raw_identifier)) { + i
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
@@ -0,0 +1,25 @@ +.. title:: clang-tidy - readability-redundant-inline-specifier + +readability-redundant-inline-specifier +== + +Checks for instances of the ``inline`` keyword in code where it is redundant felix642 wrote: Right, I was looking at the wrong file hence why I was confused. https://github.com/llvm/llvm-project/pull/73069 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/73069 From 89281ccb5354e3d6349d10e6f9446194d2d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Thu, 16 Nov 2023 22:03:15 -0500 Subject: [PATCH 1/8] =?UTF-8?q?[clang-tidy]=C2=A0Added=20check=20to=20dete?= =?UTF-8?q?ct=20redundant=20inline=20keyword?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This checks find usages of the inline keywork where it is already implicitly defined by the compiler and suggests it's removal. Fixes #72397 --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../RedundantInlineSpecifierCheck.cpp | 99 .../RedundantInlineSpecifierCheck.h | 36 ++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../redundant-inline-specifier.rst| 34 ++ .../redundant-inline-specifier.cpp| 110 ++ clang/include/clang/ASTMatchers/ASTMatchers.h | 2 +- 9 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5452c2d48a46173..811310db8c721a6 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -20,6 +20,7 @@ add_clang_library(clangTidyReadabilityModule IdentifierLengthCheck.cpp IdentifierNamingCheck.cpp ImplicitBoolConversionCheck.cpp + RedundantInlineSpecifierCheck.cpp InconsistentDeclarationParameterNameCheck.cpp IsolateDeclarationCheck.cpp MagicNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index b8e6e6414320600..3ce7bfecaecba64 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -39,6 +39,7 @@ #include "RedundantControlFlowCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" +#include "RedundantInlineSpecifierCheck.h" #include "RedundantMemberInitCheck.h" #include "RedundantPreprocessorCheck.h" #include "RedundantSmartptrGetCheck.h" @@ -93,6 +94,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-identifier-naming"); CheckFactories.registerCheck( "readability-implicit-bool-conversion"); +CheckFactories.registerCheck( +"readability-redundant-inline-specifier"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp new file mode 100644 index 000..e73b570df759153 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -0,0 +1,99 @@ +//===--- RedundantInlineSpecifierCheck.cpp - +// clang-tidy--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation CurrentLocation = RangeLocation.getBegin(); + Token CurrentToken; + while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts, + true) && + CurrentLocation < RangeLocation.getEnd() && + CurrentToken.isNot(tok::eof)) { +if (CurrentToken.is(tok::raw_identifier)) { + i
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/73069 From 89281ccb5354e3d6349d10e6f9446194d2d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Thu, 16 Nov 2023 22:03:15 -0500 Subject: [PATCH 1/9] =?UTF-8?q?[clang-tidy]=C2=A0Added=20check=20to=20dete?= =?UTF-8?q?ct=20redundant=20inline=20keyword?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This checks find usages of the inline keywork where it is already implicitly defined by the compiler and suggests it's removal. Fixes #72397 --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../RedundantInlineSpecifierCheck.cpp | 99 .../RedundantInlineSpecifierCheck.h | 36 ++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../redundant-inline-specifier.rst| 34 ++ .../redundant-inline-specifier.cpp| 110 ++ clang/include/clang/ASTMatchers/ASTMatchers.h | 2 +- 9 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5452c2d48a46173..811310db8c721a6 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -20,6 +20,7 @@ add_clang_library(clangTidyReadabilityModule IdentifierLengthCheck.cpp IdentifierNamingCheck.cpp ImplicitBoolConversionCheck.cpp + RedundantInlineSpecifierCheck.cpp InconsistentDeclarationParameterNameCheck.cpp IsolateDeclarationCheck.cpp MagicNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index b8e6e6414320600..3ce7bfecaecba64 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -39,6 +39,7 @@ #include "RedundantControlFlowCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" +#include "RedundantInlineSpecifierCheck.h" #include "RedundantMemberInitCheck.h" #include "RedundantPreprocessorCheck.h" #include "RedundantSmartptrGetCheck.h" @@ -93,6 +94,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-identifier-naming"); CheckFactories.registerCheck( "readability-implicit-bool-conversion"); +CheckFactories.registerCheck( +"readability-redundant-inline-specifier"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp new file mode 100644 index 000..e73b570df759153 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -0,0 +1,99 @@ +//===--- RedundantInlineSpecifierCheck.cpp - +// clang-tidy--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation CurrentLocation = RangeLocation.getBegin(); + Token CurrentToken; + while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts, + true) && + CurrentLocation < RangeLocation.getEnd() && + CurrentToken.isNot(tok::eof)) { +if (CurrentToken.is(tok::raw_identifier)) { + i
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/73069 From 89281ccb5354e3d6349d10e6f9446194d2d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Thu, 16 Nov 2023 22:03:15 -0500 Subject: [PATCH 01/10] =?UTF-8?q?[clang-tidy]=C2=A0Added=20check=20to=20de?= =?UTF-8?q?tect=20redundant=20inline=20keyword?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This checks find usages of the inline keywork where it is already implicitly defined by the compiler and suggests it's removal. Fixes #72397 --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../RedundantInlineSpecifierCheck.cpp | 99 .../RedundantInlineSpecifierCheck.h | 36 ++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../redundant-inline-specifier.rst| 34 ++ .../redundant-inline-specifier.cpp| 110 ++ clang/include/clang/ASTMatchers/ASTMatchers.h | 2 +- 9 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5452c2d48a46173..811310db8c721a6 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -20,6 +20,7 @@ add_clang_library(clangTidyReadabilityModule IdentifierLengthCheck.cpp IdentifierNamingCheck.cpp ImplicitBoolConversionCheck.cpp + RedundantInlineSpecifierCheck.cpp InconsistentDeclarationParameterNameCheck.cpp IsolateDeclarationCheck.cpp MagicNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index b8e6e6414320600..3ce7bfecaecba64 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -39,6 +39,7 @@ #include "RedundantControlFlowCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" +#include "RedundantInlineSpecifierCheck.h" #include "RedundantMemberInitCheck.h" #include "RedundantPreprocessorCheck.h" #include "RedundantSmartptrGetCheck.h" @@ -93,6 +94,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-identifier-naming"); CheckFactories.registerCheck( "readability-implicit-bool-conversion"); +CheckFactories.registerCheck( +"readability-redundant-inline-specifier"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp new file mode 100644 index 000..e73b570df759153 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -0,0 +1,99 @@ +//===--- RedundantInlineSpecifierCheck.cpp - +// clang-tidy--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static std::optional +getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation CurrentLocation = RangeLocation.getBegin(); + Token CurrentToken; + while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts, + true) && + CurrentLocation < RangeLocation.getEnd() && + CurrentToken.isNot(tok::eof)) { +if (CurrentToken.is(tok::raw_identifier)) { +
[clang] [clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
@@ -0,0 +1,110 @@ +// RUN: %check_clang_tidy %s readability-redundant-inline-specifier %t + +template inline T f() +// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: function 'f' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: template T f() +{ +return T{}; +} + +template <> inline double f() = delete; +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function 'f' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: template <> double f() = delete; + +inline int g(float a) +// CHECK-MESSAGES-NOT: :[[@LINE-1]]:1: warning: function 'g' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +{ +return static_cast(a - 5.F); +} + +inline int g(double) = delete; +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: function 'g' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: int g(double) = delete; + +class C +{ + public: +inline C& operator=(const C&) = delete; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'operator=' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: C& operator=(const C&) = delete; + +constexpr inline C& operator=(int a); +// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: function 'operator=' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: constexpr C& operator=(int a); + +inline C() {} +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'C' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: C() {} + +constexpr inline C(int); +// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: function 'C' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: constexpr C(int); + +inline int Get42() const { return 42; } +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'Get42' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: int Get42() const { return 42; } + +static inline constexpr int C_STATIC = 42; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'C_STATIC' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: static constexpr int C_STATIC = 42; + +static constexpr int C_STATIC_2 = 42; +// CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: variable 'C_STATIC_2' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +}; + +constexpr inline int Get42() { return 42; } +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: function 'Get42' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: constexpr int Get42() { return 42; } + + +static constexpr inline int NAMESPACE_STATIC = 42; +// CHECK-MESSAGES-NOT: :[[@LINE-1]]:18: warning: variable 'NAMESPACE_STATIC' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] + +inline static int fn0(int i) +// CHECK-MESSAGES-NOT: :[[@LINE-1]]:1: warning: function 'fn0' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +{ +return i - 1; +} + +static constexpr inline int fn1(int i) +// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: function 'fn1' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: static constexpr int fn1(int i) +{ +return i - 1; +} + +namespace +{ +inline int fn2(int i) +// CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: function 'fn2' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +{ +return i - 1; +} + +inline constexpr int fn3(int i) +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'fn3' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: constexpr int fn3(int i) +{ +return i - 1; +} +} + +namespace ns +{ +inline int fn4(int i) +// CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: function 'fn4' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +{ +return i - 1; +} + +inline constexpr int fn5(int i) +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'fn5' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: constexpr int fn5(int i) +{ +return i - 1; +} +} + +auto fn6 = [](){}; +//CHECK-MESSAGES-NOT: :[[@LINE-1]]:1: warning: function 'operator()' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] + felix642 wrote: The current implementation does not raise any warnings if the inline keyword
[clang-tools-extra] [clang-tidy] Invalid Fix-It generated for implicit-widening-multiplication-result (PR #76315)
https://github.com/felix642 created https://github.com/llvm/llvm-project/pull/76315 The check currently emits warnings for the following code: `uint64_t fn() { return 1024 * 1024; }` But the code generated after applying the notes will look like this: `uint64_t fn() { return static_cast(1024 * )1024; }` ``` [:5:12: warning: performing an implicit widening conversion to type 'uint64_t' (aka 'unsigned long') of a multiplication performed in type 'int' [bugprone-implicit-widening-of-multiplication-result]](javascript:;) 5 | return 1024 * 1024; |^ [:5:12: note: make conversion explicit to silence this warning](javascript:;) 1 | #include 2 | 3 | uint64_t fn() 4 | { 5 | return 1024 * 1024; |^~~ |static_cast( ) [:5:12: note: perform multiplication in a wider type](javascript:;) 5 | return 1024 * 1024; |^~~~ |static_cast() 1 warning generated. ``` This is because when generating the notes the check will use the beginLoc() and EndLoc() of the subexpr of the implicit cast. But in some cases the AST Node might not have a beginLoc and EndLoc. This seems to be true when the Node is composed of only 1 token (for example an integer literal). Calling the getEndLoc() on this type of node will simply return the known location which is, in this case, the beginLoc. Fixes #63070 #56728 I was not able to add tests for this commit and this is because the check currently emits two notes for every detection. The first suggestion is to silence the warning by casting the whole operation. The second suggestion is there to fix the issue (And I think the suggested code is invalid, but I've opened another issue for that: https://github.com/llvm/llvm-project/issues/76311). Since both suggestions are overlapping clang-tidy refuses to apply them and I was not able to add new tests. The first suggestion seems useless since we should not suggest a fix-it to silence an issue (the message seems sufficient to me). But this requires more discussion and the change is not trivial so I've left it how it is for now. From 7d9c4d423c00033e8c4df26b87176e3054976423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sat, 23 Dec 2023 23:17:03 -0500 Subject: [PATCH] =?UTF-8?q?[clang-tidy]=C2=A0Invalid=20Fix-It=20generated?= =?UTF-8?q?=20for=20implicit-widening-multiplication-result?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The check currently emits warnings for the following code: uint64_t fn() { return 1024 * 1024; } But the code generated after applying the notes will look like this: uint64_t fn() { return static_cast(1024 * )1024; } This is because when generating the notes the check will use the beginLoc and EndLoc of the subexpr of the implicit cast. But in some cases the AST Node might not have a beginLoc and EndLoc. This seems to be true when the Node is composed of only 1 token (for example an integer literal). Calling the getEndLoc() on this type of node will simply return the known location which is, in this case, the beginLoc. Fixes #63070 #56728 --- ...citWideningOfMultiplicationResultCheck.cpp | 27 +-- clang-tools-extra/docs/ReleaseNotes.rst | 4 +++ ...tion-result-array-subscript-expression.cpp | 4 +-- ...-widening-of-multiplication-result-int.cpp | 8 +++--- ...f-multiplication-result-pointer-offset.cpp | 4 +-- 5 files changed, 31 insertions(+), 16 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp index 95a7c521eabb0e..6f22f02f301835 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp @@ -9,6 +9,7 @@ #include "ImplicitWideningOfMultiplicationResultCheck.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" #include using namespace clang::ast_matchers; @@ -99,15 +100,16 @@ void ImplicitWideningOfMultiplicationResultCheck::handleImplicitCastExpr( "make conversion explicit to silence this warning", DiagnosticIDs::Note) << E->getSourceRange(); - +const SourceLocation EndLoc = Lexer::getLocForEndOfToken( +E->getEndLoc(), 0, *Result->SourceManager, getLangOpts()); if (ShouldUseCXXStaticCast) Diag << FixItHint::CreateInsertion( E->getBeginLoc(), "static_cast<" + Ty.getAsString() + ">(") - << FixItHint::CreateInsertion(E->getEndLoc(), ")"); + << FixItHint::CreateInsertion(EndLoc, ")"); else Diag << FixItHint::CreateInsertion(E->getBeginLoc(),
[clang-tools-extra] [clang-tidy] Invalid Fix-It generated for implicit-widening-multiplication-result (PR #76315)
felix642 wrote: A patch that proposed a similar fix was previously submitted for this issue but never landed : https://reviews.llvm.org/D141058 https://github.com/llvm/llvm-project/pull/76315 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] Fixed clang-tidy rewriter not properly handling symlinks (PR #76350)
https://github.com/felix642 created https://github.com/llvm/llvm-project/pull/76350 Rewriter would not properly fix files if they were symlinked and using --fix with clang-tidy would overwrite the symlink with the corrections rather than the file. With these changes the Rewriter now properly resolves the symlink before applying the fixes. Fixes #60845 From 37bde4bedaa6a80fb5c855a06d790cb0180fa5bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 24 Dec 2023 21:01:32 -0500 Subject: [PATCH] Fixed clang-tidy rewriter not properly handling symlinks Rewriter would not properly fix files if they were symlinked and using --fix with clang-tidy would overwrite the symlink with the corrections rather than the file. With these changes the Rewriter now properly resolves the symlink before applying the fixes. Fixes #60845 --- clang-tools-extra/docs/ReleaseNotes.rst | 2 ++ clang/lib/Rewrite/Rewriter.cpp| 24 +- clang/unittests/libclang/LibclangTest.cpp | 25 +++ 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6d91748e4cef18..8a16ce1a7988a2 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -119,6 +119,8 @@ Improvements to clang-tidy - Improved `--dump-config` to print check options in alphabetical order. +- Improved `--fix` to properly apply corrections on files that are symlinked. + - Improved :program:`clang-tidy-diff.py` script. It now returns exit code `1` if any :program:`clang-tidy` subprocess exits with a non-zero code or if exporting fixes fails. It now accepts a directory as a value for diff --git a/clang/lib/Rewrite/Rewriter.cpp b/clang/lib/Rewrite/Rewriter.cpp index 0e6ae365064463..edc147ec304801 100644 --- a/clang/lib/Rewrite/Rewriter.cpp +++ b/clang/lib/Rewrite/Rewriter.cpp @@ -14,6 +14,7 @@ #include "clang/Rewrite/Core/Rewriter.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticIDs.h" +#include "clang/Basic/FileEntry.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Lex/Lexer.h" @@ -412,16 +413,19 @@ bool Rewriter::overwriteChangedFiles() { unsigned OverwriteFailure = Diag.getCustomDiagID( DiagnosticsEngine::Error, "unable to overwrite file %0: %1"); for (buffer_iterator I = buffer_begin(), E = buffer_end(); I != E; ++I) { -OptionalFileEntryRef Entry = getSourceMgr().getFileEntryRefForID(I->first); -llvm::SmallString<128> Path(Entry->getName()); -getSourceMgr().getFileManager().makeAbsolutePath(Path); -if (auto Error = llvm::writeToOutput(Path, [&](llvm::raw_ostream &OS) { - I->second.write(OS); - return llvm::Error::success(); -})) { - Diag.Report(OverwriteFailure) - << Entry->getName() << llvm::toString(std::move(Error)); - AllWritten = false; +if (OptionalFileEntryRef fileEntry = +getSourceMgr().getFileEntryRefForID(I->first)) { + llvm::StringRef FileName = + getSourceMgr().getFileManager().getCanonicalName(*fileEntry); + if (auto Error = + llvm::writeToOutput(FileName, [&](llvm::raw_ostream &OS) { +I->second.write(OS); +return llvm::Error::success(); + })) { +Diag.Report(OverwriteFailure) +<< FileName << llvm::toString(std::move(Error)); +AllWritten = false; + } } } return !AllWritten; diff --git a/clang/unittests/libclang/LibclangTest.cpp b/clang/unittests/libclang/LibclangTest.cpp index 87075a46d75187..cde19d9004cd81 100644 --- a/clang/unittests/libclang/LibclangTest.cpp +++ b/clang/unittests/libclang/LibclangTest.cpp @@ -16,6 +16,7 @@ #include "llvm/Support/raw_ostream.h" #include "gtest/gtest.h" #include +#include #include #include #include @@ -1379,3 +1380,27 @@ TEST_F(LibclangRewriteTest, RewriteRemove) { ASSERT_EQ(clang_CXRewriter_overwriteChangedFiles(Rew), 0); EXPECT_EQ(getFileContent(Filename), "int () { return 0; }"); } + +TEST_F(LibclangRewriteTest, Symlink) { + std::filesystem::path Symlink = "link.cpp"; + std::filesystem::create_symlink(Filename, Symlink); + ASSERT_TRUE(std::filesystem::exists(Symlink)); + FilesAndDirsToRemove.emplace(Symlink); + + CXTranslationUnit SymTu = clang_parseTranslationUnit( + Index, Symlink.c_str(), nullptr, 0, nullptr, 0, TUFlags); + CXFile SymlinkFile = clang_getFile(SymTu, Symlink.c_str()); + CXRewriter SymRew = clang_CXRewriter_create(SymTu); + + CXSourceLocation B = clang_getLocation(SymTu, SymlinkFile, 1, 5); + CXSourceLocation E = clang_getLocation(SymTu, SymlinkFile, 1, 9); + CXSourceRange Rng = clang_getRange(B, E); + + clang_CXRewriter_removeText(SymRew, Rng); + + ASSERT_EQ(clang_CXRewriter_overwriteChangedFiles(SymRew), 0); + EXPECT_EQ(getFileContent(Filename), "i
[clang-tools-extra] [clang] Fixed clang-tidy rewriter not properly handling symlinks (PR #76350)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/76350 From 0a5ae9bfff7597794889c93e4da5789714be3a4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 24 Dec 2023 21:01:32 -0500 Subject: [PATCH] [clang][tidy] Fixed clang-tidy rewriter not properly handling symlinks Rewriter would not properly fix files if they were symlinked and using --fix with clang-tidy would overwrite the symlink with the corrections rather than the file. With these changes the Rewriter now properly resolves the symlink before applying the fixes. Fixes #60845 --- clang-tools-extra/docs/ReleaseNotes.rst | 2 ++ clang/lib/Rewrite/Rewriter.cpp| 24 +- clang/unittests/libclang/LibclangTest.cpp | 25 +++ 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6d91748e4cef18..8a16ce1a7988a2 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -119,6 +119,8 @@ Improvements to clang-tidy - Improved `--dump-config` to print check options in alphabetical order. +- Improved `--fix` to properly apply corrections on files that are symlinked. + - Improved :program:`clang-tidy-diff.py` script. It now returns exit code `1` if any :program:`clang-tidy` subprocess exits with a non-zero code or if exporting fixes fails. It now accepts a directory as a value for diff --git a/clang/lib/Rewrite/Rewriter.cpp b/clang/lib/Rewrite/Rewriter.cpp index 0e6ae365064463..edc147ec304801 100644 --- a/clang/lib/Rewrite/Rewriter.cpp +++ b/clang/lib/Rewrite/Rewriter.cpp @@ -14,6 +14,7 @@ #include "clang/Rewrite/Core/Rewriter.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticIDs.h" +#include "clang/Basic/FileEntry.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Lex/Lexer.h" @@ -412,16 +413,19 @@ bool Rewriter::overwriteChangedFiles() { unsigned OverwriteFailure = Diag.getCustomDiagID( DiagnosticsEngine::Error, "unable to overwrite file %0: %1"); for (buffer_iterator I = buffer_begin(), E = buffer_end(); I != E; ++I) { -OptionalFileEntryRef Entry = getSourceMgr().getFileEntryRefForID(I->first); -llvm::SmallString<128> Path(Entry->getName()); -getSourceMgr().getFileManager().makeAbsolutePath(Path); -if (auto Error = llvm::writeToOutput(Path, [&](llvm::raw_ostream &OS) { - I->second.write(OS); - return llvm::Error::success(); -})) { - Diag.Report(OverwriteFailure) - << Entry->getName() << llvm::toString(std::move(Error)); - AllWritten = false; +if (OptionalFileEntryRef fileEntry = +getSourceMgr().getFileEntryRefForID(I->first)) { + llvm::StringRef FileName = + getSourceMgr().getFileManager().getCanonicalName(*fileEntry); + if (auto Error = + llvm::writeToOutput(FileName, [&](llvm::raw_ostream &OS) { +I->second.write(OS); +return llvm::Error::success(); + })) { +Diag.Report(OverwriteFailure) +<< FileName << llvm::toString(std::move(Error)); +AllWritten = false; + } } } return !AllWritten; diff --git a/clang/unittests/libclang/LibclangTest.cpp b/clang/unittests/libclang/LibclangTest.cpp index 87075a46d75187..cde19d9004cd81 100644 --- a/clang/unittests/libclang/LibclangTest.cpp +++ b/clang/unittests/libclang/LibclangTest.cpp @@ -16,6 +16,7 @@ #include "llvm/Support/raw_ostream.h" #include "gtest/gtest.h" #include +#include #include #include #include @@ -1379,3 +1380,27 @@ TEST_F(LibclangRewriteTest, RewriteRemove) { ASSERT_EQ(clang_CXRewriter_overwriteChangedFiles(Rew), 0); EXPECT_EQ(getFileContent(Filename), "int () { return 0; }"); } + +TEST_F(LibclangRewriteTest, Symlink) { + std::filesystem::path Symlink = "link.cpp"; + std::filesystem::create_symlink(Filename, Symlink); + ASSERT_TRUE(std::filesystem::exists(Symlink)); + FilesAndDirsToRemove.emplace(Symlink); + + CXTranslationUnit SymTu = clang_parseTranslationUnit( + Index, Symlink.c_str(), nullptr, 0, nullptr, 0, TUFlags); + CXFile SymlinkFile = clang_getFile(SymTu, Symlink.c_str()); + CXRewriter SymRew = clang_CXRewriter_create(SymTu); + + CXSourceLocation B = clang_getLocation(SymTu, SymlinkFile, 1, 5); + CXSourceLocation E = clang_getLocation(SymTu, SymlinkFile, 1, 9); + CXSourceRange Rng = clang_getRange(B, E); + + clang_CXRewriter_removeText(SymRew, Rng); + + ASSERT_EQ(clang_CXRewriter_overwriteChangedFiles(SymRew), 0); + EXPECT_EQ(getFileContent(Filename), "int () { return 0; }"); + EXPECT_TRUE(std::filesystem::is_symlink(Symlink)); + + clang_CXRewriter_dispose(SymRew); +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/li
[clang-tools-extra] [clang] [clang][tidy] Fixed clang-tidy rewriter not properly handling symlinks (PR #76350)
https://github.com/felix642 edited https://github.com/llvm/llvm-project/pull/76350 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Improved --verify-config when using literal style in config file (PR #85591)
https://github.com/felix642 created https://github.com/llvm/llvm-project/pull/85591 Specifying checks using the literal style (|) in the clang-tidy config file is currently supported but was not implemented for the --verify-config options. This means that clang-tidy would work properly but, using the --verify-config option would raise an error due to some checks not being parsed properly. Fixes #53737 CC @SimplyDanny From 2ada09c8e10ecd05e3881995e96cfc2070331f05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 17 Mar 2024 20:50:17 -0400 Subject: [PATCH] [clang-tidy] Improved --verify-config when using literal style in config file Specifying checks using the literal style (|) in the clang-tidy config file is currently supported but was not implemented for the --verify-config options. This means that clang-tidy would work properly but, using the --verify-config option would raise an error due to some checks not being parsed properly. Fixes #53737 --- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp | 9 ++--- clang-tools-extra/docs/ReleaseNotes.rst | 3 +++ .../test/clang-tidy/infrastructure/verify-config.cpp | 12 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp index 9f3d6b6db6cbca..b68a6215b383a6 100644 --- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp +++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp @@ -454,11 +454,14 @@ static constexpr StringLiteral VerifyConfigWarningEnd = " [-verify-config]\n"; static bool verifyChecks(const StringSet<> &AllChecks, StringRef CheckGlob, StringRef Source) { - llvm::StringRef Cur, Rest; + llvm::StringRef Cur = CheckGlob; + llvm::StringRef Rest = CheckGlob; bool AnyInvalid = false; - for (std::tie(Cur, Rest) = CheckGlob.split(','); - !(Cur.empty() && Rest.empty()); std::tie(Cur, Rest) = Rest.split(',')) { + while (!Cur.empty() || !Rest.empty()) { +Cur = Rest.substr(0, Rest.find_first_of(",\n")); +Rest = Rest.substr(Cur.size() + 1); Cur = Cur.trim(); + if (Cur.empty()) continue; Cur.consume_front("-"); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 44680f79de6f54..a699aa9aadd908 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -251,6 +251,9 @@ Miscellaneous option is specified. Now ``clang-apply-replacements`` applies formatting only with the option. +- Fixed ``--verify-check`` option not properly parsing checks when using the + literal operator in the ``.clang-tidy`` config + Improvements to include-fixer - diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp index 421f8641281acb..3659285986482a 100644 --- a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp @@ -18,3 +18,15 @@ // CHECK-VERIFY: command-line option '-checks': warning: check glob 'bad*glob' doesn't match any known check [-verify-config] // CHECK-VERIFY: command-line option '-checks': warning: unknown check 'llvm-includeorder'; did you mean 'llvm-include-order' [-verify-config] // CHECK-VERIFY: command-line option '-checks': warning: unknown check 'my-made-up-check' [-verify-config] + +// RUN: echo -e 'Checks: |\n bugprone-argument-comment\n bugprone-assert-side-effect,\n bugprone-bool-pointer-implicit-conversion\n readability-use-anyof*' > %T/MyClangTidyConfig +// RUN: clang-tidy -verify-config \ +// RUN: --config-file=%T/MyClangTidyConfig | FileCheck %s -check-prefix=CHECK-VERIFY-BLOCK-OK +// CHECK-VERIFY-BLOCK-OK: No config errors detected. + +// RUN: echo -e 'Checks: |\n bugprone-arguments-*\n bugprone-assert-side-effects\n bugprone-bool-pointer-implicit-conversion' > %T/MyClangTidyConfigBad +// RUN: not clang-tidy -verify-config \ +// RUN: --config-file=%T/MyClangTidyConfigBad 2>&1 | FileCheck %s -check-prefix=CHECK-VERIFY-BLOCK-BAD +// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: check glob 'bugprone-arguments-*' doesn't match any known check [-verify-config] +// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: unknown check 'bugprone-assert-side-effects'; did you mean 'bugprone-assert-side-effect' [-verify-config] + ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/82947 From e67122cb2c17395e863725cc25d99afe6c7bb48c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 20:20:59 -0500 Subject: [PATCH] [clang-tidy] Improved modernize-use-using by fixing a false-negative The check needs a parent decl to match but if the typedef is in a function, the parent is a declStmt which is not a decl by itself. Improved the matcher to match on either a decl or a declstmt and extract the decl from the stmt in the latter case. fixes #72179 --- .../clang-tidy/modernize/UseUsingCheck.cpp| 24 +-- clang-tools-extra/docs/ReleaseNotes.rst | 3 ++ .../checkers/modernize/use-using.cpp | 43 ++- 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp index bb05f206c717cea..24eefdb082eb32a 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp @@ -8,6 +8,7 @@ #include "UseUsingCheck.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/DeclGroup.h" #include "clang/Lex/Lexer.h" using namespace clang::ast_matchers; @@ -24,6 +25,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = "extern-c-decl"; static constexpr llvm::StringLiteral ParentDeclName = "parent-decl"; static constexpr llvm::StringLiteral TagDeclName = "tag-decl"; static constexpr llvm::StringLiteral TypedefName = "typedef"; +static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt"; UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), @@ -41,7 +43,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { unless(isInstantiated()), optionally(hasAncestor( linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))), - hasParent(decl().bind(ParentDeclName))) + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName .bind(TypedefName), this); @@ -51,17 +54,32 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { tagDecl( anyOf(allOf(unless(anyOf(isImplicit(), classTemplateSpecializationDecl())), - hasParent(decl().bind(ParentDeclName))), + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName, // We want the parent of the ClassTemplateDecl, not the parent // of the specialization. classTemplateSpecializationDecl(hasAncestor(classTemplateDecl( -hasParent(decl().bind(ParentDeclName))) +anyOf(hasParent(decl().bind(ParentDeclName)), + hasParent(declStmt().bind(DeclStmtName .bind(TagDeclName), this); } void UseUsingCheck::check(const MatchFinder::MatchResult &Result) { const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName); + + if (!ParentDecl) { +const auto *ParentDeclStmt = Result.Nodes.getNodeAs(DeclStmtName); +if (ParentDeclStmt) { + if (ParentDeclStmt->isSingleDecl()) +ParentDecl = ParentDeclStmt->getSingleDecl(); + else +ParentDecl = +ParentDeclStmt->getDeclGroup().getDeclGroup() +[ParentDeclStmt->getDeclGroup().getDeclGroup().size() - 1]; +} + } + if (!ParentDecl) return; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 44680f79de6f543..7e4c75b5c3a2948 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -224,6 +224,9 @@ Changes in existing checks analyzed, se the check now handles the common patterns `const auto e = (*vector_ptr)[i]` and `const auto e = vector_ptr->at(i);`. +- Improved :doc:`modernize-use-using ` + check by adding support for detection of typedefs declared on function level. + - Improved :doc:`readability-implicit-bool-conversion ` check to provide valid fix suggestions for ``static_cast`` without a preceding space and diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp index 462bc984fd3ad56..925e5f9c1ca54e8 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s modernize-use-using %t -- -- -I %S/Inputs/use-using/ +// RUN: %check_clang_tidy %s modernize-use-using %t -- -- -fno-delayed-template-parsing -I %S/Inputs/use-using/ typedef int Type; // CHECK-MESSAGES: :[[@LI
[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)
felix642 wrote: @PiotrZSL I've squashed my changed. When you'll have time, I think we can merge this PR. https://github.com/llvm/llvm-project/pull/82947 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Improved --verify-config when using literal style in config file (PR #85591)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/85591 From f015496c511b4c2898b9f4d65ebfe39a34c5119c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 17 Mar 2024 20:50:17 -0400 Subject: [PATCH 1/3] [clang-tidy] Improved --verify-config when using literal style in config file Specifying checks using the literal style (|) in the clang-tidy config file is currently supported but was not implemented for the --verify-config options. This means that clang-tidy would work properly but, using the --verify-config option would raise an error due to some checks not being parsed properly. Fixes #53737 --- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp | 9 ++--- clang-tools-extra/docs/ReleaseNotes.rst | 3 +++ .../test/clang-tidy/infrastructure/verify-config.cpp | 12 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp index 9f3d6b6db6cbca..b68a6215b383a6 100644 --- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp +++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp @@ -454,11 +454,14 @@ static constexpr StringLiteral VerifyConfigWarningEnd = " [-verify-config]\n"; static bool verifyChecks(const StringSet<> &AllChecks, StringRef CheckGlob, StringRef Source) { - llvm::StringRef Cur, Rest; + llvm::StringRef Cur = CheckGlob; + llvm::StringRef Rest = CheckGlob; bool AnyInvalid = false; - for (std::tie(Cur, Rest) = CheckGlob.split(','); - !(Cur.empty() && Rest.empty()); std::tie(Cur, Rest) = Rest.split(',')) { + while (!Cur.empty() || !Rest.empty()) { +Cur = Rest.substr(0, Rest.find_first_of(",\n")); +Rest = Rest.substr(Cur.size() + 1); Cur = Cur.trim(); + if (Cur.empty()) continue; Cur.consume_front("-"); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index a604e9276668ae..3887384e713896 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -262,6 +262,9 @@ Miscellaneous option is specified. Now ``clang-apply-replacements`` applies formatting only with the option. +- Fixed ``--verify-check`` option not properly parsing checks when using the + literal operator in the ``.clang-tidy`` config + Improvements to include-fixer - diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp index 421f8641281acb..3659285986482a 100644 --- a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp @@ -18,3 +18,15 @@ // CHECK-VERIFY: command-line option '-checks': warning: check glob 'bad*glob' doesn't match any known check [-verify-config] // CHECK-VERIFY: command-line option '-checks': warning: unknown check 'llvm-includeorder'; did you mean 'llvm-include-order' [-verify-config] // CHECK-VERIFY: command-line option '-checks': warning: unknown check 'my-made-up-check' [-verify-config] + +// RUN: echo -e 'Checks: |\n bugprone-argument-comment\n bugprone-assert-side-effect,\n bugprone-bool-pointer-implicit-conversion\n readability-use-anyof*' > %T/MyClangTidyConfig +// RUN: clang-tidy -verify-config \ +// RUN: --config-file=%T/MyClangTidyConfig | FileCheck %s -check-prefix=CHECK-VERIFY-BLOCK-OK +// CHECK-VERIFY-BLOCK-OK: No config errors detected. + +// RUN: echo -e 'Checks: |\n bugprone-arguments-*\n bugprone-assert-side-effects\n bugprone-bool-pointer-implicit-conversion' > %T/MyClangTidyConfigBad +// RUN: not clang-tidy -verify-config \ +// RUN: --config-file=%T/MyClangTidyConfigBad 2>&1 | FileCheck %s -check-prefix=CHECK-VERIFY-BLOCK-BAD +// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: check glob 'bugprone-arguments-*' doesn't match any known check [-verify-config] +// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: unknown check 'bugprone-assert-side-effects'; did you mean 'bugprone-assert-side-effect' [-verify-config] + From 0e2cb54934829f42069b06f599fa9e724bdf40b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sat, 23 Mar 2024 13:30:28 -0400 Subject: [PATCH 2/3] fixup! [clang-tidy] Improved --verify-config when using literal style in config file Partially rewrote verifyChecks() to realign the checks parsing logic. Instead of manually parsing the string of globs, verifyCHecks() now uses a GlobList which is responsible to parse the string and return a list of regexes. This ensures that any changes made to the checks parsing logic has to be made at only one place rather than two. --- clang-tools-extra/clang-tidy/GlobList.cpp | 14 ++-- clang-tools-extra/clang-tidy/GlobList.h | 4 ++ .../clang-tidy/tool/ClangTid
[clang-tools-extra] [clang-tidy] Improved --verify-config when using literal style in config file (PR #85591)
felix642 wrote: Thank you for the review @SimplyDanny, as per your suggestion, I've rewrote part of my fix to unify the parsing logic rather than updating the duplicated code. The method now uses the GlobList which already handles correctly the regexes generation from the list of checks. https://github.com/llvm/llvm-project/pull/85591 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/82947 From 72f2b398d6f41372dc84579a83d273398884c869 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 20:20:59 -0500 Subject: [PATCH] [clang-tidy] Improved modernize-use-using by fixing a false-negative The check needs a parent decl to match but if the typedef is in a function, the parent is a declStmt which is not a decl by itself. Improved the matcher to match on either a decl or a declstmt and extract the decl from the stmt in the latter case. fixes #72179 --- .../clang-tidy/modernize/UseUsingCheck.cpp| 24 +-- clang-tools-extra/docs/ReleaseNotes.rst | 3 ++ .../checkers/modernize/use-using.cpp | 43 ++- 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp index bb05f206c717ce..24eefdb082eb32 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp @@ -8,6 +8,7 @@ #include "UseUsingCheck.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/DeclGroup.h" #include "clang/Lex/Lexer.h" using namespace clang::ast_matchers; @@ -24,6 +25,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = "extern-c-decl"; static constexpr llvm::StringLiteral ParentDeclName = "parent-decl"; static constexpr llvm::StringLiteral TagDeclName = "tag-decl"; static constexpr llvm::StringLiteral TypedefName = "typedef"; +static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt"; UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), @@ -41,7 +43,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { unless(isInstantiated()), optionally(hasAncestor( linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))), - hasParent(decl().bind(ParentDeclName))) + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName .bind(TypedefName), this); @@ -51,17 +54,32 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { tagDecl( anyOf(allOf(unless(anyOf(isImplicit(), classTemplateSpecializationDecl())), - hasParent(decl().bind(ParentDeclName))), + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName, // We want the parent of the ClassTemplateDecl, not the parent // of the specialization. classTemplateSpecializationDecl(hasAncestor(classTemplateDecl( -hasParent(decl().bind(ParentDeclName))) +anyOf(hasParent(decl().bind(ParentDeclName)), + hasParent(declStmt().bind(DeclStmtName .bind(TagDeclName), this); } void UseUsingCheck::check(const MatchFinder::MatchResult &Result) { const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName); + + if (!ParentDecl) { +const auto *ParentDeclStmt = Result.Nodes.getNodeAs(DeclStmtName); +if (ParentDeclStmt) { + if (ParentDeclStmt->isSingleDecl()) +ParentDecl = ParentDeclStmt->getSingleDecl(); + else +ParentDecl = +ParentDeclStmt->getDeclGroup().getDeclGroup() +[ParentDeclStmt->getDeclGroup().getDeclGroup().size() - 1]; +} + } + if (!ParentDecl) return; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index a604e9276668ae..14c7064930bbbc 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -235,6 +235,9 @@ Changes in existing checks analyzed, se the check now handles the common patterns `const auto e = (*vector_ptr)[i]` and `const auto e = vector_ptr->at(i);`. +- Improved :doc:`modernize-use-using ` + check by adding support for detection of typedefs declared on function level. + - Improved :doc:`readability-implicit-bool-conversion ` check to provide valid fix suggestions for ``static_cast`` without a preceding space and diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp index 462bc984fd3ad5..925e5f9c1ca54e 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s modernize-use-using %t -- -- -I %S/Inputs/use-using/ +// RUN: %check_clang_tidy %s modernize-use-using %t -- -- -fno-delayed-template-parsing -I %S/Inputs/use-using/ typedef int Type; // CHECK-MESSAGES: :[[@LINE-1]]
[clang-tools-extra] [clang-tidy] Improved --verify-config when using literal style in config file (PR #85591)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/85591 From f015496c511b4c2898b9f4d65ebfe39a34c5119c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 17 Mar 2024 20:50:17 -0400 Subject: [PATCH 1/4] [clang-tidy] Improved --verify-config when using literal style in config file Specifying checks using the literal style (|) in the clang-tidy config file is currently supported but was not implemented for the --verify-config options. This means that clang-tidy would work properly but, using the --verify-config option would raise an error due to some checks not being parsed properly. Fixes #53737 --- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp | 9 ++--- clang-tools-extra/docs/ReleaseNotes.rst | 3 +++ .../test/clang-tidy/infrastructure/verify-config.cpp | 12 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp index 9f3d6b6db6cbca..b68a6215b383a6 100644 --- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp +++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp @@ -454,11 +454,14 @@ static constexpr StringLiteral VerifyConfigWarningEnd = " [-verify-config]\n"; static bool verifyChecks(const StringSet<> &AllChecks, StringRef CheckGlob, StringRef Source) { - llvm::StringRef Cur, Rest; + llvm::StringRef Cur = CheckGlob; + llvm::StringRef Rest = CheckGlob; bool AnyInvalid = false; - for (std::tie(Cur, Rest) = CheckGlob.split(','); - !(Cur.empty() && Rest.empty()); std::tie(Cur, Rest) = Rest.split(',')) { + while (!Cur.empty() || !Rest.empty()) { +Cur = Rest.substr(0, Rest.find_first_of(",\n")); +Rest = Rest.substr(Cur.size() + 1); Cur = Cur.trim(); + if (Cur.empty()) continue; Cur.consume_front("-"); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index a604e9276668ae..3887384e713896 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -262,6 +262,9 @@ Miscellaneous option is specified. Now ``clang-apply-replacements`` applies formatting only with the option. +- Fixed ``--verify-check`` option not properly parsing checks when using the + literal operator in the ``.clang-tidy`` config + Improvements to include-fixer - diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp index 421f8641281acb..3659285986482a 100644 --- a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp @@ -18,3 +18,15 @@ // CHECK-VERIFY: command-line option '-checks': warning: check glob 'bad*glob' doesn't match any known check [-verify-config] // CHECK-VERIFY: command-line option '-checks': warning: unknown check 'llvm-includeorder'; did you mean 'llvm-include-order' [-verify-config] // CHECK-VERIFY: command-line option '-checks': warning: unknown check 'my-made-up-check' [-verify-config] + +// RUN: echo -e 'Checks: |\n bugprone-argument-comment\n bugprone-assert-side-effect,\n bugprone-bool-pointer-implicit-conversion\n readability-use-anyof*' > %T/MyClangTidyConfig +// RUN: clang-tidy -verify-config \ +// RUN: --config-file=%T/MyClangTidyConfig | FileCheck %s -check-prefix=CHECK-VERIFY-BLOCK-OK +// CHECK-VERIFY-BLOCK-OK: No config errors detected. + +// RUN: echo -e 'Checks: |\n bugprone-arguments-*\n bugprone-assert-side-effects\n bugprone-bool-pointer-implicit-conversion' > %T/MyClangTidyConfigBad +// RUN: not clang-tidy -verify-config \ +// RUN: --config-file=%T/MyClangTidyConfigBad 2>&1 | FileCheck %s -check-prefix=CHECK-VERIFY-BLOCK-BAD +// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: check glob 'bugprone-arguments-*' doesn't match any known check [-verify-config] +// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: unknown check 'bugprone-assert-side-effects'; did you mean 'bugprone-assert-side-effect' [-verify-config] + From 0e2cb54934829f42069b06f599fa9e724bdf40b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sat, 23 Mar 2024 13:30:28 -0400 Subject: [PATCH 2/4] fixup! [clang-tidy] Improved --verify-config when using literal style in config file Partially rewrote verifyChecks() to realign the checks parsing logic. Instead of manually parsing the string of globs, verifyCHecks() now uses a GlobList which is responsible to parse the string and return a list of regexes. This ensures that any changes made to the checks parsing logic has to be made at only one place rather than two. --- clang-tools-extra/clang-tidy/GlobList.cpp | 14 ++-- clang-tools-extra/clang-tidy/GlobList.h | 4 ++ .../clang-tidy/tool/ClangTid
[clang-tools-extra] [clang-tidy] Improved --verify-config when using literal style in config file (PR #85591)
@@ -454,52 +454,31 @@ static constexpr StringLiteral VerifyConfigWarningEnd = " [-verify-config]\n"; static bool verifyChecks(const StringSet<> &AllChecks, StringRef CheckGlob, StringRef Source) { - llvm::StringRef Cur, Rest; + GlobList Globs(CheckGlob); bool AnyInvalid = false; - for (std::tie(Cur, Rest) = CheckGlob.split(','); - !(Cur.empty() && Rest.empty()); std::tie(Cur, Rest) = Rest.split(',')) { -Cur = Cur.trim(); -if (Cur.empty()) + for (const auto &Item : Globs.getItems()) { +const llvm::Regex &Reg = Item.Regex; +const llvm::StringRef Text = Item.Text; +if (Text.starts_with("clang-diagnostic")) continue; -Cur.consume_front("-"); -if (Cur.starts_with("clang-diagnostic")) - continue; -if (Cur.contains('*')) { - SmallString<128> RegexText("^"); - StringRef MetaChars("()^$|*+?.[]\\{}"); - for (char C : Cur) { -if (C == '*') - RegexText.push_back('.'); -else if (MetaChars.contains(C)) - RegexText.push_back('\\'); -RegexText.push_back(C); - } - RegexText.push_back('$'); - llvm::Regex Glob(RegexText); - std::string Error; - if (!Glob.isValid(Error)) { -AnyInvalid = true; -llvm::WithColor::error(llvm::errs(), Source) -<< "building check glob '" << Cur << "' " << Error << "'\n"; -continue; - } - if (llvm::none_of(AllChecks.keys(), -[&Glob](StringRef S) { return Glob.match(S); })) { -AnyInvalid = true; +if (llvm::none_of(AllChecks.keys(), [&Reg](StringRef S) { + llvm::errs() << S << '\n'; felix642 wrote: No, I forgot to remove this print before committing, it should be fixed now. Thank you https://github.com/llvm/llvm-project/pull/85591 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Improved --verify-config when using literal style in config file (PR #85591)
@@ -454,52 +454,31 @@ static constexpr StringLiteral VerifyConfigWarningEnd = " [-verify-config]\n"; static bool verifyChecks(const StringSet<> &AllChecks, StringRef CheckGlob, StringRef Source) { - llvm::StringRef Cur, Rest; + GlobList Globs(CheckGlob); bool AnyInvalid = false; - for (std::tie(Cur, Rest) = CheckGlob.split(','); - !(Cur.empty() && Rest.empty()); std::tie(Cur, Rest) = Rest.split(',')) { -Cur = Cur.trim(); -if (Cur.empty()) + for (const auto &Item : Globs.getItems()) { +const llvm::Regex &Reg = Item.Regex; +const llvm::StringRef Text = Item.Text; +if (Text.starts_with("clang-diagnostic")) continue; -Cur.consume_front("-"); -if (Cur.starts_with("clang-diagnostic")) - continue; -if (Cur.contains('*')) { - SmallString<128> RegexText("^"); - StringRef MetaChars("()^$|*+?.[]\\{}"); - for (char C : Cur) { -if (C == '*') - RegexText.push_back('.'); -else if (MetaChars.contains(C)) - RegexText.push_back('\\'); -RegexText.push_back(C); - } - RegexText.push_back('$'); - llvm::Regex Glob(RegexText); - std::string Error; - if (!Glob.isValid(Error)) { -AnyInvalid = true; -llvm::WithColor::error(llvm::errs(), Source) -<< "building check glob '" << Cur << "' " << Error << "'\n"; -continue; - } - if (llvm::none_of(AllChecks.keys(), -[&Glob](StringRef S) { return Glob.match(S); })) { -AnyInvalid = true; +if (llvm::none_of(AllChecks.keys(), [&Reg](StringRef S) { + llvm::errs() << S << '\n'; + return Reg.match(S); +})) { + AnyInvalid = true; + if (Item.Text.contains('*')) felix642 wrote: You are right, I've changed slightly this function's semantics, but it shouldn't have any observable behaviour change for the end user. Before my change, we would only create a regex and try to match it if the string contains any '*' character. But, since the GlobList implicitly generates a list of Regexes we always try to match the strings through regexp now. This won't make a difference since the generated regex will only match the exact string if it doesn't contain any '*', but it will be a bit less efficient since we are going through pattern matching rather than string comparison. I've moved the condition with the '*' character at the of the method to preserve the error message that is displayed to the user in case we can't find any matching check. (Either with "check glob ... doesn't match" if it contains a '*' or "unknown check ..." if it doesn't). https://github.com/llvm/llvm-project/pull/85591 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Improved --verify-config when using literal style in config file (PR #85591)
https://github.com/felix642 edited https://github.com/llvm/llvm-project/pull/85591 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Improved --verify-config when using literal style in config file (PR #85591)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/85591 From f015496c511b4c2898b9f4d65ebfe39a34c5119c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 17 Mar 2024 20:50:17 -0400 Subject: [PATCH 1/5] [clang-tidy] Improved --verify-config when using literal style in config file Specifying checks using the literal style (|) in the clang-tidy config file is currently supported but was not implemented for the --verify-config options. This means that clang-tidy would work properly but, using the --verify-config option would raise an error due to some checks not being parsed properly. Fixes #53737 --- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp | 9 ++--- clang-tools-extra/docs/ReleaseNotes.rst | 3 +++ .../test/clang-tidy/infrastructure/verify-config.cpp | 12 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp index 9f3d6b6db6cbca..b68a6215b383a6 100644 --- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp +++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp @@ -454,11 +454,14 @@ static constexpr StringLiteral VerifyConfigWarningEnd = " [-verify-config]\n"; static bool verifyChecks(const StringSet<> &AllChecks, StringRef CheckGlob, StringRef Source) { - llvm::StringRef Cur, Rest; + llvm::StringRef Cur = CheckGlob; + llvm::StringRef Rest = CheckGlob; bool AnyInvalid = false; - for (std::tie(Cur, Rest) = CheckGlob.split(','); - !(Cur.empty() && Rest.empty()); std::tie(Cur, Rest) = Rest.split(',')) { + while (!Cur.empty() || !Rest.empty()) { +Cur = Rest.substr(0, Rest.find_first_of(",\n")); +Rest = Rest.substr(Cur.size() + 1); Cur = Cur.trim(); + if (Cur.empty()) continue; Cur.consume_front("-"); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index a604e9276668ae..3887384e713896 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -262,6 +262,9 @@ Miscellaneous option is specified. Now ``clang-apply-replacements`` applies formatting only with the option. +- Fixed ``--verify-check`` option not properly parsing checks when using the + literal operator in the ``.clang-tidy`` config + Improvements to include-fixer - diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp index 421f8641281acb..3659285986482a 100644 --- a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp @@ -18,3 +18,15 @@ // CHECK-VERIFY: command-line option '-checks': warning: check glob 'bad*glob' doesn't match any known check [-verify-config] // CHECK-VERIFY: command-line option '-checks': warning: unknown check 'llvm-includeorder'; did you mean 'llvm-include-order' [-verify-config] // CHECK-VERIFY: command-line option '-checks': warning: unknown check 'my-made-up-check' [-verify-config] + +// RUN: echo -e 'Checks: |\n bugprone-argument-comment\n bugprone-assert-side-effect,\n bugprone-bool-pointer-implicit-conversion\n readability-use-anyof*' > %T/MyClangTidyConfig +// RUN: clang-tidy -verify-config \ +// RUN: --config-file=%T/MyClangTidyConfig | FileCheck %s -check-prefix=CHECK-VERIFY-BLOCK-OK +// CHECK-VERIFY-BLOCK-OK: No config errors detected. + +// RUN: echo -e 'Checks: |\n bugprone-arguments-*\n bugprone-assert-side-effects\n bugprone-bool-pointer-implicit-conversion' > %T/MyClangTidyConfigBad +// RUN: not clang-tidy -verify-config \ +// RUN: --config-file=%T/MyClangTidyConfigBad 2>&1 | FileCheck %s -check-prefix=CHECK-VERIFY-BLOCK-BAD +// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: check glob 'bugprone-arguments-*' doesn't match any known check [-verify-config] +// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: unknown check 'bugprone-assert-side-effects'; did you mean 'bugprone-assert-side-effect' [-verify-config] + From 0e2cb54934829f42069b06f599fa9e724bdf40b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sat, 23 Mar 2024 13:30:28 -0400 Subject: [PATCH 2/5] fixup! [clang-tidy] Improved --verify-config when using literal style in config file Partially rewrote verifyChecks() to realign the checks parsing logic. Instead of manually parsing the string of globs, verifyCHecks() now uses a GlobList which is responsible to parse the string and return a list of regexes. This ensures that any changes made to the checks parsing logic has to be made at only one place rather than two. --- clang-tools-extra/clang-tidy/GlobList.cpp | 14 ++-- clang-tools-extra/clang-tidy/GlobList.h | 4 ++ .../clang-tidy/tool/ClangTid
[clang-tools-extra] [clang-tidy] Removed redundant-inline-specifier warning on static data members (PR #81423)
https://github.com/felix642 created https://github.com/llvm/llvm-project/pull/81423 Updated the check to ignore point static data members with in class initializer since removing the inline specifier would generate a compilation error Fixes #80684 From 45c89e269bd37e298dd12035dc78c986fa47e824 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Thu, 8 Feb 2024 17:07:38 -0500 Subject: [PATCH] =?UTF-8?q?[clang-tidy]=C2=A0Removed=20redundant-inline-sp?= =?UTF-8?q?ecifier=20warning=20on=20static=20data=20members?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updated the check to ignore floating point static data members with in class initializer since removing the inline specifier would generate a compilation error Fixes #80684 --- .../RedundantInlineSpecifierCheck.cpp | 17 +++-- clang-tools-extra/docs/ReleaseNotes.rst | 4 .../readability/redundant-inline-specifier.cpp | 15 +++ 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp index 0e8d17d4442478..3d6b052686c20c 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -47,6 +47,9 @@ AST_POLYMORPHIC_MATCHER_P(isInternalLinkage, return VD->isInAnonymousNamespace(); llvm_unreachable("Not a valid polymorphic type"); } + +AST_MATCHER(clang::VarDecl, hasInitialization) { return Node.hasInit(); } + } // namespace static SourceLocation getInlineTokenLocation(SourceRange RangeLocation, @@ -88,12 +91,14 @@ void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) { this); if (getLangOpts().CPlusPlus17) { -Finder->addMatcher( -varDecl(isInlineSpecified(), -anyOf(isInternalLinkage(StrictMode), - allOf(isConstexpr(), hasAncestor(recordDecl() -.bind("var_decl"), -this); +const auto IsPartOfRecordDecl = hasAncestor(recordDecl()); +Finder->addMatcher(varDecl(isInlineSpecified(), + anyOf(allOf(isInternalLinkage(StrictMode), + unless(allOf(hasInitialization(), +IsPartOfRecordDecl))), + allOf(isConstexpr(), IsPartOfRecordDecl))) + .bind("var_decl"), + this); } } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ee68c8f49b3df2..303bfef17015a0 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -160,6 +160,10 @@ Changes in existing checks `AllowStringArrays` option, enabling the exclusion of array types with deduced length initialized from string literals. +- Improved :doc:`readability-redundant-inline-specifier + ` check to properly + emit warnings for static data member with an in-class initializer. + Removed checks ^^ diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp index cdd98d8fdc20f5..4c0e063c00e73f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp @@ -135,3 +135,18 @@ INLINE_MACRO() #define INLINE_KW inline INLINE_KW void fn10() { } + +namespace { +class A +{ +public: + static inline float test = 3.0F; + static inline double test2 = 3.0; + static inline int test3 = 3; + + static inline float test4; + // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:10: warning: variable 'test4' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] + // CHECK-FIXES-STRICT: static float test4; +}; +} + ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Removed redundant-inline-specifier warning on static data members (PR #81423)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/81423 From 0fa56a4176205337270d15049dc34a8508488905 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Thu, 8 Feb 2024 17:07:38 -0500 Subject: [PATCH] =?UTF-8?q?[clang-tidy]=C2=A0Removed=20redundant-inline-sp?= =?UTF-8?q?ecifier=20warning=20on=20static=20data=20members?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updated the check to ignore point static data members with in class initializer since removing the inline specifier would generate a compilation error Fixes #80684 --- .../RedundantInlineSpecifierCheck.cpp | 17 +++-- clang-tools-extra/docs/ReleaseNotes.rst | 4 .../readability/redundant-inline-specifier.cpp | 14 ++ 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp index 0e8d17d4442478..3d6b052686c20c 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -47,6 +47,9 @@ AST_POLYMORPHIC_MATCHER_P(isInternalLinkage, return VD->isInAnonymousNamespace(); llvm_unreachable("Not a valid polymorphic type"); } + +AST_MATCHER(clang::VarDecl, hasInitialization) { return Node.hasInit(); } + } // namespace static SourceLocation getInlineTokenLocation(SourceRange RangeLocation, @@ -88,12 +91,14 @@ void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) { this); if (getLangOpts().CPlusPlus17) { -Finder->addMatcher( -varDecl(isInlineSpecified(), -anyOf(isInternalLinkage(StrictMode), - allOf(isConstexpr(), hasAncestor(recordDecl() -.bind("var_decl"), -this); +const auto IsPartOfRecordDecl = hasAncestor(recordDecl()); +Finder->addMatcher(varDecl(isInlineSpecified(), + anyOf(allOf(isInternalLinkage(StrictMode), + unless(allOf(hasInitialization(), +IsPartOfRecordDecl))), + allOf(isConstexpr(), IsPartOfRecordDecl))) + .bind("var_decl"), + this); } } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ee68c8f49b3df2..303bfef17015a0 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -160,6 +160,10 @@ Changes in existing checks `AllowStringArrays` option, enabling the exclusion of array types with deduced length initialized from string literals. +- Improved :doc:`readability-redundant-inline-specifier + ` check to properly + emit warnings for static data member with an in-class initializer. + Removed checks ^^ diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp index cdd98d8fdc20f5..14f9e88f7e7218 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp @@ -135,3 +135,17 @@ INLINE_MACRO() #define INLINE_KW inline INLINE_KW void fn10() { } + +namespace { +class A +{ +public: + static inline float test = 3.0F; + static inline double test2 = 3.0; + static inline int test3 = 3; + + static inline float test4; + // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:10: warning: variable 'test4' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] + // CHECK-FIXES-STRICT: static float test4; +}; +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Removed redundant-inline-specifier warning on static data members (PR #81423)
@@ -88,12 +91,14 @@ void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) { this); if (getLangOpts().CPlusPlus17) { -Finder->addMatcher( -varDecl(isInlineSpecified(), -anyOf(isInternalLinkage(StrictMode), - allOf(isConstexpr(), hasAncestor(recordDecl() -.bind("var_decl"), -this); +const auto IsPartOfRecordDecl = hasAncestor(recordDecl()); +Finder->addMatcher(varDecl(isInlineSpecified(), + anyOf(allOf(isInternalLinkage(StrictMode), + unless(allOf(hasInitialization(), felix642 wrote: I thought about this, but I can't find an example where a class member would have the inline keyword and not be static. If you look at the test case I added, the static keyword is required for the code to compile otherwise you will get a compilation error saying that inline can only appear on non-local variable (Which makes sense tbf). Do you have an example in mind where the inline keyword is used on non-static class member? godbolt: https://godbolt.org/z/aEGGvs9Kc https://github.com/llvm/llvm-project/pull/81423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Removed redundant-inline-specifier warning on static data members (PR #81423)
@@ -47,6 +47,9 @@ AST_POLYMORPHIC_MATCHER_P(isInternalLinkage, return VD->isInAnonymousNamespace(); llvm_unreachable("Not a valid polymorphic type"); } + +AST_MATCHER(clang::VarDecl, hasInitialization) { return Node.hasInit(); } felix642 wrote: Simply didn't know this exists. I'll update my code with your suggestion. Thank you https://github.com/llvm/llvm-project/pull/81423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Removed redundant-inline-specifier warning on static data members (PR #81423)
https://github.com/felix642 edited https://github.com/llvm/llvm-project/pull/81423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Removed redundant-inline-specifier warning on static data members (PR #81423)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/81423 From 0fa56a4176205337270d15049dc34a8508488905 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Thu, 8 Feb 2024 17:07:38 -0500 Subject: [PATCH 1/2] =?UTF-8?q?[clang-tidy]=C2=A0Removed=20redundant-inlin?= =?UTF-8?q?e-specifier=20warning=20on=20static=20data=20members?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updated the check to ignore point static data members with in class initializer since removing the inline specifier would generate a compilation error Fixes #80684 --- .../RedundantInlineSpecifierCheck.cpp | 17 +++-- clang-tools-extra/docs/ReleaseNotes.rst | 4 .../readability/redundant-inline-specifier.cpp | 14 ++ 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp index 0e8d17d4442478..3d6b052686c20c 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -47,6 +47,9 @@ AST_POLYMORPHIC_MATCHER_P(isInternalLinkage, return VD->isInAnonymousNamespace(); llvm_unreachable("Not a valid polymorphic type"); } + +AST_MATCHER(clang::VarDecl, hasInitialization) { return Node.hasInit(); } + } // namespace static SourceLocation getInlineTokenLocation(SourceRange RangeLocation, @@ -88,12 +91,14 @@ void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) { this); if (getLangOpts().CPlusPlus17) { -Finder->addMatcher( -varDecl(isInlineSpecified(), -anyOf(isInternalLinkage(StrictMode), - allOf(isConstexpr(), hasAncestor(recordDecl() -.bind("var_decl"), -this); +const auto IsPartOfRecordDecl = hasAncestor(recordDecl()); +Finder->addMatcher(varDecl(isInlineSpecified(), + anyOf(allOf(isInternalLinkage(StrictMode), + unless(allOf(hasInitialization(), +IsPartOfRecordDecl))), + allOf(isConstexpr(), IsPartOfRecordDecl))) + .bind("var_decl"), + this); } } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ee68c8f49b3df2..303bfef17015a0 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -160,6 +160,10 @@ Changes in existing checks `AllowStringArrays` option, enabling the exclusion of array types with deduced length initialized from string literals. +- Improved :doc:`readability-redundant-inline-specifier + ` check to properly + emit warnings for static data member with an in-class initializer. + Removed checks ^^ diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp index cdd98d8fdc20f5..14f9e88f7e7218 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp @@ -135,3 +135,17 @@ INLINE_MACRO() #define INLINE_KW inline INLINE_KW void fn10() { } + +namespace { +class A +{ +public: + static inline float test = 3.0F; + static inline double test2 = 3.0; + static inline int test3 = 3; + + static inline float test4; + // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:10: warning: variable 'test4' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] + // CHECK-FIXES-STRICT: static float test4; +}; +} From 3b4684a3003793e80d25e8a75c8252dbb5514cea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 11 Feb 2024 21:02:59 -0500 Subject: [PATCH 2/2] =?UTF-8?q?fixup!=20[clang-tidy]=C2=A0Removed=20redund?= =?UTF-8?q?ant-inline-specifier=20warning=20on=20static=20data=20members?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improved AST Matcher based on comments --- .../RedundantInlineSpecifierCheck.cpp | 19 +-- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp index 3d6b052686c20c..1693e5c5e9cd45 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -47,9 +47,6 @@ AST_POLYMORPHIC_MATCHER_P(isI
[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/82947 From d1cbed0e2e83bd3544067fd25d7e811f1ab3f095 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 20:20:59 -0500 Subject: [PATCH 1/3] [clang-tidy] Improved modernize-use-using by fixing a false-negative The check needs a parent decl to match but if the typedef is in a function, the parent is a declStmt which is not a decl by itself. Improved the matcher to match on either a decl or a declstmt and extract the decl from the stmt in the latter case. fixes #72179 --- .../clang-tidy/modernize/UseUsingCheck.cpp | 16 +--- clang-tools-extra/docs/ReleaseNotes.rst| 3 +++ .../checkers/modernize/use-using.cpp | 18 ++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp index bb05f206c717ce..50a07fc02e31b4 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp @@ -24,6 +24,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = "extern-c-decl"; static constexpr llvm::StringLiteral ParentDeclName = "parent-decl"; static constexpr llvm::StringLiteral TagDeclName = "tag-decl"; static constexpr llvm::StringLiteral TypedefName = "typedef"; +static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt"; UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), @@ -41,7 +42,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { unless(isInstantiated()), optionally(hasAncestor( linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))), - hasParent(decl().bind(ParentDeclName))) + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName .bind(TypedefName), this); @@ -51,17 +53,25 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { tagDecl( anyOf(allOf(unless(anyOf(isImplicit(), classTemplateSpecializationDecl())), - hasParent(decl().bind(ParentDeclName))), + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName, // We want the parent of the ClassTemplateDecl, not the parent // of the specialization. classTemplateSpecializationDecl(hasAncestor(classTemplateDecl( -hasParent(decl().bind(ParentDeclName))) +anyOf(hasParent(decl().bind(ParentDeclName)), + hasParent(declStmt().bind(DeclStmtName .bind(TagDeclName), this); } void UseUsingCheck::check(const MatchFinder::MatchResult &Result) { const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName); + + if (!ParentDecl) { +const auto *ParentDeclStmt = Result.Nodes.getNodeAs(DeclStmtName); +ParentDecl = ParentDeclStmt->getSingleDecl(); + } + if (!ParentDecl) return; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6fd01ed9d471c5..2b36ae4acb9f1d 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -183,6 +183,9 @@ Changes in existing checks ` check to also remove any trailing whitespace when deleting the ``virtual`` keyword. +- Improved :doc:`modernize-use-using ` + check by fixing false-negative in functions. + - Improved :doc:`readability-implicit-bool-conversion ` check to provide valid fix suggestions for ``static_cast`` without a preceding space and diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp index 462bc984fd3ad5..230c7c94cda1cf 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp @@ -342,3 +342,21 @@ typedef int InExternCPP; // CHECK-FIXES: using InExternCPP = int; } + +namespace ISSUE_72179 +{ + void foo() + { +typedef int a; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using a = int; + + } + + void foo2() + { +typedef struct { int a; union { int b; }; } c; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using c = struct { int a; union { int b; }; }; + } +} From 0b5c78508fbf49229d576171d7af6c6bb16b4367 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 22:14:22 -0500 Subject: [PATCH 2/3] fixup! [clang-tidy]
[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/82947 From d1cbed0e2e83bd3544067fd25d7e811f1ab3f095 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 20:20:59 -0500 Subject: [PATCH 1/4] [clang-tidy] Improved modernize-use-using by fixing a false-negative The check needs a parent decl to match but if the typedef is in a function, the parent is a declStmt which is not a decl by itself. Improved the matcher to match on either a decl or a declstmt and extract the decl from the stmt in the latter case. fixes #72179 --- .../clang-tidy/modernize/UseUsingCheck.cpp | 16 +--- clang-tools-extra/docs/ReleaseNotes.rst| 3 +++ .../checkers/modernize/use-using.cpp | 18 ++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp index bb05f206c717ce..50a07fc02e31b4 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp @@ -24,6 +24,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = "extern-c-decl"; static constexpr llvm::StringLiteral ParentDeclName = "parent-decl"; static constexpr llvm::StringLiteral TagDeclName = "tag-decl"; static constexpr llvm::StringLiteral TypedefName = "typedef"; +static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt"; UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), @@ -41,7 +42,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { unless(isInstantiated()), optionally(hasAncestor( linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))), - hasParent(decl().bind(ParentDeclName))) + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName .bind(TypedefName), this); @@ -51,17 +53,25 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { tagDecl( anyOf(allOf(unless(anyOf(isImplicit(), classTemplateSpecializationDecl())), - hasParent(decl().bind(ParentDeclName))), + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName, // We want the parent of the ClassTemplateDecl, not the parent // of the specialization. classTemplateSpecializationDecl(hasAncestor(classTemplateDecl( -hasParent(decl().bind(ParentDeclName))) +anyOf(hasParent(decl().bind(ParentDeclName)), + hasParent(declStmt().bind(DeclStmtName .bind(TagDeclName), this); } void UseUsingCheck::check(const MatchFinder::MatchResult &Result) { const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName); + + if (!ParentDecl) { +const auto *ParentDeclStmt = Result.Nodes.getNodeAs(DeclStmtName); +ParentDecl = ParentDeclStmt->getSingleDecl(); + } + if (!ParentDecl) return; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6fd01ed9d471c5..2b36ae4acb9f1d 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -183,6 +183,9 @@ Changes in existing checks ` check to also remove any trailing whitespace when deleting the ``virtual`` keyword. +- Improved :doc:`modernize-use-using ` + check by fixing false-negative in functions. + - Improved :doc:`readability-implicit-bool-conversion ` check to provide valid fix suggestions for ``static_cast`` without a preceding space and diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp index 462bc984fd3ad5..230c7c94cda1cf 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp @@ -342,3 +342,21 @@ typedef int InExternCPP; // CHECK-FIXES: using InExternCPP = int; } + +namespace ISSUE_72179 +{ + void foo() + { +typedef int a; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using a = int; + + } + + void foo2() + { +typedef struct { int a; union { int b; }; } c; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using c = struct { int a; union { int b; }; }; + } +} From 0b5c78508fbf49229d576171d7af6c6bb16b4367 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 22:14:22 -0500 Subject: [PATCH 2/4] fixup! [clang-tidy]
[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/82947 From f1d957413bcc81e9ced06ad40570859769f4c1c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 20:20:59 -0500 Subject: [PATCH 1/5] [clang-tidy] Improved modernize-use-using by fixing a false-negative The check needs a parent decl to match but if the typedef is in a function, the parent is a declStmt which is not a decl by itself. Improved the matcher to match on either a decl or a declstmt and extract the decl from the stmt in the latter case. fixes #72179 --- .../clang-tidy/modernize/UseUsingCheck.cpp | 16 +--- clang-tools-extra/docs/ReleaseNotes.rst| 3 +++ .../checkers/modernize/use-using.cpp | 18 ++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp index bb05f206c717ce..50a07fc02e31b4 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp @@ -24,6 +24,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = "extern-c-decl"; static constexpr llvm::StringLiteral ParentDeclName = "parent-decl"; static constexpr llvm::StringLiteral TagDeclName = "tag-decl"; static constexpr llvm::StringLiteral TypedefName = "typedef"; +static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt"; UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), @@ -41,7 +42,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { unless(isInstantiated()), optionally(hasAncestor( linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))), - hasParent(decl().bind(ParentDeclName))) + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName .bind(TypedefName), this); @@ -51,17 +53,25 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { tagDecl( anyOf(allOf(unless(anyOf(isImplicit(), classTemplateSpecializationDecl())), - hasParent(decl().bind(ParentDeclName))), + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName, // We want the parent of the ClassTemplateDecl, not the parent // of the specialization. classTemplateSpecializationDecl(hasAncestor(classTemplateDecl( -hasParent(decl().bind(ParentDeclName))) +anyOf(hasParent(decl().bind(ParentDeclName)), + hasParent(declStmt().bind(DeclStmtName .bind(TagDeclName), this); } void UseUsingCheck::check(const MatchFinder::MatchResult &Result) { const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName); + + if (!ParentDecl) { +const auto *ParentDeclStmt = Result.Nodes.getNodeAs(DeclStmtName); +ParentDecl = ParentDeclStmt->getSingleDecl(); + } + if (!ParentDecl) return; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 44680f79de6f54..479b9b86444184 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -224,6 +224,9 @@ Changes in existing checks analyzed, se the check now handles the common patterns `const auto e = (*vector_ptr)[i]` and `const auto e = vector_ptr->at(i);`. +- Improved :doc:`modernize-use-using ` + check by fixing false-negative in functions. + - Improved :doc:`readability-implicit-bool-conversion ` check to provide valid fix suggestions for ``static_cast`` without a preceding space and diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp index 462bc984fd3ad5..230c7c94cda1cf 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp @@ -342,3 +342,21 @@ typedef int InExternCPP; // CHECK-FIXES: using InExternCPP = int; } + +namespace ISSUE_72179 +{ + void foo() + { +typedef int a; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using a = int; + + } + + void foo2() + { +typedef struct { int a; union { int b; }; } c; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using c = struct { int a; union { int b; }; }; + } +} From e7dcbbcf92e896354e9a52da259251a7d3a74369 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 22:14:22 -05
[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)
https://github.com/felix642 ready_for_review https://github.com/llvm/llvm-project/pull/82947 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)
felix642 wrote: Thank you for the quick review @PiotrZSL. I've implemented the proposed changes and rebased on top of main. We can wait a few days to see if someone else has some comments to add and then we should be ready to merge https://github.com/llvm/llvm-project/pull/82947 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)
https://github.com/felix642 converted_to_draft https://github.com/llvm/llvm-project/pull/82947 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/82947 From f1d957413bcc81e9ced06ad40570859769f4c1c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 20:20:59 -0500 Subject: [PATCH 1/6] [clang-tidy] Improved modernize-use-using by fixing a false-negative The check needs a parent decl to match but if the typedef is in a function, the parent is a declStmt which is not a decl by itself. Improved the matcher to match on either a decl or a declstmt and extract the decl from the stmt in the latter case. fixes #72179 --- .../clang-tidy/modernize/UseUsingCheck.cpp | 16 +--- clang-tools-extra/docs/ReleaseNotes.rst| 3 +++ .../checkers/modernize/use-using.cpp | 18 ++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp index bb05f206c717ce..50a07fc02e31b4 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp @@ -24,6 +24,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = "extern-c-decl"; static constexpr llvm::StringLiteral ParentDeclName = "parent-decl"; static constexpr llvm::StringLiteral TagDeclName = "tag-decl"; static constexpr llvm::StringLiteral TypedefName = "typedef"; +static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt"; UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), @@ -41,7 +42,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { unless(isInstantiated()), optionally(hasAncestor( linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))), - hasParent(decl().bind(ParentDeclName))) + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName .bind(TypedefName), this); @@ -51,17 +53,25 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { tagDecl( anyOf(allOf(unless(anyOf(isImplicit(), classTemplateSpecializationDecl())), - hasParent(decl().bind(ParentDeclName))), + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName, // We want the parent of the ClassTemplateDecl, not the parent // of the specialization. classTemplateSpecializationDecl(hasAncestor(classTemplateDecl( -hasParent(decl().bind(ParentDeclName))) +anyOf(hasParent(decl().bind(ParentDeclName)), + hasParent(declStmt().bind(DeclStmtName .bind(TagDeclName), this); } void UseUsingCheck::check(const MatchFinder::MatchResult &Result) { const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName); + + if (!ParentDecl) { +const auto *ParentDeclStmt = Result.Nodes.getNodeAs(DeclStmtName); +ParentDecl = ParentDeclStmt->getSingleDecl(); + } + if (!ParentDecl) return; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 44680f79de6f54..479b9b86444184 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -224,6 +224,9 @@ Changes in existing checks analyzed, se the check now handles the common patterns `const auto e = (*vector_ptr)[i]` and `const auto e = vector_ptr->at(i);`. +- Improved :doc:`modernize-use-using ` + check by fixing false-negative in functions. + - Improved :doc:`readability-implicit-bool-conversion ` check to provide valid fix suggestions for ``static_cast`` without a preceding space and diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp index 462bc984fd3ad5..230c7c94cda1cf 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp @@ -342,3 +342,21 @@ typedef int InExternCPP; // CHECK-FIXES: using InExternCPP = int; } + +namespace ISSUE_72179 +{ + void foo() + { +typedef int a; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using a = int; + + } + + void foo2() + { +typedef struct { int a; union { int b; }; } c; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using c = struct { int a; union { int b; }; }; + } +} From e7dcbbcf92e896354e9a52da259251a7d3a74369 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 22:14:22 -05
[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)
https://github.com/felix642 ready_for_review https://github.com/llvm/llvm-project/pull/82947 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)
https://github.com/felix642 created https://github.com/llvm/llvm-project/pull/82947 The check needs a parent decl to match but if the typedef is in a function, the parent is a declStmt which is not a decl by itself. Improved the matcher to match on either a decl or a declstmt and extract the decl from the stmt in the latter case. fixes #72179 CC: @FabianWolff @PiotrZSL @njames93 From 003ec9d5b75cdf6de3140612e1c4b42322a52413 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 20:20:59 -0500 Subject: [PATCH] [clang-tidy] Improved modernize-use-using by fixing a false-positive The check needs a parent decl to match but if the typedef is in a function, the parent is a declStmt which is not a decl by itself. Improved the matcher to match on either a decl or a declstmt and extract the decl from the stmt in the latter case. fixes #72179 --- .../clang-tidy/modernize/UseUsingCheck.cpp | 16 +--- clang-tools-extra/docs/ReleaseNotes.rst| 3 +++ .../checkers/modernize/use-using.cpp | 18 ++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp index bb05f206c717ce..50a07fc02e31b4 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp @@ -24,6 +24,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = "extern-c-decl"; static constexpr llvm::StringLiteral ParentDeclName = "parent-decl"; static constexpr llvm::StringLiteral TagDeclName = "tag-decl"; static constexpr llvm::StringLiteral TypedefName = "typedef"; +static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt"; UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), @@ -41,7 +42,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { unless(isInstantiated()), optionally(hasAncestor( linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))), - hasParent(decl().bind(ParentDeclName))) + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName .bind(TypedefName), this); @@ -51,17 +53,25 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { tagDecl( anyOf(allOf(unless(anyOf(isImplicit(), classTemplateSpecializationDecl())), - hasParent(decl().bind(ParentDeclName))), + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName, // We want the parent of the ClassTemplateDecl, not the parent // of the specialization. classTemplateSpecializationDecl(hasAncestor(classTemplateDecl( -hasParent(decl().bind(ParentDeclName))) +anyOf(hasParent(decl().bind(ParentDeclName)), + hasParent(declStmt().bind(DeclStmtName .bind(TagDeclName), this); } void UseUsingCheck::check(const MatchFinder::MatchResult &Result) { const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName); + + if (!ParentDecl) { +const auto *ParentDeclStmt = Result.Nodes.getNodeAs(DeclStmtName); +ParentDecl = ParentDeclStmt->getSingleDecl(); + } + if (!ParentDecl) return; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6fd01ed9d471c5..2b36ae4acb9f1d 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -183,6 +183,9 @@ Changes in existing checks ` check to also remove any trailing whitespace when deleting the ``virtual`` keyword. +- Improved :doc:`modernize-use-using ` + check by fixing false-negative in functions. + - Improved :doc:`readability-implicit-bool-conversion ` check to provide valid fix suggestions for ``static_cast`` without a preceding space and diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp index 462bc984fd3ad5..230c7c94cda1cf 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp @@ -342,3 +342,21 @@ typedef int InExternCPP; // CHECK-FIXES: using InExternCPP = int; } + +namespace ISSUE_72179 +{ + void foo() + { +typedef int a; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using a = int; + + } + + void foo2() + { +typedef struct { int a; union { int b; }; } c; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef'
[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/82947 From d1cbed0e2e83bd3544067fd25d7e811f1ab3f095 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 20:20:59 -0500 Subject: [PATCH] [clang-tidy] Improved modernize-use-using by fixing a false-negative The check needs a parent decl to match but if the typedef is in a function, the parent is a declStmt which is not a decl by itself. Improved the matcher to match on either a decl or a declstmt and extract the decl from the stmt in the latter case. fixes #72179 --- .../clang-tidy/modernize/UseUsingCheck.cpp | 16 +--- clang-tools-extra/docs/ReleaseNotes.rst| 3 +++ .../checkers/modernize/use-using.cpp | 18 ++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp index bb05f206c717ce..50a07fc02e31b4 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp @@ -24,6 +24,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = "extern-c-decl"; static constexpr llvm::StringLiteral ParentDeclName = "parent-decl"; static constexpr llvm::StringLiteral TagDeclName = "tag-decl"; static constexpr llvm::StringLiteral TypedefName = "typedef"; +static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt"; UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), @@ -41,7 +42,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { unless(isInstantiated()), optionally(hasAncestor( linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))), - hasParent(decl().bind(ParentDeclName))) + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName .bind(TypedefName), this); @@ -51,17 +53,25 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { tagDecl( anyOf(allOf(unless(anyOf(isImplicit(), classTemplateSpecializationDecl())), - hasParent(decl().bind(ParentDeclName))), + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName, // We want the parent of the ClassTemplateDecl, not the parent // of the specialization. classTemplateSpecializationDecl(hasAncestor(classTemplateDecl( -hasParent(decl().bind(ParentDeclName))) +anyOf(hasParent(decl().bind(ParentDeclName)), + hasParent(declStmt().bind(DeclStmtName .bind(TagDeclName), this); } void UseUsingCheck::check(const MatchFinder::MatchResult &Result) { const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName); + + if (!ParentDecl) { +const auto *ParentDeclStmt = Result.Nodes.getNodeAs(DeclStmtName); +ParentDecl = ParentDeclStmt->getSingleDecl(); + } + if (!ParentDecl) return; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6fd01ed9d471c5..2b36ae4acb9f1d 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -183,6 +183,9 @@ Changes in existing checks ` check to also remove any trailing whitespace when deleting the ``virtual`` keyword. +- Improved :doc:`modernize-use-using ` + check by fixing false-negative in functions. + - Improved :doc:`readability-implicit-bool-conversion ` check to provide valid fix suggestions for ``static_cast`` without a preceding space and diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp index 462bc984fd3ad5..230c7c94cda1cf 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp @@ -342,3 +342,21 @@ typedef int InExternCPP; // CHECK-FIXES: using InExternCPP = int; } + +namespace ISSUE_72179 +{ + void foo() + { +typedef int a; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using a = int; + + } + + void foo2() + { +typedef struct { int a; union { int b; }; } c; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using c = struct { int a; union { int b; }; }; + } +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] bugprone-unused-return-value config now supports regexes (PR #82952)
https://github.com/felix642 created https://github.com/llvm/llvm-project/pull/82952 The parameter `CheckedReturnTypes` now supports regexes Fixes #63107 From 86bc8a57fd56a6b2593af8bcf85d50bbc0ce984e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 22:05:08 -0500 Subject: [PATCH] [clang-tidy] bugprone-unused-return-value config now support regexes Fixes #63107 --- .../bugprone/UnusedReturnValueCheck.cpp | 202 +- .../bugprone/UnusedReturnValueCheck.h | 6 +- .../hicpp/IgnoredRemoveResultCheck.cpp| 8 +- clang-tools-extra/docs/ReleaseNotes.rst | 4 + .../bugprone/unused-return-value-custom.cpp | 2 +- 5 files changed, 114 insertions(+), 108 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp index 05012c7df6a975..b4bf85c912c3ca 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp @@ -33,98 +33,98 @@ AST_MATCHER_P(FunctionDecl, isInstantiatedFrom, Matcher, UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), - CheckedFunctions(Options.get("CheckedFunctions", - "::std::async;" - "::std::launder;" - "::std::remove;" - "::std::remove_if;" - "::std::unique;" - "::std::unique_ptr::release;" - "::std::basic_string::empty;" - "::std::vector::empty;" - "::std::back_inserter;" - "::std::distance;" - "::std::find;" - "::std::find_if;" - "::std::inserter;" - "::std::lower_bound;" - "::std::make_pair;" - "::std::map::count;" - "::std::map::find;" - "::std::map::lower_bound;" - "::std::multimap::equal_range;" - "::std::multimap::upper_bound;" - "::std::set::count;" - "::std::set::find;" - "::std::setfill;" - "::std::setprecision;" - "::std::setw;" - "::std::upper_bound;" - "::std::vector::at;" - // C standard library - "::bsearch;" - "::ferror;" - "::feof;" - "::isalnum;" - "::isalpha;" - "::isblank;" - "::iscntrl;" - "::isdigit;" - "::isgraph;" - "::islower;" - "::isprint;" - "::ispunct;" - "::isspace;" - "::isupper;" - "::iswalnum;" - "::iswprint;" - "::iswspace;" - "::isxdigit;" - "::memchr;" - "::memcmp;" - "::strcmp;" - "::strcoll;" - "::strncmp;" - "::strpbrk;" - "::strrchr;" - "::strspn;" - "::strstr;" - "::wcscmp;" - // POSIX - "::access;" - "::bind;" - "::connect;" - "::difftime;" - "::dlsym;" - "::fnmatch;" - "::getaddrinfo;" - "::getopt;" - "::htonl;" - "::htons;" - "::iconv_open;" -
[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/82947 From d1cbed0e2e83bd3544067fd25d7e811f1ab3f095 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 20:20:59 -0500 Subject: [PATCH 1/2] [clang-tidy] Improved modernize-use-using by fixing a false-negative The check needs a parent decl to match but if the typedef is in a function, the parent is a declStmt which is not a decl by itself. Improved the matcher to match on either a decl or a declstmt and extract the decl from the stmt in the latter case. fixes #72179 --- .../clang-tidy/modernize/UseUsingCheck.cpp | 16 +--- clang-tools-extra/docs/ReleaseNotes.rst| 3 +++ .../checkers/modernize/use-using.cpp | 18 ++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp index bb05f206c717ce..50a07fc02e31b4 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp @@ -24,6 +24,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = "extern-c-decl"; static constexpr llvm::StringLiteral ParentDeclName = "parent-decl"; static constexpr llvm::StringLiteral TagDeclName = "tag-decl"; static constexpr llvm::StringLiteral TypedefName = "typedef"; +static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt"; UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), @@ -41,7 +42,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { unless(isInstantiated()), optionally(hasAncestor( linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))), - hasParent(decl().bind(ParentDeclName))) + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName .bind(TypedefName), this); @@ -51,17 +53,25 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) { tagDecl( anyOf(allOf(unless(anyOf(isImplicit(), classTemplateSpecializationDecl())), - hasParent(decl().bind(ParentDeclName))), + anyOf(hasParent(decl().bind(ParentDeclName)), +hasParent(declStmt().bind(DeclStmtName, // We want the parent of the ClassTemplateDecl, not the parent // of the specialization. classTemplateSpecializationDecl(hasAncestor(classTemplateDecl( -hasParent(decl().bind(ParentDeclName))) +anyOf(hasParent(decl().bind(ParentDeclName)), + hasParent(declStmt().bind(DeclStmtName .bind(TagDeclName), this); } void UseUsingCheck::check(const MatchFinder::MatchResult &Result) { const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName); + + if (!ParentDecl) { +const auto *ParentDeclStmt = Result.Nodes.getNodeAs(DeclStmtName); +ParentDecl = ParentDeclStmt->getSingleDecl(); + } + if (!ParentDecl) return; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6fd01ed9d471c5..2b36ae4acb9f1d 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -183,6 +183,9 @@ Changes in existing checks ` check to also remove any trailing whitespace when deleting the ``virtual`` keyword. +- Improved :doc:`modernize-use-using ` + check by fixing false-negative in functions. + - Improved :doc:`readability-implicit-bool-conversion ` check to provide valid fix suggestions for ``static_cast`` without a preceding space and diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp index 462bc984fd3ad5..230c7c94cda1cf 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp @@ -342,3 +342,21 @@ typedef int InExternCPP; // CHECK-FIXES: using InExternCPP = int; } + +namespace ISSUE_72179 +{ + void foo() + { +typedef int a; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using a = int; + + } + + void foo2() + { +typedef struct { int a; union { int b; }; } c; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using c = struct { int a; union { int b; }; }; + } +} From 0b5c78508fbf49229d576171d7af6c6bb16b4367 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 22:14:22 -0500 Subject: [PATCH 2/2] fixup! [clang-tidy]
[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)
https://github.com/felix642 converted_to_draft https://github.com/llvm/llvm-project/pull/82947 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] bugprone-unused-return-value config now supports regexes (PR #82952)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/82952 From 86bc8a57fd56a6b2593af8bcf85d50bbc0ce984e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 22:05:08 -0500 Subject: [PATCH 1/2] [clang-tidy] bugprone-unused-return-value config now support regexes Fixes #63107 --- .../bugprone/UnusedReturnValueCheck.cpp | 202 +- .../bugprone/UnusedReturnValueCheck.h | 6 +- .../hicpp/IgnoredRemoveResultCheck.cpp| 8 +- clang-tools-extra/docs/ReleaseNotes.rst | 4 + .../bugprone/unused-return-value-custom.cpp | 2 +- 5 files changed, 114 insertions(+), 108 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp index 05012c7df6a975..b4bf85c912c3ca 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp @@ -33,98 +33,98 @@ AST_MATCHER_P(FunctionDecl, isInstantiatedFrom, Matcher, UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), - CheckedFunctions(Options.get("CheckedFunctions", - "::std::async;" - "::std::launder;" - "::std::remove;" - "::std::remove_if;" - "::std::unique;" - "::std::unique_ptr::release;" - "::std::basic_string::empty;" - "::std::vector::empty;" - "::std::back_inserter;" - "::std::distance;" - "::std::find;" - "::std::find_if;" - "::std::inserter;" - "::std::lower_bound;" - "::std::make_pair;" - "::std::map::count;" - "::std::map::find;" - "::std::map::lower_bound;" - "::std::multimap::equal_range;" - "::std::multimap::upper_bound;" - "::std::set::count;" - "::std::set::find;" - "::std::setfill;" - "::std::setprecision;" - "::std::setw;" - "::std::upper_bound;" - "::std::vector::at;" - // C standard library - "::bsearch;" - "::ferror;" - "::feof;" - "::isalnum;" - "::isalpha;" - "::isblank;" - "::iscntrl;" - "::isdigit;" - "::isgraph;" - "::islower;" - "::isprint;" - "::ispunct;" - "::isspace;" - "::isupper;" - "::iswalnum;" - "::iswprint;" - "::iswspace;" - "::isxdigit;" - "::memchr;" - "::memcmp;" - "::strcmp;" - "::strcoll;" - "::strncmp;" - "::strpbrk;" - "::strrchr;" - "::strspn;" - "::strstr;" - "::wcscmp;" - // POSIX - "::access;" - "::bind;" - "::connect;" - "::difftime;" - "::dlsym;" - "::fnmatch;" - "::getaddrinfo;" - "::getopt;" - "::htonl;" - "::htons;" - "::iconv_open;" - "::inet_addr;" -
[clang-tools-extra] [clang-tidy] bugprone-unused-return-value config now supports regexes (PR #82952)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/82952 From 5e30d84368a05bab31a26c413fdc8092449f4111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 22:05:08 -0500 Subject: [PATCH 1/2] [clang-tidy] bugprone-unused-return-value config now support regexes Fixes #63107 --- .../bugprone/UnusedReturnValueCheck.cpp | 202 +- .../bugprone/UnusedReturnValueCheck.h | 6 +- .../hicpp/IgnoredRemoveResultCheck.cpp| 8 +- clang-tools-extra/docs/ReleaseNotes.rst | 4 + .../bugprone/unused-return-value-custom.cpp | 2 +- 5 files changed, 114 insertions(+), 108 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp index 05012c7df6a975..b4bf85c912c3ca 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp @@ -33,98 +33,98 @@ AST_MATCHER_P(FunctionDecl, isInstantiatedFrom, Matcher, UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), - CheckedFunctions(Options.get("CheckedFunctions", - "::std::async;" - "::std::launder;" - "::std::remove;" - "::std::remove_if;" - "::std::unique;" - "::std::unique_ptr::release;" - "::std::basic_string::empty;" - "::std::vector::empty;" - "::std::back_inserter;" - "::std::distance;" - "::std::find;" - "::std::find_if;" - "::std::inserter;" - "::std::lower_bound;" - "::std::make_pair;" - "::std::map::count;" - "::std::map::find;" - "::std::map::lower_bound;" - "::std::multimap::equal_range;" - "::std::multimap::upper_bound;" - "::std::set::count;" - "::std::set::find;" - "::std::setfill;" - "::std::setprecision;" - "::std::setw;" - "::std::upper_bound;" - "::std::vector::at;" - // C standard library - "::bsearch;" - "::ferror;" - "::feof;" - "::isalnum;" - "::isalpha;" - "::isblank;" - "::iscntrl;" - "::isdigit;" - "::isgraph;" - "::islower;" - "::isprint;" - "::ispunct;" - "::isspace;" - "::isupper;" - "::iswalnum;" - "::iswprint;" - "::iswspace;" - "::isxdigit;" - "::memchr;" - "::memcmp;" - "::strcmp;" - "::strcoll;" - "::strncmp;" - "::strpbrk;" - "::strrchr;" - "::strspn;" - "::strstr;" - "::wcscmp;" - // POSIX - "::access;" - "::bind;" - "::connect;" - "::difftime;" - "::dlsym;" - "::fnmatch;" - "::getaddrinfo;" - "::getopt;" - "::htonl;" - "::htons;" - "::iconv_open;" - "::inet_addr;" -
[clang-tools-extra] [clang-tidy] bugprone-unused-return-value config now supports regexes (PR #82952)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/82952 From 00885ea60007a1da88f9d476e14252f950f358a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 22:05:08 -0500 Subject: [PATCH] [clang-tidy] bugprone-unused-return-value config now support regexes Fixes #63107 --- .../bugprone/UnusedReturnValueCheck.cpp | 202 +- .../bugprone/UnusedReturnValueCheck.h | 6 +- .../hicpp/IgnoredRemoveResultCheck.cpp| 8 +- clang-tools-extra/docs/ReleaseNotes.rst | 4 + .../checks/bugprone/unused-return-value.rst | 5 +- .../bugprone/unused-return-value-custom.cpp | 2 +- 6 files changed, 117 insertions(+), 110 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp index 05012c7df6a975..b4bf85c912c3ca 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp @@ -33,98 +33,98 @@ AST_MATCHER_P(FunctionDecl, isInstantiatedFrom, Matcher, UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), - CheckedFunctions(Options.get("CheckedFunctions", - "::std::async;" - "::std::launder;" - "::std::remove;" - "::std::remove_if;" - "::std::unique;" - "::std::unique_ptr::release;" - "::std::basic_string::empty;" - "::std::vector::empty;" - "::std::back_inserter;" - "::std::distance;" - "::std::find;" - "::std::find_if;" - "::std::inserter;" - "::std::lower_bound;" - "::std::make_pair;" - "::std::map::count;" - "::std::map::find;" - "::std::map::lower_bound;" - "::std::multimap::equal_range;" - "::std::multimap::upper_bound;" - "::std::set::count;" - "::std::set::find;" - "::std::setfill;" - "::std::setprecision;" - "::std::setw;" - "::std::upper_bound;" - "::std::vector::at;" - // C standard library - "::bsearch;" - "::ferror;" - "::feof;" - "::isalnum;" - "::isalpha;" - "::isblank;" - "::iscntrl;" - "::isdigit;" - "::isgraph;" - "::islower;" - "::isprint;" - "::ispunct;" - "::isspace;" - "::isupper;" - "::iswalnum;" - "::iswprint;" - "::iswspace;" - "::isxdigit;" - "::memchr;" - "::memcmp;" - "::strcmp;" - "::strcoll;" - "::strncmp;" - "::strpbrk;" - "::strrchr;" - "::strspn;" - "::strstr;" - "::wcscmp;" - // POSIX - "::access;" - "::bind;" - "::connect;" - "::difftime;" - "::dlsym;" - "::fnmatch;" - "::getaddrinfo;" - "::getopt;" - "::htonl;" - "::htons;" - "::iconv_open;" -
[clang-tools-extra] [clang-tidy] bugprone-unused-return-value config now supports regexes (PR #82952)
felix642 wrote: > Rebase, and could be merged. Done, thank you for the quick review. https://github.com/llvm/llvm-project/pull/82952 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] bugprone-unused-return-value config now supports regexes (PR #82952)
felix642 wrote: Nevermind I broke the documentation.. Hold on https://github.com/llvm/llvm-project/pull/82952 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] bugprone-unused-return-value config now supports regexes (PR #82952)
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/82952 From 00885ea60007a1da88f9d476e14252f950f358a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= Date: Sun, 25 Feb 2024 22:05:08 -0500 Subject: [PATCH 1/2] [clang-tidy] bugprone-unused-return-value config now support regexes Fixes #63107 --- .../bugprone/UnusedReturnValueCheck.cpp | 202 +- .../bugprone/UnusedReturnValueCheck.h | 6 +- .../hicpp/IgnoredRemoveResultCheck.cpp| 8 +- clang-tools-extra/docs/ReleaseNotes.rst | 4 + .../checks/bugprone/unused-return-value.rst | 5 +- .../bugprone/unused-return-value-custom.cpp | 2 +- 6 files changed, 117 insertions(+), 110 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp index 05012c7df6a975..b4bf85c912c3ca 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp @@ -33,98 +33,98 @@ AST_MATCHER_P(FunctionDecl, isInstantiatedFrom, Matcher, UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), - CheckedFunctions(Options.get("CheckedFunctions", - "::std::async;" - "::std::launder;" - "::std::remove;" - "::std::remove_if;" - "::std::unique;" - "::std::unique_ptr::release;" - "::std::basic_string::empty;" - "::std::vector::empty;" - "::std::back_inserter;" - "::std::distance;" - "::std::find;" - "::std::find_if;" - "::std::inserter;" - "::std::lower_bound;" - "::std::make_pair;" - "::std::map::count;" - "::std::map::find;" - "::std::map::lower_bound;" - "::std::multimap::equal_range;" - "::std::multimap::upper_bound;" - "::std::set::count;" - "::std::set::find;" - "::std::setfill;" - "::std::setprecision;" - "::std::setw;" - "::std::upper_bound;" - "::std::vector::at;" - // C standard library - "::bsearch;" - "::ferror;" - "::feof;" - "::isalnum;" - "::isalpha;" - "::isblank;" - "::iscntrl;" - "::isdigit;" - "::isgraph;" - "::islower;" - "::isprint;" - "::ispunct;" - "::isspace;" - "::isupper;" - "::iswalnum;" - "::iswprint;" - "::iswspace;" - "::isxdigit;" - "::memchr;" - "::memcmp;" - "::strcmp;" - "::strcoll;" - "::strncmp;" - "::strpbrk;" - "::strrchr;" - "::strspn;" - "::strstr;" - "::wcscmp;" - // POSIX - "::access;" - "::bind;" - "::connect;" - "::difftime;" - "::dlsym;" - "::fnmatch;" - "::getaddrinfo;" - "::getopt;" - "::htonl;" - "::htons;" - "::iconv_open;" -
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/felix642 approved this pull request. LGTM, There is still some work to do to support more cases like ternary operators and if/else statements, but I think we have a decent foundation to work on and we can add these cases later in other PRs. https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
felix642 wrote: @11happy I was testing with `d5b8dc25598` but this seems to be working properly now. I would assume that this has been fixed in your latest commits https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/felix642 edited https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/felix642 requested changes to this pull request. Looks good! I was able run your code on llvm's project and I'm happy with the changes that I see. There is maybe one more use case that we should check before I am ready to approve (Except what Piotr has already highlighted) The following code generates an invalid fix : ``` #include int main() { std::vector a; int b; if(a[0] < b) { a[0] = b; } ``` ``` #include int main() { std::vector a; int b; a[0] = std::max(a[0], b); } ``` The type of `a` and `b` are considered different by your check. `a` is resolved has value_type instead of int. If we are able to fix that I would be ready accept this PR! https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,137 @@ +//===--- UseStdMinMaxCheck.cpp - clang-tidy ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "UseStdMinMaxCheck.h" +#include "../utils/ASTUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Preprocessor.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static const llvm::StringRef AlgorithmHeader(""); + +UseStdMinMaxCheck::UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + IncludeInserter(Options.getLocalOrGlobal("IncludeStyle", + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()) {} + +void UseStdMinMaxCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle()); +} + +void UseStdMinMaxCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + ifStmt( + stmt().bind("if"), + hasCondition(binaryOperator(hasAnyOperatorName("<", ">", "<=", ">="), + hasLHS(expr().bind("CondLhs")), + hasRHS(expr().bind("CondRhs", + hasThen(anyOf(stmt(binaryOperator(hasOperatorName("="), +hasLHS(expr().bind("AssignLhs")), +hasRHS(expr().bind("AssignRhs", +compoundStmt(statementCountIs(1), + has(binaryOperator( felix642 wrote: The binaryOperator AST matcher seems to be present twice. Might be a good idea to extract it into a variable ? https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/felix642 deleted https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/felix642 requested changes to this pull request. Added a few more comments regarding some nits but we're almost there https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,195 @@ +//===--- UseStdMinMaxCheck.cpp - clang-tidy ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "UseStdMinMaxCheck.h" +#include "../utils/ASTUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Preprocessor.h" +#include +using namespace std; + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +class ExprVisitor : public clang::RecursiveASTVisitor { +public: + explicit ExprVisitor(clang::ASTContext *Context) : Context(Context) {} + bool visitStmt(const clang::Stmt *S, bool &found, + clang::QualType &GlobalImplicitCastType) { + +if (isa(S) && !found) { felix642 wrote: You might want to (ignore/filter) some implicit cast types otherwise code like this one might give you some unexpected results: ``` #include class Foo {}; int bar() { std::vector> a; unsigned int b = a[0].size(); if(a[0].size() > b) b = a[0].size(); } ``` https://godbolt.org/z/M67o5hxrG https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/felix642 edited https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/felix642 requested changes to this pull request. https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,195 @@ +//===--- UseStdMinMaxCheck.cpp - clang-tidy ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "UseStdMinMaxCheck.h" +#include "../utils/ASTUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Preprocessor.h" +#include felix642 wrote: Also, do not include iostream https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/felix642 edited https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,195 @@ +//===--- UseStdMinMaxCheck.cpp - clang-tidy ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "UseStdMinMaxCheck.h" +#include "../utils/ASTUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Preprocessor.h" +#include +using namespace std; + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +class ExprVisitor : public clang::RecursiveASTVisitor { +public: + explicit ExprVisitor(clang::ASTContext *Context) : Context(Context) {} + bool visitStmt(const clang::Stmt *S, bool &found, + clang::QualType &GlobalImplicitCastType) { + +if (isa(S) && !found) { felix642 wrote: You are missing `CK_UserDefinedConversion` https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,238 @@ +//===--- UseStdMinMaxCheck.cpp - clang-tidy ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "UseStdMinMaxCheck.h" +#include "../utils/ASTUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Preprocessor.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static bool isImplicitCastType(const clang::CastKind castKind) { + switch (castKind) { + case clang::CK_CPointerToObjCPointerCast: + case clang::CK_BlockPointerToObjCPointerCast: + case clang::CK_BitCast: + case clang::CK_AnyPointerToBlockPointerCast: + case clang::CK_NullToMemberPointer: + case clang::CK_NullToPointer: + case clang::CK_IntegralToPointer: + case clang::CK_PointerToIntegral: + case clang::CK_IntegralCast: + case clang::CK_BooleanToSignedIntegral: + case clang::CK_IntegralToFloating: + case clang::CK_FloatingToIntegral: + case clang::CK_FloatingCast: + case clang::CK_ObjCObjectLValueCast: + case clang::CK_FloatingRealToComplex: + case clang::CK_FloatingComplexToReal: + case clang::CK_FloatingComplexCast: + case clang::CK_FloatingComplexToIntegralComplex: + case clang::CK_IntegralRealToComplex: + case clang::CK_IntegralComplexToReal: + case clang::CK_IntegralComplexCast: + case clang::CK_IntegralComplexToFloatingComplex: + case clang::CK_FloatingToFixedPoint: + case clang::CK_FixedPointToFloating: + case clang::CK_FixedPointCast: + case clang::CK_FixedPointToIntegral: + case clang::CK_IntegralToFixedPoint: + case clang::CK_MatrixCast: + case clang::CK_PointerToBoolean: + case clang::CK_IntegralToBoolean: + case clang::CK_FloatingToBoolean: + case clang::CK_MemberPointerToBoolean: + case clang::CK_FloatingComplexToBoolean: + case clang::CK_IntegralComplexToBoolean: +return true; + default: +return false; + } +} + +class ExprVisitor : public clang::RecursiveASTVisitor { +public: + explicit ExprVisitor(clang::ASTContext *Context) : Context(Context) {} + bool visitStmt(const clang::Stmt *S, bool &found, + clang::QualType &GlobalImplicitCastType) { + +if (isa(S) && !found) { + const auto CastKind = cast(S)->getCastKind(); + if (isImplicitCastType(CastKind)) { +found = true; +const clang::ImplicitCastExpr *ImplicitCast = +cast(S); +GlobalImplicitCastType = ImplicitCast->getType(); +// Stop visiting children. +return false; + } +} +// Continue visiting children. +for (const clang::Stmt *Child : S->children()) { + if (Child) { +this->visitStmt(Child, found, GlobalImplicitCastType); + } +} + +return true; // Continue visiting other nodes. + } + +private: + clang::ASTContext *Context; +}; + +static const llvm::StringRef AlgorithmHeader(""); + +static bool minCondition(const BinaryOperator::Opcode Op, const Expr *CondLhs, + const Expr *CondRhs, const Expr *AssignLhs, + const Expr *AssignRhs, const ASTContext &Context) { + if ((Op == BO_LT || Op == BO_LE) && + (tidy::utils::areStatementsIdentical(CondLhs, AssignRhs, Context) && + tidy::utils::areStatementsIdentical(CondRhs, AssignLhs, Context))) +return true; + + if ((Op == BO_GT || Op == BO_GE) && + (tidy::utils::areStatementsIdentical(CondLhs, AssignLhs, Context) && + tidy::utils::areStatementsIdentical(CondRhs, AssignRhs, Context))) +return true; + + return false; +} + +static bool maxCondition(const BinaryOperator::Opcode Op, const Expr *CondLhs, + const Expr *CondRhs, const Expr *AssignLhs, + const Expr *AssignRhs, const ASTContext &Context) { + if ((Op == BO_LT || Op == BO_LE) && + (tidy::utils::areStatementsIdentical(CondLhs, AssignLhs, Context) && + tidy::utils::areStatementsIdentical(CondRhs, AssignRhs, Context))) +return true; + + if ((Op == BO_GT || Op == BO_GE) && + (tidy::utils::areStatementsIdentical(CondLhs, AssignRhs, Context) && + tidy::utils::areStatementsIdentical(CondRhs, AssignLhs, Context))) +return true; + + return false; +} + +static std::string +createReplacement(const BinaryOperator::Opcode Op, const Expr *CondLhs, + const Expr *CondRhs, const Expr *AssignLhs, + const ASTContext &Context, const SourceManager &Source, + const LangOptions &LO, StringRef FunctionName, + QualType GlobalImplicitCastType) { + const llvm::StringRef CondLhsStr = Lexer::getSourceText( + Source.getExpansionRange(CondLhs->getSourceRange()), S
[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/felix642 requested changes to this pull request. https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,238 @@ +//===--- UseStdMinMaxCheck.cpp - clang-tidy ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "UseStdMinMaxCheck.h" +#include "../utils/ASTUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Preprocessor.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static bool isImplicitCastType(const clang::CastKind castKind) { + switch (castKind) { + case clang::CK_CPointerToObjCPointerCast: + case clang::CK_BlockPointerToObjCPointerCast: + case clang::CK_BitCast: + case clang::CK_AnyPointerToBlockPointerCast: + case clang::CK_NullToMemberPointer: + case clang::CK_NullToPointer: + case clang::CK_IntegralToPointer: + case clang::CK_PointerToIntegral: + case clang::CK_IntegralCast: + case clang::CK_BooleanToSignedIntegral: + case clang::CK_IntegralToFloating: + case clang::CK_FloatingToIntegral: + case clang::CK_FloatingCast: + case clang::CK_ObjCObjectLValueCast: + case clang::CK_FloatingRealToComplex: + case clang::CK_FloatingComplexToReal: + case clang::CK_FloatingComplexCast: + case clang::CK_FloatingComplexToIntegralComplex: + case clang::CK_IntegralRealToComplex: + case clang::CK_IntegralComplexToReal: + case clang::CK_IntegralComplexCast: + case clang::CK_IntegralComplexToFloatingComplex: + case clang::CK_FloatingToFixedPoint: + case clang::CK_FixedPointToFloating: + case clang::CK_FixedPointCast: + case clang::CK_FixedPointToIntegral: + case clang::CK_IntegralToFixedPoint: + case clang::CK_MatrixCast: + case clang::CK_PointerToBoolean: + case clang::CK_IntegralToBoolean: + case clang::CK_FloatingToBoolean: + case clang::CK_MemberPointerToBoolean: + case clang::CK_FloatingComplexToBoolean: + case clang::CK_IntegralComplexToBoolean: +return true; + default: +return false; + } +} + +class ExprVisitor : public clang::RecursiveASTVisitor { +public: + explicit ExprVisitor(clang::ASTContext *Context) : Context(Context) {} + bool visitStmt(const clang::Stmt *S, bool &found, + clang::QualType &GlobalImplicitCastType) { + +if (isa(S) && !found) { + const auto CastKind = cast(S)->getCastKind(); + if (isImplicitCastType(CastKind)) { +found = true; +const clang::ImplicitCastExpr *ImplicitCast = +cast(S); +GlobalImplicitCastType = ImplicitCast->getType(); +// Stop visiting children. +return false; + } +} +// Continue visiting children. +for (const clang::Stmt *Child : S->children()) { + if (Child) { +this->visitStmt(Child, found, GlobalImplicitCastType); + } +} + +return true; // Continue visiting other nodes. + } + +private: + clang::ASTContext *Context; +}; + +static const llvm::StringRef AlgorithmHeader(""); + +static bool minCondition(const BinaryOperator::Opcode Op, const Expr *CondLhs, + const Expr *CondRhs, const Expr *AssignLhs, + const Expr *AssignRhs, const ASTContext &Context) { + if ((Op == BO_LT || Op == BO_LE) && + (tidy::utils::areStatementsIdentical(CondLhs, AssignRhs, Context) && + tidy::utils::areStatementsIdentical(CondRhs, AssignLhs, Context))) +return true; + + if ((Op == BO_GT || Op == BO_GE) && + (tidy::utils::areStatementsIdentical(CondLhs, AssignLhs, Context) && + tidy::utils::areStatementsIdentical(CondRhs, AssignRhs, Context))) +return true; + + return false; +} + +static bool maxCondition(const BinaryOperator::Opcode Op, const Expr *CondLhs, + const Expr *CondRhs, const Expr *AssignLhs, + const Expr *AssignRhs, const ASTContext &Context) { + if ((Op == BO_LT || Op == BO_LE) && + (tidy::utils::areStatementsIdentical(CondLhs, AssignLhs, Context) && + tidy::utils::areStatementsIdentical(CondRhs, AssignRhs, Context))) +return true; + + if ((Op == BO_GT || Op == BO_GE) && + (tidy::utils::areStatementsIdentical(CondLhs, AssignRhs, Context) && + tidy::utils::areStatementsIdentical(CondRhs, AssignLhs, Context))) +return true; + + return false; +} + +static std::string +createReplacement(const BinaryOperator::Opcode Op, const Expr *CondLhs, + const Expr *CondRhs, const Expr *AssignLhs, + const ASTContext &Context, const SourceManager &Source, + const LangOptions &LO, StringRef FunctionName, + QualType GlobalImplicitCastType) { + const llvm::StringRef CondLhsStr = Lexer::getSourceText( + Source.getExpansionRange(CondLhs->getSourceRange()), S
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,189 @@ +//===--- UseStdMinMaxCheck.cpp - clang-tidy ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "UseStdMinMaxCheck.h" +#include "../utils/ASTUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Preprocessor.h" +#include +using namespace std; + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +class ExprVisitor : public clang::RecursiveASTVisitor { +public: + explicit ExprVisitor(clang::ASTContext *Context) : Context(Context) {} + bool visitStmt(const clang::Stmt *S, bool &found, + clang::QualType &GlobalImplicitCastType) { + +if (isa(S) && !found) { + found = true; + const clang::ImplicitCastExpr *ImplicitCast = + cast(S); + GlobalImplicitCastType = ImplicitCast->getType(); + // Stop visiting children. + return false; +} +// Continue visiting children. +for (const clang::Stmt *Child : S->children()) { + if (Child) { +this->visitStmt(Child, found, GlobalImplicitCastType); + } +} + +return true; // Continue visiting other nodes. + } + +private: + clang::ASTContext *Context; felix642 wrote: Would you be able to tell me where are you using this ASTContext variable? I'm not seeing any usage and I feel like I'm missing something https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,189 @@ +//===--- UseStdMinMaxCheck.cpp - clang-tidy ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "UseStdMinMaxCheck.h" +#include "../utils/ASTUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Preprocessor.h" +#include +using namespace std; + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +class ExprVisitor : public clang::RecursiveASTVisitor { +public: + explicit ExprVisitor(clang::ASTContext *Context) : Context(Context) {} + bool visitStmt(const clang::Stmt *S, bool &found, + clang::QualType &GlobalImplicitCastType) { + +if (isa(S) && !found) { + found = true; + const clang::ImplicitCastExpr *ImplicitCast = + cast(S); + GlobalImplicitCastType = ImplicitCast->getType(); + // Stop visiting children. + return false; +} +// Continue visiting children. +for (const clang::Stmt *Child : S->children()) { + if (Child) { +this->visitStmt(Child, found, GlobalImplicitCastType); + } +} + +return true; // Continue visiting other nodes. + } + +private: + clang::ASTContext *Context; felix642 wrote: I can see that, but you're not using that variable anywhere. What's the point of declaring it if you're only initializing it and not using it? https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,27 @@ +// RUN: %check_clang_tidy %s readability-ConditionalToStdMinMax %t + +void foo() { + int value1,value2; + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::max instead of < [readability-ConditionalToStdMinMax] + if (value1 < value2) +value1 = value2; // CHECK-FIXES: value1 = std::max(value1, value2); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::min instead of < [readability-ConditionalToStdMinMax] + if (value1 < value2) +value2 = value1; // CHECK-FIXES: value2 = std::min(value1, value2); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::min instead of > [readability-ConditionalToStdMinMax] + if (value2 > value1) +value2 = value1; // CHECK-FIXES: value2 = std::min(value2, value1); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::max instead of > [readability-ConditionalToStdMinMax] + if (value2 > value1) +value1 = value2; // CHECK-FIXES: value1 = std::max(value2, value1); + + // No suggestion needed here + if (value1 == value2) +value1 = value2; + + felix642 wrote: What about classes ? We should add a test case that includes a comparison with a class member. ``` struct Foo { int x; }; void func() { int bar; Foo foo; if(bar < foo.x) bar = foo.x; } ``` I'm guessing that ideally we would also need to support this type of operation : ``` struct Foo { int x; auto operator<=>(const Foo& rhs) const = default; }; void func() { Foo foo1; Foo foo2; if(foo2 < foo1) foo2 = foo1; } ``` https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Invalid Fix-It generated for implicit-widening-multiplication-result (PR #76315)
@@ -18,7 +18,7 @@ char *t0(char *base, int a, int b) { // CHECK-NOTES-CXX: static_cast( ) // CHECK-NOTES-ALL: :[[@LINE-5]]:17: note: perform multiplication in a wider type // CHECK-NOTES-C:(ptrdiff_t) - // CHECK-NOTES-CXX: static_cast() + // CHECK-NOTES-CXX: static_cast( ) felix642 wrote: I agree with you, like I've mentionned : > Since both suggestions are overlapping clang-tidy refuses to apply them [...] This is probably the reason why they are currently not using `CHECK-FIXES`. It would be a good idea to update the tests to use `CHECK-FIXES` but we would probably need to remove one of the FIX-IT for it to work. https://github.com/llvm/llvm-project/pull/76315 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Invalid Fix-It generated for implicit-widening-multiplication-result (PR #76315)
felix642 wrote: @PiotrZSL thank you for the review. I would greatly appreciate if you could merge this PR for me since I do not have the rights to do it. https://github.com/llvm/llvm-project/pull/76315 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
felix642 wrote: ping @EugeneZelenko https://github.com/llvm/llvm-project/pull/73069 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Fix missing parentheses in readability-implicit-bool-conversion fixes (PR #74891)
https://github.com/felix642 approved this pull request. https://github.com/llvm/llvm-project/pull/74891 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Fix missing parentheses in readability-implicit-bool-conversion fixes (PR #74891)
felix642 wrote: LGTM ! You'll need to update your branch with master but the PR looks good. The only thing that seems odd is the changes that you've made to the method `areParensNeededForOverloadedOperator` where the Operators `OO_New`, `OO_Delete`, `OO_Array_New` and `OO_ArrayDelete` now need parenthesis where they did not before. I'm assuming that you have a used case in mind where parenthesis would be needed and this is why you've changed the behaviour. https://github.com/llvm/llvm-project/pull/74891 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang-tools-extra] [clang-tidy] Fix missing parentheses in readability-implicit-bool-conversion fixes (PR #74891)
https://github.com/felix642 approved this pull request. https://github.com/llvm/llvm-project/pull/74891 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/felix642 requested changes to this pull request. Overall looks good, but left a few comments regarding some nits that I think should be addressed before landing this PR. https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits