On Sat, Jul 28, 2012 at 5:51 AM, Matthieu Monrocq <[email protected]> wrote: > > > 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 ?
I'd hazard a guess that it's so we don't have to special case for the STL container names (& just assuming any type called "size_type" is going to be arch-size-dependent might hit a lot of cases where the name is used but that property is not true (in user code, etc)). Finding a general mechanism where the STL case falls out is usually preferable - oh, and it works the other way too. Now if a user writes code with their own typedef of size_t that happens to have some other spelling we've never seen before, we'll still issue the right suggestion. - David _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
