[clang] [clang] Update -Wformat warnings for fixed-point format specifiers (PR #82855)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 23 17:01:51 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: None (PiJoules)
<details>
<summary>Changes</summary>
ISO/IEC TR 18037 defines %r, %R, %k, and %K for fixed point format specifiers. -Wformat should not warn on these when they are provided.
---
Full diff: https://github.com/llvm/llvm-project/pull/82855.diff
6 Files Affected:
- (modified) clang/include/clang/AST/ASTContext.h (+4)
- (modified) clang/include/clang/AST/FormatString.h (+11)
- (modified) clang/lib/AST/ASTContext.cpp (+36)
- (modified) clang/lib/AST/FormatString.cpp (+25)
- (modified) clang/lib/AST/PrintfFormatString.cpp (+86-3)
- (added) clang/test/Sema/format-fixed-point.c (+134)
``````````diff
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 12ce9af1e53f63..ff6b64c7f72d57 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -2981,6 +2981,10 @@ class ASTContext : public RefCountedBase<ASTContext> {
// corresponding saturated type for a given fixed point type.
QualType getCorrespondingSaturatedType(QualType Ty) const;
+ // Per ISO N1169, this method accepts fixed point types and returns the
+ // corresponding non-saturated type for a given fixed point type.
+ QualType getCorrespondingUnsaturatedType(QualType Ty) const;
+
// This method accepts fixed point types and returns the corresponding signed
// type. Unlike getCorrespondingUnsignedType(), this only accepts unsigned
// fixed point types because there are unsigned integer types like bool and
diff --git a/clang/include/clang/AST/FormatString.h b/clang/include/clang/AST/FormatString.h
index 5c4ad9baaef608..e2232fb4a47153 100644
--- a/clang/include/clang/AST/FormatString.h
+++ b/clang/include/clang/AST/FormatString.h
@@ -171,6 +171,14 @@ class ConversionSpecifier {
ZArg, // MS extension
+ // ISO/IEC TR 18037 (fixed-point) specific specifiers.
+ kArg, // %k for signed accum types
+ KArg, // %K for unsigned accum types
+ rArg, // %r for signed fract types
+ RArg, // %R for unsigned fract types
+ FixedPointArgBeg = kArg,
+ FixedPointArgEnd = RArg,
+
// Objective-C specific specifiers.
ObjCObjArg, // '@'
ObjCBeg = ObjCObjArg,
@@ -237,6 +245,9 @@ class ConversionSpecifier {
bool isDoubleArg() const {
return kind >= DoubleArgBeg && kind <= DoubleArgEnd;
}
+ bool isFixedPointArg() const {
+ return kind >= FixedPointArgBeg && kind <= FixedPointArgEnd;
+ }
const char *toString() const;
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index c475c841233c59..5a8fae76a43a4d 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -13314,6 +13314,42 @@ QualType ASTContext::getCommonSugaredType(QualType X, QualType Y,
return R;
}
+QualType ASTContext::getCorrespondingUnsaturatedType(QualType Ty) const {
+ assert(Ty->isFixedPointType());
+
+ if (Ty->isUnsaturatedFixedPointType())
+ return Ty;
+
+ switch (Ty->castAs<BuiltinType>()->getKind()) {
+ default:
+ llvm_unreachable("Not a saturated fixed point type!");
+ case BuiltinType::SatShortAccum:
+ return ShortAccumTy;
+ case BuiltinType::SatAccum:
+ return AccumTy;
+ case BuiltinType::SatLongAccum:
+ return LongAccumTy;
+ case BuiltinType::SatUShortAccum:
+ return UnsignedShortAccumTy;
+ case BuiltinType::SatUAccum:
+ return UnsignedAccumTy;
+ case BuiltinType::SatULongAccum:
+ return UnsignedLongAccumTy;
+ case BuiltinType::SatShortFract:
+ return ShortFractTy;
+ case BuiltinType::SatFract:
+ return FractTy;
+ case BuiltinType::SatLongFract:
+ return LongFractTy;
+ case BuiltinType::SatUShortFract:
+ return UnsignedShortFractTy;
+ case BuiltinType::SatUFract:
+ return UnsignedFractTy;
+ case BuiltinType::SatULongFract:
+ return UnsignedLongFractTy;
+ }
+}
+
QualType ASTContext::getCorrespondingSaturatedType(QualType Ty) const {
assert(Ty->isFixedPointType());
diff --git a/clang/lib/AST/FormatString.cpp b/clang/lib/AST/FormatString.cpp
index c5d14b4af7ff15..0c80ad109ccbb2 100644
--- a/clang/lib/AST/FormatString.cpp
+++ b/clang/lib/AST/FormatString.cpp
@@ -403,6 +403,10 @@ ArgType::matchesType(ASTContext &C, QualType argTy) const {
else if (ETy->isUnscopedEnumerationType())
argTy = ETy->getDecl()->getIntegerType();
}
+
+ if (argTy->isSaturatedFixedPointType())
+ argTy = C.getCorrespondingUnsaturatedType(argTy);
+
argTy = C.getCanonicalType(argTy).getUnqualifiedType();
if (T == argTy)
@@ -761,6 +765,16 @@ const char *ConversionSpecifier::toString() const {
// MS specific specifiers.
case ZArg: return "Z";
+
+ // ISO/IEC TR 18037 (fixed-point) specific specifiers.
+ case rArg:
+ return "r";
+ case RArg:
+ return "R";
+ case kArg:
+ return "k";
+ case KArg:
+ return "K";
}
return nullptr;
}
@@ -825,6 +839,9 @@ bool FormatSpecifier::hasValidLengthModifier(const TargetInfo &Target,
if (LO.OpenCL && CS.isDoubleArg())
return !VectorNumElts.isInvalid();
+ if (CS.isFixedPointArg())
+ return true;
+
if (Target.getTriple().isOSMSVCRT()) {
switch (CS.getKind()) {
case ConversionSpecifier::cArg:
@@ -877,6 +894,9 @@ bool FormatSpecifier::hasValidLengthModifier(const TargetInfo &Target,
return true;
}
+ if (CS.isFixedPointArg())
+ return true;
+
switch (CS.getKind()) {
case ConversionSpecifier::bArg:
case ConversionSpecifier::BArg:
@@ -1043,6 +1063,11 @@ bool FormatSpecifier::hasStandardConversionSpecifier(
case ConversionSpecifier::UArg:
case ConversionSpecifier::ZArg:
return false;
+ case ConversionSpecifier::rArg:
+ case ConversionSpecifier::RArg:
+ case ConversionSpecifier::kArg:
+ case ConversionSpecifier::KArg:
+ return LangOpt.FixedPoint;
}
llvm_unreachable("Invalid ConversionSpecifier Kind!");
}
diff --git a/clang/lib/AST/PrintfFormatString.cpp b/clang/lib/AST/PrintfFormatString.cpp
index 3b09ca40bd2a53..3eb497767f2ec1 100644
--- a/clang/lib/AST/PrintfFormatString.cpp
+++ b/clang/lib/AST/PrintfFormatString.cpp
@@ -348,6 +348,8 @@ static PrintfSpecifierResult ParsePrintfSpecifier(FormatStringHandler &H,
case 'r':
if (isFreeBSDKPrintf)
k = ConversionSpecifier::FreeBSDrArg; // int
+ else if (LO.FixedPoint)
+ k = ConversionSpecifier::rArg;
break;
case 'y':
if (isFreeBSDKPrintf)
@@ -373,6 +375,20 @@ static PrintfSpecifierResult ParsePrintfSpecifier(FormatStringHandler &H,
if (Target.getTriple().isOSMSVCRT())
k = ConversionSpecifier::ZArg;
break;
+ // ISO/IEC TR 18037 (fixed-point) specific.
+ // NOTE: 'r' is handled up above since FreeBSD also supports %r.
+ case 'k':
+ if (LO.FixedPoint)
+ k = ConversionSpecifier::kArg;
+ break;
+ case 'K':
+ if (LO.FixedPoint)
+ k = ConversionSpecifier::KArg;
+ break;
+ case 'R':
+ if (LO.FixedPoint)
+ k = ConversionSpecifier::RArg;
+ break;
}
// Check to see if we used the Objective-C modifier flags with
@@ -627,6 +643,9 @@ ArgType PrintfSpecifier::getScalarArgType(ASTContext &Ctx,
}
}
+ if (CS.isFixedPointArg() && !Ctx.getLangOpts().FixedPoint)
+ return ArgType::Invalid();
+
switch (CS.getKind()) {
case ConversionSpecifier::sArg:
if (LM.getKind() == LengthModifier::AsWideChar) {
@@ -658,6 +677,50 @@ ArgType PrintfSpecifier::getScalarArgType(ASTContext &Ctx,
return ArgType::CPointerTy;
case ConversionSpecifier::ObjCObjArg:
return ArgType::ObjCPointerTy;
+ case ConversionSpecifier::kArg:
+ switch (LM.getKind()) {
+ case LengthModifier::None:
+ return Ctx.AccumTy;
+ case LengthModifier::AsShort:
+ return Ctx.ShortAccumTy;
+ case LengthModifier::AsLong:
+ return Ctx.LongAccumTy;
+ default:
+ return ArgType::Invalid();
+ }
+ case ConversionSpecifier::KArg:
+ switch (LM.getKind()) {
+ case LengthModifier::None:
+ return Ctx.UnsignedAccumTy;
+ case LengthModifier::AsShort:
+ return Ctx.UnsignedShortAccumTy;
+ case LengthModifier::AsLong:
+ return Ctx.UnsignedLongAccumTy;
+ default:
+ return ArgType::Invalid();
+ }
+ case ConversionSpecifier::rArg:
+ switch (LM.getKind()) {
+ case LengthModifier::None:
+ return Ctx.FractTy;
+ case LengthModifier::AsShort:
+ return Ctx.ShortFractTy;
+ case LengthModifier::AsLong:
+ return Ctx.LongFractTy;
+ default:
+ return ArgType::Invalid();
+ }
+ case ConversionSpecifier::RArg:
+ switch (LM.getKind()) {
+ case LengthModifier::None:
+ return Ctx.UnsignedFractTy;
+ case LengthModifier::AsShort:
+ return Ctx.UnsignedShortFractTy;
+ case LengthModifier::AsLong:
+ return Ctx.UnsignedLongFractTy;
+ default:
+ return ArgType::Invalid();
+ }
default:
break;
}
@@ -955,6 +1018,10 @@ bool PrintfSpecifier::hasValidPlusPrefix() const {
case ConversionSpecifier::AArg:
case ConversionSpecifier::FreeBSDrArg:
case ConversionSpecifier::FreeBSDyArg:
+ case ConversionSpecifier::rArg:
+ case ConversionSpecifier::RArg:
+ case ConversionSpecifier::kArg:
+ case ConversionSpecifier::KArg:
return true;
default:
@@ -966,7 +1033,7 @@ bool PrintfSpecifier::hasValidAlternativeForm() const {
if (!HasAlternativeForm)
return true;
- // Alternate form flag only valid with the bBoxXaAeEfFgG conversions
+ // Alternate form flag only valid with the bBoxXaAeEfFgGrRkK conversions
switch (CS.getKind()) {
case ConversionSpecifier::bArg:
case ConversionSpecifier::BArg:
@@ -984,6 +1051,10 @@ bool PrintfSpecifier::hasValidAlternativeForm() const {
case ConversionSpecifier::GArg:
case ConversionSpecifier::FreeBSDrArg:
case ConversionSpecifier::FreeBSDyArg:
+ case ConversionSpecifier::rArg:
+ case ConversionSpecifier::RArg:
+ case ConversionSpecifier::kArg:
+ case ConversionSpecifier::KArg:
return true;
default:
@@ -995,7 +1066,7 @@ bool PrintfSpecifier::hasValidLeadingZeros() const {
if (!HasLeadingZeroes)
return true;
- // Leading zeroes flag only valid with the bBdiouxXaAeEfFgG conversions
+ // Leading zeroes flag only valid with the bBdiouxXaAeEfFgGrRkK conversions
switch (CS.getKind()) {
case ConversionSpecifier::bArg:
case ConversionSpecifier::BArg:
@@ -1018,6 +1089,10 @@ bool PrintfSpecifier::hasValidLeadingZeros() const {
case ConversionSpecifier::GArg:
case ConversionSpecifier::FreeBSDrArg:
case ConversionSpecifier::FreeBSDyArg:
+ case ConversionSpecifier::rArg:
+ case ConversionSpecifier::RArg:
+ case ConversionSpecifier::kArg:
+ case ConversionSpecifier::KArg:
return true;
default:
@@ -1044,6 +1119,10 @@ bool PrintfSpecifier::hasValidSpacePrefix() const {
case ConversionSpecifier::AArg:
case ConversionSpecifier::FreeBSDrArg:
case ConversionSpecifier::FreeBSDyArg:
+ case ConversionSpecifier::rArg:
+ case ConversionSpecifier::RArg:
+ case ConversionSpecifier::kArg:
+ case ConversionSpecifier::KArg:
return true;
default:
@@ -1089,7 +1168,7 @@ bool PrintfSpecifier::hasValidPrecision() const {
if (Precision.getHowSpecified() == OptionalAmount::NotSpecified)
return true;
- // Precision is only valid with the bBdiouxXaAeEfFgGsP conversions
+ // Precision is only valid with the bBdiouxXaAeEfFgGsPrRkK conversions
switch (CS.getKind()) {
case ConversionSpecifier::bArg:
case ConversionSpecifier::BArg:
@@ -1114,6 +1193,10 @@ bool PrintfSpecifier::hasValidPrecision() const {
case ConversionSpecifier::FreeBSDrArg:
case ConversionSpecifier::FreeBSDyArg:
case ConversionSpecifier::PArg:
+ case ConversionSpecifier::rArg:
+ case ConversionSpecifier::RArg:
+ case ConversionSpecifier::kArg:
+ case ConversionSpecifier::KArg:
return true;
default:
diff --git a/clang/test/Sema/format-fixed-point.c b/clang/test/Sema/format-fixed-point.c
new file mode 100644
index 00000000000000..a90a205ab1adb7
--- /dev/null
+++ b/clang/test/Sema/format-fixed-point.c
@@ -0,0 +1,134 @@
+// RUN: %clang_cc1 -ffixed-point -fsyntax-only -verify -Wformat -isystem %S/Inputs %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wformat -isystem %S/Inputs %s -DWITHOUT_FIXED_POINT
+
+int printf(const char *restrict, ...);
+
+short s;
+unsigned short us;
+int i;
+unsigned int ui;
+long l;
+unsigned long ul;
+float fl;
+double d;
+char c;
+unsigned char uc;
+
+#ifndef WITHOUT_FIXED_POINT
+short _Fract sf;
+_Fract f;
+long _Fract lf;
+unsigned short _Fract usf;
+unsigned _Fract uf;
+unsigned long _Fract ulf;
+short _Accum sa;
+_Accum a;
+long _Accum la;
+unsigned short _Accum usa;
+unsigned _Accum ua;
+unsigned long _Accum ula;
+_Sat short _Fract sat_sf;
+_Sat _Fract sat_f;
+_Sat long _Fract sat_lf;
+_Sat unsigned short _Fract sat_usf;
+_Sat unsigned _Fract sat_uf;
+_Sat unsigned long _Fract sat_ulf;
+_Sat short _Accum sat_sa;
+_Sat _Accum sat_a;
+_Sat long _Accum sat_la;
+_Sat unsigned short _Accum sat_usa;
+_Sat unsigned _Accum sat_ua;
+_Sat unsigned long _Accum sat_ula;
+
+void test_invalid_args(void) {
+ /// None of these should match against a fixed point type.
+ printf("%r", s); // expected-warning{{format specifies type '_Fract' but the argument has type 'short'}}
+ printf("%r", us); // expected-warning{{format specifies type '_Fract' but the argument has type 'unsigned short'}}
+ printf("%r", i); // expected-warning{{format specifies type '_Fract' but the argument has type 'int'}}
+ printf("%r", ui); // expected-warning{{format specifies type '_Fract' but the argument has type 'unsigned int'}}
+ printf("%r", l); // expected-warning{{format specifies type '_Fract' but the argument has type 'long'}}
+ printf("%r", ul); // expected-warning{{format specifies type '_Fract' but the argument has type 'unsigned long'}}
+ printf("%r", fl); // expected-warning{{format specifies type '_Fract' but the argument has type 'float'}}
+ printf("%r", d); // expected-warning{{format specifies type '_Fract' but the argument has type 'double'}}
+ printf("%r", c); // expected-warning{{format specifies type '_Fract' but the argument has type 'char'}}
+ printf("%r", uc); // expected-warning{{format specifies type '_Fract' but the argument has type 'unsigned char'}}
+}
+
+void test_fixed_point_specifiers(void) {
+ printf("%r", f);
+ printf("%R", uf);
+ printf("%k", a);
+ printf("%K", ua);
+
+ /// Test different sizes.
+ printf("%r", sf); // expected-warning{{format specifies type '_Fract' but the argument has type 'short _Fract'}}
+ printf("%r", lf); // expected-warning{{format specifies type '_Fract' but the argument has type 'long _Fract'}}
+ printf("%R", usf); // expected-warning{{format specifies type 'unsigned _Fract' but the argument has type 'unsigned short _Fract'}}
+ printf("%R", ulf); // expected-warning{{format specifies type 'unsigned _Fract' but the argument has type 'unsigned long _Fract'}}
+ printf("%k", sa); // expected-warning{{format specifies type '_Accum' but the argument has type 'short _Accum'}}
+ printf("%k", la); // expected-warning{{format specifies type '_Accum' but the argument has type 'long _Accum'}}
+ printf("%K", usa); // expected-warning{{format specifies type 'unsigned _Accum' but the argument has type 'unsigned short _Accum'}}
+ printf("%K", ula); // expected-warning{{format specifies type 'unsigned _Accum' but the argument has type 'unsigned long _Accum'}}
+
+ /// Test signs.
+ printf("%r", uf); // expected-warning{{format specifies type '_Fract' but the argument has type 'unsigned _Fract'}}
+ printf("%R", f); // expected-warning{{format specifies type 'unsigned _Fract' but the argument has type '_Fract'}}
+ printf("%k", ua); // expected-warning{{format specifies type '_Accum' but the argument has type 'unsigned _Accum'}}
+ printf("%K", a); // expected-warning{{format specifies type 'unsigned _Accum' but the argument has type '_Accum'}}
+
+ /// Test between types.
+ printf("%r", a); // expected-warning{{format specifies type '_Fract' but the argument has type '_Accum'}}
+ printf("%R", ua); // expected-warning{{format specifies type 'unsigned _Fract' but the argument has type 'unsigned _Accum'}}
+ printf("%k", f); // expected-warning{{format specifies type '_Accum' but the argument has type '_Fract'}}
+ printf("%K", uf); // expected-warning{{format specifies type 'unsigned _Accum' but the argument has type 'unsigned _Fract'}}
+
+ /// Test saturated types.
+ printf("%r", sat_f);
+ printf("%R", sat_uf);
+ printf("%k", sat_a);
+ printf("%K", sat_ua);
+}
+
+void test_length_modifiers_and_flags(void) {
+ printf("%hr", sf);
+ printf("%lr", lf);
+ printf("%hR", usf);
+ printf("%lR", ulf);
+ printf("%hk", sa);
+ printf("%lk", la);
+ printf("%hK", usa);
+ printf("%lK", ula);
+
+ printf("%hr", sat_sf);
+ printf("%lr", sat_lf);
+ printf("%hR", sat_usf);
+ printf("%lR", sat_ulf);
+ printf("%hk", sat_sa);
+ printf("%lk", sat_la);
+ printf("%hK", sat_usa);
+ printf("%lK", sat_ula);
+
+ printf("%10r", f);
+ printf("%10.10r", f);
+ printf("%010r", f);
+ printf("%-10r", f);
+ printf("%.10r", f);
+ printf("%+r", f);
+ printf("% r", f);
+ printf("%#r", f);
+ printf("%#.r", f);
+ printf("%#.0r", f);
+
+ /// Test some invalid length modifiers.
+ printf("%zr", f); // expected-warning{{length modifier 'z' results in undefined behavior or no effect with 'r' conversion specifier}}
+ printf("%llr", f); // expected-warning{{length modifier 'll' results in undefined behavior or no effect with 'r' conversion specifier}}
+ printf("%hhr", f); // expected-warning{{length modifier 'hh' results in undefined behavior or no effect with 'r' conversion specifier}}
+}
+#else
+void test_fixed_point_specifiers_no_printf() {
+ printf("%k", i); // expected-warning{{invalid conversion specifier 'k'}}
+ printf("%K", i); // expected-warning{{invalid conversion specifier 'K'}}
+ printf("%r", i); // expected-warning{{invalid conversion specifier 'r'}}
+ printf("%R", i); // expected-warning{{invalid conversion specifier 'R'}}
+}
+#endif // WITHOUT_FIXED_POINT
``````````
</details>
https://github.com/llvm/llvm-project/pull/82855
More information about the cfe-commits
mailing list