llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
@llvm/pr-subscribers-clang-tidy
Author: None (DaveBrantonCTCT)
<details>
<summary>Changes</summary>
These two examples illustrate the two problems
```
int8_t int_8_value;
printf("%d", int_8_value) ;
```
Will fail to replace correctly. The int8_t will cause this error message, and
the replacement will be skipped.
`Unknown corresponding signed type for non-BuiltinType 'int8_t'`
```
SomeEnum enum_value;
printf("%x", enum_value) ;
```
This example will proceed with the replacement, but will fail to add the
necessary cast to the enum's underlying type, and so the format string will
break your build. That is, the replacement will be
```
SomeEnum enum_value;
fmt::print("{:x}", enum_value) ;
```
Instead of
```
SomeEnum enum_value;
fmt::print("{:x}", static_cast<int>(enum_value)) ;
``
---
Full diff: https://github.com/llvm/llvm-project/pull/155200.diff
1 Files Affected:
- (modified) clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
(+68-11)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
index 0df8e913100fc..76a14c5800589 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 <cstdint>
static std::optional<std::string>
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,38 @@ 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
@@ -478,8 +512,22 @@ bool FormatStringConverter::emitIntegerArgument(
// unsigned unless we cast it.
if (const std::optional<std::string> MaybeCastType =
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
return conversionNotPossible(
(Twine("argument ") + Twine(ArgIndex) + " cannot be cast to " +
@@ -488,9 +536,22 @@ 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;
}
@@ -514,6 +575,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;
@@ -526,12 +589,6 @@ bool FormatStringConverter::emitType(const PrintfSpecifier
&FS, const Expr *Arg,
"static_cast<const void *>(");
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;
``````````
</details>
https://github.com/llvm/llvm-project/pull/155200
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
