Skip to content

Commit 4c900c6

Browse files
committed
[C] Handle comma operator for implicit int->enum conversions
In C++, the type of an enumerator is the type of the enumeration, whereas in C, the type of the enumerator is 'int'. The type of a comma operator is the type of the right-hand operand, which means you can get an implicit conversion with this code in C but not in C++: enum E { Zero }; enum E foo() { return ((void)0, Zero); } We were previously incorrectly diagnosing this code as being incompatible with C++ because the type of the paren expression would be 'int' there, whereas in C++ the type is 'E'. So now we handle the comma operator with special logic when analyzing implicit conversions in C. When analyzing the left-hand operand of a comma operator, we do not need to check for that operand causing an implicit conversion for the entire comma expression. So we only check for that case with the right-hand operand. This addresses a concern brought up post-commit: llvm#137658 (comment)
1 parent aec3929 commit 4c900c6

File tree

3 files changed

+55
-2
lines changed

3 files changed

+55
-2
lines changed

clang/lib/Sema/SemaChecking.cpp

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11647,6 +11647,29 @@ static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T,
1164711647
}
1164811648
}
1164911649

11650+
static void CheckCommaOperand(Sema &S, Expr *E, QualType T, SourceLocation CC,
11651+
bool Check) {
11652+
E = E->IgnoreParenImpCasts();
11653+
AnalyzeImplicitConversions(S, E, CC);
11654+
11655+
if (Check && E->getType() != T)
11656+
S.CheckImplicitConversion(E, T, CC);
11657+
}
11658+
11659+
/// Analyze the given comma operator. The basic idea behind the analysis is to
11660+
/// analyze the left and right operands slightly differently. The left operand
11661+
/// needs to check whether the operand itself has an implicit conversion, but
11662+
/// not whether the left operand induces an implicit conversion for the entire
11663+
/// comma expression itself. This is similar to how CheckConditionalOperand
11664+
/// behaves; it's as-if the correct operand were directly used for the implicit
11665+
/// conversion check.
11666+
static void AnalyzeCommaOperator(Sema &S, BinaryOperator *E, QualType T) {
11667+
assert(E->isCommaOp() && "Must be a comma operator");
11668+
11669+
CheckCommaOperand(S, E->getLHS(), T, E->getOperatorLoc(), false);
11670+
CheckCommaOperand(S, E->getRHS(), T, E->getOperatorLoc(), true);
11671+
}
11672+
1165011673
/// Analyze the given compound assignment for the possible losing of
1165111674
/// floating-point precision.
1165211675
static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
@@ -12464,7 +12487,7 @@ static void AnalyzeImplicitConversions(
1246412487
<< OrigE->getSourceRange() << T->isBooleanType()
1246512488
<< FixItHint::CreateReplacement(UO->getBeginLoc(), "!");
1246612489

12467-
if (const auto *BO = dyn_cast<BinaryOperator>(SourceExpr))
12490+
if (auto *BO = dyn_cast<BinaryOperator>(SourceExpr))
1246812491
if ((BO->getOpcode() == BO_And || BO->getOpcode() == BO_Or) &&
1246912492
BO->getLHS()->isKnownToHaveBooleanValue() &&
1247012493
BO->getRHS()->isKnownToHaveBooleanValue() &&
@@ -12490,6 +12513,9 @@ static void AnalyzeImplicitConversions(
1249012513
(BO->getOpcode() == BO_And ? "&&" : "||"));
1249112514
S.Diag(BO->getBeginLoc(), diag::note_cast_operand_to_int);
1249212515
}
12516+
} else if (BO->isCommaOp() && !S.getLangOpts().CPlusPlus) {
12517+
AnalyzeCommaOperator(S, BO, T);
12518+
return;
1249312519
}
1249412520

1249512521
// For conditional operators, we analyze the arguments as if they

clang/test/Sema/implicit-cast.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
1-
// RUN: %clang_cc1 -fsyntax-only %s
1+
// RUN: %clang_cc1 -fsyntax-only -verify %s
22

33
static char *test1(int cf) {
44
return cf ? "abc" : 0;
55
}
66
static char *test2(int cf) {
77
return cf ? 0 : "abc";
88
}
9+
10+
int baz(void) {
11+
int f;
12+
return ((void)0, f = 1.4f); // expected-warning {{implicit conversion from 'float' to 'int' changes value from 1.4 to 1}}
13+
}

clang/test/Sema/implicit-int-enum-conversion.c

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,25 @@ enum E1 quux(void) {
5050
return E2_Zero; // expected-warning {{implicit conversion from enumeration type 'enum E2' to different enumeration type 'enum E1'}} \
5151
cxx-error {{cannot initialize return object of type 'enum E1' with an rvalue of type 'E2'}}
5252
}
53+
54+
enum E1 comma1(void) {
55+
return ((void)0, E1_One);
56+
}
57+
58+
enum E1 comma2(void) {
59+
enum E1 x;
60+
return
61+
(x = 12, // expected-warning {{implicit conversion from 'int' to enumeration type 'enum E1' is invalid in C++}} \
62+
cxx-error {{assigning to 'enum E1' from incompatible type 'int'}}
63+
E1_One);
64+
}
65+
66+
enum E1 comma3(void) {
67+
enum E1 x;
68+
return ((void)0, foo()); // Okay, no conversion in C++
69+
}
70+
71+
enum E1 comma4(void) {
72+
return ((void)1, 2); // expected-warning {{implicit conversion from 'int' to enumeration type 'enum E1' is invalid in C++}} \
73+
cxx-error {{cannot initialize return object of type 'enum E1' with an rvalue of type 'int'}}
74+
}

0 commit comments

Comments
 (0)