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