modernize-use-std-format clang tidy warning not working on example code from documentation

[belobrov-andrey]

After upgrading project to С++20 and enabling modernize-use-std-format warning there is modernize-use-std-format clang tidy warning:
Unable to use 'std::format' instead of 'StrFormat' because first argument is not a narrow string literal [modernize-use-std-format]
The minimal code sample to reproduce problem (sample from documentation):

#include <absl/strings/str_format.h>

#include <string>

std::string getDescription(const std::string& description, int value)
{
    return absl::StrFormat("The %s is %3d", description.c_str(), value);
}

Compile options: -g -stdlib=libc++ -std=c++20

[belobrov-andrey]

Reproduced with Abseil LTS 20240722.1 from Conan receipt abseil/20240722.1
Compile options from compile_commands.json: -m64 -stdlib=libc++ -g -std=gnu++20 -arch x86_64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk

[llvmbot]

@llvm/issue-subscribers-clang-tidy

Author: Andrey (belobrov-andrey)

After upgrading project to С++20 and enabling modernize-use-std-format warning there is modernize-use-std-format clang tidy warning: Unable to use 'std::format' instead of 'StrFormat' because first argument is not a narrow string literal [modernize-use-std-format] The minimal code sample to reproduce problem (sample from [documentation](https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-std-format.html)): ``` #include <absl/strings/str_format.h>

#include <string>

std::string getDescription(const std::string& description, int value)
{
return absl::StrFormat("The %s is %3d", description.c_str(), value);
}

Compile options: -g -stdlib=libc++ -std=c++20
</details>

[gle8098]

Hello! I looked into the issue a bit.

Here's the AST dump for a call to absl::StrFormat from Abseil LTS 20240722.1, compiled the listing from the original post:

  `-CallExpr 0x5ad75e33fdd8 <col:12, col:71> 'std::string'
    |-<...ref to absl::StrFormat...>
    |-MaterializeTemporaryExpr 0x5ad75e340258 <col:28> 'const FormatSpec<const char *, int>':'const absl::str_format_internal::FormatSpecTemplate<262148, 655355>' lvalue
    | `-CXXConstructExpr 0x5ad75e340000 <col:28> 'const FormatSpec<const char *, int>':'const absl::str_format_internal::FormatSpecTemplate<262148, 655355>' 'void (const char *)'
    |   `-ImplicitCastExpr 0x5ad75e33fec8 <col:28> 'const char *' <ArrayToPointerDecay>
    |     `-StringLiteral 0x5ad75e32c050 <col:28> 'const char[14]' lvalue "The %s is %3d"
    |-<...arg...>
    `-<...arg...>

Note the implicit constructor call of absl::str_format_internal::FormatSpecTemplate. The original signature of StrFormat is complex enough that it constructs a new object from a const char[].

As a result, FormatStringConverter fails to ignore this implicit call here. I suggest replacing IgnoreImplicitAsWritten with IgnoreUnlessSpelledInSource.

The tests for modernize-use-std-format are wrong because mocked version of absl::StrFormat does not invoke any constructor calls. It might be better to change it to something like this:

namespace absl
{
template <typename... Args>
std::string StrFormat(std::string_view format, const Args&... args);
} // namespace absl

In this case it would need to construct a std::string_view.

---

I made a patch with the changes above, but it did not pass the tests. In #92896 it was explicitly forbidden to use modernize-use-std-format for functions that construct something implicitly from the format arg. Perhaps we should consider relaxing this restriction? Otherwise, the check is completely useless for absl::StrFormat.
cc @mikecrowe

[mikecrowe]

Thanks for reporting this and the detailed analysis. It seems that getting this right is going to take me more attempts. I'll look into it.

[mikecrowe]

@gle8098's recommendations were spot on, except it took me a while to work out that I needed to use a std::string_view reference for the argument to make the fake function prototype match the Abseil one closely enough for the failure to occur. The suggested fix seems to work except AFAICT it is now impossible to generate the "first argument is not a narrow string literal" error, so I'll replace it with an assert and sort out the test cases.

Thanks again!

[gle8098]

@mikecrowe Thanks for fixing the issue!