[clang-tidy] support return c ? a : b; in bugprone-return-const-ref-from-parameter
#107657
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-clang-tidy Author: Julian Schmidt (5chmidti) ChangesA Full diff: https://github.com/llvm/llvm-project/pull/107657.diff 3 Files Affected:
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 ¶m, const float ¶mf, int &mut_param, float &mut_paramf) {
itf1(0);
itf1(param);
|
|
@llvm/pr-subscribers-clang-tools-extra Author: Julian Schmidt (5chmidti) ChangesA Full diff: https://github.com/llvm/llvm-project/pull/107657.diff 3 Files Affected:
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 ¶m, const float ¶mf, int &mut_param, float &mut_paramf) {
itf1(0);
itf1(param);
|
…-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.
5chmidti
force-pushed
the
clang_tidy_return_const_ref_conditional_operator
branch
from
4c18d09 to
b00b02b
Compare
HerrCai0907
reviewed
Oct 3, 2024
| .bind("func"); | ||
|
|
||
| Finder->addMatcher(returnStmt(hasReturnValue(DRef), hasAncestor(Func)), this); | ||
| Finder->addMatcher(conditionalOperator(eachOf(hasTrueExpression(DRef), |
There was a problem hiding this comment.
return should also in this pattern. we should not warn for the condition expression, instead, we should warn for return c?a:b
There was a problem hiding this comment.
Done. I've also wrapped the conditionalOperator inside an ignoresParens
There was a problem hiding this comment.
I've also wrapped the
conditionalOperatorinside anignoresParens
- Added
ignoreParensaround thedeclRefExprmatcher.
Maybe these should be done as a new commit? WDYT?
HerrCai0907
approved these changes
Oct 28, 2024
smallp-o-p
pushed a commit
to smallp-o-p/llvm-project
that referenced
this pull request
Nov 3, 2024
…-from-parameter (llvm#107657) 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.
NoumanAmir657
pushed a commit
to NoumanAmir657/llvm-project
that referenced
this pull request
Nov 4, 2024
…-from-parameter (llvm#107657) 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A
const ¶meter can also be returned via a conditional operator:c ? a : b. This change adds support for diagnosing returning these parameterswith conditional operators.