Skip to content

Commit 0657f74

Browse files
committed
[clang] Implemented C/C++ tests for assign-warning
This is still a draft since other tests fail and the Objective-C tests also still need to be implemented
1 parent 7b82fe7 commit 0657f74

File tree

6 files changed

+131
-9
lines changed

6 files changed

+131
-9
lines changed

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8477,7 +8477,7 @@ def err_incomplete_object_call : Error<
84778477
def warn_condition_is_assignment : Warning<"using the result of an "
84788478
"assignment as a condition without parentheses">,
84798479
InGroup<Parentheses>;
8480-
def warn_parens_bool_assign : Warning<"suggest parentheses around assignment used as truth value">,
8480+
def warn_assignment_bool_context : Warning<"suggest parentheses around assignment used as truth value">,
84818481
InGroup<Parentheses>;
84828482
def warn_free_nonheap_object
84838483
: Warning<"attempt to call %0 on non-heap %select{object %2|object: block expression|object: lambda-to-function-pointer conversion}1">,

clang/include/clang/Sema/Sema.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7232,7 +7232,7 @@ class Sema final : public SemaBase {
72327232
/// being used as a boolean condition, warn if it's an assignment.
72337233
void DiagnoseAssignmentAsCondition(Expr *E);
72347234

7235-
void DiagnoseImplicitCastBoolAssignment(Expr *E, QualType ToType);
7235+
void DiagnoseAssignmentBoolContext(Expr *E, QualType ToType);
72367236

72377237
/// Redundant parentheses over an equality comparison can indicate
72387238
/// that the user intended an assignment used as condition.

clang/lib/Sema/Sema.cpp

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -687,13 +687,27 @@ void Sema::diagnoseZeroToNullptrConversion(CastKind Kind, const Expr *E) {
687687
<< FixItHint::CreateReplacement(E->getSourceRange(), "nullptr");
688688
}
689689

690-
void Sema::DiagnoseImplicitCastBoolAssignment(Expr* E,QualType Ty) {
690+
void Sema::DiagnoseAssignmentBoolContext(Expr* E,QualType Ty) {
691691
if (Ty->isBooleanType()) {
692+
// `bool(x=0)` and if (x=0){} emit:
693+
// -ImplicitCastExpr bool IntegralToBoolean
694+
// -- ImplicitCastExpr int LValueToRValue
695+
// --- Assignment ...
696+
// But should still emit this warning (at least gcc does), even if bool-cast is not directly followed by assignment
697+
// NOTE: Is this robust enough or can there be other semantic expression until the assignment?
698+
while (auto *ICE = dyn_cast<ImplicitCastExpr>(E)) {
699+
// If there is another implicit cast to bool then this warning would have been already emitted
700+
if (ICE->getType()->isBooleanType())
701+
return;
702+
E = ICE->getSubExpr();
703+
}
704+
692705
if (BinaryOperator* Op = dyn_cast<BinaryOperator>(E)) {
693-
// should only be issued for regular assignment `=`, not for e.g `+=`
706+
// Should only be issued for regular assignment `=`, not for compound-assign like `+=`.
707+
// NOTE: Might make sense to emit for all assignments even if gcc only does for regular assignment
694708
if (Op->getOpcode() == BO_Assign) {
695709
SourceLocation Loc = Op->getOperatorLoc();
696-
Diag(Loc,diag::warn_parens_bool_assign) << E->getSourceRange();
710+
Diag(Loc,diag::warn_assignment_bool_context) << E->getSourceRange();
697711
}
698712
}
699713
}
@@ -773,7 +787,13 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
773787
}
774788
}
775789

776-
DiagnoseImplicitCastBoolAssignment(E,Ty);
790+
// FIXME: Doesn't include C89, so this warning isn't emitted when passing `std=c99`.
791+
auto isC = getLangOpts().C99 || getLangOpts().C11 || getLangOpts().C17 || getLangOpts().C23;
792+
// Do not emit this warning for Objective-C, since it's a common idiom.
793+
// NOTE: Are there other languages that this could affect besides C and C++?
794+
// Ideally would check `getLangOpts().Cplusplus || getLangOpts().C` but there is no option for C (only C99 etc.).
795+
if ((getLangOpts().CPlusPlus || isC) && !getLangOpts().ObjC)
796+
DiagnoseAssignmentBoolContext(E,Ty);
777797

778798
if (ImplicitCastExpr *ImpCast = dyn_cast<ImplicitCastExpr>(E)) {
779799
if (ImpCast->getCastKind() == Kind && (!BasePath || BasePath->empty())) {

clang/lib/Sema/SemaExpr.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20276,7 +20276,11 @@ void Sema::DiagnoseEqualityWithExtraParens(ParenExpr *ParenE) {
2027620276

2027720277
ExprResult Sema::CheckBooleanCondition(SourceLocation Loc, Expr *E,
2027820278
bool IsConstexpr) {
20279-
DiagnoseAssignmentAsCondition(E);
20279+
// This warning is already covered by `warn_assignment_bool_context` in C++.
20280+
// NOTE: Ideally both warnings would be combined
20281+
if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
20282+
DiagnoseAssignmentAsCondition(E);
20283+
2028020284
if (ParenExpr *parenE = dyn_cast<ParenExpr>(E))
2028120285
DiagnoseEqualityWithExtraParens(parenE);
2028220286

clang/lib/Sema/SemaExprCXX.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4393,8 +4393,6 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
43934393
FromType = From->getType();
43944394
}
43954395

4396-
DiagnoseImplicitCastBoolAssignment(From,ToType);
4397-
43984396
// If we're converting to an atomic type, first convert to the corresponding
43994397
// non-atomic type.
44004398
QualType ToAtomicType;
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
// RUN: %clang_cc1 -x c++ -fsyntax-only -Wparentheses -verify %s
2+
// RUN: %clang_cc1 -x c -fsyntax-only -Wparentheses -verify %s
3+
4+
// INFO: This warning is only issued for C and C++.
5+
// This file is executed 3 times once for each language mention in the run-command.
6+
7+
#ifdef __cplusplus
8+
9+
// Do not emit the warning for compound-assignments.
10+
bool f(int x) { return x = 0; } // expected-warning {{suggest parentheses around assignment used as truth value}}
11+
bool f2(int x) { return x += 0; }
12+
13+
bool f3(bool x) { return x = 0; }
14+
15+
void test() {
16+
int x;
17+
if (x = 0) {} // expected-warning {{suggest parentheses around assignment used as truth value}}
18+
if (x = 4 && x){} // expected-warning {{suggest parentheses around assignment used as truth value}}
19+
20+
(void)bool(x = 1); // expected-warning {{suggest parentheses around assignment used as truth value}}
21+
(void)(bool)(x = 1);
22+
23+
// This should still emit since the RHS is casted to `int` before being casted back to `bool`.
24+
(void)bool(x = false); // expected-warning {{suggest parentheses around assignment used as truth value}}
25+
26+
// Should only issue warning once, even if multiple implicit casts.
27+
// FIXME: This only checks that warning occurs not how often.
28+
(void)bool(bool(x = 1)); // expected-warning {{suggest parentheses around assignment used as truth value}}
29+
(void)bool(int(bool(x = 1))); // expected-warning {{suggest parentheses around assignment used as truth value}}
30+
(void)bool(int(x = 1));
31+
32+
bool _a = x = 3; // expected-warning {{suggest parentheses around assignment used as truth value}}
33+
34+
// Shouldn't warn for above cases if parentheses were provided.
35+
if ((x = 0)) {}
36+
(void)bool((x = 1));
37+
bool _b= (x = 3);
38+
}
39+
40+
#elif defined(__OBJC__)
41+
42+
// NOTE: This warning shouldn't affect Objective-C
43+
#import <Foundation/Foundation.h>
44+
45+
BOOL f(int x) { return x = 0; }
46+
BOOL f2(int x) { return x += 0; }
47+
48+
BOOL f3(BOOL x) { return x = 0; }
49+
50+
void test() {
51+
int x;
52+
53+
if (x = 0) {} // expected-warning {{using the result of an assignment as a condition without parentheses}}
54+
if (x = 4 && x){} // expected-warning {{using the result of an assignment as a condition without parentheses}}
55+
56+
(void)(BOOL)(x = 1);
57+
(void)(BOOL)(int)(x = 1);
58+
59+
BOOL _a = x = 3;
60+
61+
if ((x = 0)) {}
62+
(void)BOOL((x = 1));
63+
BOOL _b= (x = 3);
64+
}
65+
66+
#else
67+
68+
// NOTE: Don't know if tests allow includes.
69+
#include <stdbool.h>
70+
71+
// Do not emit the warning for compound-assignments.
72+
bool f(int x) { return x = 0; } // expected-warning {{suggest parentheses around assignment used as truth value}}
73+
bool f2(int x) { return x += 0; }
74+
75+
bool f3(bool x) { return x = 0; }
76+
77+
void test() {
78+
int x;
79+
80+
// This should emit the `warn_condition_is_assignment` warning since
81+
// C doesn't do implicit conversion booleans for conditions
82+
if (x = 0) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \
83+
// expected-note{{use '==' to turn this assignment into an equality comparison}} \
84+
// expected-note{{place parentheses around the assignment to silence this warning}}
85+
if (x = 4 && x){} // expected-warning {{using the result of an assignment as a condition without parentheses}} \
86+
// expected-note{{use '==' to turn this assignment into an equality comparison}} \
87+
// expected-note{{place parentheses around the assignment to silence this warning}}
88+
89+
(void)(bool)(x = 1);
90+
(void)(bool)(int)(x = 1);
91+
92+
93+
bool _a = x = 3; // expected-warning {{suggest parentheses around assignment used as truth value}}
94+
95+
// Shouldn't warn for above cases if parentheses were provided.
96+
if ((x = 0)) {}
97+
bool _b = (x = 3);
98+
}
99+
100+
#endif

0 commit comments

Comments
 (0)