Skip to content

Commit

Permalink
[clang-tidy] Fix RedundantStringCStrCheck with r values
Browse files Browse the repository at this point in the history
The previous fix for this, https://reviews.llvm.org/D76761, Passed test cases but failed in the real world as std::string has a non trivial destructor so creates a CXXBindTemporaryExpr.

This handles that shortfall and updates the test case std::basic_string implementation to use a non trivial destructor to reflect real world behaviour.

Reviewed By: gribozavr2

Differential Revision: https://reviews.llvm.org/D84831
  • Loading branch information
njames93 committed Jul 29, 2020
1 parent 8d27be8 commit b99630e
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 6 deletions.
Expand Up @@ -92,16 +92,18 @@ void RedundantStringCStrCheck::registerMatchers(
callee(memberExpr().bind("member")),
callee(cxxMethodDecl(hasAnyName("c_str", "data"))))
.bind("call");

const auto HasRValueTempParent =
hasParent(materializeTemporaryExpr(unless(isBoundToLValue())));
// Detect redundant 'c_str()' calls through a string constructor.
// If CxxConstructExpr is the part of some CallExpr we need to
// check that matched ParamDecl of the ancestor CallExpr is not rvalue.
Finder->addMatcher(
traverse(ast_type_traits::TK_AsIs,
cxxConstructExpr(StringConstructorExpr,
hasArgument(0, StringCStrCallExpr),
unless(hasParent(materializeTemporaryExpr(
unless(isBoundToLValue())))))),
traverse(
ast_type_traits::TK_AsIs,
cxxConstructExpr(
StringConstructorExpr, hasArgument(0, StringCStrCallExpr),
unless(anyOf(HasRValueTempParent, hasParent(cxxBindTemporaryExpr(
HasRValueTempParent)))))),
this);

// Detect: 's == str.c_str()' -> 's == str'
Expand Down
Expand Up @@ -15,6 +15,8 @@ struct basic_string {
basic_string();
basic_string(const C *p, const A &a = A());

~basic_string();

const C *c_str() const;
const C *data() const;

Expand Down

0 comments on commit b99630e

Please sign in to comment.