[clang-tidy] Fix c_str() removal and cast addition when re-ordering arguments

[mikecrowe]

Summary

The modernize-use-std-print check would get confused if it had to
re-order field-width and precision arguments at the same time as adding
casts or removing calls to c_str().

Fix this by tracking the argument indices and combining c_str() removal
with argument re-ordering. Add missing test cases to lit check.

Fixes https://github.com/llvm/llvm-project/issues/64033

[mikecrowe]

Fixing this was rather messier than I expected, but this solution does seem to work for the test cases. Here's my original commit message before I trimmed it, in case it provides any more insight.

[clang-tidy] Fix c_str() removal and cast addition when re-ordering arguments

The modernize-use-std-print check would get confused if it had to
re-order field-width and precision arguments at the same time as adding
casts or removing calls to c_str(). For example applying the check with
StrictMode enabled to:

 printf("%*s=%*d\n", 4, s.c_str(), 3, ui);

yields:

 std::println("{:>{}}={:{}}", s.c_str(), s4, ui, static_cast<int>(3));

rather than the expected:

 std::println("{:>{}}={:{}}", s, 4, static_cast<int>(ui), 3);

Fix this by:

- storing the ArgIndex rather than the Expr pointer for any arguments
  that need casts to be added in ArgFixes so that the index can be
  modified when the arguments are re-ordered. Use a struct rather than a
  tuple for clarity.

- Making applyFixes do argument re-ordering first, but also taking care
  of c_str() removal at the same time if necessary. Update the argument
  index of any ArgFixes too.

- Apply the ArgFixes afterwards.

- Finally apply and c_str() removals that remain.

- Add lit check test cases for the combinations of argument reordering,
  casts and c_str() removal. This required moving the definition of
  struct iterator earlier in use-std-print.cpp.

[mikecrowe]

Rebase on top of D156616 and add more tests

[mikecrowe]

Reinstate version I incorrectly replaced with D154287

[mikecrowe]

@PiotrZSL, I think that this is quite an important fix since without it the check completely mangles the code. Should it be put in the 17.x release branch too?

[PiotrZSL]

In D156616#4621914, @mikecrowe wrote:

@PiotrZSL, I think that this is quite an important fix since without it the check completely mangles the code. Should it be put in the 17.x release branch too?

If issue happens only when StrictMode is set to True, then I wouldn't worry too much, and then this could wait for Clang 18 that is just half year away.

[mikecrowe]

In D156616#4621945, @PiotrZSL wrote:

In D156616#4621914, @mikecrowe wrote:

@PiotrZSL, I think that this is quite an important fix since without it the check completely mangles the code. Should it be put in the 17.x release branch too?

If issue happens only when StrictMode is set to True, then I wouldn't worry too much, and then this could wait for Clang 18 that is just half year away.

The removal of c_str() happens even without StrictMode set to True.

[PiotrZSL]

In D156616#4621975, @mikecrowe wrote:

The removal of c_str() happens even without StrictMode set to True.

I still think we will be fine. There are not many situations when arguments need to be reorder. And most probably not many users will change default configuration.
LLVM 17 is already past RC3, this issue is not critical, nor a regression, user can always disable check, or just manually fix few problematic cases in code.
Additionally user could always take "main" version, and use it just to apply fixes from this check.

[mikecrowe]

Additionally user could always take "main" version, and use it just to apply fixes from this check.

OK. Can this land on main then?

Thanks.

[mikecrowe]

Thanks for the review and landing this.

Mike.

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

private:
   /// Lazily-created c_str() call matcher
   std::optional<clang::ast_matchers::StatementMatcher>
       StringCStrCallExprMatcher;
 
   const StringLiteral *FormatExpr;
   std::string StandardFormatString;
 
   /// Casts to be used to wrap arguments to retain printf compatibility.
