From c42f34515794b4cb5881d3b63a49e58111d0a46f Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Thu, 9 Sep 2010 00:05:53 +0000 Subject: [PATCH] When building SwitchStmts in Sema, record whether all the enum values of a switch(enum) where covered by individual case statements. Flow-based analyses may wish to consult this information, and recording this in the AST allows us to obviate reconstructing this information later when we build the CFG. llvm-svn: 113447 --- clang/include/clang/AST/Stmt.h | 18 +++++ clang/lib/Sema/SemaStmt.cpp | 80 +++++++++++++---------- clang/lib/Serialization/ASTReaderStmt.cpp | 3 + clang/lib/Serialization/ASTWriterStmt.cpp | 1 + 4 files changed, 68 insertions(+), 34 deletions(-) diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h index 62a6b6463df5..856a14c51a1d 100644 --- a/clang/include/clang/AST/Stmt.h +++ b/clang/include/clang/AST/Stmt.h @@ -662,6 +662,11 @@ class SwitchStmt : public Stmt { SwitchCase *FirstCase; SourceLocation SwitchLoc; + /// If the SwitchStmt is a switch on an enum value, this records whether + /// all the enum values were covered by CaseStmts. This value is meant to + /// be a hint for possible clients. + unsigned AllEnumCasesCovered : 1; + public: SwitchStmt(ASTContext &C, VarDecl *Var, Expr *cond); @@ -709,6 +714,19 @@ public: SC->setNextSwitchCase(FirstCase); FirstCase = SC; } + + /// Set a flag in the SwitchStmt indicating that if the 'switch (X)' is a + /// switch over an enum value then all cases have been explicitly covered. + void setAllEnumCasesCovered() { + AllEnumCasesCovered = 1; + } + + /// Returns true if the SwitchStmt is a switch of an enum value and all cases + /// have been explicitly covered. + bool isAllEnumCasesCovered() const { + return (bool) AllEnumCasesCovered; + } + virtual SourceRange getSourceRange() const { return SourceRange(SwitchLoc, SubExprs[BODY]->getLocEnd()); } diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 083e4dba85e3..fa23654e0d7d 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -696,14 +696,14 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch, } // 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. + // values. We only issue a warning if there is not 'default:', but + // we still do the analysis to preserve this information in the AST + // (which can be used by flow-based analyes). // - // 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(); + // If switch has default case, then ignore it. - if (!CaseListIsErroneous && !TheDefaultStmt && !HasConstantCond && ET) { + if (!CaseListIsErroneous && !HasConstantCond && ET) { const EnumDecl *ED = ET->getDecl(); typedef llvm::SmallVector, 64> EnumValsTy; EnumValsTy EnumVals; @@ -723,40 +723,46 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch, std::stable_sort(EnumVals.begin(), EnumVals.end(), CmpEnumVals); 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(); + + // See which case values aren't in enum. + // 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. + if (!TheDefaultStmt) { + EnumValsTy::const_iterator EI = EnumVals.begin(); + 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) + while (EI != EIend && EI->first < CI->first) + EI++; + if (EI == EIend || EI->first > CI->first) 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(); + } + // 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++) { - while (EI != EIend && EI->first < RI->first) - EI++; + while (EI != EIend && EI->first < RI->first) + EI++; - if (EI == EIend || EI->first != RI->first) { - Diag(RI->second->getLHS()->getExprLoc(), diag::warn_not_in_enum) - << ED->getDeclName(); - } + if (EI == EIend || EI->first != RI->first) { + 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::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::warn_not_in_enum) + << ED->getDeclName(); + } } - //Check which enum vals aren't in switch + // Check which enum vals aren't in switch CaseValsTy::const_iterator CI = CaseVals.begin(); CaseRangesTy::const_iterator RI = CaseRanges.begin(); - EI = EnumVals.begin(); - for (; EI != EIend; EI++) { + bool hasCasesNotInSwitch = false; + + for (EnumValsTy::const_iterator EI = EnumVals.begin(); EI != EIend; EI++){ //Drop unneeded case values llvm::APSInt CIVal; while (CI != CaseVals.end() && CI->first < EI->first) @@ -765,17 +771,23 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch, if (CI != CaseVals.end() && CI->first == EI->first) continue; - //Drop unneeded case ranges + // Drop unneeded case ranges for (; RI != CaseRanges.end(); RI++) { llvm::APSInt Hi = RI->second->getRHS()->EvaluateAsInt(Context); if (EI->first <= Hi) break; } - if (RI == CaseRanges.end() || EI->first < RI->first) - Diag(CondExpr->getExprLoc(), diag::warn_missing_cases) - << EI->second->getDeclName(); + if (RI == CaseRanges.end() || EI->first < RI->first) { + hasCasesNotInSwitch = true; + if (!TheDefaultStmt) + Diag(CondExpr->getExprLoc(), diag::warn_missing_cases) + << EI->second->getDeclName(); + } } + + if (!hasCasesNotInSwitch) + SS->setAllEnumCasesCovered(); } } diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp index 93664452d92c..9c6d4b96dd8f 100644 --- a/clang/lib/Serialization/ASTReaderStmt.cpp +++ b/clang/lib/Serialization/ASTReaderStmt.cpp @@ -239,6 +239,9 @@ void ASTStmtReader::VisitSwitchStmt(SwitchStmt *S) { S->setCond(Reader.ReadSubExpr()); S->setBody(Reader.ReadSubStmt()); S->setSwitchLoc(SourceLocation::getFromRawEncoding(Record[Idx++])); + if (Record[Idx++]) + S->setAllEnumCasesCovered(); + SwitchCase *PrevSC = 0; for (unsigned N = Record.size(); Idx != N; ++Idx) { SwitchCase *SC = Reader.getSwitchCaseWithID(Record[Idx]); diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp index 403311b8c1af..6ea2c4bd0c0b 100644 --- a/clang/lib/Serialization/ASTWriterStmt.cpp +++ b/clang/lib/Serialization/ASTWriterStmt.cpp @@ -233,6 +233,7 @@ void ASTStmtWriter::VisitSwitchStmt(SwitchStmt *S) { Writer.AddStmt(S->getCond()); Writer.AddStmt(S->getBody()); Writer.AddSourceLocation(S->getSwitchLoc(), Record); + Record.push_back(S->isAllEnumCasesCovered()); for (SwitchCase *SC = S->getSwitchCaseList(); SC; SC = SC->getNextSwitchCase()) Record.push_back(Writer.RecordSwitchCaseID(SC));