diff --git a/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp b/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp @@ -113,7 +113,8 @@ return false; // Ignore implicit casts, since enums implicitly cast to integer types. Cond = Cond->IgnoreParenImpCasts(); - EnumT = Cond->getType()->getAsAdjusted(); + // Get the canonical type to handle typedefs. + EnumT = Cond->getType().getCanonicalType()->getAsAdjusted(); if (!EnumT) return false; EnumD = EnumT->getDecl(); @@ -152,14 +153,30 @@ if (CS->caseStmtIsGNURange()) return false; + // Support for direct references to enum constants. This is required to + // support C and ObjC which don't contain values in their ConstantExprs. + // The general way to get the value of a case is EvaluateAsRValue, but we'd + // rather not deal with that in case the AST is broken. + if (auto *DRE = dyn_cast(CS->getLHS()->IgnoreParenCasts())) { + if (auto *Enumerator = dyn_cast(DRE->getDecl())) { + auto Iter = ExpectedCases.find(Normalize(Enumerator->getInitVal())); + if (Iter != ExpectedCases.end()) + Iter->second.setCovered(); + continue; + } + } + + // ConstantExprs with values are expected for C++, otherwise the storage + // kind will be None. + // Case expression is not a constant expression or is value-dependent, // so we may not be able to work out which cases are covered. const ConstantExpr *CE = dyn_cast(CS->getLHS()); if (!CE || CE->isValueDependent()) return false; - // Unsure if this case could ever come up, but prevents an unreachable - // executing in getResultAsAPSInt. + // We need a storage kind in order to be able to fetch the value type, + // currently both C and ObjC enums will return none. if (CE->getResultStorageKind() == ConstantExpr::RSK_None) return false; auto Iter = ExpectedCases.find(Normalize(CE->getResultAsAPSInt())); diff --git a/clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp --- a/clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp +++ b/clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp @@ -23,6 +23,7 @@ CodeContext Context; llvm::StringRef TestSource; llvm::StringRef ExpectedSource; + llvm::StringRef FileName = "TestTU.cpp"; }; Case Cases[]{ @@ -206,10 +207,56 @@ R""(template void f() {enum Enum {A}; ^switch (A) {}})"", "unavailable", }, + {// C: Only filling in missing enumerators + Function, + R""( + enum CEnum {A,B,C}; + enum CEnum val = A; + ^switch (val) {case B:break;} + )"", + R""( + enum CEnum {A,B,C}; + enum CEnum val = A; + switch (val) {case B:break;case A:case C:break;} + )"", + "TestTU.c"}, + {// ObjC: Only filling in missing enumerators + Function, + R""( + enum ObjCEnum {A,B,C}; + enum ObjCEnum val = B; + ^switch (val) {case A:break;} + )"", + R""( + enum ObjCEnum {A,B,C}; + enum ObjCEnum val = B; + switch (val) {case A:break;case B:case C:break;} + )"", + "TestTU.m"}, + {// ObjC: Only filling in missing enumerators w/ typedefs + Function, + R""( + typedef unsigned long UInteger; + enum ControlState : UInteger; + typedef enum ControlState ControlState; + enum ControlState : UInteger {A,B,C}; + ControlState controlState = A; + switch (^controlState) {case A:break;} + )"", + R""( + typedef unsigned long UInteger; + enum ControlState : UInteger; + typedef enum ControlState ControlState; + enum ControlState : UInteger {A,B,C}; + ControlState controlState = A; + switch (controlState) {case A:break;case B:case C:break;} + )"", + "TestTU.m"}, }; for (const auto &Case : Cases) { Context = Case.Context; + FileName = Case.FileName; EXPECT_EQ(apply(Case.TestSource), Case.ExpectedSource); } }