aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, dblaikie.
aaron.ballman added a subscriber: cfe-commits.
This patch improves the literal conversion diagnostics where the target type is
a Boolean by handling literals more consistently, and telling the user whether
the resulting value will be true or false.
This patch changes the behavior of literal conversions from floating-point
literals in two ways: (1) we now always diagnose instead of silently accepting
conversions where the float can be converted to an exact integer value, and (2)
we drop the information about what the source value of the floating-point
literal is (it's unimportant information in the context of the Boolean
conversion).
This patch removes the string literal conversion diagnostic (and
-Wstring-conversion) which is off by default, and rolls its functionality into
-Wliteral-conversion, which is on by default. Since we already handle string
literal to bool conversion through logical && to not diagnose, I do not believe
this will increase the chattiness of the diagnostic. In fact, turning the
diagnostic to on by default caught a few tests that were accidentally using
string->bool literal conversions. This does change the wording of the previous
diagnostic, but I think the new wording is an improvement.
This patch now diagnoses character literal conversions to Boolean types where
they used to be silently accepted.
http://reviews.llvm.org/D15935
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaChecking.cpp
test/Analysis/casts.c
test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
test/CXX/expr/expr.unary/expr.unary.op/p6.cpp
test/Sema/exprs.c
test/Sema/warn-string-conversion.c
test/SemaCXX/condition.cpp
test/SemaCXX/overload-call.cpp
test/SemaCXX/warn-literal-conversion.cpp
test/SemaCXX/warn-string-conversion.cpp
test/SemaCXX/warn-thread-safety-analysis.cpp
Index: test/SemaCXX/warn-thread-safety-analysis.cpp
===================================================================
--- test/SemaCXX/warn-thread-safety-analysis.cpp
+++ test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -1046,8 +1046,8 @@
struct Bas { void operator& (Foo &) {} };
void mumble()
{
- Bas() & Bar().func() << "" << "";
- Bas() & Bar().func() << "";
+ Bas() & Bar().func() << true << true;
+ Bas() & Bar().func() << true;
}
} // end namespace thread_annot_lock_61_modified
Index: test/SemaCXX/warn-string-conversion.cpp
===================================================================
--- test/SemaCXX/warn-string-conversion.cpp
+++ test/SemaCXX/warn-string-conversion.cpp
@@ -1,24 +1,24 @@
-// RUN: %clang_cc1 -fsyntax-only -Wstring-conversion -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s
// Warn on cases where a string literal is converted into a bool.
// An exception is made for this in logical and operators.
void assert(bool condition);
void test0() {
- bool b0 = "hi"; // expected-warning{{implicit conversion turns string literal into bool: 'const char [3]' to 'bool'}}
- b0 = ""; // expected-warning{{implicit conversion turns string literal into bool: 'const char [1]' to 'bool'}}
- b0 = 0 || ""; // expected-warning{{implicit conversion turns string literal into bool: 'const char [1]' to 'bool'}}
- b0 = "" || 0; // expected-warning{{implicit conversion turns string literal into bool: 'const char [1]' to 'bool'}}
+ bool b0 = "hi"; // expected-warning{{implicit conversion from 'const char [3]' to 'bool'; will always evaluate to 'true'}}
+ b0 = ""; // expected-warning{{implicit conversion from 'const char [1]' to 'bool'; will always evaluate to 'true'}}
+ b0 = 0 || ""; // expected-warning{{implicit conversion from 'const char [1]' to 'bool'; will always evaluate to 'true'}}
+ b0 = "" || 0; // expected-warning{{implicit conversion from 'const char [1]' to 'bool'; will always evaluate to 'true'}}
b0 = 0 && "";
b0 = "" && 0;
- assert("error"); // expected-warning{{implicit conversion turns string literal into bool: 'const char [6]' to 'bool'}}
- assert(0 || "error"); // expected-warning{{implicit conversion turns string literal into bool: 'const char [6]' to 'bool'}}
- assert("error" || 0); // expected-warning{{implicit conversion turns string literal into bool: 'const char [6]' to 'bool'}}
+ assert("error"); // expected-warning{{implicit conversion from 'const char [6]' to 'bool'; will always evaluate to 'true'}}
+ assert(0 || "error"); // expected-warning{{implicit conversion from 'const char [6]' to 'bool'; will always evaluate to 'true'}}
+ assert("error" || 0); // expected-warning{{implicit conversion from 'const char [6]' to 'bool'; will always evaluate to 'true'}}
assert(0 && "error");
assert("error" && 0);
- while("hi") {} // expected-warning{{implicit conversion turns string literal into bool: 'const char [3]' to 'bool'}}
- do {} while("hi"); // expected-warning{{implicit conversion turns string literal into bool: 'const char [3]' to 'bool'}}
- for (;"hi";); // expected-warning{{implicit conversion turns string literal into bool: 'const char [3]' to 'bool'}}
- if("hi") {} // expected-warning{{implicit conversion turns string literal into bool: 'const char [3]' to 'bool'}}
+ while("hi") {} // expected-warning{{implicit conversion from 'const char [3]' to 'bool'; will always evaluate to 'true'}}
+ do {} while("hi"); // expected-warning{{implicit conversion from 'const char [3]' to 'bool'; will always evaluate to 'true'}}
+ for (;"hi";); // expected-warning{{implicit conversion from 'const char [3]' to 'bool'; will always evaluate to 'true'}}
+ if("hi") {} // expected-warning{{implicit conversion from 'const char [3]' to 'bool'; will always evaluate to 'true'}}
}
Index: test/SemaCXX/warn-literal-conversion.cpp
===================================================================
--- test/SemaCXX/warn-literal-conversion.cpp
+++ test/SemaCXX/warn-literal-conversion.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wliteral-conversion -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wliteral-conversion -std=c++1z -verify %s
void foo(int y);
@@ -42,10 +42,26 @@
// Similarly, test floating point conversion to bool. Only float values of zero
// are converted to false; everything else is converted to true.
void test1() {
- bool b1 = 0.99f; // expected-warning {{implicit conversion from 'float' to 'bool' changes value from 0.99 to true}}
- bool b2 = 0.99; // expected-warning {{implicit conversion from 'double' to 'bool' changes value from 0.99 to true}}
- // These do not warn because they can be directly converted to integral
- // values.
- bool b3 = 0.0f;
- bool b4 = 0.0;
+ bool b1 = 0.99f; // expected-warning {{implicit conversion from 'float' to 'bool'; will always evaluate to 'true'}}
+ bool b2 = 0.99; // expected-warning {{implicit conversion from 'double' to 'bool'; will always evaluate to 'true'}}
+ bool b3 = 0.0f; // expected-warning {{implicit conversion from 'float' to 'bool'; will always evaluate to 'false'}}
+ bool b4 = 0.0; // expected-warning {{implicit conversion from 'double' to 'bool'; will always evaluate to 'false'}}
+
+ bool b5 = "false"; // expected-warning {{implicit conversion from 'const char [6]' to 'bool'; will always evaluate to 'true'}}
+ bool b6 = L"false"; // expected-warning {{implicit conversion from 'const wchar_t [6]' to 'bool'; will always evaluate to 'true'}}
+ bool b7 = u"0"; // expected-warning {{implicit conversion from 'const char16_t [2]' to 'bool'; will always evaluate to 'true'}}
+ bool b8 = U"false"; // expected-warning {{implicit conversion from 'const char32_t [6]' to 'bool'; will always evaluate to 'true'}}
+ bool b9 = u8"0"; // expected-warning {{implicit conversion from 'const char [2]' to 'bool'; will always evaluate to 'true'}}
+
+ bool b10 = '\0'; // expected-warning {{implicit conversion from 'char' to 'bool'; will always evaluate to 'false'}}
+ bool b11 = L'1'; // expected-warning {{implicit conversion from 'wchar_t' to 'bool'; will always evaluate to 'true'}}
+ bool b12 = U'\0'; // expected-warning {{implicit conversion from 'char32_t' to 'bool'; will always evaluate to 'false'}}
+ bool b13 = u'1'; // expected-warning {{implicit conversion from 'char16_t' to 'bool'; will always evaluate to 'true'}}
+ bool b14 = u8'\0'; // expected-warning {{implicit conversion from 'char' to 'bool'; will always evaluate to 'false'}}
}
+
+#define assert(x) (void)(!!(x))
+void should_not_diagnose() {
+ assert(1 && "false");
+ assert(1 && "some random string");
+}
Index: test/SemaCXX/overload-call.cpp
===================================================================
--- test/SemaCXX/overload-call.cpp
+++ test/SemaCXX/overload-call.cpp
@@ -69,7 +69,7 @@
#else
// expected-error@-4 {{cannot initialize a variable of type 'int *' with an rvalue of type 'double *'}}
#endif
- double* dp1 = k(L"foo");
+ double* dp1 = k(L"foo"); // expected-warning {{implicit conversion from 'const wchar_t [4]' to 'bool'; will always evaluate to 'true'}}
}
int* l(wchar_t*);
@@ -82,7 +82,7 @@
#else
// expected-error@-4 {{cannot initialize a variable of type 'int *' with an rvalue of type 'double *'}}
#endif
- double* dp1 = l("foo");
+ double* dp1 = l("foo"); // expected-warning {{implicit conversion from 'const char [4]' to 'bool'; will always evaluate to 'true'}}
}
int* m(const char*);
Index: test/SemaCXX/condition.cpp
===================================================================
--- test/SemaCXX/condition.cpp
+++ test/SemaCXX/condition.cpp
@@ -45,7 +45,7 @@
// Make sure we do function/array decay.
void test3() {
- if ("help")
+ if ("help") // expected-warning {{implicit conversion from 'const char [5]' to 'bool'; will always evaluate to 'true'}}
(void) 0;
if (test3) // expected-warning {{address of function 'test3' will always evaluate to 'true'}} \
Index: test/Sema/warn-string-conversion.c
===================================================================
--- test/Sema/warn-string-conversion.c
+++ test/Sema/warn-string-conversion.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -fsyntax-only -Wstring-conversion %s
+// RUN: %clang_cc1 -verify -fsyntax-only %s
void do_nothing();
void assert_error();
@@ -16,19 +16,19 @@
void test1() {
assert1(0 && "foo");
assert1("foo" && 0);
- assert1(0 || "foo"); // expected-warning {{string literal}}
- assert1("foo"); // expected-warning {{string literal}}
+ assert1(0 || "foo"); // expected-warning {{implicit conversion from 'char [4]' to '_Bool'; will always evaluate to 'true'}}
+ assert1("foo"); // expected-warning {{implicit conversion from 'char [4]' to '_Bool'; will always evaluate to 'true'}}
assert2(0 && "foo");
assert2("foo" && 0);
- assert2(0 || "foo"); // expected-warning {{string literal}}
- assert2("foo"); // expected-warning {{string literal}}
+ assert2(0 || "foo"); // expected-warning {{implicit conversion from 'char [4]' to '_Bool'; will always evaluate to 'true'}}
+ assert2("foo"); // expected-warning {{implicit conversion from 'char [4]' to '_Bool'; will always evaluate to 'true'}}
}
void test2() {
- if ("hi") {} // expected-warning {{string literal}}
- while ("hello") {} // expected-warning {{string literal}}
- for (;"howdy";) {} // expected-warning {{string literal}}
- do { } while ("hey"); // expected-warning {{string literal}}
- int x = "hey" ? 1 : 2; // expected-warning {{string literal}}
+ if ("hi") {} // expected-warning {{implicit conversion from 'char [3]' to '_Bool'; will always evaluate to 'true'}}
+ while ("hello") {} // expected-warning {{implicit conversion from 'char [6]' to '_Bool'; will always evaluate to 'true'}}
+ for (;"howdy";) {} // expected-warning {{implicit conversion from 'char [6]' to '_Bool'; will always evaluate to 'true'}}
+ do { } while ("hey"); // expected-warning {{implicit conversion from 'char [4]' to '_Bool'; will always evaluate to 'true'}}
+ int x = "hey" ? 1 : 2; // expected-warning {{implicit conversion from 'char [4]' to '_Bool'; will always evaluate to 'true'}}
}
Index: test/Sema/exprs.c
===================================================================
--- test/Sema/exprs.c
+++ test/Sema/exprs.c
@@ -242,7 +242,7 @@
// Make sure we do function/array decay.
void test22() {
- if ("help")
+ if ("help") // expected-warning {{implicit conversion from 'char [5]' to '_Bool'; will always evaluate to 'true'}}
(void) 0;
if (test22) // expected-warning {{address of function 'test22' will always evaluate to 'true'}} \
Index: test/CXX/expr/expr.unary/expr.unary.op/p6.cpp
===================================================================
--- test/CXX/expr/expr.unary/expr.unary.op/p6.cpp
+++ test/CXX/expr/expr.unary/expr.unary.op/p6.cpp
@@ -4,7 +4,7 @@
bool b = !0;
-bool b2 = !1.2; //expected-warning{{implicit conversion from 'double' to 'bool' changes value from 1.2 to true}}
+bool b2 = !1.2; //expected-warning{{implicit conversion from 'double' to 'bool'; will always evaluate to 'true'}}
bool b3 = !4;
Index: test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
===================================================================
--- test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
+++ test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -triple x86_64-apple-macosx10.6.7 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-literal-conversion -std=c++11 -triple x86_64-apple-macosx10.6.7 -verify %s
// Verify that narrowing conversions in initializer lists cause errors in C++0x
// mode.
Index: test/Analysis/casts.c
===================================================================
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -analyzer-store=region -verify %s
-// RUN: %clang_cc1 -triple i386-apple-darwin9 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -Wno-literal-conversion -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -Wno-literal-conversion -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -analyzer-store=region -verify %s
extern void clang_analyzer_eval(_Bool);
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -6982,10 +6982,7 @@
Value.toString(PrettySourceValue, precision);
SmallString<16> PrettyTargetValue;
- if (T->isSpecificBuiltinType(BuiltinType::Bool))
- PrettyTargetValue = Value.isZero() ? "false" : "true";
- else
- IntegerValue.toString(PrettyTargetValue);
+ IntegerValue.toString(PrettyTargetValue);
S.Diag(FL->getExprLoc(), diag::warn_impcast_literal_float_to_integer)
<< FL->getType() << T.getUnqualifiedType() << PrettySourceValue
@@ -7190,12 +7187,31 @@
// Diagnose implicit casts to bool.
if (Target->isSpecificBuiltinType(BuiltinType::Bool)) {
- if (isa<StringLiteral>(E))
- // Warn on string literal to bool. Checks for string literals in logical
- // and expressions, for instance, assert(0 && "error here"), are
- // prevented by a check in AnalyzeImplicitConversions().
- return DiagnoseImpCast(S, E, T, CC,
- diag::warn_impcast_string_literal_to_bool);
+ if (isa<StringLiteral>(E)) {
+ // Warn on string literal to bool. Checks for string literals in logical
+ // AND expressions, for instance, assert(0 && "error here"), are
+ // prevented by a check in AnalyzeImplicitConversions(). All string
+ // literal implicit conversions to bool evaluate to true.
+ S.Diag(E->getLocStart(), diag::warn_impcast_literal_to_bool)
+ << E->getType() << T << true << E->getSourceRange()
+ << SourceRange(CC);
+ return;
+ }
+ if (const auto *CharLit = dyn_cast<CharacterLiteral>(E)) {
+ // Warn on char literal to bool.
+ S.Diag(CharLit->getLocation(), diag::warn_impcast_literal_to_bool)
+ << E->getType() << T << (bool)CharLit->getValue()
+ << E->getSourceRange() << SourceRange(CC);
+ return;
+ }
+ if (const auto *FL = dyn_cast<FloatingLiteral>(E)) {
+ // When converting from a floating-point literal to boolean, always
+ // diagnose, even if the literal will exactly convert to an interger.
+ S.Diag(FL->getExprLoc(), diag::warn_impcast_literal_to_bool)
+ << FL->getType() << T.getUnqualifiedType() << !FL->getValue().isZero()
+ << FL->getSourceRange() << SourceRange(CC);
+ return;
+ }
if (isa<ObjCStringLiteral>(E) || isa<ObjCArrayLiteral>(E) ||
isa<ObjCDictionaryLiteral>(E) || isa<ObjCBoxedExpr>(E)) {
// This covers the literal expressions that evaluate to Objective-C
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2704,12 +2704,12 @@
def warn_impcast_bitfield_precision_constant : Warning<
"implicit truncation from %2 to bitfield changes value from %0 to %1">,
InGroup<BitFieldConstantConversion>;
+def warn_impcast_literal_to_bool : Warning<
+ "implicit conversion from %0 to %1; will always evaluate to "
+ "'%select{false|true}2'">, InGroup<LiteralConversion>;
def warn_impcast_literal_float_to_integer : Warning<
"implicit conversion from %0 to %1 changes value from %2 to %3">,
InGroup<LiteralConversion>;
-def warn_impcast_string_literal_to_bool : Warning<
- "implicit conversion turns string literal into bool: %0 to %1">,
- InGroup<StringConversion>, DefaultIgnore;
def warn_impcast_different_enum_types : Warning<
"implicit conversion from enumeration type %0 to different enumeration type "
"%1">, InGroup<EnumConversion>;
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -38,7 +38,6 @@
def ConstantConversion :
DiagGroup<"constant-conversion", [ BitFieldConstantConversion ] >;
def LiteralConversion : DiagGroup<"literal-conversion">;
-def StringConversion : DiagGroup<"string-conversion">;
def SignConversion : DiagGroup<"sign-conversion">;
def PointerBoolConversion : DiagGroup<"pointer-bool-conversion">;
def UndefinedBoolConversion : DiagGroup<"undefined-bool-conversion">;
@@ -568,8 +567,7 @@
NonLiteralNullConversion, // (1-1)->pointer (etc)
NullConversion, // NULL->non-pointer
ObjCLiteralConversion,
- SignConversion,
- StringConversion]>,
+ SignConversion]>,
DiagCategory<"Value Conversion Issue">;
def Unused : DiagGroup<"unused",
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
