[PATCH] D59859: [clang-tidy] FIXIT for implicit bool conversion now obeys upper case suffixes if enforced.
pnappa updated this revision to Diff 196795.
pnappa added a comment.
Apologies for taking a while, I'm afraid I hadn't had a moment of spare time
over the past few weeks.
I believe I've applied the requested changes, bar one that I'm unsure about:
@alexfh to be sure, would adding an Option to
"readability-implicit-bool-conversion" called say, "EnforceUppercase", suffice?
With that option enabled the synthesis of the explicit comparison would be
uppercase.
I'll make the appropriate changes to documentation when I get everything
finalised.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59859/new/
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,124 @@
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix,readability-implicit-bool-conversion %t
+
+template
+void functionTaking(T);
+
+
+// Implicit conversion to bool with enforced uppercase suffix.
+
+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(unsignedInteger);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'unsigned int' -> bool [readability-implicit-bool-conversion]
+ // CHECK-FIXES: functionTaking(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(unsignedLong);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'unsigned long' -> bool [readability-implicit-bool-conversion]
+ // CHECK-FIXES: functionTaking(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(floating);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'float' -> bool [readability-implicit-bool-conversion]
+ // CHECK-FIXES: functionTaking(floating != 0.0F);
+
+ double doubleFloating = 10.0;
+ functionTaking(doubleFloating);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'double' -> bool [readability-implicit-bool-conversion]
+ // CHECK-FIXES: functionTaking(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 !=
[PATCH] D59859: [clang-tidy] FIXIT for implicit bool conversion now obeys upper case suffixes if enforced.
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
+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(unsignedInteger);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'unsigned int' -> bool [readability-implicit-bool-conversion]
+ // CHECK-FIXES: functionTaking(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(unsignedLong);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'unsigned long' -> bool [readability-implicit-bool-conversion]
+ // CHECK-FIXES: functionTaking(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(floating);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'float' -> bool [readability-implicit-bool-conversion]
+ // CHECK-FIXES: functionTaking(floating != 0.0F);
+
+ double doubleFloating = 10.0;
+ functionTaking(doubleFloating);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'double' -> bool [readability-implicit-bool-conversion]
+ // CHECK-FIXES: functionTaking(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:
