[clang-tidy/google-runtime-int] Allow passing non-bitwidth types to printf()-style APIs

[benhamilton]

Summary

The google-runtime-int check currently fires on calls like:

printf("%lu", (unsigned long)foo);

However, the style guide says:

Where possible, avoid passing arguments of types specified by
bitwidth typedefs to printf-based APIs.

http://google.github.io/styleguide/cppguide.html#64-bit_Portability

This diff relaxes the check to not fire on parameters to functions
with the __format__ attribute. (I didn't specifically check
for __printf__ since there are a few variations.)

Test Plan: New tests added. Ran tests with:

% make -j16 check-clang-tools

[alexfh]

LG with one comment. Thank you for the fix!

test/clang-tidy/google-runtime-int.cpp

77 ↗

(On Diff #144645)

Please add another function similar to this one, but without the attribute and verify that the check fires on arguments to that function.

[alexfh]

clang-tidy/google/IntegerTypesCheck.cpp

66 ↗

(On Diff #144645)

I vaguely remember that there were some problems matching ancestors of TypeLocs. I hope this has been solved since then, but making a test more robust would be useful. See the other comment.

[benhamilton]

Add another test for printf without attribute

[benhamilton]

Thanks for the review. Added another test.

test/clang-tidy/google-runtime-int.cpp

77 ↗

(On Diff #144645)

Sure, done.

clang-tools-extra/trunk/clang-tidy/google/IntegerTypesCheck.cpp

 //===--- IntegerTypesCheck.cpp - clang-tidy -------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "IntegerTypesCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/AttrKinds.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Lexer.h"
 
 namespace clang {
 
 using namespace ast_matchers;
void IntegerTypesCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "SignedTypePrefix", SignedTypePrefix);
   Options.store(Opts, "TypeSuffix", TypeSuffix);
 }
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
   if (!getLangOpts().CPlusPlus)
     return;
-  Finder->addMatcher(typeLoc(loc(isInteger())).bind("tl"), this);
+  // Match any integer types, unless they are passed to a printf-based API:
+  //
+  // http://google.github.io/styleguide/cppguide.html#64-bit_Portability
+  // "Where possible, avoid passing arguments of types specified by
+  // bitwidth typedefs to printf-based APIs."
+  Finder->addMatcher(typeLoc(loc(isInteger()),
+                             unless(hasAncestor(callExpr(
+                                 callee(functionDecl(hasAttr(attr::Format)))))))
+                         .bind("tl"),
+                     this);
   IdentTable = llvm::make_unique<IdentifierTable>(getLangOpts());
 }
 
 void IntegerTypesCheck::check(const MatchFinder::MatchResult &Result) {
   auto TL = *Result.Nodes.getNodeAs<TypeLoc>("tl");
   SourceLocation Loc = TL.getLocStart();
 
   if (Loc.isInvalid() || Loc.isMacroID())

clang-tools-extra/trunk/test/clang-tidy/google-runtime-int.cpp

 
 struct A { A& operator=(const A&); };
 class B { A a[0]; };
 
 void fff() {
   B a, b;
   a = b;
 }
+
+__attribute__((__format__ (__printf__, 1, 2)))
+void myprintf(const char* s, ...);
+
+void doprint_no_warning() {
+  uint64 foo = 23;
+  myprintf("foo %lu %lu", (unsigned long)42, (unsigned long)foo);
+}
+
+void myprintf_no_attribute(const char* s, ...);
+
+void doprint_warning() {
+  uint64 foo = 23;
+  myprintf_no_attribute("foo %lu %lu", (unsigned long)42, (unsigned long)foo);
+// CHECK-MESSAGES: [[@LINE-1]]:41: warning: consider replacing 'unsigned long'
+// CHECK-MESSAGES: [[@LINE-2]]:60: warning: consider replacing 'unsigned long'
+}