-  std::vector<std::tuple<const Expr *, std::string>> ArgFixes;
+  struct ArgumentFix {
+    unsigned ArgIndex;
+    std::string Fix;
+
+    // We currently need this for emplace_back. Roll on C++20.
+    explicit ArgumentFix(unsigned ArgIndex, std::string Fix)
+        : ArgIndex(ArgIndex), Fix(std::move(Fix)) {}
+  };
+
+  std::vector<ArgumentFix> ArgFixes;
   std::vector<clang::ast_matchers::BoundNodes> ArgCStrRemovals;
 
   // Argument rotations to cope with the fact that std::print puts the value to
   // be formatted first and the width and precision afterwards whereas printf
   // puts the width and preicision first.
   std::vector<std::tuple<unsigned, unsigned>> ArgRotates;
 
   void emitAlignment(const PrintfSpecifier &FS, std::string &FormatSpec);
   void emitSign(const PrintfSpecifier &FS, std::string &FormatSpec);
   void emitAlternativeForm(const PrintfSpecifier &FS, std::string &FormatSpec);
   void emitFieldWidth(const PrintfSpecifier &FS, std::string &FormatSpec);
   void emitPrecision(const PrintfSpecifier &FS, std::string &FormatSpec);
-  void emitStringArgument(const Expr *Arg);
+  void emitStringArgument(unsigned ArgIndex, const Expr *Arg);
   bool emitIntegerArgument(ConversionSpecifier::Kind ArgKind, const Expr *Arg,
                            unsigned ArgIndex, std::string &FormatSpec);
 
   bool emitType(const PrintfSpecifier &FS, const Expr *Arg,
                 std::string &FormatSpec);
   bool convertArgument(const PrintfSpecifier &FS, const Expr *Arg,
                        std::string &StandardFormatString);
 

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

void FormatStringConverter::maybeRotateArguments(const PrintfSpecifier &FS) {
   if (FieldPrecision.getHowSpecified() == OptionalAmount::Arg &&
       !FieldPrecision.usesPositionalArg())
     ++ArgCount;
 
   if (ArgCount)
     ArgRotates.emplace_back(FS.getArgIndex() + ArgsOffset, ArgCount);
 }
 
