On Tue, Sep 13, 2011 at 5:54 PM, Richard Trieu <[email protected]> wrote:
> On Fri, Sep 9, 2011 at 4:13 PM, Richard Trieu <[email protected]> wrote:
>> Add a new warning and bundle it into -Wliteral-conversion. This
>> warning triggers on cases where a string literal is implicitly
>> converted to a bool. For the common case of:
>>
>> assert(condition && "Error text");
>>
>> an exception for string literals in logical operations has also been
>> included.
>>
>
> Currently, the patch has this new warning bundled in with
> -Wliteral-conversion. It's been brought up that -Wbool-conversions
> might be a more suitable place for this warning. Any thoughts on
> this?
>
A few changes to the new warning and catching a few more things in
LLVM/Clang. Now, it should be able to catch string literal to bool in
if, while, do-while, and for statements and in function calls.
r140232 & r140234 were minor changes caught by this warning, basically
changing assert(!"error") to assert(0 && "error"). r140231 was a
change for a string argument for a bool parameter in a function call.
Index: test/SemaCXX/warn-literal-conversion.cpp
===================================================================
--- test/SemaCXX/warn-literal-conversion.cpp (revision 139441)
+++ test/SemaCXX/warn-literal-conversion.cpp (working copy)
@@ -43,3 +43,19 @@
int y = (24*60*60) * 0.25;
int pennies = 123.45 * 100;
}
+
+// Warn on cases where a string literal is converted into a bool.
+// An exception is made for this in logical operators.
+void assert(bool condition);
+void test1() {
+ 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 && "";
+ assert("error"); // expected-warning{{implicit conversion turns string literal into bool: 'const char [6]' to 'bool'}}
+ assert(0 && "error");
+
+ 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'}}
+}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td (revision 139441)
+++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
@@ -1463,6 +1463,9 @@
"implicit conversion turns literal floating-point number into integer: "
"%0 to %1">,
InGroup<DiagGroup<"literal-conversion">>, DefaultIgnore;
+def warn_impcast_string_literal_to_bool : Warning<
+ "implicit conversion turns string literal into bool: %0 to %1">,
+ InGroup<DiagGroup<"literal-conversion">>, DefaultIgnore;
def note_fix_integral_float_as_integer : Note<
"this can be rewritten as an integer literal with the exact same value">;
def warn_impcast_different_enum_types : Warning<
Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp (revision 139441)
+++ lib/Sema/SemaExprCXX.cpp (working copy)
@@ -4631,7 +4631,7 @@
if (FullExpr.isInvalid())
return ExprError();
- CheckImplicitConversions(FullExpr.get());
+ CheckImplicitConversions(FullExpr.get(), FullExpr.get()->getExprLoc());
return MaybeCreateExprWithCleanups(FullExpr);
}
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp (revision 139441)
+++ lib/Sema/SemaChecking.cpp (working copy)
@@ -3240,9 +3240,17 @@
if (CC.isInvalid())
return;
- // Never diagnose implicit casts to bool.
- if (Target->isSpecificBuiltinType(BuiltinType::Bool))
- return;
+ // Diagnose implicit casts to to bool.
+ if (Target->isSpecificBuiltinType(BuiltinType::Bool)) {
+ if (isa<StringLiteral>(E))
+ // Warn on string literal to bool. Checks for string literals in logical
+ // expressions, for instances, assert(0 && "error here"), is prevented
+ // by a check in AnalyzeImplicitConversions().
+ return DiagnoseImpCast(S, E, T, CC,
+ diag::warn_impcast_string_literal_to_bool);
+ else // Other casts to bool are not checked.
+ return;
+ }
// Strip vector types.
if (isa<VectorType>(Source)) {
@@ -3511,8 +3519,16 @@
// Now just recurse over the expression's children.
CC = E->getExprLoc();
- for (Stmt::child_range I = E->children(); I; ++I)
- AnalyzeImplicitConversions(S, cast<Expr>(*I), CC);
+ BinaryOperator *BO = dyn_cast<BinaryOperator>(E);
+ bool IsLogicalOperator = BO && BO->isLogicalOp();
+ for (Stmt::child_range I = E->children(); I; ++I) {
+ Expr *ChildExpr = cast<Expr>(*I);
+ if (IsLogicalOperator &&
+ isa<StringLiteral>(ChildExpr->IgnoreParenImpCasts()))
+ // Ignore checking string literals that are in logical operators.
+ continue;
+ AnalyzeImplicitConversions(S, ChildExpr, CC);
+ }
}
} // end anonymous namespace
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
