Author: hokein Date: Wed Jun 26 01:10:26 2019 New Revision: 364392 URL: http://llvm.org/viewvc/llvm-project?rev=364392&view=rev Log: [clangd] Don't rename the namespace.
Summary: Also fix a small bug -- the extra argument "-xc++" doesn't overwrite the language if the argument is present after the file name in the compiler command. Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D63759 Modified: clang-tools-extra/trunk/clangd/refactor/Rename.cpp clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp clang-tools-extra/trunk/clangd/unittests/TestTU.cpp Modified: clang-tools-extra/trunk/clangd/refactor/Rename.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/Rename.cpp?rev=364392&r1=364391&r2=364392&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/refactor/Rename.cpp (original) +++ clang-tools-extra/trunk/clangd/refactor/Rename.cpp Wed Jun 26 01:10:26 2019 @@ -93,12 +93,15 @@ enum ReasonToReject { NoIndexProvided, NonIndexable, UsedOutsideFile, + UnsupportedSymbol, }; // Check the symbol Decl is renameable (per the index) within the file. llvm::Optional<ReasonToReject> renamableWithinFile(const NamedDecl &Decl, StringRef MainFile, const SymbolIndex *Index) { + if (llvm::isa<NamespaceDecl>(&Decl)) + return ReasonToReject::UnsupportedSymbol; auto &ASTCtx = Decl.getASTContext(); const auto &SM = ASTCtx.getSourceManager(); bool MainFileIsHeader = ASTCtx.getLangOpts().IsHeaderFile; @@ -160,6 +163,8 @@ renameWithinFile(ParsedAST &AST, llvm::S return "the symbol is used outside main file"; case NonIndexable: return "symbol may be used in other files (not eligible for indexing)"; + case UnsupportedSymbol: + return "symbol is not a supported kind (e.g. namespace)"; } llvm_unreachable("unhandled reason kind"); }; Modified: clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp?rev=364392&r1=364391&r2=364392&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp (original) +++ clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp Wed Jun 26 01:10:26 2019 @@ -93,28 +93,41 @@ TEST(RenameTest, SingleFile) { TEST(RenameTest, Renameable) { // Test cases where the symbol is declared in header. - const char *Headers[] = { - R"cpp(// allow -- function-local + struct Case { + const char* HeaderCode; + const char* ErrorMessage; // null if no error + }; + Case Cases[] = { + {R"cpp(// allow -- function-local void f(int [[Lo^cal]]) { [[Local]] = 2; } )cpp", + nullptr}, - R"cpp(// allow -- symbol is indexable and has no refs in index. + {R"cpp(// allow -- symbol is indexable and has no refs in index. void [[On^lyInThisFile]](); )cpp", + nullptr}, - R"cpp(// disallow -- symbol is indexable and has other refs in index. + {R"cpp(// disallow -- symbol is indexable and has other refs in index. void f() { Out^side s; } )cpp", + "used outside main file"}, - R"cpp(// disallow -- symbol is not indexable. + {R"cpp(// disallow -- symbol is not indexable. namespace { class Unin^dexable {}; } )cpp", + "not eligible for indexing"}, + + {R"cpp(// disallow -- namespace symbol isn't supported + namespace fo^o {} + )cpp", + "not a supported kind"}, }; const char *CommonHeader = "class Outside {};"; TestTU OtherFile = TestTU::withCode("Outside s;"); @@ -123,13 +136,14 @@ TEST(RenameTest, Renameable) { // The index has a "Outside" reference. auto OtherFileIndex = OtherFile.index(); - for (const char *Header : Headers) { - Annotations T(Header); + for (const auto& Case : Cases) { + Annotations T(Case.HeaderCode); // We open the .h file as the main file. TestTU TU = TestTU::withCode(T.code()); TU.Filename = "test.h"; TU.HeaderCode = CommonHeader; - TU.ExtraArgs.push_back("-xc++"); + // Parsing the .h file as C++ include. + TU.ExtraArgs.push_back("-xobjective-c++-header"); auto AST = TU.build(); auto Results = renameWithinFile(AST, testPath(TU.Filename), T.point(), @@ -138,9 +152,11 @@ TEST(RenameTest, Renameable) { if (T.ranges().empty()) WantRename = false; if (!WantRename) { + assert(Case.ErrorMessage && "Error message must be set!"); EXPECT_FALSE(Results) << "expected renameWithinFile returned an error: " << T.code(); - llvm::consumeError(Results.takeError()); + auto ActualMessage = llvm::toString(Results.takeError()); + EXPECT_THAT(ActualMessage, testing::HasSubstr(Case.ErrorMessage)); } else { EXPECT_TRUE(bool(Results)) << "renameWithinFile returned an error: " << llvm::toString(Results.takeError()); Modified: clang-tools-extra/trunk/clangd/unittests/TestTU.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/TestTU.cpp?rev=364392&r1=364391&r2=364392&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/unittests/TestTU.cpp (original) +++ clang-tools-extra/trunk/clangd/unittests/TestTU.cpp Wed Jun 26 01:10:26 2019 @@ -31,7 +31,7 @@ ParsedAST TestTU::build() const { Files[FullHeaderName] = HeaderCode; Files[ImportThunk] = ThunkContents; - std::vector<const char *> Cmd = {"clang", FullFilename.c_str()}; + std::vector<const char *> Cmd = {"clang"}; // FIXME: this shouldn't need to be conditional, but it breaks a // GoToDefinition test for some reason (getMacroArgExpandedLocation fails). if (!HeaderCode.empty()) { @@ -40,6 +40,9 @@ ParsedAST TestTU::build() const { : FullHeaderName.c_str()); } Cmd.insert(Cmd.end(), ExtraArgs.begin(), ExtraArgs.end()); + // Put the file name at the end -- this allows the extra arg (-xc++) to + // override the language setting. + Cmd.push_back(FullFilename.c_str()); ParseInputs Inputs; Inputs.CompileCommand.Filename = FullFilename; Inputs.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