-void FormatStringConverter::emitStringArgument(const Expr *Arg) {
+void FormatStringConverter::emitStringArgument(unsigned ArgIndex,
+                                               const Expr *Arg) {
   // If the argument is the result of a call to std::string::c_str() or
   // data() with a return type of char then we can remove that call and
   // pass the std::string directly. We don't want to do so if the return
   // type is not a char pointer (though it's unlikely that such code would
   // compile without warnings anyway.) See RedundantStringCStrCheck.
 
   if (!StringCStrCallExprMatcher) {
     // Lazily create the matcher
void FormatStringConverter::emitStringArgument(unsigned ArgIndex,
   auto CStrMatches = match(*StringCStrCallExprMatcher, *Arg, *Context);
   if (CStrMatches.size() == 1)
     ArgCStrRemovals.push_back(CStrMatches.front());
   else if (Arg->getType()->isPointerType()) {
     const QualType Pointee = Arg->getType()->getPointeeType();
     // printf is happy to print signed char and unsigned char strings, but
     // std::format only likes char strings.
     if (Pointee->isCharType() && !isRealCharType(Pointee))
-      ArgFixes.emplace_back(Arg, "reinterpret_cast<const char *>(");
+      ArgFixes.emplace_back(ArgIndex, "reinterpret_cast<const char *>(");
   }
 }
 
 bool FormatStringConverter::emitIntegerArgument(
     ConversionSpecifier::Kind ArgKind, const Expr *Arg, unsigned ArgIndex,
     std::string &FormatSpec) {
   const clang::QualType &ArgType = Arg->getType();
   if (ArgType->isBooleanType()) {
     // std::format will print bool as either "true" or "false" by default,
     // but printf prints them as "0" or "1". Be compatible with printf by
     // requesting decimal output.
     FormatSpec.push_back('d');
   } else if (ArgType->isEnumeralType()) {
     // std::format will try to find a specialization to print the enum
     // (and probably fail), whereas printf would have just expected it to
     // be passed as its underlying type. However, printf will have forced
     // the signedness based on the format string, so we need to do the
     // same.
     if (const auto *ET = ArgType->getAs<EnumType>()) {
       if (const std::optional<std::string> MaybeCastType =
               castTypeForArgument(ArgKind, ET->getDecl()->getIntegerType()))
         ArgFixes.emplace_back(
-            Arg, (Twine("static_cast<") + *MaybeCastType + ">(").str());
+            ArgIndex, (Twine("static_cast<") + *MaybeCastType + ">(").str());
       else
         return conversionNotPossible(
             (Twine("argument ") + Twine(ArgIndex) + " has unexpected enum type")
                 .str());
     }
   } else if (CastMismatchedIntegerTypes &&
              !isMatchingSignedness(ArgKind, ArgType)) {
     // printf will happily print an unsigned type as signed if told to.
     // Even -Wformat doesn't warn for this. std::format will format as
     // unsigned unless we cast it.
     if (const std::optional<std::string> MaybeCastType =
             castTypeForArgument(ArgKind, ArgType))
       ArgFixes.emplace_back(
-          Arg, (Twine("static_cast<") + *MaybeCastType + ">(").str());
+          ArgIndex, (Twine("static_cast<") + *MaybeCastType + ">(").str());
     else
       return conversionNotPossible(
           (Twine("argument ") + Twine(ArgIndex) + " cannot be cast to " +
            Twine(ArgKind == ConversionSpecifier::Kind::uArg ? "unsigned"
                                                             : "signed") +
            " integer type to match format"
            " specifier and StrictMode is enabled")
               .str());
   } else if (isRealCharType(ArgType) || !ArgType->isIntegerType()) {
     // Only specify integer if the argument is of a different type
     FormatSpec.push_back('d');
   }
   return true;
 }
 
 /// Append the corresponding standard format string type fragment to FormatSpec,
 /// and store any argument fixes for later application.
 /// @returns true on success, false on failure
 bool FormatStringConverter::emitType(const PrintfSpecifier &FS, const Expr *Arg,
                                      std::string &FormatSpec) {
   ConversionSpecifier::Kind ArgKind = FS.getConversionSpecifier().getKind();
   switch (ArgKind) {
   case ConversionSpecifier::Kind::sArg:
-    emitStringArgument(Arg);
+    emitStringArgument(FS.getArgIndex() + ArgsOffset, Arg);
     break;
   case ConversionSpecifier::Kind::cArg:
     // The type must be "c" to get a character unless the type is exactly
     // char (whether that be signed or unsigned for the target.)
     if (!isRealCharType(Arg->getType()))
       FormatSpec.push_back('c');
     break;
   case ConversionSpecifier::Kind::dArg:
   case ConversionSpecifier::Kind::iArg:
   case ConversionSpecifier::Kind::uArg:
     if (!emitIntegerArgument(ArgKind, Arg, FS.getArgIndex() + ArgsOffset,
                              FormatSpec))
       return false;
     break;
   case ConversionSpecifier::Kind::pArg: {
     const clang::QualType &ArgType = Arg->getType();
     // std::format knows how to format void pointers and nullptrs
     if (!ArgType->isNullPtrType() && !ArgType->isVoidPointerType())
-      ArgFixes.emplace_back(Arg, "static_cast<const void *>(");
+      ArgFixes.emplace_back(FS.getArgIndex() + ArgsOffset,
+                            "static_cast<const void *>(");
     break;
   }
   case ConversionSpecifier::Kind::xArg:
     FormatSpec.push_back('x');
     break;
   case ConversionSpecifier::Kind::XArg:
     FormatSpec.push_back('X');
     break;
else if (Ch == '{') {
       StandardFormatString += "\\x";
       StandardFormatString += llvm::hexdigit(Ch >> 4, true);
       StandardFormatString += llvm::hexdigit(Ch & 0xf, true);
     } else
       StandardFormatString += Ch;
   }
 }
 
+static std::string withoutCStrReplacement(const BoundNodes &CStrRemovalMatch,
+                                          ASTContext &Context) {
+  const auto *Arg = CStrRemovalMatch.getNodeAs<Expr>("arg");
+  const auto *Member = CStrRemovalMatch.getNodeAs<MemberExpr>("member");
+  const bool Arrow = Member->isArrow();
+  return Arrow ? utils::fixit::formatDereference(*Arg, Context)
+               : tooling::fixit::getText(*Arg, Context).str();
+}
+
 /// Called by the check when it is ready to apply the fixes.
 void FormatStringConverter::applyFixes(DiagnosticBuilder &Diag,
                                        SourceManager &SM) {
   if (FormatStringNeededRewriting) {
     Diag << FixItHint::CreateReplacement(
         CharSourceRange::getTokenRange(FormatExpr->getBeginLoc(),
                                        FormatExpr->getEndLoc()),
         StandardFormatString);
   }
 
-  for (const auto &[Arg, Replacement] : ArgFixes) {
-    SourceLocation AfterOtherSide =
-        Lexer::findNextToken(Arg->getEndLoc(), SM, LangOpts)->getLocation();
-
-    Diag << FixItHint::CreateInsertion(Arg->getBeginLoc(), Replacement)
-         << FixItHint::CreateInsertion(AfterOtherSide, ")");
-  }
-
-  for (const auto &Match : ArgCStrRemovals) {
-    const auto *Call = Match.getNodeAs<CallExpr>("call");
-    const auto *Arg = Match.getNodeAs<Expr>("arg");
-    const auto *Member = Match.getNodeAs<MemberExpr>("member");
-    const bool Arrow = Member->isArrow();
-    const std::string ArgText =
-        Arrow ? utils::fixit::formatDereference(*Arg, *Context)
-              : tooling::fixit::getText(*Arg, *Context).str();
-    if (!ArgText.empty())
-      Diag << FixItHint::CreateReplacement(Call->getSourceRange(), ArgText);
-  }
-
   // ArgCount is one less than the number of arguments to be rotated.
   for (auto [ValueArgIndex, ArgCount] : ArgRotates) {
     assert(ValueArgIndex < NumArgs);
     assert(ValueArgIndex > ArgCount);
 
-    // First move the value argument to the right place.
+    // First move the value argument to the right place. But if there's a
+    // pending c_str() removal then we must do that at the same time.
+    if (const auto CStrRemovalMatch =
+            std::find_if(ArgCStrRemovals.cbegin(), ArgCStrRemovals.cend(),
+                         [ArgStartPos = Args[ValueArgIndex]->getBeginLoc()](
+                             const BoundNodes &Match) {
+                           // This c_str() removal corresponds to the argument
+                           // being moved if they start at the same location.
+                           const Expr *CStrArg = Match.getNodeAs<Expr>("arg");
+                           return ArgStartPos == CStrArg->getBeginLoc();
+                         });
+        CStrRemovalMatch != ArgCStrRemovals.end()) {
+      const std::string ArgText =
+          withoutCStrReplacement(*CStrRemovalMatch, *Context);
+      assert(!ArgText.empty());
+
+      Diag << FixItHint::CreateReplacement(
+          Args[ValueArgIndex - ArgCount]->getSourceRange(), ArgText);
+
+      // That c_str() removal is now dealt with, so we don't need to do it again
+      ArgCStrRemovals.erase(CStrRemovalMatch);
+    } else
       Diag << tooling::fixit::createReplacement(*Args[ValueArgIndex - ArgCount],
                                                 *Args[ValueArgIndex], *Context);
 
     // Now shift down the field width and precision (if either are present) to
     // accommodate it.
     for (size_t Offset = 0; Offset < ArgCount; ++Offset)
       Diag << tooling::fixit::createReplacement(
           *Args[ValueArgIndex - Offset], *Args[ValueArgIndex - Offset - 1],
           *Context);
+
+    // Now we need to modify the ArgFix index too so that we fix the right
+    // argument. We don't need to care about the width and precision indices
+    // since they never need fixing.
+    for (auto &ArgFix : ArgFixes) {
+      if (ArgFix.ArgIndex == ValueArgIndex)
+        ArgFix.ArgIndex = ValueArgIndex - ArgCount;
+    }
+  }
+
+  for (const auto &[ArgIndex, Replacement] : ArgFixes) {
+    SourceLocation AfterOtherSide =
+        Lexer::findNextToken(Args[ArgIndex]->getEndLoc(), SM, LangOpts)
+            ->getLocation();
+
+    Diag << FixItHint::CreateInsertion(Args[ArgIndex]->getBeginLoc(),
+                                       Replacement, true)
+         << FixItHint::CreateInsertion(AfterOtherSide, ")", true);
+  }
+
+  for (const auto &Match : ArgCStrRemovals) {
+    const auto *Call = Match.getNodeAs<CallExpr>("call");
+    const std::string ArgText = withoutCStrReplacement(Match, *Context);
+    if (!ArgText.empty())
+      Diag << FixItHint::CreateReplacement(Call->getSourceRange(), ArgText);
   }
 }
 } // namespace clang::tidy::utils

clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp

 // RUN: %check_clang_tidy -check-suffixes=,STRICT \
 // RUN:   -std=c++23 %s modernize-use-std-print %t -- \
 // RUN:   -config="{CheckOptions: {StrictMode: true}}" \
 // RUN:   -- -isystem %clang_tidy_headers -fexceptions
 // RUN: %check_clang_tidy -check-suffixes=,NOTSTRICT \
 // RUN:   -std=c++23 %s modernize-use-std-print %t -- \
 // RUN:   -config="{CheckOptions: {StrictMode: false}}" \
 // RUN:   -- -isystem %clang_tidy_headers -fexceptions
 #include <cstddef>
 #include <cstdint>
 #include <cstdio>
 // CHECK-FIXES: #include <print>
 #include <inttypes.h>
 #include <string.h>
 #include <string>
 
+template <typename T>
+struct iterator {
+  T *operator->();
+  T &operator*();
+};
+
 void printf_simple() {
   printf("Hello");
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print]
   // CHECK-FIXES: std::print("Hello");
 }
 
 void printf_newline() {
   printf("Hello\n");
void printf_precision() {
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
   // CHECK-FIXES: std::println("Hello {:.5}", "Goodbye");
 
   printf("Hello %.5c\n", 'G');
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
   // CHECK-FIXES: std::println("Hello {:.5}", 'G');
 }
 
-void printf_field_width_and_precision() {
+void printf_field_width_and_precision(const std::string &s1, const std::string &s2, const std::string &s3)
+{
   printf("width only:%*d width and precision:%*.*f precision only:%.*f\n", 3, 42, 4, 2, 3.14159265358979323846, 5, 2.718);
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
   // CHECK-FIXES: std::println("width only:{:{}} width and precision:{:{}.{}f} precision only:{:.{}f}", 42, 3, 3.14159265358979323846, 4, 2, 2.718, 5);
 
+  const unsigned int ui1 = 42, ui2 = 43, ui3 = 44;
+  printf("casts width only:%*d width and precision:%*.*d precision only:%.*d\n", 3, ui1, 4, 2, ui2, 5, ui3);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES-NOTSTRICT: std::println("casts width only:{:{}} width and precision:{:{}.{}} precision only:{:.{}}", ui1, 3, ui2, 4, 2, ui3, 5);
+  // CHECK-FIXES-STRICT: std::println("casts width only:{:{}} width and precision:{:{}.{}} precision only:{:.{}}", static_cast<int>(ui1), 3, static_cast<int>(ui2), 4, 2, static_cast<int>(ui3), 5);
+
+  printf("c_str removal width only:%*s width and precision:%*.*s precision only:%.*s\n", 3, s1.c_str(), 4, 2, s2.c_str(), 5, s3.c_str());
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::println("c_str removal width only:{:>{}} width and precision:{:>{}.{}} precision only:{:.{}}", s1, 3, s2, 4, 2, s3, 5);
+
+  const std::string *ps1 = &s1, *ps2 = &s2, *ps3 = &s3;
+  printf("c_str() removal pointer width only:%-*s width and precision:%-*.*s precision only:%-.*s\n", 3, ps1->c_str(), 4, 2, ps2->c_str(), 5, ps3->c_str());
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::println("c_str() removal pointer width only:{:{}} width and precision:{:{}.{}} precision only:{:.{}}", *ps1, 3, *ps2, 4, 2, *ps3, 5);
+
+  iterator<std::string> is1, is2, is3;
+  printf("c_str() removal iterator width only:%-*s width and precision:%-*.*s precision only:%-.*s\n", 3, is1->c_str(), 4, 2, is2->c_str(), 5, is3->c_str());
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::println("c_str() removal iterator width only:{:{}} width and precision:{:{}.{}} precision only:{:.{}}", *is1, 3, *is2, 4, 2, *is3, 5);
+
   printf("width and precision positional:%1$*2$.*3$f after\n", 3.14159265358979323846, 4, 2);
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
   // CHECK-FIXES: std::println("width and precision positional:{0:{1}.{2}f} after", 3.14159265358979323846, 4, 2);
 
   const int width = 10, precision = 3;
   printf("width only:%3$*1$d width and precision:%4$*1$.*2$f precision only:%5$.*2$f\n", width, precision, 42, 3.1415926, 2.718);
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
   // CHECK-FIXES: std::println("width only:{2:{0}} width and precision:{3:{0}.{1}f} precision only:{4:.{1}f}", width, precision, 42, 3.1415926, 2.718);
+
+  printf("c_str removal width only:%3$*1$s width and precision:%4$*1$.*2$s precision only:%5$.*2$s\n", width, precision, s1.c_str(), s2.c_str(), s3.c_str());
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::println("c_str removal width only:{2:>{0}} width and precision:{3:>{0}.{1}} precision only:{4:.{1}}", width, precision, s1, s2, s3);
 }
 
-void fprintf_field_width_and_precision() {
+void fprintf_field_width_and_precision(const std::string &s1, const std::string &s2, const std::string &s3) {
   fprintf(stderr, "width only:%*d width and precision:%*.*f precision only:%.*f\n", 3, 42, 4, 2, 3.14159265358979323846, 5, 2.718);
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print]
   // CHECK-FIXES: std::println(stderr, "width only:{:{}} width and precision:{:{}.{}f} precision only:{:.{}f}", 42, 3, 3.14159265358979323846, 4, 2, 2.718, 5);
 
   fprintf(stderr, "width and precision positional:%1$*2$.*3$f after\n", 3.14159265358979323846, 4, 2);
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print]
   // CHECK-FIXES: std::println(stderr, "width and precision positional:{0:{1}.{2}f} after", 3.14159265358979323846, 4, 2);
 
+  fprintf(stderr, "c_str removal width only:%*s width and precision:%*.*s precision only:%.*s\n", 3, s1.c_str(), 4, 2, s2.c_str(), 5, s3.c_str());
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print]
+  // CHECK-FIXES: std::println(stderr, "c_str removal width only:{:>{}} width and precision:{:>{}.{}} precision only:{:.{}}", s1, 3, s2, 4, 2, s3, 5);
+
+  const std::string *ps1 = &s1, *ps2 = &s2, *ps3 = &s3;
+  fprintf(stderr, "c_str() removal pointer width only:%-*s width and precision:%-*.*s precision only:%-.*s\n", 3, ps1->c_str(), 4, 2, ps2->c_str(), 5, ps3->c_str());
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print]
+  // CHECK-FIXES: std::println(stderr, "c_str() removal pointer width only:{:{}} width and precision:{:{}.{}} precision only:{:.{}}", *ps1, 3, *ps2, 4, 2, *ps3, 5);
+
+  iterator<std::string> is1, is2, is3;
+  fprintf(stderr, "c_str() removal iterator width only:%-*s width and precision:%-*.*s precision only:%-.*s\n", 3, is1->c_str(), 4, 2, is2->c_str(), 5, is3->c_str());
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print]
+  // CHECK-FIXES: std::println(stderr, "c_str() removal iterator width only:{:{}} width and precision:{:{}.{}} precision only:{:.{}}", *is1, 3, *is2, 4, 2, *is3, 5);
+
   const int width = 10, precision = 3;
   fprintf(stderr, "width only:%3$*1$d width and precision:%4$*1$.*2$f precision only:%5$.*2$f\n", width, precision, 42, 3.1415926, 2.718);
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print]
   // CHECK-FIXES: std::println(stderr, "width only:{2:{0}} width and precision:{3:{0}.{1}f} precision only:{4:.{1}f}", width, precision, 42, 3.1415926, 2.718);
