[clang-tools-extra] [clang-tidy] support `return c ? a : b;` in bugprone-return-const-ref-from-parameter (PR #107657)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tidy

Author: Julian Schmidt (5chmidti)

<details>
<summary>Changes</summary>

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.


---
Full diff: https://github.com/llvm/llvm-project/pull/107657.diff


3 Files Affected:

- (modified) clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp (+21-16) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp (+9) 


``````````diff
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 8d028f8863cb7a..969e514ee630ea 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,12 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/casting-through-void>` check to suggest replacing
   the offending code with ``reinterpret_cast``, to more clearly express intent.
 
+- 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:`modernize-use-std-format
   <clang-tidy/checks/modernize/use-std-format>` check to support replacing
   member function calls too.
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 &param, const float &paramf, int &mut_param, float &mut_paramf) {
         itf1(0);
         itf1(param);

``````````

</details>


https://github.com/llvm/llvm-project/pull/107657