[clang-tools-extra] [clang-tidy] support `return c ? a : b;` in bugprone-return-const-ref-from-parameter (PR #107657)
Julian Schmidt via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 10 14:16:00 PDT 2024
https://github.com/5chmidti updated https://github.com/llvm/llvm-project/pull/107657
>From b00b02b3d92a88fcf7d688b39d52e74e59f76ecd Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schmidt at gmail.com>
Date: Sat, 7 Sep 2024 01:54:38 +0200
Subject: [PATCH 1/3] [clang-tidy] support `return c ? a : b;` in
bugprone-return-const-ref-from-parameter
A `const &` parameter can also be returned via a conditional operator:
`c ? a : b`. This change adds support for diagnosing returning these parameters
with conditional operators.
---
.../ReturnConstRefFromParameterCheck.cpp | 37 +++++++++++--------
clang-tools-extra/docs/ReleaseNotes.rst | 5 +++
.../return-const-ref-from-parameter.cpp | 9 +++++
3 files changed, 35 insertions(+), 16 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
index adb26ade955c5e..9c12b8fb12838c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "ReturnConstRefFromParameterCheck.h"
+#include "clang/AST/Expr.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
@@ -15,20 +16,24 @@ using namespace clang::ast_matchers;
namespace clang::tidy::bugprone {
void ReturnConstRefFromParameterCheck::registerMatchers(MatchFinder *Finder) {
- Finder->addMatcher(
- returnStmt(
- hasReturnValue(declRefExpr(
- to(parmVarDecl(hasType(hasCanonicalType(
- qualType(lValueReferenceType(pointee(
- qualType(isConstQualified()))))
- .bind("type"))))
- .bind("param")))),
- hasAncestor(
- functionDecl(hasReturnTypeLoc(loc(qualType(
- hasCanonicalType(equalsBoundNode("type"))))))
- .bind("func")))
- .bind("ret"),
- this);
+ const auto DRef =
+ declRefExpr(
+ to(parmVarDecl(hasType(hasCanonicalType(
+ qualType(lValueReferenceType(pointee(
+ qualType(isConstQualified()))))
+ .bind("type"))))
+ .bind("param")))
+ .bind("dref");
+ const auto Func =
+ functionDecl(hasReturnTypeLoc(loc(
+ qualType(hasCanonicalType(equalsBoundNode("type"))))))
+ .bind("func");
+
+ Finder->addMatcher(returnStmt(hasReturnValue(DRef), hasAncestor(Func)), this);
+ Finder->addMatcher(conditionalOperator(eachOf(hasTrueExpression(DRef),
+ hasFalseExpression(DRef)),
+ hasAncestor(Func)),
+ this);
}
static bool isSameTypeIgnoringConst(QualType A, QualType B) {
@@ -85,8 +90,8 @@ void ReturnConstRefFromParameterCheck::check(
const MatchFinder::MatchResult &Result) {
const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("func");
const auto *PD = Result.Nodes.getNodeAs<ParmVarDecl>("param");
- const auto *R = Result.Nodes.getNodeAs<ReturnStmt>("ret");
- const SourceRange Range = R->getRetValue()->getSourceRange();
+ const auto *DRef = Result.Nodes.getNodeAs<DeclRefExpr>("dref");
+ const SourceRange Range = DRef->getSourceRange();
if (Range.isInvalid())
return;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 97300f12eab628..3b8dc2d1d76b33 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -137,6 +137,11 @@ Changes in existing checks
<clang-tidy/checks/bugprone/posix-return>` check to support integer literals
as LHS and posix call as RHS of comparison.
+- Improved :doc:`bugprone-return-const-ref-from-parameter
+ <clang-tidy/checks/bugprone/return-const-ref-from-parameter>` check to
+ diagnose potential dangling references when returning a ``const &`` parameter
+ by using the conditional operator ``cond ? var1 : var2``.
+
- Improved :doc:`bugprone-sizeof-expression
<clang-tidy/checks/bugprone/sizeof-expression>` check to find suspicious
usages of ``sizeof()``, ``alignof()``, and ``offsetof()`` when adding or
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp
index d13c127da7c2a1..d83d997a455d50 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp
@@ -27,6 +27,10 @@ int const &f3(TConstRef a) { return a; }
int const &f4(TConst &a) { return a; }
// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: returning a constant reference parameter
+int const &f5(TConst &a) { return true ? a : a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:42: warning: returning a constant reference parameter
+// CHECK-MESSAGES: :[[@LINE-2]]:46: warning: returning a constant reference parameter
+
template <typename T>
const T& tf1(const T &a) { return a; }
// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: returning a constant reference parameter
@@ -47,6 +51,11 @@ template <typename T>
const T& itf4(typename ConstRef<T>::type a) { return a; }
// CHECK-MESSAGES: :[[@LINE-1]]:54: warning: returning a constant reference parameter
+template <typename T>
+const T& itf5(const T &a) { return true ? a : a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: returning a constant reference parameter
+// CHECK-MESSAGES: :[[@LINE-2]]:47: warning: returning a constant reference parameter
+
void instantiate(const int ¶m, const float ¶mf, int &mut_param, float &mut_paramf) {
itf1(0);
itf1(param);
>From 1c3d6275de45b9c523ad0eaf61b8a63bce5f5aba Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schmidt at gmail.com>
Date: Wed, 9 Oct 2024 21:45:29 +0200
Subject: [PATCH 2/3] the conditional operator has to be the returned value
---
.../bugprone/ReturnConstRefFromParameterCheck.cpp | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
index 9c12b8fb12838c..ea7432f2549cca 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
@@ -30,10 +30,11 @@ void ReturnConstRefFromParameterCheck::registerMatchers(MatchFinder *Finder) {
.bind("func");
Finder->addMatcher(returnStmt(hasReturnValue(DRef), hasAncestor(Func)), this);
- Finder->addMatcher(conditionalOperator(eachOf(hasTrueExpression(DRef),
- hasFalseExpression(DRef)),
- hasAncestor(Func)),
- this);
+ Finder->addMatcher(
+ returnStmt(hasReturnValue(ignoringParens(conditionalOperator(
+ eachOf(hasTrueExpression(DRef), hasFalseExpression(DRef)),
+ hasAncestor(Func))))),
+ this);
}
static bool isSameTypeIgnoringConst(QualType A, QualType B) {
>From f47238fec6d8a84fb968cc2c4eb6f78a76ebd6e0 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schmidt at gmail.com>
Date: Thu, 10 Oct 2024 23:15:03 +0200
Subject: [PATCH 3/3] ignore parens for the DeclRefExpr of the parameter
---
.../clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
index ea7432f2549cca..7cc4fe519d3a64 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
@@ -16,14 +16,14 @@ using namespace clang::ast_matchers;
namespace clang::tidy::bugprone {
void ReturnConstRefFromParameterCheck::registerMatchers(MatchFinder *Finder) {
- const auto DRef =
+ const auto DRef = ignoringParens(
declRefExpr(
to(parmVarDecl(hasType(hasCanonicalType(
qualType(lValueReferenceType(pointee(
qualType(isConstQualified()))))
.bind("type"))))
.bind("param")))
- .bind("dref");
+ .bind("dref"));
const auto Func =
functionDecl(hasReturnTypeLoc(loc(
qualType(hasCanonicalType(equalsBoundNode("type"))))))
More information about the cfe-commits
mailing list