+
+  fprintf(stderr, "c_str removal width only:%3$*1$s width and precision:%4$*1$.*2$s precision only:%5$.*2$s\n", width, precision, s1.c_str(), s2.c_str(), s3.c_str());
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print]
+  // CHECK-FIXES: std::println(stderr, "c_str removal width only:{2:>{0}} width and precision:{3:>{0}.{1}} precision only:{4:.{1}}", width, precision, s1, s2, s3);
 }
 
 void printf_alternative_form() {
   printf("Wibble %#x\n", 42);
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
   // CHECK-FIXES: std::println("Wibble {:#x}", 42);
 
   printf("Wibble %#20x\n", 42);

 }
 
 void fprintf_string_pointer_cstr(const std::string *s1) {
   fprintf(stderr, "fprintf string pointer c_str %s", s1->c_str());
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'fprintf' [modernize-use-std-print]
   // CHECK-FIXES: std::print(stderr, "fprintf string pointer c_str {}", *s1);
 }
 
-template <typename T>
-struct iterator {
-  T *operator->();
-  T &operator*();
-};
-
 void printf_iterator_cstr(iterator<std::string> i1, iterator<std::string> i2)
 {
   printf("printf iterator c_str %s %s\n", i1->c_str(), i2->data());
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
   // CHECK-FIXES: std::println("printf iterator c_str {} {}", *i1, *i2);
 }
 
 // Something that isn't std::string, so the calls to c_str() and data() must not