[clang-tidy] Move formatDereference to FixitHintUtils

[mikecrowe]

Summary

I'd like to use RedundantStringCStrCheck's formatDereference function
from the up-coming modernize-use-std-print check. Let's move it to
FixItHintUtils so that the implementation can be shared.

[PiotrZSL]

LGTM, code compiles locally, restarted CI job (maybe will pass).
Add [NFC] to change title before committing as this changes does not impact functionality.

clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp

230

NOTE: make this internal function static so it would have internal linkage.

[mikecrowe]

Thanks for the review. New version with the static added coming shortly.

[mikecrowe]

Make needParensAfterUnaryOperator static

[mikecrowe]

Rebase, no changes.

[PiotrZSL]

I will push this change, no point to keep it open.

[PiotrZSL]

Looks like there is missing dependency in CMake:
https://lab.llvm.org/buildbot/#/builders/57/builds/27531

FAILED: lib/libclangTidyUtils.so.17git 
: && /home/buildbots/clang.15.0.4/bin/clang++ --gcc-toolchain=/usr -fPIC -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -DNDEBUG  -Wl,-z,defs -Wl,-z,nodelete -Wl,--color-diagnostics   -Wl,--gc-sections -shared -Wl,-soname,libclangTidyUtils.so.17git -o lib/libclangTidyUtils.so.17git tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/Aliasing.cpp.o tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/ASTUtils.cpp.o tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/DeclRefExprUtils.cpp.o tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/ExceptionAnalyzer.cpp.o tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/ExceptionSpecAnalyzer.cpp.o tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/ExprSequence.cpp.o tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/FileExtensionsUtils.cpp.o tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/FixItHintUtils.cpp.o tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/HeaderGuard.cpp.o tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/IncludeInserter.cpp.o tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/IncludeSorter.cpp.o tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/LexerUtils.cpp.o tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/NamespaceAliaser.cpp.o tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/OptionsUtils.cpp.o tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/RenamerClangTidyCheck.cpp.o tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/TransformerClangTidyCheck.cpp.o tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/TypeTraits.cpp.o tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/UsingInserter.cpp.o  -Wl,-rpath,"\$ORIGIN/../lib:/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/lib:"  lib/libclangTidy.so.17git  lib/libclangTransformer.so.17git  lib/libclangSema.so.17git  lib/libclangASTMatchers.so.17git  lib/libclangAST.so.17git  lib/libLLVMFrontendOpenMP.so.17git  lib/libclangLex.so.17git  lib/libclangBasic.so.17git  lib/libLLVMSupport.so.17git  -Wl,-rpath-link,/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/lib && :
ld.lld: error: undefined symbol: clang::tooling::fixit::internal::getText(clang::CharSourceRange, clang::ASTContext const&)
>>> referenced by FixItHintUtils.cpp
>>>               tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/FixItHintUtils.cpp.o:(clang::tidy::utils::fixit::formatDereference[abi:cxx11](clang::Expr const&, clang::ASTContext const&))
>>> referenced by FixItHintUtils.cpp
>>>               tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/FixItHintUtils.cpp.o:(clang::tidy::utils::fixit::formatDereference[abi:cxx11](clang::Expr const&, clang::ASTContext const&))
clang-15: error: linker command failed with exit code 1 (use -v to see invocation)

[mikecrowe]

Add clangTooling to LINK_LIBS for tooling::fixit::getText for clangTidyUtils to fix failure to build with BUILD_SHARED_LIBS=ON:

ld.lld: error: undefined symbol: clang::tooling::fixit::internal::getText(clang::CharSourceRange, clang::ASTContext const&)
>>> referenced by FixItHintUtils.cpp
>>>               tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/FixItHintUtils.cpp.o:(clang::tidy::utils::fixit::formatDereference[abi:cxx11](clang::Expr const&, clang::ASTContext const&))
>>> referenced by FixItHintUtils.cpp
>>>               tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/obj.clangTidyUtils.dir/FixItHintUtils.cpp.o:(clang::tidy::utils::fixit::formatDereference[abi:cxx11](clang::Expr const&, clang::ASTContext const&))

I've no idea whether this is a sensible solution, but it does seem to work in my testing.

[PiotrZSL]

I think it should be under clang_target_link_libraries

[mikecrowe]

Move clangTooling to clang_target_link_libraries

clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp

 //===- RedundantStringCStrCheck.cpp - Check for redundant c_str calls -----===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file implements a check for redundant calls of c_str() on strings.
 //
 //===----------------------------------------------------------------------===//
 
 #include "RedundantStringCStrCheck.h"
