<br><br><div class="gmail_quote">On Fri, Jul 27, 2012 at 9:17 PM, Hans Wennborg <span dir="ltr"><<a href="mailto:hans@hanshq.net" target="_blank">hans@hanshq.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: hans<br>
Date: Fri Jul 27 14:17:46 2012<br>
New Revision: 160886<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=160886&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=160886&view=rev</a><br>
Log:<br>
Make -Wformat walk the typedef chain when looking for size_t, etc.<br>
<br>
Clang's -Wformat fix-its currently suggest using "%zu" for values of<br>
type size_t (in C99 or C++11 mode). However, for a type such as<br>
std::vector<T>::size_type, it does not notice that type is actually<br>
typedeffed to size_t, and instead suggests a format for the underlying<br>
type, such as "%lu" or "%u".<br>
<br>
This commit makes the format string fix mechanism walk the typedef chain<br>
so that it notices if the type is size_t, even if that isn't "at the<br>
top".<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Analysis/Analyses/FormatString.h<br>
cfe/trunk/lib/Analysis/FormatString.cpp<br>
cfe/trunk/lib/Analysis/PrintfFormatString.cpp<br>
cfe/trunk/lib/Analysis/ScanfFormatString.cpp<br>
cfe/trunk/test/Sema/format-strings-fixit.c<br>
<br>
Modified: cfe/trunk/include/clang/Analysis/Analyses/FormatString.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/FormatString.h?rev=160886&r1=160885&r2=160886&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/FormatString.h?rev=160886&r1=160885&r2=160886&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Analysis/Analyses/FormatString.h (original)<br>
+++ cfe/trunk/include/clang/Analysis/Analyses/FormatString.h Fri Jul 27 14:17:46 2012<br>
@@ -355,6 +355,10 @@<br>
bool hasStandardConversionSpecifier(const LangOptions &LangOpt) const;<br>
<br>
bool hasStandardLengthConversionCombination() const;<br>
+<br>
+ /// For a TypedefType QT, if it is a named integer type such as size_t,<br>
+ /// assign the appropriate value to LM and return true.<br>
+ static bool namedTypeToLengthModifier(QualType QT, LengthModifier &LM);<br>
};<br>
<br>
} // end analyze_format_string namespace<br>
<br>
Modified: cfe/trunk/lib/Analysis/FormatString.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/FormatString.cpp?rev=160886&r1=160885&r2=160886&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/FormatString.cpp?rev=160886&r1=160885&r2=160886&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Analysis/FormatString.cpp (original)<br>
+++ cfe/trunk/lib/Analysis/FormatString.cpp Fri Jul 27 14:17:46 2012<br>
@@ -681,3 +681,37 @@<br>
}<br>
return true;<br>
}<br>
+<br>
+bool FormatSpecifier::namedTypeToLengthModifier(QualType QT,<br>
+ LengthModifier &LM) {<br>
+ assert(isa<TypedefType>(QT) && "Expected a TypedefType");<br>
+ const TypedefNameDecl *Typedef = cast<TypedefType>(QT)->getDecl();<br>
+<br>
+ for (;;) {<br>
+ const IdentifierInfo *Identifier = Typedef->getIdentifier();<br>
+ if (Identifier->getName() == "size_t") {<br>
+ LM.setKind(LengthModifier::AsSizeT);<br>
+ return true;<br>
+ } else if (Identifier->getName() == "ssize_t") {<br>
+ // Not C99, but common in Unix.<br>
+ LM.setKind(LengthModifier::AsSizeT);<br>
+ return true;<br>
+ } else if (Identifier->getName() == "intmax_t") {<br>
+ LM.setKind(LengthModifier::AsIntMax);<br>
+ return true;<br>
+ } else if (Identifier->getName() == "uintmax_t") {<br>
+ LM.setKind(LengthModifier::AsIntMax);<br>
+ return true;<br>
+ } else if (Identifier->getName() == "ptrdiff_t") {<br>
+ LM.setKind(LengthModifier::AsPtrDiff);<br>
+ return true;<br>
+ }<br>
+<br>
+ QualType T = Typedef->getUnderlyingType();<br>
+ if (!isa<TypedefType>(T))<br>
+ break;<br>
+<br>
+ Typedef = cast<TypedefType>(T)->getDecl();<br>
+ }<br>
+ return false;<br>
+}<br>
<br>
Modified: cfe/trunk/lib/Analysis/PrintfFormatString.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/PrintfFormatString.cpp?rev=160886&r1=160885&r2=160886&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/PrintfFormatString.cpp?rev=160886&r1=160885&r2=160886&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Analysis/PrintfFormatString.cpp (original)<br>
+++ cfe/trunk/lib/Analysis/PrintfFormatString.cpp Fri Jul 27 14:17:46 2012<br>
@@ -447,21 +447,8 @@<br>
}<br>
<br>
// Handle size_t, ptrdiff_t, etc. that have dedicated length modifiers in C99.<br>
- if (isa<TypedefType>(QT) && (LangOpt.C99 || LangOpt.CPlusPlus0x)) {<br>
- const IdentifierInfo *Identifier = QT.getBaseTypeIdentifier();<br>
- if (Identifier->getName() == "size_t") {<br>
- LM.setKind(LengthModifier::AsSizeT);<br>
- } else if (Identifier->getName() == "ssize_t") {<br>
- // Not C99, but common in Unix.<br>
- LM.setKind(LengthModifier::AsSizeT);<br>
- } else if (Identifier->getName() == "intmax_t") {<br>
- LM.setKind(LengthModifier::AsIntMax);<br>
- } else if (Identifier->getName() == "uintmax_t") {<br>
- LM.setKind(LengthModifier::AsIntMax);<br>
- } else if (Identifier->getName() == "ptrdiff_t") {<br>
- LM.setKind(LengthModifier::AsPtrDiff);<br>
- }<br>
- }<br>
+ if (isa<TypedefType>(QT) && (LangOpt.C99 || LangOpt.CPlusPlus0x))<br>
+ namedTypeToLengthModifier(QT, LM);<br>
<br>
// If fixing the length modifier was enough, we are done.<br>
const analyze_printf::ArgTypeResult &ATR = getArgType(Ctx, IsObjCLiteral);<br>
<br>
Modified: cfe/trunk/lib/Analysis/ScanfFormatString.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ScanfFormatString.cpp?rev=160886&r1=160885&r2=160886&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ScanfFormatString.cpp?rev=160886&r1=160885&r2=160886&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Analysis/ScanfFormatString.cpp (original)<br>
+++ cfe/trunk/lib/Analysis/ScanfFormatString.cpp Fri Jul 27 14:17:46 2012<br>
@@ -382,21 +382,8 @@<br>
}<br>
<br>
// Handle size_t, ptrdiff_t, etc. that have dedicated length modifiers in C99.<br>
- if (isa<TypedefType>(PT) && (LangOpt.C99 || LangOpt.CPlusPlus0x)) {<br>
- const IdentifierInfo *Identifier = QT.getBaseTypeIdentifier();<br>
- if (Identifier->getName() == "size_t") {<br>
- LM.setKind(LengthModifier::AsSizeT);<br>
- } else if (Identifier->getName() == "ssize_t") {<br>
- // Not C99, but common in Unix.<br>
- LM.setKind(LengthModifier::AsSizeT);<br>
- } else if (Identifier->getName() == "intmax_t") {<br>
- LM.setKind(LengthModifier::AsIntMax);<br>
- } else if (Identifier->getName() == "uintmax_t") {<br>
- LM.setKind(LengthModifier::AsIntMax);<br>
- } else if (Identifier->getName() == "ptrdiff_t") {<br>
- LM.setKind(LengthModifier::AsPtrDiff);<br>
- }<br>
- }<br>
+ if (isa<TypedefType>(PT) && (LangOpt.C99 || LangOpt.CPlusPlus0x))<br>
+ namedTypeToLengthModifier(PT, LM);<br>
<br>
// If fixing the length modifier was enough, we are done.<br>
const analyze_scanf::ScanfArgTypeResult &ATR = getArgType(Ctx);<br>
<br>
Modified: cfe/trunk/test/Sema/format-strings-fixit.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-fixit.c?rev=160886&r1=160885&r2=160886&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-fixit.c?rev=160886&r1=160885&r2=160886&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Sema/format-strings-fixit.c (original)<br>
+++ cfe/trunk/test/Sema/format-strings-fixit.c Fri Jul 27 14:17:46 2012<br>
@@ -64,6 +64,18 @@<br>
printf("%f", (uintmax_t) 42);<br>
printf("%f", (ptrdiff_t) 42);<br>
<br>
+ // Look beyond the first typedef.<br>
+ typedef size_t my_size_type;<br>
+ typedef intmax_t my_intmax_type;<br>
+ typedef uintmax_t my_uintmax_type;<br>
+ typedef ptrdiff_t my_ptrdiff_type;<br>
+ typedef int my_int_type;<br>
+ printf("%f", (my_size_type) 42);<br>
+ printf("%f", (my_intmax_type) 42);<br>
+ printf("%f", (my_uintmax_type) 42);<br>
+ printf("%f", (my_ptrdiff_type) 42);<br>
+ printf("%f", (my_int_type) 42);<br>
+<br>
// string<br>
printf("%ld", "foo");<br>
<br>
@@ -122,6 +134,18 @@<br>
scanf("%f", &uIntmaxVar);<br>
scanf("%f", &ptrdiffVar);<br>
<br>
+ // Look beyond the first typedef for named integer types.<br>
+ typedef size_t my_size_type;<br>
+ typedef intmax_t my_intmax_type;<br>
+ typedef uintmax_t my_uintmax_type;<br>
+ typedef ptrdiff_t my_ptrdiff_type;<br>
+ typedef int my_int_type;<br>
+ scanf("%f", (my_size_type*)&sizeVar);<br>
+ scanf("%f", (my_intmax_type*)&intmaxVar);<br>
+ scanf("%f", (my_uintmax_type*)&uIntmaxVar);<br>
+ scanf("%f", (my_ptrdiff_type*)&ptrdiffVar);<br>
+ scanf("%f", (my_int_type*)&intVar);<br>
+<br>
// Preserve the original formatting.<br>
scanf("%o", &longVar);<br>
scanf("%u", &longVar);<br>
@@ -162,6 +186,11 @@<br>
// CHECK: printf("%jd", (intmax_t) 42);<br>
// CHECK: printf("%ju", (uintmax_t) 42);<br>
// CHECK: printf("%td", (ptrdiff_t) 42);<br>
+// CHECK: printf("%zu", (my_size_type) 42);<br>
+// CHECK: printf("%jd", (my_intmax_type) 42);<br>
+// CHECK: printf("%ju", (my_uintmax_type) 42);<br>
+// CHECK: printf("%td", (my_ptrdiff_type) 42);<br>
+// CHECK: printf("%d", (my_int_type) 42);<br>
// CHECK: printf("%s", "foo");<br>
// CHECK: printf("%lo", (long) 42);<br>
// CHECK: printf("%lu", (long) 42);<br>
@@ -193,6 +222,11 @@<br>
// CHECK: scanf("%jd", &intmaxVar);<br>
// CHECK: scanf("%ju", &uIntmaxVar);<br>
// CHECK: scanf("%td", &ptrdiffVar);<br>
+// CHECK: scanf("%zu", (my_size_type*)&sizeVar);<br>
+// CHECK: scanf("%jd", (my_intmax_type*)&intmaxVar);<br>
+// CHECK: scanf("%ju", (my_uintmax_type*)&uIntmaxVar);<br>
+// CHECK: scanf("%td", (my_ptrdiff_type*)&ptrdiffVar);<br>
+// CHECK: scanf("%d", (my_int_type*)&intVar);<br>
// CHECK: scanf("%lo", &longVar);<br>
// CHECK: scanf("%lu", &longVar);<br>
// CHECK: scanf("%lx", &longVar);<br>
<br><br></blockquote><div><br>Hi Hans,<br><br>I wonder why it would be necessary to "know" that the std::vector<...>::size_type is a size_t instead of being a long unsigned int. The very purpose of the typedef to `size_type` in the first place is to abstract the underlying type, and different STL implementations are free to alias it differently as far as I know, so...<br>
<br>... could you explain why it is important for you ?<br><br>-- Matthieu <br></div></div>