[cfe-commits] r104010 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaStmt.cpp test/Sema/switch.c test/SemaCXX/constant-expression.cpp test/SemaCXX/i-c-e-cxx.cpp test/SemaCXX/switch.cpp
John McCall
rjmccall at apple.com
Mon May 17 20:19:21 PDT 2010
Author: rjmccall
Date: Mon May 17 22:19:21 2010
New Revision: 104010
URL: http://llvm.org/viewvc/llvm-project?rev=104010&view=rev
Log:
If a switch condition is constant, don't warn about missing enum cases.
If a switch condition is constant, warn if there's no case for it.
Constant switch conditions do come up in reasonable template code.
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/test/Sema/switch.c
cfe/trunk/test/SemaCXX/constant-expression.cpp
cfe/trunk/test/SemaCXX/i-c-e-cxx.cpp
cfe/trunk/test/SemaCXX/switch.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=104010&r1=104009&r2=104010&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon May 17 22:19:21 2010
@@ -2892,9 +2892,11 @@
InGroup<DiagGroup<"switch">>;
def err_duplicate_case : Error<"duplicate case value '%0'">;
def warn_case_empty_range : Warning<"empty case range specified">;
+def warn_missing_case_for_condition :
+ Warning<"no case matching constant switch condition '%0'">;
def warn_missing_cases : Warning<"enumeration value %0 not handled in switch">,
InGroup<DiagGroup<"switch-enum"> >;
-def not_in_enum : Warning<"case value not in enumerated type %0">,
+def warn_not_in_enum : Warning<"case value not in enumerated type %0">,
InGroup<DiagGroup<"switch-enum"> >;
def err_typecheck_statement_requires_scalar : Error<
"statement requires expression of scalar type (%0 invalid)">;
Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=104010&r1=104009&r2=104010&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Mon May 17 22:19:21 2010
@@ -567,6 +567,7 @@
}
Expr *CondExpr = SS->getCond();
+ Expr *CondExprBeforePromotion = CondExpr;
QualType CondTypeBeforePromotion =
GetTypeBeforeIntegralPromotion(CondExpr);
@@ -675,16 +676,38 @@
}
if (!HasDependentValue) {
+ // If we don't have a default statement, check whether the
+ // condition is constant.
+ llvm::APSInt ConstantCondValue;
+ bool HasConstantCond = false;
+ bool ShouldCheckConstantCond = false;
+ if (!HasDependentValue && !TheDefaultStmt) {
+ Expr::EvalResult Result;
+ HasConstantCond = CondExprBeforePromotion->Evaluate(Result, Context);
+ if (HasConstantCond) {
+ assert(Result.Val.isInt() && "switch condition evaluated to non-int");
+ ConstantCondValue = Result.Val.getInt();
+ ShouldCheckConstantCond = true;
+
+ assert(ConstantCondValue.getBitWidth() == CondWidth &&
+ ConstantCondValue.isSigned() == CondIsSigned);
+ }
+ }
+
// Sort all the scalar case values so we can easily detect duplicates.
std::stable_sort(CaseVals.begin(), CaseVals.end(), CmpCaseVals);
if (!CaseVals.empty()) {
- for (unsigned i = 0, e = CaseVals.size()-1; i != e; ++i) {
- if (CaseVals[i].first == CaseVals[i+1].first) {
+ for (unsigned i = 0, e = CaseVals.size(); i != e; ++i) {
+ if (ShouldCheckConstantCond &&
+ CaseVals[i].first == ConstantCondValue)
+ ShouldCheckConstantCond = false;
+
+ if (i != 0 && CaseVals[i].first == CaseVals[i-1].first) {
// If we have a duplicate, report it.
- Diag(CaseVals[i+1].second->getLHS()->getLocStart(),
- diag::err_duplicate_case) << CaseVals[i].first.toString(10);
Diag(CaseVals[i].second->getLHS()->getLocStart(),
+ diag::err_duplicate_case) << CaseVals[i].first.toString(10);
+ Diag(CaseVals[i-1].second->getLHS()->getLocStart(),
diag::note_duplicate_case_prev);
// FIXME: We really want to remove the bogus case stmt from the
// substmt, but we have no way to do this right now.
@@ -703,6 +726,7 @@
// Scan the ranges, computing the high values and removing empty ranges.
std::vector<llvm::APSInt> HiVals;
for (unsigned i = 0, e = CaseRanges.size(); i != e; ++i) {
+ llvm::APSInt &LoVal = CaseRanges[i].first;
CaseStmt *CR = CaseRanges[i].second;
Expr *Hi = CR->getRHS();
llvm::APSInt HiVal = Hi->EvaluateAsInt(Context);
@@ -718,7 +742,7 @@
CR->setRHS(Hi);
// If the low value is bigger than the high value, the case is empty.
- if (CaseRanges[i].first > HiVal) {
+ if (LoVal > HiVal) {
Diag(CR->getLHS()->getLocStart(), diag::warn_case_empty_range)
<< SourceRange(CR->getLHS()->getLocStart(),
CR->getRHS()->getLocEnd());
@@ -726,6 +750,12 @@
--i, --e;
continue;
}
+
+ if (ShouldCheckConstantCond &&
+ LoVal <= ConstantCondValue &&
+ ConstantCondValue <= HiVal)
+ ShouldCheckConstantCond = false;
+
HiVals.push_back(HiVal);
}
@@ -779,18 +809,32 @@
}
}
- // Check to see if switch is over an Enum and handles all of its
- // values
+ // Complain if we have a constant condition and we didn't find a match.
+ if (!CaseListIsErroneous && ShouldCheckConstantCond) {
+ // TODO: it would be nice if we printed enums as enums, chars as
+ // chars, etc.
+ Diag(CondExpr->getExprLoc(), diag::warn_missing_case_for_condition)
+ << ConstantCondValue.toString(10)
+ << CondExpr->getSourceRange();
+ }
+
+ // Check to see if switch is over an Enum and handles all of its
+ // values. We don't need to do this if there's a default
+ // statement or if we have a constant condition.
+ //
+ // TODO: we might want to check whether case values are out of the
+ // enum even if we don't want to check whether all cases are handled.
const EnumType* ET = CondTypeBeforePromotion->getAs<EnumType>();
// If switch has default case, then ignore it.
- if (!CaseListIsErroneous && !TheDefaultStmt && ET) {
+ if (!CaseListIsErroneous && !TheDefaultStmt && !HasConstantCond && ET) {
const EnumDecl *ED = ET->getDecl();
typedef llvm::SmallVector<std::pair<llvm::APSInt, EnumConstantDecl*>, 64> EnumValsTy;
EnumValsTy EnumVals;
- // Gather all enum values, set their type and sort them, allowing easier comparison
- // with CaseVals.
- for (EnumDecl::enumerator_iterator EDI = ED->enumerator_begin(); EDI != ED->enumerator_end(); EDI++) {
+ // Gather all enum values, set their type and sort them,
+ // allowing easier comparison with CaseVals.
+ for (EnumDecl::enumerator_iterator EDI = ED->enumerator_begin();
+ EDI != ED->enumerator_end(); EDI++) {
llvm::APSInt Val = (*EDI)->getInitVal();
if(Val.getBitWidth() < CondWidth)
Val.extend(CondWidth);
@@ -798,30 +842,36 @@
EnumVals.push_back(std::make_pair(Val, (*EDI)));
}
std::stable_sort(EnumVals.begin(), EnumVals.end(), CmpEnumVals);
- EnumValsTy::iterator EIend = std::unique(EnumVals.begin(), EnumVals.end(), EqEnumVals);
+ EnumValsTy::iterator EIend =
+ std::unique(EnumVals.begin(), EnumVals.end(), EqEnumVals);
// See which case values aren't in enum
EnumValsTy::const_iterator EI = EnumVals.begin();
- for (CaseValsTy::const_iterator CI = CaseVals.begin(); CI != CaseVals.end(); CI++) {
+ for (CaseValsTy::const_iterator CI = CaseVals.begin();
+ CI != CaseVals.end(); CI++) {
while (EI != EIend && EI->first < CI->first)
EI++;
if (EI == EIend || EI->first > CI->first)
- Diag(CI->second->getLHS()->getExprLoc(), diag::not_in_enum) << ED->getDeclName();
+ Diag(CI->second->getLHS()->getExprLoc(), diag::warn_not_in_enum)
+ << ED->getDeclName();
}
// See which of case ranges aren't in enum
EI = EnumVals.begin();
- for (CaseRangesTy::const_iterator RI = CaseRanges.begin(); RI != CaseRanges.end() && EI != EIend; RI++) {
+ for (CaseRangesTy::const_iterator RI = CaseRanges.begin();
+ RI != CaseRanges.end() && EI != EIend; RI++) {
while (EI != EIend && EI->first < RI->first)
EI++;
if (EI == EIend || EI->first != RI->first) {
- Diag(RI->second->getLHS()->getExprLoc(), diag::not_in_enum) << ED->getDeclName();
+ Diag(RI->second->getLHS()->getExprLoc(), diag::warn_not_in_enum)
+ << ED->getDeclName();
}
llvm::APSInt Hi = RI->second->getRHS()->EvaluateAsInt(Context);
while (EI != EIend && EI->first < Hi)
EI++;
if (EI == EIend || EI->first != Hi)
- Diag(RI->second->getRHS()->getExprLoc(), diag::not_in_enum) << ED->getDeclName();
+ Diag(RI->second->getRHS()->getExprLoc(), diag::warn_not_in_enum)
+ << ED->getDeclName();
}
//Check which enum vals aren't in switch
CaseValsTy::const_iterator CI = CaseVals.begin();
@@ -844,7 +894,8 @@
}
if (RI == CaseRanges.end() || EI->first < RI->first)
- Diag(CondExpr->getExprLoc(), diag::warn_missing_cases) << EI->second->getDeclName();
+ Diag(CondExpr->getExprLoc(), diag::warn_missing_cases)
+ << EI->second->getDeclName();
}
}
}
Modified: cfe/trunk/test/Sema/switch.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/switch.c?rev=104010&r1=104009&r2=104010&view=diff
==============================================================================
--- cfe/trunk/test/Sema/switch.c (original)
+++ cfe/trunk/test/Sema/switch.c Mon May 17 22:19:21 2010
@@ -24,36 +24,37 @@
void test3(void) {
// empty switch;
- switch (0);
+ switch (0); // expected-warning {{no case matching constant switch condition '0'}}
}
extern int g();
void test4()
{
- switch (1) {
+ int cond;
+ switch (cond) {
case 0 && g():
case 1 || g():
break;
}
- switch(1) {
+ switch(cond) {
case g(): // expected-error {{expression is not an integer constant expression}}
case 0 ... g(): // expected-error {{expression is not an integer constant expression}}
break;
}
- switch (1) {
+ switch (cond) {
case 0 && g() ... 1 || g():
break;
}
- switch (1) {
+ switch (cond) {
case g() && 0: // expected-error {{expression is not an integer constant expression}} // expected-note {{subexpression not valid in an integer constant expression}}
break;
}
- switch (1) {
+ switch (cond) {
case 0 ... g() || 1: // expected-error {{expression is not an integer constant expression}} // expected-note {{subexpression not valid in an integer constant expression}}
break;
}
@@ -68,7 +69,7 @@
}
void test6() {
- const char ch = 'a';
+ char ch = 'a';
switch(ch) {
case 1234: // expected-warning {{overflow converting case value}}
break;
@@ -261,3 +262,18 @@
default: break;
}
}
+
+void test15() {
+ int i = 0;
+ switch (1) { // expected-warning {{no case matching constant switch condition '1'}}
+ case 0: i = 0; break;
+ case 2: i++; break;
+ }
+}
+
+void test16() {
+ const char c = '5';
+ switch (c) { // expected-warning {{no case matching constant switch condition '53'}}
+ case '6': return;
+ }
+}
Modified: cfe/trunk/test/SemaCXX/constant-expression.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression.cpp?rev=104010&r1=104009&r2=104010&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/constant-expression.cpp (original)
+++ cfe/trunk/test/SemaCXX/constant-expression.cpp Mon May 17 22:19:21 2010
@@ -57,8 +57,8 @@
i10 = sizeof(Struct),
i11 = true? 1 + cval * Struct::sval ^ itval / (int)1.5 - sizeof(Struct) : 0
;
- void f() {
- switch(0) {
+ void f(int cond) {
+ switch(cond) {
case 0 + 1:
case 100 + eval:
case 200 + cval:
Modified: cfe/trunk/test/SemaCXX/i-c-e-cxx.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/i-c-e-cxx.cpp?rev=104010&r1=104009&r2=104010&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/i-c-e-cxx.cpp (original)
+++ cfe/trunk/test/SemaCXX/i-c-e-cxx.cpp Mon May 17 22:19:21 2010
@@ -17,7 +17,7 @@
int a() {
const int t=t; // expected-note {{subexpression not valid}}
- switch(1) {
+ switch(1) { // expected-warning {{no case matching constant switch condition '1'}}
case t:; // expected-error {{not an integer constant expression}}
}
}
Modified: cfe/trunk/test/SemaCXX/switch.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/switch.cpp?rev=104010&r1=104009&r2=104010&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/switch.cpp (original)
+++ cfe/trunk/test/SemaCXX/switch.cpp Mon May 17 22:19:21 2010
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wswitch-enum %s
void test() {
bool x = true;
@@ -40,3 +40,20 @@
switch (c) { // expected-error{{incomplete class type}}
}
}
+
+namespace test3 {
+ enum En { A, B, C };
+ template <En how> void foo() {
+ int x = 0, y = 5;
+
+ switch (how) { //expected-warning {{no case matching constant switch condition '2'}}
+ case A: x *= y; break;
+ case B: x += y; break;
+ // No case for C, but it's okay because we have a constant condition.
+ }
+ }
+
+ template void foo<A>();
+ template void foo<B>();
+ template void foo<C>(); //expected-note {{in instantiation}}
+}
More information about the cfe-commits
mailing list