Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-tidy] Support functional cast in bugprone-dangling-handle #69067

Conversation

PiotrZSL
Copy link
Member

Add support for constructor conversion based functional
cast. Allows to detect issues like:
const std::string_view test1 = std::string(a);

Add support for constructor conversion based functional
cast. Allows to detect issues like:
const std::string_view test1 = std::string(a);
Add info about change that were introduced in
commit f2e5000.
@PiotrZSL PiotrZSL self-assigned this Oct 14, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 14, 2023

@llvm/pr-subscribers-clang-tidy

Author: Piotr Zegar (PiotrZSL)

Changes

Add support for constructor conversion based functional
cast. Allows to detect issues like:
const std::string_view test1 = std::string(a);


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

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/bugprone/DanglingHandleCheck.cpp (+8-3)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp (+8)
diff --git a/clang-tools-extra/clang-tidy/bugprone/DanglingHandleCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/DanglingHandleCheck.cpp
index 9ded699ba78e66b..ffaa1edc83e7299 100644
--- a/clang-tools-extra/clang-tidy/bugprone/DanglingHandleCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/DanglingHandleCheck.cpp
@@ -33,14 +33,19 @@ handleFrom(const ast_matchers::internal::Matcher<RecordDecl> &IsAHandle,
 
 ast_matchers::internal::Matcher<Stmt> handleFromTemporaryValue(
     const ast_matchers::internal::Matcher<RecordDecl> &IsAHandle) {
+
+  const auto TemporaryExpr =
+      anyOf(cxxBindTemporaryExpr(),
+            cxxFunctionalCastExpr(hasCastKind(CK_ConstructorConversion),
+                                  hasSourceExpression(cxxBindTemporaryExpr())));
   // If a ternary operator returns a temporary value, then both branches hold a
   // temporary value. If one of them is not a temporary then it must be copied
   // into one to satisfy the type of the operator.
   const auto TemporaryTernary = conditionalOperator(
-      hasTrueExpression(ignoringParenImpCasts(cxxBindTemporaryExpr())),
-      hasFalseExpression(ignoringParenImpCasts(cxxBindTemporaryExpr())));
+      hasTrueExpression(ignoringParenImpCasts(TemporaryExpr)),
+      hasFalseExpression(ignoringParenImpCasts(TemporaryExpr)));
 
-  return handleFrom(IsAHandle, anyOf(cxxBindTemporaryExpr(), TemporaryTernary));
+  return handleFrom(IsAHandle, anyOf(TemporaryExpr, TemporaryTernary));
 }
 
 ast_matchers::internal::Matcher<RecordDecl> isASequence() {
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 03e5dc6f164af2a..35d02f7c42f82fd 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -190,6 +190,11 @@ New check aliases
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- Improved :doc:`bugprone-dangling-handle
+  <clang-tidy/checks/bugprone/dangling-handle>` check to support functional
+  casting during type conversions at variable initialization, now with improved
+  compatibility for C++17 and later versions.
+
 - Improved :doc:`bugprone-lambda-function-name
   <clang-tidy/checks/bugprone/lambda-function-name>` check by adding option
   `IgnoreMacros` to ignore warnings in macros.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp
index 23cda5321764383..96c812617038a37 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp
@@ -108,6 +108,14 @@ void Positives() {
   std::string_view view4(ReturnsAString());
   // CHECK-MESSAGES-CXX14: [[@LINE-1]]:20: warning: std::basic_string_view outlives
   // CHECK-MESSAGES-CXX17: [[@LINE-2]]:26: warning: std::basic_string_view outlives
+
+  std::string_view view5 = std::string("test");
+  // CHECK-MESSAGES-CXX14: [[@LINE-1]]:20: warning: std::basic_string_view outlives its value [bugprone-dangling-handle]
+  // CHECK-MESSAGES-CXX17: [[@LINE-2]]:28: warning: std::basic_string_view outlives its value [bugprone-dangling-handle]
+
+  std::string_view view6 = std::string{"test"};
+  // CHECK-MESSAGES-CXX14: [[@LINE-1]]:20: warning: std::basic_string_view outlives its value [bugprone-dangling-handle]
+  // CHECK-MESSAGES-CXX17: [[@LINE-2]]:28: warning: std::basic_string_view outlives its value [bugprone-dangling-handle]
 }
 
 void OtherTypes() {

@PiotrZSL PiotrZSL merged commit 1097c71 into llvm:main Oct 26, 2023
4 checks passed
@PiotrZSL PiotrZSL deleted the 68637-clang-tidy-bugprone-dangling-handle-works-with-fooa-but-not-fooa branch October 26, 2023 05:58
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Oct 26, 2023
…m#69067)

Add support for constructor conversion based functional
cast. Allows to detect issues like:
const std::string_view test1 = std::string(a);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clang-tidy: bugprone-dangling-handle works with foo{"a"} but not foo("a")
3 participants