diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp --- a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp @@ -70,27 +70,53 @@ IncludeInserter.registerPreprocessor(PP); } +static clang::ast_matchers::StatementMatcher +unusedReturnValue(clang::ast_matchers::StatementMatcher MatchedCallExpr) { + auto UnusedInCompoundStmt = + compoundStmt(forEach(MatchedCallExpr), + // The checker can't currently differentiate between the + // return statement and other statements inside GNU statement + // expressions, so disable the checker inside them to avoid + // false positives. + unless(hasParent(stmtExpr()))); + auto UnusedInIfStmt = + ifStmt(eachOf(hasThen(MatchedCallExpr), hasElse(MatchedCallExpr))); + auto UnusedInWhileStmt = whileStmt(hasBody(MatchedCallExpr)); + auto UnusedInDoStmt = doStmt(hasBody(MatchedCallExpr)); + auto UnusedInForStmt = + forStmt(eachOf(hasLoopInit(MatchedCallExpr), + hasIncrement(MatchedCallExpr), hasBody(MatchedCallExpr))); + auto UnusedInRangeForStmt = cxxForRangeStmt(hasBody(MatchedCallExpr)); + auto UnusedInCaseStmt = switchCase(forEach(MatchedCallExpr)); + + return stmt(anyOf(UnusedInCompoundStmt, UnusedInIfStmt, UnusedInWhileStmt, + UnusedInDoStmt, UnusedInForStmt, UnusedInRangeForStmt, + UnusedInCaseStmt)); +} + void UseStdPrintCheck::registerMatchers(MatchFinder *Finder) { if (!PrintfLikeFunctions.empty()) Finder->addMatcher( - callExpr(argumentCountAtLeast(1), - hasArgument(0, stringLiteral(isOrdinary())), - callee(functionDecl( - unless(cxxMethodDecl()), - matchers::matchesAnyListedName(PrintfLikeFunctions)) - .bind("func_decl"))) - .bind("printf"), + unusedReturnValue( + callExpr(argumentCountAtLeast(1), + hasArgument(0, stringLiteral(isOrdinary())), + callee(functionDecl(unless(cxxMethodDecl()), + matchers::matchesAnyListedName( + PrintfLikeFunctions)) + .bind("func_decl"))) + .bind("printf")), this); if (!FprintfLikeFunctions.empty()) Finder->addMatcher( - callExpr(argumentCountAtLeast(2), - hasArgument(1, stringLiteral(isOrdinary())), - callee(functionDecl(unless(cxxMethodDecl()), - matchers::matchesAnyListedName( - FprintfLikeFunctions)) - .bind("func_decl"))) - .bind("fprintf"), + unusedReturnValue( + callExpr(argumentCountAtLeast(2), + hasArgument(1, stringLiteral(isOrdinary())), + callee(functionDecl(unless(cxxMethodDecl()), + matchers::matchesAnyListedName( + FprintfLikeFunctions)) + .bind("func_decl"))) + .bind("fprintf")), this); } diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst @@ -49,6 +49,13 @@ - The glibc extension ``%m``. +- ``printf`` and similar functions return the number of characters printed. + ``std::print`` does not. This means that any invocations that use the + return value will not be converted. Unfortunately this currently includes + explicitly-casting to ``void``. Deficiencies in this check mean that any + invocations inside ``GCC`` compound statements cannot be converted even + if the resulting value is not used. + If conversion would be incomplete or unsafe then the entire invocation will be left unchanged. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-absl.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-absl.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-absl.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-absl.cpp @@ -55,6 +55,12 @@ // CHECK-FIXES: std::println("Unsigned integer {} from short", s); } +int printf_uses_return_value(int i) { + using namespace absl; + + return PrintF("return value %d\n", i); +} + void fprintf_simple(FILE *fp) { absl::FPrintF(fp, "Hello %s %d", "world", 42); // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'FPrintF' [modernize-use-std-print] @@ -85,3 +91,9 @@ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'FPrintF' [modernize-use-std-print] // CHECK-FIXES: std::println(fp, "Unsigned integer {} from short", s); } + +int fprintf_uses_return_value(FILE *fp, int i) { + using namespace absl; + + return FPrintF(fp, "return value %d\n", i); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp @@ -48,6 +48,12 @@ printf("Hello %s %d\n", "world", 42); } +int printf_uses_return_value(int i) { + return myprintf("return value %d\n", i); + // CHECK-MESSAGES-NOT: [[@LINE-1]]:10: warning: use 'std::println' instead of 'myprintf' [modernize-use-std-print] + // CHECK-FIXES-NOT: std::println("return value {}", i); +} + void fprintf_simple(FILE *fp) { myfprintf(stderr, "Hello %s %d", "world", 42); @@ -73,3 +79,9 @@ // When using custom options leave fprintf alone fprintf(stderr, "Hello %s %d\n", "world", 42); } + +int fprintf_uses_return_value(int i) { + return myfprintf(stderr, "return value %d\n", i); + // CHECK-MESSAGES-NOT: [[@LINE-1]]:10: warning: use 'std::println' instead of 'myprintf' [modernize-use-std-print] + // CHECK-FIXES-NOT: std::println(stderr, "return value {}", i); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp @@ -1,11 +1,11 @@ // RUN: %check_clang_tidy -check-suffixes=,STRICT \ // RUN: -std=c++23 %s modernize-use-std-print %t -- \ // RUN: -config="{CheckOptions: [{key: StrictMode, value: true}]}" \ -// RUN: -- -isystem %clang_tidy_headers +// 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: [{key: StrictMode, value: false}]}" \ -// RUN: -- -isystem %clang_tidy_headers +// RUN: -- -isystem %clang_tidy_headers -fexceptions #include #include #include @@ -44,6 +44,239 @@ // CHECK-FIXES: std::println("Hello"); } +// std::print returns nothing, so any callers that use the return +// value cannot be automatically translated. +int printf_uses_return_value(int choice) { + const int i = printf("Return value assigned to variable %d\n", 42); + + extern void accepts_int(int); + accepts_int(printf("Return value passed to function %d\n", 42)); + + if (choice == 0) + printf("if body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: std::println("if body {}", i); + else if (choice == 1) + printf("else if body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: std::println("else if body {}", i); + else + printf("else body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: std::println("else body {}", i); + + if (printf("Return value used as boolean in if statement")) + if (printf("Return value used in expression if statement") == 44) + if (const int j = printf("Return value used in assignment in if statement")) + if (const int k = printf("Return value used with initializer in if statement"); k == 44) + ; + + int d = 0; + while (printf("%d", d) < 2) + ++d; + + while (true) + printf("while body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: std::println("while body {}", i); + + do + printf("do body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: std::println("do body {}", i); + while (true); + + for (;;) + printf("for body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: std::println("for body {}", i); + + for (printf("for init statement %d\n", i);;) + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: std::println("for init statement {}", i); + ;; + + for (int j = printf("for init statement %d\n", i);;) + ;; + + for (; printf("for condition %d\n", i);) + ;; + + for (;; printf("for expression %d\n", i)) + // CHECK-MESSAGES: [[@LINE-1]]:11: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: std::println("for expression {}", i) + ;; + + for (auto C : "foo") + printf("ranged-for body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: std::println("ranged-for body {}", i); + + switch (1) { + case 1: + printf("switch case body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: std::println("switch case body {}", i); + break; + default: + printf("switch default body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: std::println("switch default body {}", i); + break; + } + + try { + printf("try body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: std::println("try body {}", i); + } catch (int) { + printf("catch body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: std::println("catch body {}", i); + } + + (printf("Parenthesised expression %d\n", i)); + // CHECK-MESSAGES: [[@LINE-1]]:4: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: (std::println("Parenthesised expression {}", i)); + + // Ideally we would convert these two, but the current check doesn't cope with + // that. + (void)printf("cast to void %d\n", i); + // CHECK-MESSAGES-NOT: [[@LINE-1]]:9: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES-NOT: std::println("cast to void {}", i); + + static_cast(printf("static_cast to void %d\n", i)); + // CHECK-MESSAGES-NOT: [[@LINE-1]]:9: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES-NOT: std::println("static cast to void {}", i); + + const int x = ({ printf("GCC statement expression using return value immediately %d\n", i); }); + const int y = ({ const int y = printf("GCC statement expression using return value immediately %d\n", i); y; }); + + // Ideally we would convert this one, but the current check doesn't cope with + // that. + ({ printf("GCC statement expression with unused result %d\n", i); }); + // CHECK-MESSAGES-NOT: [[@LINE-1]]:6: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES-NOT: std::println("GCC statement expression with unused result {}", i); + + return printf("Return value used in return\n"); +} + +int fprintf_uses_return_value(int choice) { + const int i = fprintf(stderr, "Return value assigned to variable %d\n", 42); + + extern void accepts_int(int); + accepts_int(fprintf(stderr, "Return value passed to function %d\n", 42)); + + if (choice == 0) + fprintf(stderr, "if body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES: std::println(stderr, "if body {}", i); + else if (choice == 1) + fprintf(stderr, "else if body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES: std::println(stderr, "else if body {}", i); + else + fprintf(stderr, "else body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES: std::println(stderr, "else body {}", i); + + if (fprintf(stderr, "Return value used as boolean in if statement")) + if (fprintf(stderr, "Return value used in expression if statement") == 44) + if (const int j = fprintf(stderr, "Return value used in assignment in if statement")) + if (const int k = fprintf(stderr, "Return value used with initializer in if statement"); k == 44) + ; + + int d = 0; + while (fprintf(stderr, "%d", d) < 2) + ++d; + + while (true) + fprintf(stderr, "while body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES: std::println(stderr, "while body {}", i); + + do + fprintf(stderr, "do body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES: std::println(stderr, "do body {}", i); + while (true); + + for (;;) + fprintf(stderr, "for body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES: std::println(stderr, "for body {}", i); + + for (fprintf(stderr, "for init statement %d\n", i);;) + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES: std::println(stderr, "for init statement {}", i); + ;; + + for (int j = fprintf(stderr, "for init statement %d\n", i);;) + ;; + + for (; fprintf(stderr, "for condition %d\n", i);) + ;; + + for (;; fprintf(stderr, "for expression %d\n", i)) + // CHECK-MESSAGES: [[@LINE-1]]:11: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES: std::println(stderr, "for expression {}", i) + ;; + + for (auto C : "foo") + fprintf(stderr, "ranged-for body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES: std::println(stderr, "ranged-for body {}", i); + + switch (1) { + case 1: + fprintf(stderr, "switch case body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES: std::println(stderr, "switch case body {}", i); + break; + default: + fprintf(stderr, "switch default body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES: std::println(stderr, "switch default body {}", i); + break; + } + + try { + fprintf(stderr, "try body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES: std::println(stderr, "try body {}", i); + } catch (int) { + fprintf(stderr, "catch body %d\n", i); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES: std::println(stderr, "catch body {}", i); + } + + + (printf("Parenthesised expression %d\n", i)); + // CHECK-MESSAGES: [[@LINE-1]]:4: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: (std::println("Parenthesised expression {}", i)); + + // Ideally we would convert these two, but the current check doesn't cope with + // that. + (void)fprintf(stderr, "cast to void %d\n", i); + // CHECK-MESSAGES-NOT: [[@LINE-1]]:9: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES-NOT: std::println(stderr, "cast to void {}", i); + + static_cast(fprintf(stderr, "static_cast to void %d\n", i)); + // CHECK-MESSAGES-NOT: [[@LINE-1]]:9: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES-NOT: std::println(stderr, "static cast to void {}", i); + + const int x = ({ fprintf(stderr, "GCC statement expression using return value immediately %d\n", i); }); + const int y = ({ const int y = fprintf(stderr, "GCC statement expression using return value immediately %d\n", i); y; }); + + // Ideally we would convert this one, but the current check doesn't cope with + // that. + ({ fprintf(stderr, "GCC statement expression with unused result %d\n", i); }); + // CHECK-MESSAGES-NOT: [[@LINE-1]]:6: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES-NOT: std::println("GCC statement expression with unused result {}", i); + + return fprintf(stderr, "Return value used in return\n"); +} + void fprintf_simple() { fprintf(stderr, "Hello"); // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'fprintf' [modernize-use-std-print]