This revision was automatically updated to reflect the committed changes.
Closed by commit rL364392: [clangd] Don't rename the namespace. (authored
by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D63759?vs=206437&id=206608#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63759/new/
https://reviews.llvm.org/D63759
Files:
clang-tools-extra/trunk/clangd/refactor/Rename.cpp
clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp
clang-tools-extra/trunk/clangd/unittests/TestTU.cpp
Index: clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp
@@ -93,28 +93,41 @@
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 @@
// 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 @@
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());
Index: clang-tools-extra/trunk/clangd/unittests/TestTU.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/trunk/clangd/unittests/TestTU.cpp
@@ -31,7 +31,7 @@
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 @@
: 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()};
Index: clang-tools-extra/trunk/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/refactor/Rename.cpp
+++ clang-tools-extra/trunk/clangd/refactor/Rename.cpp
@@ -93,12 +93,15 @@
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 @@
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");
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
