[clang-tools-extra] [clang-tidy] fix false-positives for templates in `bugprone-return-const-ref-from-parameter` (PR #90273)

Julian Schmidt via cfe-commits cfe-commits at lists.llvm.org
Thu May 2 03:42:49 PDT 2024 

https://github.com/5chmidti updated https://github.com/llvm/llvm-project/pull/90273

>From 9b5bf4e1d53b3a55f9290199aeccb02c20a1e2cc Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schmidt at gmail.com>
Date: Fri, 26 Apr 2024 22:49:38 +0200
Subject: [PATCH 1/2] [clang-tidy] fix false-positives for templates in
 `bugprone-return-const-ref-from-parameter`

In the AST for function templates, the return will be a DeclRefExpr,
even if the return type differs from that of the returned variable.
Protect against false-positives by constraining the canonical return
type to be that of the parameter.
Also streams the source range of the returned expression into the
diagnostic.
---
 .../ReturnConstRefFromParameterCheck.cpp      |  10 +-
 .../return-const-ref-from-parameter.cpp       | 114 ++++++++++++++++++
 2 files changed, 121 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
index 8ae37d4f774d23..b3f7dd6d1c86f8 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
@@ -17,8 +17,11 @@ namespace clang::tidy::bugprone {
 
 void ReturnConstRefFromParameterCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-      returnStmt(hasReturnValue(declRefExpr(to(parmVarDecl(hasType(
-                     hasCanonicalType(matchers::isReferenceToConst())))))))
+      returnStmt(
+          hasReturnValue(declRefExpr(to(parmVarDecl(hasType(hasCanonicalType(
+              qualType(matchers::isReferenceToConst()).bind("type"))))))),
+          hasAncestor(functionDecl(hasReturnTypeLoc(
+              loc(qualType(hasCanonicalType(equalsBoundNode("type"))))))))
           .bind("ret"),
       this);
 }
@@ -28,7 +31,8 @@ void ReturnConstRefFromParameterCheck::check(
   const auto *R = Result.Nodes.getNodeAs<ReturnStmt>("ret");
   diag(R->getRetValue()->getBeginLoc(),
        "returning a constant reference parameter may cause a use-after-free "
-       "when the parameter is constructed from a temporary");
+       "when the parameter is constructed from a temporary")
+      << R->getRetValue()->getSourceRange();
 }
 
 } // namespace clang::tidy::bugprone
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 a83a019ec7437d..205d93606993e1 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
@@ -4,6 +4,15 @@ using T = int;
 using TConst = int const;
 using TConstRef = int const&;
 
+template <typename T>
+struct Wrapper { Wrapper(T); };
+
+template <typename T>
+struct Identity { using type = T; };
+
+template <typename T>
+struct ConstRef { using type = const T&; };
+
 namespace invalid {
 
 int const &f1(int const &a) { return a; }
@@ -18,8 +27,59 @@ 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
 
+template <typename T>
+const T& tf1(const T &a) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: returning a constant reference parameter
+
+template <typename T>
+const T& itf1(const T &a) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:36: warning: returning a constant reference parameter
+
+template <typename T>
+typename ConstRef<T>::type itf2(const T &a) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:54: warning: returning a constant reference parameter
+
+template <typename T>
+typename ConstRef<T>::type itf3(typename ConstRef<T>::type a) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:72: warning: returning a constant reference parameter
+
+template <typename T>
+const T& itf4(typename ConstRef<T>::type a) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:54: warning: returning a constant reference parameter
+
+void instantiate(const int &param, const float &paramf, int &mut_param, float &mut_paramf) {
+        itf1(0);
+        itf1(param);
+        itf1(paramf);
+        itf2(0);
+        itf2(param);
+        itf2(paramf);
+        itf3<int>(0);
+        itf3<int>(param);
+        itf3<float>(paramf);
+        itf4<int>(0);
+        itf4<int>(param);
+        itf4<float>(paramf);
+}
+
+struct C {
+    const C& foo(const C&c) { return c; }
+// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: returning a constant reference parameter
+};
+
 } // namespace invalid
 
+namespace false_negative_because_dependent_and_not_instantiated {
+template <typename T>
+typename ConstRef<T>::type tf2(const T &a) { return a; }
+
+template <typename T>
+typename ConstRef<T>::type tf3(typename ConstRef<T>::type a) { return a; }
+
+template <typename T>
+const T& tf4(typename ConstRef<T>::type a) { return a; }
+} // false_negative_because_dependent_and_not_instantiated
+
 namespace valid {
 
 int const &f1(int &a) { return a; }
@@ -28,4 +88,58 @@ int const &f2(int &&a) { return a; }
 
 int f1(int const &a) { return a; }
 
+template <typename T>
+T tf1(T a) { return a; }
+
+template <typename T>
+T tf2(const T a) { return a; }
+
+template <typename T>
+T tf3(const T &a) { return a; }
+
+template <typename T>
+Identity<T>::type tf4(const T &a) { return a; }
+
+template <typename T>
+T itf1(T a) { return a; }
+
+template <typename T>
+T itf2(const T a) { return a; }
+
+template <typename T>
+T itf3(const T &a) { return a; }
+
+template <typename T>
+Wrapper<T> itf4(const T& a) { return a; }
+
+template <typename T>
+const T& itf5(T& a) { return a; }
+
+template <typename T>
+T itf6(T& a) { return a; }
+
+void instantiate(const int &param, const float &paramf, int &mut_param, float &mut_paramf) {
+        itf1(0);
+        itf1(param);
+        itf1(paramf);
+        itf2(0);
+        itf2(param);
+        itf2(paramf);
+        itf3(0);
+        itf3(param);
+        itf3(paramf);
+        itf2(0);
+        itf2(param);
+        itf2(paramf);
+        itf3(0);
+        itf3(param);
+        itf3(paramf);
+        itf4(param);
+        itf4(paramf);
+        itf5(mut_param);
+        itf5(mut_paramf);
+        itf6(mut_param);
+        itf6(mut_paramf);
+}
+
 } // namespace valid

>From e777cafa35f49ab2409f2e5615dc69ba1f88b558 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schmidt at gmail.com>
Date: Thu, 2 May 2024 12:42:15 +0200
Subject: [PATCH 2/2] fix Windows CI failure because of delayed template
 parsing

---
 .../checkers/bugprone/return-const-ref-from-parameter.cpp       | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 205d93606993e1..ca41bdf74a1073 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
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s bugprone-return-const-ref-from-parameter %t
+// RUN: %check_clang_tidy %s bugprone-return-const-ref-from-parameter %t -- -- -fno-delayed-template-parsing
 
 using T = int;
 using TConst = int const;

More information about the cfe-commits mailing list