diff --git a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp index 0d0834dc38fc6..24c34c4208e4b 100644 --- a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp +++ b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp @@ -42,7 +42,7 @@ static bool isRealCharType(const clang::QualType &Ty) { /// If possible, return the text name of the signed type that corresponds to the /// passed integer type. If the passed type is already signed then its name is -/// just returned. Only supports BuiltinTypes. +/// just returned. Supports BuiltinTypes and types from static std::optional getCorrespondingSignedTypeName(const clang::QualType &QT) { using namespace clang; @@ -80,6 +80,10 @@ getCorrespondingSignedTypeName(const clang::QualType &QT) { const bool InStd = SimplifiedTypeName.consume_front("std::"); const StringRef Prefix = InStd ? "std::" : ""; + if (SimplifiedTypeName.starts_with("int") && + SimplifiedTypeName.ends_with("_t")) + return (Twine(Prefix) + SimplifiedTypeName).str(); + if (SimplifiedTypeName.starts_with("uint") && SimplifiedTypeName.ends_with("_t")) return (Twine(Prefix) + SimplifiedTypeName.drop_front()).str(); @@ -453,8 +457,36 @@ bool FormatStringConverter::emitIntegerArgument( // std::format will print bool as either "true" or "false" by default, // but printf prints them as "0" or "1". Be compatible with printf by // requesting decimal output. - FormatSpec.push_back('d'); + + // In cases where `x` or `X` was specified in the format string + // these will technically have no effect, since the bool can only be zero or + // one. However, it seems best to leave them as-is anyway. + switch (ArgKind) { + case ConversionSpecifier::Kind::xArg: + FormatSpec.push_back('x'); // Not strictly needed + break; + case ConversionSpecifier::Kind::XArg: + FormatSpec.push_back('X'); + break; + default: + FormatSpec.push_back('d'); + } + } else if (ArgType->isEnumeralType()) { + + // If the format string contained `x` or `X`, then use these + // format modifiers. Otherwise the default will work. + switch (ArgKind) { + case ConversionSpecifier::Kind::xArg: + FormatSpec.push_back('x'); + break; + case ConversionSpecifier::Kind::XArg: + FormatSpec.push_back('X'); + break; + default: + break; + } + // std::format will try to find a specialization to print the enum // (and probably fail), whereas printf would have just expected it to // be passed as its underlying type. However, printf will have forced @@ -476,10 +508,21 @@ bool FormatStringConverter::emitIntegerArgument( // Even -Wformat doesn't warn for this. std::format will format as // unsigned unless we cast it. if (const std::optional MaybeCastType = - castTypeForArgument(ArgKind, ArgType)) + castTypeForArgument(ArgKind, ArgType)) { + switch (ArgKind) { + case ConversionSpecifier::Kind::xArg: + FormatSpec.push_back('x'); + break; + case ConversionSpecifier::Kind::XArg: + FormatSpec.push_back('X'); + break; + default: + break; + } + ArgFixes.emplace_back( ArgIndex, (Twine("static_cast<") + *MaybeCastType + ">(").str()); - else + } else return conversionNotPossible( (Twine("argument ") + Twine(ArgIndex) + " cannot be cast to " + Twine(ArgKind == ConversionSpecifier::Kind::uArg ? "unsigned" @@ -487,9 +530,20 @@ bool FormatStringConverter::emitIntegerArgument( " integer type to match format" " specifier and StrictMode is enabled") .str()); - } else if (isRealCharType(ArgType) || !ArgType->isIntegerType()) { - // Only specify integer if the argument is of a different type - FormatSpec.push_back('d'); + } else { + switch (ArgKind) { + case ConversionSpecifier::Kind::xArg: + FormatSpec.push_back('x'); + break; + case ConversionSpecifier::Kind::XArg: + FormatSpec.push_back('X'); + break; + default: + if (isRealCharType(ArgType) || !ArgType->isIntegerType()) { + // Only specify integer if the argument is of a different type + FormatSpec.push_back('d'); + } + } } return true; } @@ -513,6 +567,8 @@ bool FormatStringConverter::emitType(const PrintfSpecifier &FS, const Expr *Arg, case ConversionSpecifier::Kind::dArg: case ConversionSpecifier::Kind::iArg: case ConversionSpecifier::Kind::uArg: + case ConversionSpecifier::Kind::xArg: + case ConversionSpecifier::Kind::XArg: if (!emitIntegerArgument(ArgKind, Arg, FS.getArgIndex() + ArgsOffset, FormatSpec)) return false; @@ -525,12 +581,6 @@ bool FormatStringConverter::emitType(const PrintfSpecifier &FS, const Expr *Arg, "static_cast("); break; } - case ConversionSpecifier::Kind::xArg: - FormatSpec.push_back('x'); - break; - case ConversionSpecifier::Kind::XArg: - FormatSpec.push_back('X'); - break; case ConversionSpecifier::Kind::oArg: FormatSpec.push_back('o'); break; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 32e4dfb8aa329..de7cbde037d68 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -211,7 +211,8 @@ Changes in existing checks - Improved :doc:`modernize-use-std-format ` check to correctly match when the format string is converted to a different type by an implicit - constructor call. + constructor call. Also correctly replaces signed types, and correctly + adds a ``static_cast`` to the underlying type when being printed in hex. - Improved :doc:`modernize-use-std-print ` check to correctly match