pnappa created this revision.
pnappa added a reviewer: lebedev.ri.
pnappa added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, jdoerfert, xazax.hun.
Herald added a project: clang.
Fix for https://bugs.llvm.org/show_bug.cgi?id=41199#c1
Previously, if a user implicitly cast to a bool (say, in a conditional
statement), the fix would be to add an explicit comparison. So, for a floating
point implicit case to bool, from `if (f)`, the synthesised code would be `if
(f != 0.0f)`.
Even if the flag "readability-uppercase-literal-suffix" was enabled, the
synthesised suffix would be lowercase. This commit changes that, such that if
that flag is enabled when "readability-implicit-bool-conversion" is enabled,
the synthesised suffix is uppercase.
A non-covered case is when "modernize-use-default-member-init" is enabled,
lower-case suffixes may still be synthesised (I think, based off the code). Any
RFC whether this should be made a different issue or whether or not this
behaviour should be added in?
Intended as my first commit to the llvm-project.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D59859
Files:
clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h
clang-tools-extra/test/clang-tidy/readability-implicit-bool-conversion.cpp
clang-tools-extra/test/clang-tidy/readability-uppercase-literal-suffix-implicit-bool-conversion.cpp
Index: clang-tools-extra/test/clang-tidy/readability-uppercase-literal-suffix-implicit-bool-conversion.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/readability-uppercase-literal-suffix-implicit-bool-conversion.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix,readability-implicit-bool-conversion %t
+
+template<typename T>
+void functionTaking(T);
+
+
+////////// Implicit conversion to bool with enforced uppercase suffix.
+
+// TODO: should we add complex number suffix tests? I don't see any here. It may be an annoyance thing due to it's a C++17 test. I should ask in phabricator when I submit.
+// XXX: ensure that tabs are 2 wide
+// XXX: run clang tidy on this using the clangtidy config at the root of the repo
+void implicitConversionToBoolSimpleCases() {
+ unsigned int unsignedInteger = 10u;
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: integer literal has suffix 'u', which is not uppercase
+ // CHECK-MESSAGES-NEXT: unsigned int unsignedInteger = 10u;
+ // CHECK-MESSAGES-NEXT: ^ ~
+ // CHECK-MESSAGES-NEXT: {{^ *}}U{{$}}
+ // CHECK-FIXES: unsigned int unsignedInteger = 10U;
+
+ functionTaking<bool>(unsignedInteger);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'unsigned int' -> bool [readability-implicit-bool-conversion]
+ // CHECK-FIXES: functionTaking<bool>(unsignedInteger != 0U);
+
+ unsigned long unsignedLong = 10ul;
+ // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: integer literal has suffix 'ul', which is not uppercase [readability-uppercase-literal-suffix]
+ // CHECK-MESSAGES-NEXT: unsigned long unsignedLong = 10ul;
+ // CHECK-MESSAGES-NEXT: ^ ~~
+ // CHECK-MESSAGES-NEXT: {{^ *}}UL{{$}}
+ // CHECK-FIXES: unsigned long unsignedLong = 10UL;
+
+ functionTaking<bool>(unsignedLong);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'unsigned long' -> bool [readability-implicit-bool-conversion]
+ // CHECK-FIXES: functionTaking<bool>(unsignedLong != 0U);
+
+ float floating = 10.0f;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]
+ // CHECK-MESSAGES-NEXT: float floating = 10.0f;
+ // CHECK-MESSAGES-NEXT: ^ ~
+ // CHECK-MESSAGES-NEXT: {{^ *}}F{{$}}
+ // CHECK-FIXES: float floating = 10.0F;
+
+ functionTaking<bool>(floating);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'float' -> bool [readability-implicit-bool-conversion]
+ // CHECK-FIXES: functionTaking<bool>(floating != 0.0F);
+
+ double doubleFloating = 10.0;
+ functionTaking<bool>(doubleFloating);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'double' -> bool [readability-implicit-bool-conversion]
+ // CHECK-FIXES: functionTaking<bool>(doubleFloating != 0.0);
+}
+
+void implicitConversionToBoolInComplexExpressions() {
+ bool boolean = true;
+
+ unsigned int integer = 10U;
+ unsigned int anotherInteger = 20U;
+ bool boolComingFromUnsignedInteger = integer + anotherInteger;
+ // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: implicit conversion 'unsigned int' -> bool
+ // CHECK-FIXES: bool boolComingFromUnsignedInteger = (integer + anotherInteger) != 0U;
+
+ float floating = 0.2F;
+ // combination of two errors on the same line
+ bool boolComingFromFloating = floating - 0.3f || boolean;
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit conversion 'float' -> bool
+ // CHECK-MESSAGES: :[[@LINE-2]]:44: warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]
+ // CHECK-FIXES: bool boolComingFromFloating = ((floating - 0.3F) != 0.0F) || boolean;
+}
+
+void implicitConversionInNegationExpressions() {
+ // ensure that in negation the replaced suffix is also capitalized
+ float floating = 10.0F;
+ bool boolComingFromNegatedFloat = ! floating;
+ // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: implicit conversion 'float' -> bool
+ // CHECK-FIXES: bool boolComingFromNegatedFloat = floating == 0.0F;
+}
+
+void implicitConversionToBoolInControlStatements() {
+ unsigned long longInteger = 2U;
+ for (;longInteger;) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: implicit conversion 'unsigned long' -> bool
+ // CHECK-FIXES: for (;longInteger != 0U;) {}
+
+ float floating = 0.3F;
+ while (floating) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit conversion 'float' -> bool
+ // CHECK-FIXES: while (floating != 0.0F) {}
+}
+
+void implicitConversionToBoolFromUnaryOperatorLiterals() {
+ functionTaking<bool>(-0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int' -> bool
+ // CHECK-FIXES: functionTaking<bool>((-0) != 0);
+
+ functionTaking<bool>(-0.0F);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'float' -> bool
+ // CHECK-FIXES: functionTaking<bool>((-0.0F) != 0.0F);
+
+ functionTaking<bool>(-0.0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'double' -> bool
+ // CHECK-FIXES: functionTaking<bool>((-0.0) != 0.0);
+}
+
+bool implicitConversionToBoolFromUnsignedInReturnValue() {
+ unsigned integer = 1u;
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: integer literal has suffix 'u', which is not uppercase
+ // CHECK-MESSAGES-NEXT: unsigned integer = 1u;
+ // CHECK-MESSAGES-NEXT: ^~
+ // CHECK-MESSAGES-NEXT: {{^ *}}U{{$}}
+ // CHECK-FIXES: unsigned integer = 1U;
+
+ return integer;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit conversion 'unsigned int' -> bool [readability-implicit-bool-conversion]
+ // CHECK-FIXES: return integer != 0U;
+}
+
+bool implicitConversionToBoolFromFloatInReturnValue() {
+ float floating = 1.0f;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]
+ // CHECK-MESSAGES-NEXT: float floating = 1.0f;
+ // CHECK-MESSAGES-NEXT: ^ ~
+ // CHECK-MESSAGES-NEXT: {{^ *}}F{{$}}
+ // CHECK-FIXES: float floating = 1.0F;
+
+ return floating;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit conversion 'float' -> bool [readability-implicit-bool-conversion]
+ // CHECK-FIXES: return floating != 0.0F;
+}
Index: clang-tools-extra/test/clang-tidy/readability-implicit-bool-conversion.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/readability-implicit-bool-conversion.cpp
+++ clang-tools-extra/test/clang-tidy/readability-implicit-bool-conversion.cpp
@@ -70,7 +70,7 @@
// CHECK-FIXES: char character = static_cast<char>(boolean);
}
-void implicitConversionFromBoollInComplexBoolExpressions() {
+void implicitConversionFromBoolInComplexBoolExpressions() {
bool boolean = true;
bool anotherBoolean = false;
Index: clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h
+++ clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h
@@ -36,6 +36,7 @@
const bool AllowIntegerConditions;
const bool AllowPointerConditions;
+ const bool UseUppercaseLiteralSuffix;
};
} // namespace readability
Index: clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
@@ -41,13 +41,22 @@
StringRef getZeroLiteralToCompareWithForType(CastKind CastExprKind,
QualType Type,
- ASTContext &Context) {
+ ASTContext &Context,
+ bool UppercaseSuffix) {
switch (CastExprKind) {
case CK_IntegralToBoolean:
- return Type->isUnsignedIntegerType() ? "0u" : "0";
+ if (UppercaseSuffix) {
+ return Type->isUnsignedIntegerType() ? "0U" : "0";
+ } else {
+ return Type->isUnsignedIntegerType() ? "0u" : "0";
+ }
case CK_FloatingToBoolean:
- return Context.hasSameType(Type, Context.FloatTy) ? "0.0f" : "0.0";
+ if (UppercaseSuffix) {
+ return Context.hasSameType(Type, Context.FloatTy) ? "0.0F" : "0.0";
+ } else {
+ return Context.hasSameType(Type, Context.FloatTy) ? "0.0f" : "0.0";
+ }
case CK_PointerToBoolean:
case CK_MemberPointerToBoolean: // Fall-through on purpose.
@@ -90,7 +99,7 @@
void fixGenericExprCastToBool(DiagnosticBuilder &Diag,
const ImplicitCastExpr *Cast, const Stmt *Parent,
- ASTContext &Context) {
+ ASTContext &Context, bool UppercaseSuffix) {
// In case of expressions like (! integer), we should remove the redundant not
// operator and use inverted comparison (integer == 0).
bool InvertComparison =
@@ -137,7 +146,7 @@
}
EndLocInsertion += getZeroLiteralToCompareWithForType(
- Cast->getCastKind(), SubExpr->getType(), Context);
+ Cast->getCastKind(), SubExpr->getType(), Context, UppercaseSuffix);
if (NeedOuterParens) {
EndLocInsertion += ")";
@@ -196,7 +205,8 @@
}
StringRef getEquivalentForBoolLiteral(const CXXBoolLiteralExpr *BoolLiteral,
- QualType DestType, ASTContext &Context) {
+ QualType DestType, ASTContext &Context,
+ bool UppercaseSuffix) {
// Prior to C++11, false literal could be implicitly converted to pointer.
if (!Context.getLangOpts().CPlusPlus11 &&
(DestType->isPointerType() || DestType->isMemberPointerType()) &&
@@ -206,13 +216,21 @@
if (DestType->isFloatingType()) {
if (Context.hasSameType(DestType, Context.FloatTy)) {
- return BoolLiteral->getValue() ? "1.0f" : "0.0f";
+ if (UppercaseSuffix) {
+ return BoolLiteral->getValue() ? "1.0F" : "0.0F";
+ } else {
+ return BoolLiteral->getValue() ? "1.0f" : "0.0f";
+ }
}
return BoolLiteral->getValue() ? "1.0" : "0.0";
}
if (DestType->isUnsignedIntegerType()) {
- return BoolLiteral->getValue() ? "1u" : "0u";
+ if (UppercaseSuffix) {
+ return BoolLiteral->getValue() ? "1U" : "0U";
+ } else {
+ return BoolLiteral->getValue() ? "1u" : "0u";
+ }
}
return BoolLiteral->getValue() ? "1" : "0";
}
@@ -248,7 +266,8 @@
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
AllowIntegerConditions(Options.get("AllowIntegerConditions", false)),
- AllowPointerConditions(Options.get("AllowPointerConditions", false)) {}
+ AllowPointerConditions(Options.get("AllowPointerConditions", false)),
+ UseUppercaseLiteralSuffix(Context->isCheckEnabled("readability-uppercase-literal-suffix")) {}
void ImplicitBoolConversionCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
@@ -368,7 +387,7 @@
if (!EquivalentLiteral.empty()) {
Diag << tooling::fixit::createReplacement(*Cast, EquivalentLiteral);
} else {
- fixGenericExprCastToBool(Diag, Cast, Parent, Context);
+ fixGenericExprCastToBool(Diag, Cast, Parent, Context, UseUppercaseLiteralSuffix);
}
}
@@ -383,7 +402,8 @@
if (const auto *BoolLiteral =
dyn_cast<CXXBoolLiteralExpr>(Cast->getSubExpr())) {
Diag << tooling::fixit::createReplacement(
- *Cast, getEquivalentForBoolLiteral(BoolLiteral, DestType, Context));
+ *Cast, getEquivalentForBoolLiteral(BoolLiteral, DestType, Context,
+ UseUppercaseLiteralSuffix));
} else {
fixGenericExprCastFromBool(Diag, Cast, Context, DestType.getAsString());
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
