Skip to content

[clang-tidy] Fix modernize-use-std-format lit test signature #102759

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

Merged
merged 1 commit into from
Aug 11, 2024

Conversation

mikecrowe
Copy link
Contributor

My fix for my original fix of issue #92896 in
666d224 modified the function signature for fmt::sprintf to more accurately match the real implementation in libfmt but failed to do the same for absl::StrFormat. The latter fix applied equally well to absl::StrFormat so it's important that its test verifies that the bug is fixed too.

My fix for my original fix of issue llvm#92896 in
666d224 modified the function signature
for fmt::sprintf to more accurately match the real implementation in
libfmt but failed to do the same for absl::StrFormat. The latter fix
applied equally well to absl::StrFormat so it's important that its test
verifies that the bug is fixed too.
@llvmbot
Copy link
Member

llvmbot commented Aug 10, 2024

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Mike Crowe (mikecrowe)

Changes

My fix for my original fix of issue #92896 in
666d224 modified the function signature for fmt::sprintf to more accurately match the real implementation in libfmt but failed to do the same for absl::StrFormat. The latter fix applied equally well to absl::StrFormat so it's important that its test verifies that the bug is fixed too.


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

1 Files Affected:

  • (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp (+2-3)
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp
index e8dea1dce2c972..800a95062e8f1a 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp
@@ -11,9 +11,8 @@
 
 namespace absl
 {
-// Use const char * for the format since the real type is hard to mock up.
-template <typename... Args>
-std::string StrFormat(const char *format, const Args&... args);
+template <typename S, typename... Args>
+std::string StrFormat(const S &format, const Args&... args);
 } // namespace absl
 
 template <typename T>

@mikecrowe
Copy link
Contributor Author

Thanks for the reviews. Please can someone land this for me when convenient?

@HerrCai0907 HerrCai0907 merged commit 4589bf9 into llvm:main Aug 11, 2024
11 checks passed
@mikecrowe mikecrowe deleted the mac/fix-absl-strformat-signature branch August 11, 2024 13:05
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.

4 participants