+#include "../utils/FixItHintUtils.h"
 #include "../utils/Matchers.h"
 #include "../utils/OptionsUtils.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/FixIt.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang::tidy::readability {
 
 namespace {
 
-// Return true if expr needs to be put in parens when it is an argument of a
-// prefix unary operator, e.g. when it is a binary or ternary operator
-// syntactically.
-bool needParensAfterUnaryOperator(const Expr &ExprNode) {
-  if (isa<clang::BinaryOperator>(&ExprNode) ||
-      isa<clang::ConditionalOperator>(&ExprNode)) {
-    return true;
-  }
-  if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(&ExprNode)) {
-    return Op->getNumArgs() == 2 && Op->getOperator() != OO_PlusPlus &&
-           Op->getOperator() != OO_MinusMinus && Op->getOperator() != OO_Call &&
-           Op->getOperator() != OO_Subscript;
-  }
-  return false;
-}
-
-// Format a pointer to an expression: prefix with '*' but simplify
-// when it already begins with '&'.  Return empty string on failure.
-std::string
-formatDereference(const ast_matchers::MatchFinder::MatchResult &Result,
-                  const Expr &ExprNode) {
-  if (const auto *Op = dyn_cast<clang::UnaryOperator>(&ExprNode)) {
-    if (Op->getOpcode() == UO_AddrOf) {
-      // Strip leading '&'.
-      return std::string(tooling::fixit::getText(
-          *Op->getSubExpr()->IgnoreParens(), *Result.Context));
-    }
-  }
-  StringRef Text = tooling::fixit::getText(ExprNode, *Result.Context);
-
-  if (Text.empty())
-    return std::string();
-
-  // Remove remaining '->' from overloaded operator call
-  Text.consume_back("->");
-
-  // Add leading '*'.
-  if (needParensAfterUnaryOperator(ExprNode)) {
-    return (llvm::Twine("*(") + Text + ")").str();
-  }
-  return (llvm::Twine("*") + Text).str();
-}
-
 AST_MATCHER(MaterializeTemporaryExpr, isBoundToLValue) {
   return Node.isBoundToLvalueReference();
 }
 
 } // end namespace
 
 RedundantStringCStrCheck::RedundantStringCStrCheck(StringRef Name,
                                                    ClangTidyContext *Context)

 void RedundantStringCStrCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call");
   const auto *Arg = Result.Nodes.getNodeAs<Expr>("arg");
   const auto *Member = Result.Nodes.getNodeAs<MemberExpr>("member");
   bool Arrow = Member->isArrow();
   // Replace the "call" node with the "arg" node, prefixed with '*'
   // if the call was using '->' rather than '.'.
   std::string ArgText =
-      Arrow ? formatDereference(Result, *Arg)
+      Arrow ? utils::fixit::formatDereference(*Arg, *Result.Context)
             : tooling::fixit::getText(*Arg, *Result.Context).str();
   if (ArgText.empty())
     return;
 
   diag(Call->getBeginLoc(), "redundant call to %0")
       << Member->getMemberDecl()
       << FixItHint::CreateReplacement(Call->getSourceRange(), ArgText);
 }
 
 } // namespace clang::tidy::readability

clang-tools-extra/clang-tidy/utils/CMakeLists.txt

 
 clang_target_link_libraries(clangTidyUtils
   PRIVATE
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangSema
+  clangTooling
   clangTransformer
   )

clang-tools-extra/clang-tidy/utils/FixItHintUtils.h

 
 /// \brief Creates fix to qualify ``VarDecl`` with the specified \c Qualifier.
 /// Requires that `Var` is isolated in written code like in `int foo = 42;`.
 std::optional<FixItHint>
 addQualifierToVarDecl(const VarDecl &Var, const ASTContext &Context,
                       DeclSpec::TQ Qualifier,
                       QualifierTarget CT = QualifierTarget::Pointee,
                       QualifierPolicy CP = QualifierPolicy::Left);
+
+// \brief Format a pointer to an expression
+std::string formatDereference(const Expr &ExprNode, const ASTContext &Context);
 } // namespace clang::tidy::utils::fixit
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_FIXITHINTUTILS_H

clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp

 //===--- FixItHintUtils.cpp - clang-tidy-----------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "FixItHintUtils.h"
 #include "LexerUtils.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/Type.h"
+#include "clang/Tooling/FixIt.h"
 #include <optional>
 
 namespace clang::tidy::utils::fixit {
 
 FixItHint changeVarDeclToReference(const VarDecl &Var, ASTContext &Context) {
   SourceLocation AmpLocation = Var.getLocation();
   auto Token = utils::lexer::getPreviousToken(
       AmpLocation, Context.getSourceManager(), Context.getLangOpts());
if (ParenStrippedType->isArrayType()) {
 
     if (AT->isPointerType())
       return changePointer(Var, Qualifier, AT->getPointeeType().getTypePtr(),
                            QualTarget, QualPolicy, Context);
   }
 
   return std::nullopt;
 }
+
+// Return true if expr needs to be put in parens when it is an argument of a
+// prefix unary operator, e.g. when it is a binary or ternary operator
+// syntactically.
+static bool needParensAfterUnaryOperator(const Expr &ExprNode) {
+  if (isa<clang::BinaryOperator>(&ExprNode) ||
+      isa<clang::ConditionalOperator>(&ExprNode)) {
+    return true;
+  }
+  if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(&ExprNode)) {
+    return Op->getNumArgs() == 2 && Op->getOperator() != OO_PlusPlus &&
+           Op->getOperator() != OO_MinusMinus && Op->getOperator() != OO_Call &&
+           Op->getOperator() != OO_Subscript;
+  }
+  return false;
+}
+
+// Format a pointer to an expression: prefix with '*' but simplify
+// when it already begins with '&'.  Return empty string on failure.
+std::string formatDereference(const Expr &ExprNode, const ASTContext &Context) {
+  if (const auto *Op = dyn_cast<clang::UnaryOperator>(&ExprNode)) {
+    if (Op->getOpcode() == UO_AddrOf) {
+      // Strip leading '&'.
+      return std::string(
+          tooling::fixit::getText(*Op->getSubExpr()->IgnoreParens(), Context));
+    }
+  }
+  StringRef Text = tooling::fixit::getText(ExprNode, Context);
+
+  if (Text.empty())
+    return std::string();
+
+  // Remove remaining '->' from overloaded operator call
+  Text.consume_back("->");
+
+  // Add leading '*'.
+  if (needParensAfterUnaryOperator(ExprNode)) {
+    return (llvm::Twine("*(") + Text + ")").str();
+  }
+  return (llvm::Twine("*") + Text).str();
+}
+
 } // namespace clang::tidy::utils::fixit