hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: clang.
hokein requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D92082
Files:
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1031,6 +1031,7 @@
EXPECT_THAT(FooCC.ranges(),
testing::UnorderedElementsAreArray(Results->LocalChanges));
+ trace::TestTracer Tracer;
// Name validation.
Results =
runPrepareRename(Server, FooCCPath, FooCC.point(),
@@ -1038,6 +1039,8 @@
EXPECT_FALSE(Results);
EXPECT_THAT(llvm::toString(Results.takeError()),
testing::HasSubstr("keyword"));
+ EXPECT_THAT(Tracer.takeMetric("rename_name_invalid", "Keywords"),
+ ElementsAre(1));
// Single-file rename on global symbols, we should report an error.
Results = runPrepareRename(Server, FooCCPath, FooCC.point(),
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -341,6 +341,15 @@
Kind K;
std::string Details;
};
+std::string toString(InvalidName::Kind K) {
+ switch (K) {
+ case InvalidName::Keywords:
+ return "Keywords";
+ case InvalidName::Conflict:
+ return "Conflict";
+ }
+ llvm_unreachable("unhandled InvalidName kind");
+}
llvm::Error makeError(InvalidName Reason) {
auto Message = [](InvalidName Reason) {
@@ -361,18 +370,25 @@
llvm::Optional<InvalidName> checkName(const NamedDecl &RenameDecl,
llvm::StringRef NewName) {
trace::Span Tracer("CheckName");
+ static constexpr trace::Metric InvalidNameMetric(
+ "rename_name_invalid", trace::Metric::Counter, "invalid_kind");
auto &ASTCtx = RenameDecl.getASTContext();
+ llvm::Optional<InvalidName> Result;
if (isKeyword(NewName, ASTCtx.getLangOpts()))
- return InvalidName{InvalidName::Keywords, NewName.str()};
- // Name conflict detection.
- // Function conflicts are subtle (overloading), so ignore them.
- if (RenameDecl.getKind() != Decl::Function) {
- if (auto *Conflict = lookupSiblingWithName(ASTCtx, RenameDecl, NewName))
- return InvalidName{
- InvalidName::Conflict,
- Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
+ Result = InvalidName{InvalidName::Keywords, NewName.str()};
+ else {
+ // Name conflict detection.
+ // Function conflicts are subtle (overloading), so ignore them.
+ if (RenameDecl.getKind() != Decl::Function) {
+ if (auto *Conflict = lookupSiblingWithName(ASTCtx, RenameDecl, NewName))
+ Result = InvalidName{
+ InvalidName::Conflict,
+ Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
+ }
}
- return llvm::None;
+ if (Result)
+ InvalidNameMetric.record(1, toString(Result->K));
+ return Result;
}
// AST-based rename, it renames all occurrences in the main file.
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1031,6 +1031,7 @@
EXPECT_THAT(FooCC.ranges(),
testing::UnorderedElementsAreArray(Results->LocalChanges));
+ trace::TestTracer Tracer;
// Name validation.
Results =
runPrepareRename(Server, FooCCPath, FooCC.point(),
@@ -1038,6 +1039,8 @@
EXPECT_FALSE(Results);
EXPECT_THAT(llvm::toString(Results.takeError()),
testing::HasSubstr("keyword"));
+ EXPECT_THAT(Tracer.takeMetric("rename_name_invalid", "Keywords"),
+ ElementsAre(1));
// Single-file rename on global symbols, we should report an error.
Results = runPrepareRename(Server, FooCCPath, FooCC.point(),
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -341,6 +341,15 @@
Kind K;
std::string Details;
};
+std::string toString(InvalidName::Kind K) {
+ switch (K) {
+ case InvalidName::Keywords:
+ return "Keywords";
+ case InvalidName::Conflict:
+ return "Conflict";
+ }
+ llvm_unreachable("unhandled InvalidName kind");
+}
llvm::Error makeError(InvalidName Reason) {
auto Message = [](InvalidName Reason) {
@@ -361,18 +370,25 @@
llvm::Optional<InvalidName> checkName(const NamedDecl &RenameDecl,
llvm::StringRef NewName) {
trace::Span Tracer("CheckName");
+ static constexpr trace::Metric InvalidNameMetric(
+ "rename_name_invalid", trace::Metric::Counter, "invalid_kind");
auto &ASTCtx = RenameDecl.getASTContext();
+ llvm::Optional<InvalidName> Result;
if (isKeyword(NewName, ASTCtx.getLangOpts()))
- return InvalidName{InvalidName::Keywords, NewName.str()};
- // Name conflict detection.
- // Function conflicts are subtle (overloading), so ignore them.
- if (RenameDecl.getKind() != Decl::Function) {
- if (auto *Conflict = lookupSiblingWithName(ASTCtx, RenameDecl, NewName))
- return InvalidName{
- InvalidName::Conflict,
- Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
+ Result = InvalidName{InvalidName::Keywords, NewName.str()};
+ else {
+ // Name conflict detection.
+ // Function conflicts are subtle (overloading), so ignore them.
+ if (RenameDecl.getKind() != Decl::Function) {
+ if (auto *Conflict = lookupSiblingWithName(ASTCtx, RenameDecl, NewName))
+ Result = InvalidName{
+ InvalidName::Conflict,
+ Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
+ }
}
- return llvm::None;
+ if (Result)
+ InvalidNameMetric.record(1, toString(Result->K));
+ return Result;
}
// AST-based rename, it renames all occurrences in the main file.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
