[clang-tools-extra] [clangd] Let DefineOutline tweak handle member functions (PR #95235)
https://github.com/ckandeler created
https://github.com/llvm/llvm-project/pull/95235
... of class templates.
>From fc3a907ab2550a999801d37400268fdc31df054d Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Tue, 14 Nov 2023 16:35:46 +0100
Subject: [PATCH] [clangd] Let DefineOutline tweak handle member functions
... of class templates.
---
clang-tools-extra/clangd/AST.cpp | 7 ++-
.../clangd/refactor/tweaks/DefineOutline.cpp | 47 ++--
.../unittests/tweaks/DefineOutlineTests.cpp | 55 ++-
3 files changed, 89 insertions(+), 20 deletions(-)
diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp
index fda1e5fdf8d82..2f2f8437d6ed9 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -144,8 +144,13 @@ getQualification(ASTContext , const DeclContext
*DestContext,
// since we stored inner-most parent first.
std::string Result;
llvm::raw_string_ostream OS(Result);
- for (const auto *Parent : llvm::reverse(Parents))
+ for (const auto *Parent : llvm::reverse(Parents)) {
+if (Parent != *Parents.rbegin() && Parent->isDependent() &&
+Parent->getAsRecordDecl() &&
+Parent->getAsRecordDecl()->getDescribedClassTemplate())
+ OS << "template ";
Parent->print(OS, Context.getPrintingPolicy());
+ }
return OS.str();
}
diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index f43f2417df8fc..15cb586e6c21e 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -128,7 +128,27 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD,
SM.getBufferData(SM.getMainFileID()), Replacements);
if (!QualifiedFunc)
return QualifiedFunc.takeError();
- return QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
+
+ std::string TemplatePrefix;
+ if (auto *MD = llvm::dyn_cast(FD)) {
+for (const CXXRecordDecl *Parent = MD->getParent(); Parent;
+ Parent =
+ llvm::dyn_cast_or_null(Parent->getParent()))
{
+ if (auto Params = Parent->getDescribedTemplateParams()) {
+std::string S;
+llvm::raw_string_ostream Stream(S);
+Params->print(Stream, FD->getASTContext());
+if (!S.empty())
+ *S.rbegin() = '\n'; // Replace space with newline
+TemplatePrefix.insert(0, S);
+ }
+}
+ }
+
+ auto Source = QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
+ if (!TemplatePrefix.empty())
+Source.insert(0, TemplatePrefix);
+ return Source;
}
// Returns replacements to delete tokens with kind `Kind` in the range
@@ -212,9 +232,13 @@ getFunctionSourceCode(const FunctionDecl *FD, const
DeclContext *TargetContext,
}
}
const NamedDecl *ND = Ref.Targets.front();
-const std::string Qualifier =
+std::string Qualifier =
getQualification(AST, TargetContext,
SM.getLocForStartOfFile(SM.getMainFileID()), ND);
+if (ND->getDeclContext()->isDependentContext()) {
+ if (llvm::isa(ND))
+Qualifier.insert(0, "typename ");
+}
if (auto Err = DeclarationCleanups.add(
tooling::Replacement(SM, Ref.NameLoc, 0, Qualifier)))
Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
@@ -407,10 +431,21 @@ class DefineOutline : public Tweak {
return !SameFile;
}
-// Bail out in templated classes, as it is hard to spell the class name,
-// i.e if the template parameter is unnamed.
-if (MD->getParent()->isTemplated())
- return false;
+for (const CXXRecordDecl *Parent = MD->getParent(); Parent;
+ Parent =
+ llvm::dyn_cast_or_null(Parent->getParent()))
{
+ if (auto Params = Parent->getDescribedTemplateParams()) {
+
+// Class template member functions must be defined in the
+// same file.
+SameFile = true;
+
+for (NamedDecl *P : *Params) {
+ if (!P->getIdentifier())
+return false;
+}
+ }
+}
// The refactoring is meaningless for unnamed classes and namespaces,
// unless we're outlining in the same file
diff --git a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
index 906ff33db8734..6a9e90c3bfa70 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
@@ -105,8 +105,8 @@ TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
F^oo(const Foo&) = delete;
};)cpp");
- // Not available within templated classes, as it is hard to spell class name
- // out-of-line in such cases.
+ // Not available within templated classes with unnamed parameters, as it is
+ // hard
[clang-tools-extra] [clangd] Allow "move function body out-of-line" in non-header files (PR #69704)
https://github.com/ckandeler closed https://github.com/llvm/llvm-project/pull/69704 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow "move function body out-of-line" in non-header files (PR #69704)
ckandeler wrote: I'm going to merge this without explicit format approval based on the following: - There was a general LGTM. - I addressed the remaining issues. - There was no further feedback for more than half a year. https://github.com/llvm/llvm-project/pull/69704 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Support callHierarchy/outgoingCalls (PR #91191)
ckandeler wrote: > If I'm understanding correctly, the implementation approach in this PR only > finds callees in the current translation > unit. > The approach in #77556 uses the project's index to find callees across > translation unit boundaries. Right, that's obviously nicer. > Regarding reviews: yes, it seems quite unfortunate that the original > developers seem to have largely moved on to > other things. I will do my best to make some progress of the project's review > backlog (including in particular > #86629 and #67802) as time permits. Your work is highly appreciated, but I don't think it's reasonable to expect a single unpaid contributor to maintain the entire project, as appears to be the case right now. https://github.com/llvm/llvm-project/pull/91191 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Support callHierarchy/outgoingCalls (PR #91191)
ckandeler wrote: > I remembered @HighCommander4 had filed a similar PR at #77556. Is this one > separate, or are they actually the same (i.e. both are salvaged from > https://reviews.llvm.org/D93829)? I didn't know about that one. Seems nothing gets reviewed anymore these days. https://github.com/llvm/llvm-project/pull/91191 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Support callHierarchy/outgoingCalls (PR #91191)
https://github.com/ckandeler closed https://github.com/llvm/llvm-project/pull/91191 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Support callHierarchy/outgoingCalls (PR #91191)
https://github.com/ckandeler created
https://github.com/llvm/llvm-project/pull/91191
None
>From d7cfeb6599fd507eed8935e452efd782fc3f0481 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Tue, 30 Apr 2024 18:20:05 +0200
Subject: [PATCH] [clangd] Support callHierarchy/outgoingCalls
---
clang-tools-extra/clangd/ClangdLSPServer.cpp | 7 +
clang-tools-extra/clangd/ClangdLSPServer.h| 3 +
clang-tools-extra/clangd/ClangdServer.cpp | 13 +
clang-tools-extra/clangd/ClangdServer.h | 4 +
clang-tools-extra/clangd/XRefs.cpp| 71 ++
clang-tools-extra/clangd/XRefs.h | 3 +
.../clangd/unittests/CallHierarchyTests.cpp | 226 +++---
7 files changed, 293 insertions(+), 34 deletions(-)
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 7fd599d4e1a0b0..5820a644088e3e 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1368,6 +1368,12 @@ void ClangdLSPServer::onCallHierarchyIncomingCalls(
Server->incomingCalls(Params.item, std::move(Reply));
}
+void ClangdLSPServer::onCallHierarchyOutgoingCalls(
+const CallHierarchyOutgoingCallsParams ,
+Callback> Reply) {
+ Server->outgoingCalls(Params.item, std::move(Reply));
+}
+
void ClangdLSPServer::onClangdInlayHints(const InlayHintsParams ,
Callback Reply) {
// Our extension has a different representation on the wire than the
standard.
@@ -1688,6 +1694,7 @@ void ClangdLSPServer::bindMethods(LSPBinder ,
Bind.method("typeHierarchy/subtypes", this, ::onSubTypes);
Bind.method("textDocument/prepareCallHierarchy", this,
::onPrepareCallHierarchy);
Bind.method("callHierarchy/incomingCalls", this,
::onCallHierarchyIncomingCalls);
+ Bind.method("callHierarchy/outgoingCalls", this,
::onCallHierarchyOutgoingCalls);
Bind.method("textDocument/selectionRange", this,
::onSelectionRange);
Bind.method("textDocument/documentLink", this,
::onDocumentLink);
Bind.method("textDocument/semanticTokens/full", this,
::onSemanticTokens);
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h
b/clang-tools-extra/clangd/ClangdLSPServer.h
index 8bcb29522509b7..4981027372cb57 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -153,6 +153,9 @@ class ClangdLSPServer : private ClangdServer::Callbacks,
void onCallHierarchyIncomingCalls(
const CallHierarchyIncomingCallsParams &,
Callback>);
+ void onCallHierarchyOutgoingCalls(
+ const CallHierarchyOutgoingCallsParams &,
+ Callback>);
void onClangdInlayHints(const InlayHintsParams &,
Callback);
void onInlayHint(const InlayHintsParams &, Callback>);
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp
b/clang-tools-extra/clangd/ClangdServer.cpp
index 1c4c2a79b5c051..19d01dfbd873e2 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -898,6 +898,19 @@ void ClangdServer::incomingCalls(
});
}
+void ClangdServer::outgoingCalls(
+const CallHierarchyItem ,
+Callback> CB) {
+ auto Action = [Item,
+ CB = std::move(CB)](Expected InpAST) mutable {
+if (!InpAST)
+ return CB(InpAST.takeError());
+CB(clangd::outgoingCalls(InpAST->AST, Item));
+ };
+ WorkScheduler->runWithAST("Outgoing Calls", Item.uri.file(),
+std::move(Action));
+}
+
void ClangdServer::inlayHints(PathRef File, std::optional RestrictRange,
Callback> CB) {
auto Action = [RestrictRange(std::move(RestrictRange)),
diff --git a/clang-tools-extra/clangd/ClangdServer.h
b/clang-tools-extra/clangd/ClangdServer.h
index 1661028be88b4e..4caef917c1ec16 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -288,6 +288,10 @@ class ClangdServer {
void incomingCalls(const CallHierarchyItem ,
Callback>);
+ /// Resolve outgoing calls for a given call hierarchy item.
+ void outgoingCalls(const CallHierarchyItem ,
+ Callback>);
+
/// Resolve inlay hints for a given document.
void inlayHints(PathRef File, std::optional RestrictRange,
Callback>);
diff --git a/clang-tools-extra/clangd/XRefs.cpp
b/clang-tools-extra/clangd/XRefs.cpp
index cd909266489a85..b9bf944a7bba98 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -42,6 +42,7 @@
#include "clang/AST/StmtCXX.h"
#include "clang/AST/StmtVisitor.h"
#include "clang/AST/Type.h"
+#include "clang/Analysis/CallGraph.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
@@ -2303,6 +2304,76 @@ incomingCalls(const CallHierarchyItem , const
SymbolIndex *Index) {
return Results;
}
[clang] [clang-tools-extra] [clangd] Collect comments from function definitions into the index (PR #67802)
ckandeler wrote: ping https://github.com/llvm/llvm-project/pull/67802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow "move function body out-of-line" in non-header files (PR #69704)
ckandeler wrote: ping https://github.com/llvm/llvm-project/pull/69704 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/ckandeler closed https://github.com/llvm/llvm-project/pull/83412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/ckandeler reopened https://github.com/llvm/llvm-project/pull/83412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/ckandeler closed https://github.com/llvm/llvm-project/pull/83412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
ckandeler wrote: Underscore separators get removed now. https://github.com/llvm/llvm-project/pull/83412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/ckandeler updated
https://github.com/llvm/llvm-project/pull/83412
>From 349edc160e9c13068c42b35321814296bb713a09 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Thu, 29 Feb 2024 12:26:52 +0100
Subject: [PATCH] [clangd] Remove potential prefix from enum value names
... when converting unscoped to scoped enums.
With traditional enums, a popular technique to guard against potential
name clashes is to use the enum name as a pseudo-namespace, like this:
enum MyEnum { MyEnumValue1, MyEnumValue2 };
With scoped enums, this makes no sense, making it extremely unlikely
that the user wants to keep such a prefix when modernizing. Therefore, our
tweak now removes it.
---
.../clangd/refactor/tweaks/ScopifyEnum.cpp| 61 -
.../unittests/tweaks/ScopifyEnumTests.cpp | 66 +--
clang-tools-extra/docs/ReleaseNotes.rst | 3 +
3 files changed, 108 insertions(+), 22 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
index e36b3249bc7b92..2be2bbc422af75 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
@@ -40,15 +40,12 @@ namespace {
/// void f() { E e1 = EV1; }
///
/// After:
-/// enum class E { EV1, EV2 };
-/// void f() { E e1 = E::EV1; }
+/// enum class E { V1, V2 };
+/// void f() { E e1 = E::V1; }
///
/// Note that the respective project code might not compile anymore
/// if it made use of the now-gone implicit conversion to int.
/// This is out of scope for this tweak.
-///
-/// TODO: In the above example, we could detect that the values
-/// start with the enum name, and remove that prefix.
class ScopifyEnum : public Tweak {
const char *id() const final;
@@ -63,7 +60,8 @@ class ScopifyEnum : public Tweak {
std::function;
llvm::Error addClassKeywordToDeclarations();
llvm::Error scopifyEnumValues();
- llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef Prefix);
+ llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef EnumName,
+ bool StripPrefix);
llvm::Expected getContentForFile(StringRef FilePath);
unsigned getOffsetFromPosition(const Position , StringRef Content) const;
llvm::Error addReplacementForReference(const ReferencesResult::Reference
,
@@ -125,25 +123,45 @@ llvm::Error ScopifyEnum::addClassKeywordToDeclarations() {
}
llvm::Error ScopifyEnum::scopifyEnumValues() {
- std::string PrefixToInsert(D->getName());
- PrefixToInsert += "::";
- for (auto E : D->enumerators()) {
-if (auto Err = scopifyEnumValue(*E, PrefixToInsert))
+ StringRef EnumName(D->getName());
+ bool StripPrefix = true;
+ for (const EnumConstantDecl *E : D->enumerators()) {
+if (!E->getName().starts_with(EnumName)) {
+ StripPrefix = false;
+ break;
+}
+ }
+ for (const EnumConstantDecl *E : D->enumerators()) {
+if (auto Err = scopifyEnumValue(*E, EnumName, StripPrefix))
return Err;
}
return llvm::Error::success();
}
llvm::Error ScopifyEnum::scopifyEnumValue(const EnumConstantDecl ,
- StringRef Prefix) {
+ StringRef EnumName,
+ bool StripPrefix) {
for (const auto :
findReferences(*S->AST, getPosition(CD), 0, S->Index, false)
.References) {
-if (Ref.Attributes & ReferencesResult::Declaration)
+if (Ref.Attributes & ReferencesResult::Declaration) {
+ if (StripPrefix) {
+const auto MakeReplacement = [](StringRef FilePath,
+ StringRef Content,
+ unsigned Offset) {
+ unsigned Length = EnumName.size();
+ if (Content[Offset + Length] == '_')
+++Length;
+ return tooling::Replacement(FilePath, Offset, Length, {});
+};
+if (auto Err = addReplacementForReference(Ref, MakeReplacement))
+ return Err;
+ }
continue;
+}
-const auto MakeReplacement = [](StringRef FilePath,
- StringRef Content, unsigned Offset)
{
+const auto MakeReplacement = [&](StringRef FilePath, StringRef Content,
+ unsigned Offset) {
const auto IsAlreadyScoped = [Content, Offset] {
if (Offset < 2)
return false;
@@ -164,9 +182,18 @@ llvm::Error ScopifyEnum::scopifyEnumValue(const
EnumConstantDecl ,
}
return false;
};
- return IsAlreadyScoped()
- ? tooling::Replacement()
- : tooling::Replacement(FilePath, Offset, 0, Prefix);
+ if (StripPrefix) {
+const int ExtraLength =
+Content[Offset + EnumName.size()] == '_' ? 1 : 0;
+if (IsAlreadyScoped())
+
[clang-tools-extra] [clangd] Let DefineOutline tweak create a definition from scratch (PR #71950)
ckandeler wrote: We should probably rekindle the discussion on the associated bug report; I think users (rightly) expect this feature. https://github.com/llvm/llvm-project/pull/71950 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] use existing function for code locations in the scopify enum tweak (PR #88737)
https://github.com/ckandeler approved this pull request. https://github.com/llvm/llvm-project/pull/88737 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
ckandeler wrote: Incorporated review comments. https://github.com/llvm/llvm-project/pull/83412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/ckandeler updated
https://github.com/llvm/llvm-project/pull/83412
>From 78393d26e62f2f9a30f366501f8e61b1a32d6af4 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Thu, 29 Feb 2024 12:26:52 +0100
Subject: [PATCH] [clangd] Remove potential prefix from enum value names
... when converting unscoped to scoped enums.
With traditional enums, a popular technique to guard against potential
name clashes is to use the enum name as a pseudo-namespace, like this:
enum MyEnum { MyEnumValue1, MyEnumValue2 };
With scoped enums, this makes no sense, making it extremely unlikely
that the user wants to keep such a prefix when modernizing. Therefore, our
tweak now removes it.
---
.../clangd/refactor/tweaks/ScopifyEnum.cpp| 55 +--
.../unittests/tweaks/ScopifyEnumTests.cpp | 38 +++--
clang-tools-extra/docs/ReleaseNotes.rst | 3 +
3 files changed, 74 insertions(+), 22 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
index e36b3249bc7b92..d44f2bb862ed07 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
@@ -40,15 +40,12 @@ namespace {
/// void f() { E e1 = EV1; }
///
/// After:
-/// enum class E { EV1, EV2 };
-/// void f() { E e1 = E::EV1; }
+/// enum class E { V1, V2 };
+/// void f() { E e1 = E::V1; }
///
/// Note that the respective project code might not compile anymore
/// if it made use of the now-gone implicit conversion to int.
/// This is out of scope for this tweak.
-///
-/// TODO: In the above example, we could detect that the values
-/// start with the enum name, and remove that prefix.
class ScopifyEnum : public Tweak {
const char *id() const final;
@@ -63,7 +60,8 @@ class ScopifyEnum : public Tweak {
std::function;
llvm::Error addClassKeywordToDeclarations();
llvm::Error scopifyEnumValues();
- llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef Prefix);
+ llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef EnumName,
+ bool StripPrefix);
llvm::Expected getContentForFile(StringRef FilePath);
unsigned getOffsetFromPosition(const Position , StringRef Content) const;
llvm::Error addReplacementForReference(const ReferencesResult::Reference
,
@@ -125,25 +123,42 @@ llvm::Error ScopifyEnum::addClassKeywordToDeclarations() {
}
llvm::Error ScopifyEnum::scopifyEnumValues() {
- std::string PrefixToInsert(D->getName());
- PrefixToInsert += "::";
- for (auto E : D->enumerators()) {
-if (auto Err = scopifyEnumValue(*E, PrefixToInsert))
+ StringRef EnumName(D->getName());
+ bool StripPrefix = true;
+ for (const EnumConstantDecl *E : D->enumerators()) {
+if (!E->getName().starts_with(EnumName)) {
+ StripPrefix = false;
+ break;
+}
+ }
+ for (const EnumConstantDecl *E : D->enumerators()) {
+if (auto Err = scopifyEnumValue(*E, EnumName, StripPrefix))
return Err;
}
return llvm::Error::success();
}
llvm::Error ScopifyEnum::scopifyEnumValue(const EnumConstantDecl ,
- StringRef Prefix) {
+ StringRef EnumName,
+ bool StripPrefix) {
for (const auto :
findReferences(*S->AST, getPosition(CD), 0, S->Index, false)
.References) {
-if (Ref.Attributes & ReferencesResult::Declaration)
+if (Ref.Attributes & ReferencesResult::Declaration) {
+ if (StripPrefix) {
+const auto MakeReplacement = [](StringRef FilePath,
+ StringRef /* Content */,
+ unsigned Offset) {
+ return tooling::Replacement(FilePath, Offset, EnumName.size(), {});
+};
+if (auto Err = addReplacementForReference(Ref, MakeReplacement))
+ return Err;
+ }
continue;
+}
-const auto MakeReplacement = [](StringRef FilePath,
- StringRef Content, unsigned Offset)
{
+const auto MakeReplacement = [&](StringRef FilePath, StringRef Content,
+ unsigned Offset) {
const auto IsAlreadyScoped = [Content, Offset] {
if (Offset < 2)
return false;
@@ -164,9 +179,15 @@ llvm::Error ScopifyEnum::scopifyEnumValue(const
EnumConstantDecl ,
}
return false;
};
- return IsAlreadyScoped()
- ? tooling::Replacement()
- : tooling::Replacement(FilePath, Offset, 0, Prefix);
+ if (StripPrefix) {
+if (IsAlreadyScoped())
+ return tooling::Replacement(FilePath, Offset, EnumName.size(), {});
+return tooling::Replacement(FilePath, Offset + EnumName.size(), 0,
+"::");
+ }
[clang-tools-extra] [clangd] Allow "move function body out-of-line" in non-header files (PR #69704)
ckandeler wrote: ping https://github.com/llvm/llvm-project/pull/69704 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Collect comments from function definitions into the index (PR #67802)
ckandeler wrote: ping https://github.com/llvm/llvm-project/pull/67802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Fix JSON conversion for symbol tags (PR #84747)
https://github.com/ckandeler created
https://github.com/llvm/llvm-project/pull/84747
The wrong constructor of json::Value got called, making every tag an array
instead of a number.
>From f67994902314acd8ae0f0c561b07b8c014172e17 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Mon, 11 Mar 2024 13:04:21 +0100
Subject: [PATCH] [clangd] Fix JSON conversion for symbol tags
The wrong constructor of json::Value got called, making every tag an
array instead of a number.
---
clang-tools-extra/clangd/Protocol.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/Protocol.cpp
b/clang-tools-extra/clangd/Protocol.cpp
index 8aa18bb0058abe..c6553e00dcae28 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -1412,7 +1412,7 @@ bool fromJSON(const llvm::json::Value ,
ReferenceParams ,
}
llvm::json::Value toJSON(SymbolTag Tag) {
- return llvm::json::Value{static_cast(Tag)};
+ return llvm::json::Value(static_cast(Tag));
}
llvm::json::Value toJSON(const CallHierarchyItem ) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow "move function body out-of-line" in non-header files (PR #69704)
ckandeler wrote: ping https://github.com/llvm/llvm-project/pull/69704 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/ckandeler updated
https://github.com/llvm/llvm-project/pull/83412
>From 01f74ddece947755938ccecbcc5f9d18a41eb793 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Thu, 29 Feb 2024 12:26:52 +0100
Subject: [PATCH] [clangd] Remove potential prefix from enum value names
... when converting unscoped to scoped enums.
With traditional enums, a popular technique to guard against potential
name clashes is to use the enum name as a pseudo-namespace, like this:
enum MyEnum { MyEnumValue1, MyEnumValue2 };
With scoped enums, this makes no sense, making it extremely unlikely
that the user wants to keep such a prefix when modernizing. Therefore, our
tweak now removes it.
---
.../clangd/refactor/tweaks/ScopifyEnum.cpp| 53 +--
.../unittests/tweaks/ScopifyEnumTests.cpp | 38 +++--
2 files changed, 70 insertions(+), 21 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
index e36b3249bc7b92..8e13ae52d121a6 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
@@ -40,15 +40,12 @@ namespace {
/// void f() { E e1 = EV1; }
///
/// After:
-/// enum class E { EV1, EV2 };
-/// void f() { E e1 = E::EV1; }
+/// enum class E { V1, V2 };
+/// void f() { E e1 = E::V1; }
///
/// Note that the respective project code might not compile anymore
/// if it made use of the now-gone implicit conversion to int.
/// This is out of scope for this tweak.
-///
-/// TODO: In the above example, we could detect that the values
-/// start with the enum name, and remove that prefix.
class ScopifyEnum : public Tweak {
const char *id() const final;
@@ -63,7 +60,8 @@ class ScopifyEnum : public Tweak {
std::function;
llvm::Error addClassKeywordToDeclarations();
llvm::Error scopifyEnumValues();
- llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef Prefix);
+ llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef EnumName,
+ bool StripPrefix);
llvm::Expected getContentForFile(StringRef FilePath);
unsigned getOffsetFromPosition(const Position , StringRef Content) const;
llvm::Error addReplacementForReference(const ReferencesResult::Reference
,
@@ -125,25 +123,42 @@ llvm::Error ScopifyEnum::addClassKeywordToDeclarations() {
}
llvm::Error ScopifyEnum::scopifyEnumValues() {
- std::string PrefixToInsert(D->getName());
- PrefixToInsert += "::";
+ StringRef EnumName(D->getName());
+ bool StripPrefix = true;
+ for (auto E : D->enumerators()) {
+if (!E->getName().starts_with(EnumName)) {
+ StripPrefix = false;
+ break;
+}
+ }
for (auto E : D->enumerators()) {
-if (auto Err = scopifyEnumValue(*E, PrefixToInsert))
+if (auto Err = scopifyEnumValue(*E, EnumName, StripPrefix))
return Err;
}
return llvm::Error::success();
}
llvm::Error ScopifyEnum::scopifyEnumValue(const EnumConstantDecl ,
- StringRef Prefix) {
+ StringRef EnumName,
+ bool StripPrefix) {
for (const auto :
findReferences(*S->AST, getPosition(CD), 0, S->Index, false)
.References) {
-if (Ref.Attributes & ReferencesResult::Declaration)
+if (Ref.Attributes & ReferencesResult::Declaration) {
+ if (StripPrefix) {
+const auto MakeReplacement = [](StringRef FilePath,
+ StringRef /* Content */,
+ unsigned Offset) {
+ return tooling::Replacement(FilePath, Offset, EnumName.size(), {});
+};
+if (auto Err = addReplacementForReference(Ref, MakeReplacement))
+ return Err;
+ }
continue;
+}
-const auto MakeReplacement = [](StringRef FilePath,
- StringRef Content, unsigned Offset)
{
+const auto MakeReplacement = [&](StringRef FilePath, StringRef Content,
+ unsigned Offset) {
const auto IsAlreadyScoped = [Content, Offset] {
if (Offset < 2)
return false;
@@ -164,9 +179,15 @@ llvm::Error ScopifyEnum::scopifyEnumValue(const
EnumConstantDecl ,
}
return false;
};
- return IsAlreadyScoped()
- ? tooling::Replacement()
- : tooling::Replacement(FilePath, Offset, 0, Prefix);
+ if (StripPrefix) {
+if (IsAlreadyScoped())
+ return tooling::Replacement(FilePath, Offset, EnumName.size(), {});
+return tooling::Replacement(FilePath, Offset + EnumName.size(), 0,
+"::");
+ }
+ return IsAlreadyScoped() ? tooling::Replacement()
+ : tooling::Replacement(FilePath, Offset,
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/ckandeler created
https://github.com/llvm/llvm-project/pull/83412
... when converting unscoped to scoped enums.
With traditional enums, a popular technique to guard against potential name
clashes is to use the enum name as a pseudo-namespace, like this:
enum MyEnum { MyEnumValue1, MyEnumValue2 };
With scoped enums, this makes no sense, making it extremely unlikely that the
user wants to keep such a prefix when modernizing. Therefore, our tweak now
removes it.
>From c687b73939821eba285b35e50c241738ba7915e4 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Thu, 29 Feb 2024 12:26:52 +0100
Subject: [PATCH] [clangd] Remove potential prefix from enum value names
... when converting unscoped to scoped enums.
With traditional enums, a popular technique to guard against potential
name clashes is to use the enum name as a pseudo-namespace, like this:
enum MyEnum { MyEnumValue1, MyEnumValue2 };
With scoped enums, this makes no sense, making it extremely unlikely
that the user wants to keep such a prefix when modernizing. Therefore, our
tweak now removes it.
---
.../clangd/refactor/tweaks/ScopifyEnum.cpp| 49 +--
.../unittests/tweaks/ScopifyEnumTests.cpp | 8 +--
2 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
index e36b3249bc7b92..5c042ee7b782b9 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
@@ -40,15 +40,12 @@ namespace {
/// void f() { E e1 = EV1; }
///
/// After:
-/// enum class E { EV1, EV2 };
-/// void f() { E e1 = E::EV1; }
+/// enum class E { V1, V2 };
+/// void f() { E e1 = E::V1; }
///
/// Note that the respective project code might not compile anymore
/// if it made use of the now-gone implicit conversion to int.
/// This is out of scope for this tweak.
-///
-/// TODO: In the above example, we could detect that the values
-/// start with the enum name, and remove that prefix.
class ScopifyEnum : public Tweak {
const char *id() const final;
@@ -63,7 +60,8 @@ class ScopifyEnum : public Tweak {
std::function;
llvm::Error addClassKeywordToDeclarations();
llvm::Error scopifyEnumValues();
- llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef Prefix);
+ llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef EnumName,
+ bool StripPrefix);
llvm::Expected getContentForFile(StringRef FilePath);
unsigned getOffsetFromPosition(const Position , StringRef Content) const;
llvm::Error addReplacementForReference(const ReferencesResult::Reference
,
@@ -125,25 +123,42 @@ llvm::Error ScopifyEnum::addClassKeywordToDeclarations() {
}
llvm::Error ScopifyEnum::scopifyEnumValues() {
- std::string PrefixToInsert(D->getName());
- PrefixToInsert += "::";
+ StringRef EnumName(D->getName());
+ bool StripPrefix = true;
+ for (auto E : D->enumerators()) {
+if (!E->getName().starts_with(EnumName)) {
+ StripPrefix = false;
+ break;
+}
+ }
for (auto E : D->enumerators()) {
-if (auto Err = scopifyEnumValue(*E, PrefixToInsert))
+if (auto Err = scopifyEnumValue(*E, EnumName, StripPrefix))
return Err;
}
return llvm::Error::success();
}
llvm::Error ScopifyEnum::scopifyEnumValue(const EnumConstantDecl ,
- StringRef Prefix) {
+ StringRef EnumName,
+ bool StripPrefix) {
for (const auto :
findReferences(*S->AST, getPosition(CD), 0, S->Index, false)
.References) {
-if (Ref.Attributes & ReferencesResult::Declaration)
+if (Ref.Attributes & ReferencesResult::Declaration) {
+ if (StripPrefix) {
+const auto MakeReplacement = [](StringRef FilePath,
+ StringRef /* Content */,
+ unsigned Offset) {
+ return tooling::Replacement(FilePath, Offset, EnumName.size(), {});
+};
+if (auto Err = addReplacementForReference(Ref, MakeReplacement))
+ return Err;
+ }
continue;
+}
-const auto MakeReplacement = [](StringRef FilePath,
- StringRef Content, unsigned Offset)
{
+const auto MakeReplacement = [&](StringRef FilePath, StringRef Content,
+ unsigned Offset) {
const auto IsAlreadyScoped = [Content, Offset] {
if (Offset < 2)
return false;
@@ -164,9 +179,15 @@ llvm::Error ScopifyEnum::scopifyEnumValue(const
EnumConstantDecl ,
}
return false;
};
+ if (StripPrefix) {
+if (IsAlreadyScoped())
+ return tooling::Replacement(FilePath, Offset, EnumName.size(), {});
+
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
https://github.com/ckandeler closed https://github.com/llvm/llvm-project/pull/69477 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
@@ -490,6 +491,13 @@ bool eligibleForExtraction(const SelectionTree::Node *N) {
BO->getRHS() == OuterImplicit.ASTNode.get())
return false;
}
+ if (const auto *Decl = Parent->ASTNode.get()) {
+if (!Decl->isInitCapture() &&
ckandeler wrote:
Note also that there are already (pre-existing) tests for such selections, e.g.:
```
{R"cpp(void f() {
int x = 1 + 2 + [[3 + 4 + 5]];
})cpp",
R"cpp(void f() {
auto placeholder = 3 + 4 + 5; int x = 1 + 2 + placeholder;
})cpp"},
```
https://github.com/llvm/llvm-project/pull/69477
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
@@ -490,6 +491,13 @@ bool eligibleForExtraction(const SelectionTree::Node *N) {
BO->getRHS() == OuterImplicit.ASTNode.get())
return false;
}
+ if (const auto *Decl = Parent->ASTNode.get()) {
+if (!Decl->isInitCapture() &&
ckandeler wrote:
> * "end":{"character":18,"
But that's [[2 + ]]3, no? I get an end character position of 19 with both Qt
Creator and VSCode for the [[2 + 3]] selection.
https://github.com/llvm/llvm-project/pull/69477
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
@@ -490,6 +491,13 @@ bool eligibleForExtraction(const SelectionTree::Node *N) {
BO->getRHS() == OuterImplicit.ASTNode.get())
return false;
}
+ if (const auto *Decl = Parent->ASTNode.get()) {
+if (!Decl->isInitCapture() &&
ckandeler wrote:
> For this case in particular, even if you said the refactoring is available
> for selection of `x = 1 + [[2 + 3]]` through
> these heuristics, clangd will still end up extracting the full RHS, because
> that's the AST node we associated with that
> selection.
I'm not sure I follow. In the example given above, invoking the tweak on the
selection results in:
```
auto placeholder = 2 + 3;
int x = 1 + placeholder;
```
Which is exactly what I (the user) wanted. Am I misunderstanding something?
https://github.com/llvm/llvm-project/pull/69477
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Collect comments from function definitions into the index (PR #67802)
ckandeler wrote: I have removed the extra Symbol member and extended the Flags enum instead. I benchmarked with Qt as the test project (qtbase, to be exact), which is heavily documented and has all its function documentation in the cpp files, so it should provide an upper bound on the effects of this patch. Index size before this patch : 68MB on-disk, 552MB in-memory With original patch (extra Symbol member): 70MB/576MB With latest patch set: 70MB/564 MB I only did one measurement each, so I don't know if there might be unrelated fluctuations there. https://github.com/llvm/llvm-project/pull/67802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Collect comments from function definitions into the index (PR #67802)
https://github.com/ckandeler updated
https://github.com/llvm/llvm-project/pull/67802
>From fb3711aa040aa2a986ed7d0c503042adecc6662a Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Fri, 29 Sep 2023 15:01:58 +0200
Subject: [PATCH] [clangd] Collect comments from function definitions into the
index
This is useful with projects that put their (doxygen) comments at the
implementation site, rather than the header.
---
clang-tools-extra/clangd/index/Symbol.h | 4 ++-
.../clangd/index/SymbolCollector.cpp | 29 +--
clang/lib/AST/ASTContext.cpp | 11 ++-
3 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clangd/index/Symbol.h
b/clang-tools-extra/clangd/index/Symbol.h
index 1aa5265299231..62c47ddfc5758 100644
--- a/clang-tools-extra/clangd/index/Symbol.h
+++ b/clang-tools-extra/clangd/index/Symbol.h
@@ -145,9 +145,11 @@ struct Symbol {
ImplementationDetail = 1 << 2,
/// Symbol is visible to other files (not e.g. a static helper function).
VisibleOutsideFile = 1 << 3,
+/// Symbol has an attached documentation comment.
+HasDocComment = 1 << 4
};
-
SymbolFlag Flags = SymbolFlag::None;
+
/// FIXME: also add deprecation message and fixit?
};
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp
b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index bf838e53f2a21..d071228455836 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -1020,15 +1020,17 @@ const Symbol *SymbolCollector::addDeclaration(const
NamedDecl , SymbolID ID,
*ASTCtx, *PP, CodeCompletionContext::CCC_Symbol, *CompletionAllocator,
*CompletionTUInfo,
/*IncludeBriefComments*/ false);
- std::string Documentation =
- formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion,
- /*CommentsFromHeaders=*/true));
+ std::string DocComment = getDocComment(Ctx, SymbolCompletion,
+ /*CommentsFromHeaders=*/true);
+ std::string Documentation = formatDocumentation(*CCS, DocComment);
if (!(S.Flags & Symbol::IndexedForCodeCompletion)) {
if (Opts.StoreAllDocumentation)
S.Documentation = Documentation;
Symbols.insert(S);
return Symbols.find(S.ID);
}
+ if (!DocComment.empty())
+S.Flags |= Symbol::HasDocComment;
S.Documentation = Documentation;
std::string Signature;
std::string SnippetSuffix;
@@ -1069,6 +1071,27 @@ void SymbolCollector::addDefinition(const NamedDecl ,
Symbol S = DeclSym;
// FIXME: use the result to filter out symbols.
S.Definition = *DefLoc;
+
+ std::string DocComment;
+ std::string Documentation;
+ if (!(S.Flags & Symbol::HasDocComment) &&
+ (llvm::isa(ND) || llvm::isa(ND))) {
+CodeCompletionResult SymbolCompletion((ND), 0);
+const auto *CCS = SymbolCompletion.CreateCodeCompletionString(
+*ASTCtx, *PP, CodeCompletionContext::CCC_Symbol, *CompletionAllocator,
+*CompletionTUInfo,
+/*IncludeBriefComments*/ false);
+DocComment = getDocComment(ND.getASTContext(), SymbolCompletion,
+ /*CommentsFromHeaders=*/true);
+if (!S.Documentation.empty())
+ Documentation = S.Documentation.str() + '\n' + DocComment;
+else
+ Documentation = formatDocumentation(*CCS, DocComment);
+if (!DocComment.empty())
+ S.Flags |= Symbol::HasDocComment;
+S.Documentation = Documentation;
+ }
+
Symbols.insert(S);
}
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index f7c9e4521b5f1..aa06a1ada4ba7 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -448,8 +448,17 @@ const RawComment *ASTContext::getRawCommentForAnyRedecl(
if (LastCheckedRedecl) {
if (LastCheckedRedecl == Redecl) {
LastCheckedRedecl = nullptr;
+continue;
+ }
+ if (auto F = llvm::dyn_cast(Redecl)) {
+if (!F->isThisDeclarationADefinition())
+ continue;
+ } else if (auto M = llvm::dyn_cast(Redecl)) {
+if (!M->isThisDeclarationADefinition())
+ continue;
+ } else {
+continue;
}
- continue;
}
const RawComment *RedeclComment = getRawCommentForDeclNoCache(Redecl);
if (RedeclComment) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clangd] Collect comments from function definitions into the index (PR #67802)
ckandeler wrote: > Ok, I see. (I was confused because nothing in the patch looks at the contents > of `Symbol::DocComment` other than > an `empty()` check; maybe a `bool HasDocComment` flag is sufficient?) Right, we just need to save the information whether there was a doc comment before clangd put default values into the Documentation field. > I'll have a more detailed look when I get a chance, but one suggestion I > wanted to make in the meantime: for > changes that add new information to the index, it helps to have a sense of > how large of an increase to the index's > disk and memory footprint they entail. In the past, I've measured this with > the LLVM codebase's index as a "test > case". Will check. https://github.com/llvm/llvm-project/pull/67802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clangd] Collect comments from function definitions into the index (PR #67802)
ckandeler wrote: > Do you have another patch where you use the new `DocComment` field? Is it > for showing in a hover? Yes, it is for showing documentation in a hover. clangd already supports that; it's just that it currently works only if the comments are attached to the declaration. With this patch it works also for comments at the implementation site, (which I think was the intended behavior all along). No additional patch is necessary. https://github.com/llvm/llvm-project/pull/67802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow "move function body out-of-line" in non-header files (PR #69704)
ckandeler wrote: ping https://github.com/llvm/llvm-project/pull/69704 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
ckandeler wrote: ping https://github.com/llvm/llvm-project/pull/69477 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clangd] Collect comments from function definitions into the index (PR #67802)
ckandeler wrote: ping https://github.com/llvm/llvm-project/pull/67802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
ckandeler wrote: The latest patch set addresses all the comments. https://github.com/llvm/llvm-project/pull/69477 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
@@ -490,6 +491,13 @@ bool eligibleForExtraction(const SelectionTree::Node *N) {
BO->getRHS() == OuterImplicit.ASTNode.get())
return false;
}
+ if (const auto *Decl = Parent->ASTNode.get()) {
+if (!Decl->isInitCapture() &&
ckandeler wrote:
I think I found a not-too-horrible solution.
https://github.com/llvm/llvm-project/pull/69477
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
@@ -504,68 +502,6 @@ TEST_F(ExtractVariableTest, Test) {
int main() {
auto placeholder = []() { return 42; }(); return placeholder ;
})cpp"},
- {R"cpp(
-template
-void foo(Ts ...args) {
- auto x = [[ []() {} ]];
-}
- )cpp",
- R"cpp(
-template
-void foo(Ts ...args) {
- auto placeholder = []() {}; auto x = placeholder ;
-}
- )cpp"},
- {R"cpp(
ckandeler wrote:
Done.
https://github.com/llvm/llvm-project/pull/69477
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
@@ -504,68 +502,6 @@ TEST_F(ExtractVariableTest, Test) {
int main() {
auto placeholder = []() { return 42; }(); return placeholder ;
})cpp"},
- {R"cpp(
ckandeler wrote:
Done.
https://github.com/llvm/llvm-project/pull/69477
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
@@ -422,8 +422,6 @@ TEST_F(ExtractVariableTest, Test) {
int member = 42;
};
)cpp"},
- {R"cpp(void f() { auto x = [[ [](){ return 42; }]]; })cpp",
ckandeler wrote:
Done.
https://github.com/llvm/llvm-project/pull/69477
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
@@ -27,10 +27,10 @@ TEST_F(ExtractVariableTest, Test) {
return t.b[[a]]r]]([[t.z]])]];
}
void f() {
- int a = [[5 +]] [[4 * xyz]]();
+ int a = 5 + [[4 * xyz]]();
// multivariable initialization
if(1)
-int x = [[1]], y = [[a + 1]], a = [[1]], z = a + 1;
+int x = [[1]] + 1, y = [[a + 1]], a = [[1]] + 2, z = a + 1;
ckandeler wrote:
Yes, I got side-tracked by fixing all the regressions, and in the end the patch
didn't fulfill its original purpose anymore.
Fixed now.
https://github.com/llvm/llvm-project/pull/69477
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
https://github.com/ckandeler updated
https://github.com/llvm/llvm-project/pull/69477
>From 0ad973446c970e110f1b9c1213e97a7d3da8 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Wed, 18 Oct 2023 17:24:04 +0200
Subject: [PATCH] [clangd] Do not offer extraction to variable for decl init
expression
That would turn:
int x = f() + 1;
into:
auto placeholder = f() + 1;
int x = placeholder;
which makes little sense and is clearly not intended, as stated
explicitly by a comment in eligibleForExtraction(). It appears that the
declaration case was simply forgotten (the assignment case was already
implemented).
---
.../refactor/tweaks/ExtractVariable.cpp | 44 +--
.../unittests/tweaks/ExtractVariableTests.cpp | 27 +++-
2 files changed, 55 insertions(+), 16 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
index 79bf962544242b..3b378153eafd52 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -468,7 +468,8 @@ bool eligibleForExtraction(const SelectionTree::Node *N) {
// Extracting Exprs like a = 1 gives placeholder = a = 1 which isn't useful.
// FIXME: we could still hoist the assignment, and leave the variable there?
ParsedBinaryOperator BinOp;
- if (BinOp.parse(*N) && BinaryOperator::isAssignmentOp(BinOp.Kind))
+ bool IsBinOp = BinOp.parse(*N);
+ if (IsBinOp && BinaryOperator::isAssignmentOp(BinOp.Kind))
return false;
const SelectionTree::Node = N->outerImplicit();
@@ -483,13 +484,48 @@ bool eligibleForExtraction(const SelectionTree::Node *N) {
OuterImplicit.ASTNode.get()))
return false;
+ std::function IsFullySelected =
+ [&](const SelectionTree::Node *N) {
+if (N->ASTNode.getSourceRange().isValid() &&
+N->Selected != SelectionTree::Complete)
+ return false;
+for (const auto *Child : N->Children) {
+ if (!IsFullySelected(Child))
+return false;
+}
+return true;
+ };
+ auto ExprIsFullySelectedTargetNode = [&](const Expr *E) {
+if (E != OuterImplicit.ASTNode.get())
+ return false;
+
+// The above condition is the only relevant one except for binary
operators.
+// Without the following code, we would fail to offer extraction for e.g.:
+// int x = 1 + 2 + [[3 + 4 + 5]];
+// See the documentation of ParsedBinaryOperator for further details.
+if (!IsBinOp)
+ return true;
+return IsFullySelected(N);
+ };
+
// Disable extraction of full RHS on assignment operations, e.g:
- // auto x = [[RHS_EXPR]];
+ // x = [[RHS_EXPR]];
// This would just result in duplicating the code.
if (const auto *BO = Parent->ASTNode.get()) {
-if (BO->isAssignmentOp() &&
-BO->getRHS() == OuterImplicit.ASTNode.get())
+if (BO->isAssignmentOp() && ExprIsFullySelectedTargetNode(BO->getRHS()))
+ return false;
+ }
+
+ // The same logic as for assignments applies to initializations.
+ // However, we do allow extracting the RHS of an init capture, as it is
+ // a valid use case to move non-trivial expressions out of the capture
clause.
+ // FIXME: In that case, the extracted variable should be captured directly,
+ //rather than an explicit copy.
+ if (const auto *Decl = Parent->ASTNode.get()) {
+if (!Decl->isInitCapture() &&
+ExprIsFullySelectedTargetNode(Decl->getInit())) {
return false;
+}
}
return true;
diff --git a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
index eb5b06cfee4316..42dd612c46 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
@@ -27,10 +27,10 @@ TEST_F(ExtractVariableTest, Test) {
return t.b[[a]]r]]([[t.z]])]];
}
void f() {
- int a = [[5 +]] [[4 * xyz]]();
+ int a = 5 + [[4 * xyz]]();
// multivariable initialization
if(1)
-int x = [[1]], y = [[a + 1]], a = [[1]], z = a + 1;
+int x = [[1]] + 1, y = a + [[1]], a = [[1]] + 2, z = a + 1;
// if without else
if([[1]])
a = [[1]] + 1;
@@ -61,7 +61,7 @@ TEST_F(ExtractVariableTest, Test) {
ExtraArgs = {"-xc"};
const char *AvailableC = R"cpp(
void foo() {
- int x = [[1]];
+ int x = [[1]] + 1;
})cpp";
EXPECT_AVAILABLE(AvailableC);
@@ -79,7 +79,7 @@ TEST_F(ExtractVariableTest, Test) {
@end
@implementation Foo
- (void)method {
- int x = [[1 + 2]];
+ int x = [[1]] + 2;
}
@end)cpp";
EXPECT_AVAILABLE(AvailableObjC);
@@ -103,6 +103,9 @@ TEST_F(ExtractVariableTest, Test) {
}
int z = [[1]];
} t;
+ int x = [[1 + 2]];
+
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
@@ -490,6 +491,13 @@ bool eligibleForExtraction(const SelectionTree::Node *N) {
BO->getRHS() == OuterImplicit.ASTNode.get())
return false;
}
+ if (const auto *Decl = Parent->ASTNode.get()) {
+if (!Decl->isInitCapture() &&
ckandeler wrote:
Presumably, with the BinOp check I intended to prevent a false negative for
this case:
`int x = 1 + 2 + [[3 + 4 + 5]];`
Apparently, the parser considers the last "+" to be the top-level expression,
so we would erroneously fail to offer extraction for the above expression.
However, the same problem already existed for the assignment case, so it's
probably wrong anyway to add such a workaround for initialization only. On the
other hand, I don't want to introduce a regression either. Do you have a good
idea as to how a proper check could look like?
https://github.com/llvm/llvm-project/pull/69477
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow "move function body out-of-line" in non-header files (PR #69704)
https://github.com/ckandeler updated
https://github.com/llvm/llvm-project/pull/69704
>From 40df0527b2a3af8012f32d771a1bb2c861d42ed3 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Thu, 19 Oct 2023 17:51:11 +0200
Subject: [PATCH] [clangd] Allow "move function body out-of-line" in non-header
files
Moving the body of member functions out-of-line makes sense for classes
defined in implementation files too.
---
.../clangd/refactor/tweaks/DefineOutline.cpp | 150 +++---
.../unittests/tweaks/DefineOutlineTests.cpp | 73 -
2 files changed, 164 insertions(+), 59 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index b84ae04072f2c19..98cb3a8770c696d 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -179,14 +179,11 @@ deleteTokensWithKind(const syntax::TokenBuffer ,
tok::TokenKind Kind,
// looked up in the context containing the function/method.
// FIXME: Drop attributes in function signature.
llvm::Expected
-getFunctionSourceCode(const FunctionDecl *FD, llvm::StringRef TargetNamespace,
+getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext,
const syntax::TokenBuffer ,
const HeuristicResolver *Resolver) {
auto = FD->getASTContext();
auto = AST.getSourceManager();
- auto TargetContext = findContextForNS(TargetNamespace, FD->getDeclContext());
- if (!TargetContext)
-return error("define outline: couldn't find a context for target");
llvm::Error Errors = llvm::Error::success();
tooling::Replacements DeclarationCleanups;
@@ -216,7 +213,7 @@ getFunctionSourceCode(const FunctionDecl *FD,
llvm::StringRef TargetNamespace,
}
const NamedDecl *ND = Ref.Targets.front();
const std::string Qualifier =
-getQualification(AST, *TargetContext,
+getQualification(AST, TargetContext,
SM.getLocForStartOfFile(SM.getMainFileID()), ND);
if (auto Err = DeclarationCleanups.add(
tooling::Replacement(SM, Ref.NameLoc, 0, Qualifier)))
@@ -232,7 +229,7 @@ getFunctionSourceCode(const FunctionDecl *FD,
llvm::StringRef TargetNamespace,
if (const auto *Destructor = llvm::dyn_cast(FD)) {
if (auto Err = DeclarationCleanups.add(tooling::Replacement(
SM, Destructor->getLocation(), 0,
-getQualification(AST, *TargetContext,
+getQualification(AST, TargetContext,
SM.getLocForStartOfFile(SM.getMainFileID()),
Destructor
Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
@@ -319,29 +316,9 @@ getFunctionSourceCode(const FunctionDecl *FD,
llvm::StringRef TargetNamespace,
}
struct InsertionPoint {
- std::string EnclosingNamespace;
+ const DeclContext *EnclosingNamespace = nullptr;
size_t Offset;
};
-// Returns the most natural insertion point for \p QualifiedName in \p
Contents.
-// This currently cares about only the namespace proximity, but in feature it
-// should also try to follow ordering of declarations. For example, if decls
-// come in order `foo, bar, baz` then this function should return some point
-// between foo and baz for inserting bar.
-llvm::Expected getInsertionPoint(llvm::StringRef Contents,
- llvm::StringRef QualifiedName,
- const LangOptions ) {
- auto Region = getEligiblePoints(Contents, QualifiedName, LangOpts);
-
- assert(!Region.EligiblePoints.empty());
- // FIXME: This selection can be made smarter by looking at the definition
- // locations for adjacent decls to Source. Unfortunately pseudo parsing in
- // getEligibleRegions only knows about namespace begin/end events so we
- // can't match function start/end positions yet.
- auto Offset = positionToOffset(Contents, Region.EligiblePoints.back());
- if (!Offset)
-return Offset.takeError();
- return InsertionPoint{Region.EnclosingNamespace, *Offset};
-}
// Returns the range that should be deleted from declaration, which always
// contains function body. In addition to that it might contain constructor
@@ -409,14 +386,9 @@ class DefineOutline : public Tweak {
}
bool prepare(const Selection ) override {
-// Bail out if we are not in a header file.
-// FIXME: We might want to consider moving method definitions below class
-// definition even if we are inside a source file.
-if (!isHeaderFile(Sel.AST->getSourceManager().getFilename(Sel.Cursor),
- Sel.AST->getLangOpts()))
- return false;
-
+SameFile = !isHeaderFile(Sel.AST->tuPath(), Sel.AST->getLangOpts());
Source = getSelectedFunction(Sel.ASTSelection.commonAncestor());
+
// Bail out if the selection is not
[clang-tools-extra] [clangd] Allow "move function body out-of-line" in non-header files (PR #69704)
@@ -349,6 +358,36 @@ TEST_F(DefineOutlineTest, ApplyTest) {
}
}
+TEST_F(DefineOutlineTest, InCppFile) {
+ FileName = "Test.cpp";
+
+ struct {
+llvm::StringRef Test;
+llvm::StringRef ExpectedSource;
+ } Cases[] = {
+ {
+ R"cpp(
+namespace foo {
+namespace {
+struct Foo { void ba^r() {} };
ckandeler wrote:
Hm, the SourceLocations are weird: getEndLoc() points to the closing brace of
the class. Do I really have to look for the semicolon manually?
https://github.com/llvm/llvm-project/pull/69704
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow "move function body out-of-line" in non-header files (PR #69704)
@@ -349,6 +358,36 @@ TEST_F(DefineOutlineTest, ApplyTest) {
}
}
+TEST_F(DefineOutlineTest, InCppFile) {
+ FileName = "Test.cpp";
+
+ struct {
+llvm::StringRef Test;
+llvm::StringRef ExpectedSource;
+ } Cases[] = {
+ {
+ R"cpp(
+namespace foo {
+namespace {
+struct Foo { void ba^r() {} };
ckandeler wrote:
Ah, maybe getOuterLexicalRecordContext().
https://github.com/llvm/llvm-project/pull/69704
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow "move function body out-of-line" in non-header files (PR #69704)
@@ -349,6 +358,36 @@ TEST_F(DefineOutlineTest, ApplyTest) {
}
}
+TEST_F(DefineOutlineTest, InCppFile) {
+ FileName = "Test.cpp";
+
+ struct {
+llvm::StringRef Test;
+llvm::StringRef ExpectedSource;
+ } Cases[] = {
+ {
+ R"cpp(
+namespace foo {
+namespace {
+struct Foo { void ba^r() {} };
ckandeler wrote:
> so what about changing this logic to get to enclosing decl instead (i.e.
> TagDecl) and insert right after its end location?
That will break with nested classes, won't it?
https://github.com/llvm/llvm-project/pull/69704
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow "move function body out-of-line" in non-header files (PR #69704)
https://github.com/ckandeler updated
https://github.com/llvm/llvm-project/pull/69704
>From 27af98b5a9b71255b2873f25943ed23e42946b27 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Thu, 19 Oct 2023 17:51:11 +0200
Subject: [PATCH] [clangd] Allow "move function body out-of-line" in non-header
files
Moving the body of member functions out-of-line makes sense for classes
defined in implementation files too.
---
.../clangd/refactor/tweaks/DefineOutline.cpp | 135 ++
.../unittests/tweaks/DefineOutlineTests.cpp | 43 +-
2 files changed, 119 insertions(+), 59 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index b84ae04072f2c19..67d13f3eb4a6b07 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -179,14 +179,11 @@ deleteTokensWithKind(const syntax::TokenBuffer ,
tok::TokenKind Kind,
// looked up in the context containing the function/method.
// FIXME: Drop attributes in function signature.
llvm::Expected
-getFunctionSourceCode(const FunctionDecl *FD, llvm::StringRef TargetNamespace,
+getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext,
const syntax::TokenBuffer ,
const HeuristicResolver *Resolver) {
auto = FD->getASTContext();
auto = AST.getSourceManager();
- auto TargetContext = findContextForNS(TargetNamespace, FD->getDeclContext());
- if (!TargetContext)
-return error("define outline: couldn't find a context for target");
llvm::Error Errors = llvm::Error::success();
tooling::Replacements DeclarationCleanups;
@@ -216,7 +213,7 @@ getFunctionSourceCode(const FunctionDecl *FD,
llvm::StringRef TargetNamespace,
}
const NamedDecl *ND = Ref.Targets.front();
const std::string Qualifier =
-getQualification(AST, *TargetContext,
+getQualification(AST, TargetContext,
SM.getLocForStartOfFile(SM.getMainFileID()), ND);
if (auto Err = DeclarationCleanups.add(
tooling::Replacement(SM, Ref.NameLoc, 0, Qualifier)))
@@ -232,7 +229,7 @@ getFunctionSourceCode(const FunctionDecl *FD,
llvm::StringRef TargetNamespace,
if (const auto *Destructor = llvm::dyn_cast(FD)) {
if (auto Err = DeclarationCleanups.add(tooling::Replacement(
SM, Destructor->getLocation(), 0,
-getQualification(AST, *TargetContext,
+getQualification(AST, TargetContext,
SM.getLocForStartOfFile(SM.getMainFileID()),
Destructor
Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
@@ -319,29 +316,9 @@ getFunctionSourceCode(const FunctionDecl *FD,
llvm::StringRef TargetNamespace,
}
struct InsertionPoint {
- std::string EnclosingNamespace;
+ const DeclContext *EnclosingNamespace = nullptr;
size_t Offset;
};
-// Returns the most natural insertion point for \p QualifiedName in \p
Contents.
-// This currently cares about only the namespace proximity, but in feature it
-// should also try to follow ordering of declarations. For example, if decls
-// come in order `foo, bar, baz` then this function should return some point
-// between foo and baz for inserting bar.
-llvm::Expected getInsertionPoint(llvm::StringRef Contents,
- llvm::StringRef QualifiedName,
- const LangOptions ) {
- auto Region = getEligiblePoints(Contents, QualifiedName, LangOpts);
-
- assert(!Region.EligiblePoints.empty());
- // FIXME: This selection can be made smarter by looking at the definition
- // locations for adjacent decls to Source. Unfortunately pseudo parsing in
- // getEligibleRegions only knows about namespace begin/end events so we
- // can't match function start/end positions yet.
- auto Offset = positionToOffset(Contents, Region.EligiblePoints.back());
- if (!Offset)
-return Offset.takeError();
- return InsertionPoint{Region.EnclosingNamespace, *Offset};
-}
// Returns the range that should be deleted from declaration, which always
// contains function body. In addition to that it might contain constructor
@@ -409,14 +386,9 @@ class DefineOutline : public Tweak {
}
bool prepare(const Selection ) override {
-// Bail out if we are not in a header file.
-// FIXME: We might want to consider moving method definitions below class
-// definition even if we are inside a source file.
-if (!isHeaderFile(Sel.AST->getSourceManager().getFilename(Sel.Cursor),
- Sel.AST->getLangOpts()))
- return false;
-
+SameFile = !isHeaderFile(Sel.AST->tuPath(), Sel.AST->getLangOpts());
Source = getSelectedFunction(Sel.ASTSelection.commonAncestor());
+
// Bail out if the selection is not a
[clang-tools-extra] [clangd] Allow "move function body out-of-line" in non-header files (PR #69704)
@@ -19,7 +19,7 @@ TWEAK_TEST(DefineOutline);
TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
FileName = "Test.cpp";
- // Not available unless in a header file.
+ // Not available for free function unless in a header file.
ckandeler wrote:
Do you have suggestions? anon namespace in header is already covered.
https://github.com/llvm/llvm-project/pull/69704
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow "move function body out-of-line" in non-header files (PR #69704)
@@ -499,17 +473,64 @@ class DefineOutline : public Tweak {
HeaderUpdates = HeaderUpdates.merge(*DelInline);
}
-auto HeaderFE = Effect::fileEdit(SM, SM.getMainFileID(), HeaderUpdates);
-if (!HeaderFE)
- return HeaderFE.takeError();
-
-Effect->ApplyEdits.try_emplace(HeaderFE->first,
- std::move(HeaderFE->second));
+if (SameFile) {
+ tooling::Replacements = Effect->ApplyEdits[*CCFile].Replacements;
+ R = R.merge(HeaderUpdates);
+} else {
+ auto HeaderFE = Effect::fileEdit(SM, SM.getMainFileID(), HeaderUpdates);
+ if (!HeaderFE)
+return HeaderFE.takeError();
+ Effect->ApplyEdits.try_emplace(HeaderFE->first,
+ std::move(HeaderFE->second));
+}
return std::move(*Effect);
}
+ // Returns the most natural insertion point for \p QualifiedName in \p
+ // Contents. This currently cares about only the namespace proximity, but in
+ // feature it should also try to follow ordering of declarations. For
example,
+ // if decls come in order `foo, bar, baz` then this function should return
+ // some point between foo and baz for inserting bar.
+ llvm::Expected getInsertionPoint(llvm::StringRef Contents,
+ const Selection ) {
+// If the definition goes to the same file and there is a namespace,
+// we should (and, in the case of anonymous namespaces, need to)
+// put the definition into the original namespace block.
+// Within this constraint, the same considerations apply as in
+// the FIXME below.
+if (SameFile) {
+ if (auto *Namespace = Source->getEnclosingNamespaceContext()) {
ckandeler wrote:
You mean this also covers the global namespace? I wasn't sure about that. But
don't we need special handling for that case, then? As I'd assume it won't have
a proper location?
https://github.com/llvm/llvm-project/pull/69704
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 535f34d - [index][clangd] Consider labels when indexing function bodies
Author: Christian Kandeler
Date: 2023-08-01T09:07:05+02:00
New Revision: 535f34dd80c2200c35971632021a5ed375774d9b
URL:
https://github.com/llvm/llvm-project/commit/535f34dd80c2200c35971632021a5ed375774d9b
DIFF:
https://github.com/llvm/llvm-project/commit/535f34dd80c2200c35971632021a5ed375774d9b.diff
LOG: [index][clangd] Consider labels when indexing function bodies
It's valuable to have document highlights for labels and be able to find
references to them.
Reviewed By: nridge
Differential Revision: https://reviews.llvm.org/D150124
Added:
Modified:
clang-tools-extra/clangd/unittests/XRefsTests.cpp
clang/lib/Index/IndexBody.cpp
clang/unittests/Index/IndexTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 05cf891e71db40..6d7fd62016991a 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -125,6 +125,13 @@ TEST(HighlightsTest, All) {
[Foo [[x]]:2 [[^y]]:4];
}
)cpp",
+ R"cpp( // Label
+int main() {
+ goto [[^theLabel]];
+ [[theLabel]]:
+return 1;
+}
+ )cpp",
};
for (const char *Test : Tests) {
Annotations T(Test);
diff --git a/clang/lib/Index/IndexBody.cpp b/clang/lib/Index/IndexBody.cpp
index e5f1764550ffa7..e88f321f18a712 100644
--- a/clang/lib/Index/IndexBody.cpp
+++ b/clang/lib/Index/IndexBody.cpp
@@ -144,6 +144,17 @@ class BodyIndexer : public
RecursiveASTVisitor {
Parent, ParentDC, Roles, Relations, E);
}
+ bool VisitGotoStmt(GotoStmt *S) {
+return IndexCtx.handleReference(S->getLabel(), S->getLabelLoc(), Parent,
+ParentDC);
+ }
+
+ bool VisitLabelStmt(LabelStmt *S) {
+if (IndexCtx.shouldIndexFunctionLocalSymbols())
+ return IndexCtx.handleDecl(S->getDecl());
+return true;
+ }
+
bool VisitMemberExpr(MemberExpr *E) {
SourceLocation Loc = E->getMemberLoc();
if (Loc.isInvalid())
diff --git a/clang/unittests/Index/IndexTests.cpp
b/clang/unittests/Index/IndexTests.cpp
index 690673a1072b00..4d19f47283c285 100644
--- a/clang/unittests/Index/IndexTests.cpp
+++ b/clang/unittests/Index/IndexTests.cpp
@@ -218,6 +218,28 @@ TEST(IndexTest, IndexParametersInDecls) {
EXPECT_THAT(Index->Symbols, Not(Contains(QName("bar";
}
+TEST(IndexTest, IndexLabels) {
+ std::string Code = R"cpp(
+int main() {
+ goto theLabel;
+ theLabel:
+return 1;
+}
+ )cpp";
+ auto Index = std::make_shared();
+ IndexingOptions Opts;
+ Opts.IndexFunctionLocals = true;
+ tooling::runToolOnCode(std::make_unique(Index, Opts), Code);
+ EXPECT_THAT(Index->Symbols,
+ Contains(AllOf(QName("theLabel"), WrittenAt(Position(3, 16)),
+ DeclAt(Position(4, 11);
+
+ Opts.IndexFunctionLocals = false;
+ Index->Symbols.clear();
+ tooling::runToolOnCode(std::make_unique(Index, Opts), Code);
+ EXPECT_THAT(Index->Symbols, Not(Contains(QName("theLabel";
+}
+
TEST(IndexTest, IndexExplicitTemplateInstantiation) {
std::string Code = R"cpp(
template
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] e72baa7 - [clangd] Add semantic token for labels
Author: Christian Kandeler
Date: 2023-06-07T12:28:06+02:00
New Revision: e72baa76b91fbcb2b16747cb7d2088723478a754
URL:
https://github.com/llvm/llvm-project/commit/e72baa76b91fbcb2b16747cb7d2088723478a754
DIFF:
https://github.com/llvm/llvm-project/commit/e72baa76b91fbcb2b16747cb7d2088723478a754.diff
LOG: [clangd] Add semantic token for labels
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D143260
Added:
Modified:
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/SemanticHighlighting.h
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index ec37476cf94ea..6a835f31f064b 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -166,6 +166,8 @@ std::optional kindForDecl(const NamedDecl
*D,
return HighlightingKind::TemplateParameter;
if (isa(D))
return HighlightingKind::Concept;
+ if (isa(D))
+return HighlightingKind::Label;
if (const auto *UUVD = dyn_cast(D)) {
auto Targets = Resolver->resolveUsingValueDecl(UUVD);
if (!Targets.empty() && Targets[0] != UUVD) {
@@ -1271,6 +1273,8 @@ llvm::raw_ostream <<(llvm::raw_ostream ,
HighlightingKind K) {
return OS << "Operator";
case HighlightingKind::Bracket:
return OS << "Bracket";
+ case HighlightingKind::Label:
+return OS << "Label";
case HighlightingKind::InactiveCode:
return OS << "InactiveCode";
}
@@ -1470,6 +1474,8 @@ llvm::StringRef toSemanticTokenType(HighlightingKind
Kind) {
return "operator";
case HighlightingKind::Bracket:
return "bracket";
+ case HighlightingKind::Label:
+return "label";
case HighlightingKind::InactiveCode:
return "comment";
}
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.h
b/clang-tools-extra/clangd/SemanticHighlighting.h
index c9db598ff08c9..59d742b83ee52 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.h
+++ b/clang-tools-extra/clangd/SemanticHighlighting.h
@@ -52,6 +52,7 @@ enum class HighlightingKind {
Modifier,
Operator,
Bracket,
+ Label,
// This one is
diff erent from the other kinds as it's a line style
// rather than a token style.
diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index 9c6e5246f5c37..ff052e6be9549 100644
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -1028,6 +1028,16 @@ sizeof...($TemplateParameter[[Elements]]);
template $Bracket[[<]]$Concept[[C2]]$Bracket[[<]]int$Bracket[[>]]
$TemplateParameter_def[[A]]$Bracket[[>]]
class $Class_def[[B]] {};
)cpp",
+ // Labels
+ R"cpp(
+bool $Function_def[[funcWithGoto]](bool $Parameter_def[[b]]) {
+ if ($Parameter[[b]])
+goto $Label[[return_true]];
+ return false;
+ $Label_decl[[return_true]]:
+return true;
+}
+ )cpp",
// no crash
R"cpp(
struct $Class_def[[Foo]] {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] 008cb29 - [clangd] Renaming: Treat member functions like other functions
Author: Christian Kandeler
Date: 2023-05-22T12:50:38+02:00
New Revision: 008cb29f87f3af391eb6c3747bdad16f2e386161
URL:
https://github.com/llvm/llvm-project/commit/008cb29f87f3af391eb6c3747bdad16f2e386161
DIFF:
https://github.com/llvm/llvm-project/commit/008cb29f87f3af391eb6c3747bdad16f2e386161.diff
LOG: [clangd] Renaming: Treat member functions like other functions
... by skipping the conflict check. The same considerations apply.
Reviewed By: hokein
Differential Revision: https://reviews.llvm.org/D150685
Added:
Modified:
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp
b/clang-tools-extra/clangd/refactor/Rename.cpp
index 6362768f9b475..b3270534b13b1 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -515,7 +515,8 @@ std::optional checkName(const NamedDecl
,
else {
// Name conflict detection.
// Function conflicts are subtle (overloading), so ignore them.
-if (RenameDecl.getKind() != Decl::Function) {
+if (RenameDecl.getKind() != Decl::Function &&
+RenameDecl.getKind() != Decl::CXXMethod) {
if (auto *Conflict = lookupSiblingWithName(ASTCtx, RenameDecl, NewName))
Result = InvalidName{
InvalidName::Conflict,
diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp
b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index 5b99f8c4fc44c..9be4a970a7cfb 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1062,6 +1062,19 @@ TEST(RenameTest, Renameable) {
)cpp",
"conflict", !HeaderFile, "Conflict"},
+ {R"cpp(
+void func(int);
+void [[o^therFunc]](double);
+ )cpp",
+ nullptr, !HeaderFile, "func"},
+ {R"cpp(
+struct S {
+ void func(int);
+ void [[o^therFunc]](double);
+};
+ )cpp",
+ nullptr, !HeaderFile, "func"},
+
{R"cpp(
int V^ar;
)cpp",
@@ -1121,9 +1134,7 @@ TEST(RenameTest, Renameable) {
} else {
EXPECT_TRUE(bool(Results)) << "rename returned an error: "
<< llvm::toString(Results.takeError());
- ASSERT_EQ(1u, Results->GlobalChanges.size());
- EXPECT_EQ(applyEdits(std::move(Results->GlobalChanges)).front().second,
-expectedResult(T, NewName));
+ EXPECT_EQ(Results->LocalChanges, T.ranges());
}
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] bbddbe5 - [clangd] Add semantic token for angle brackets
Author: Christian Kandeler
Date: 2023-01-31T17:15:31+01:00
New Revision: bbddbe580bca5fa1c0478a3ec6edd0e0c40b9d96
URL:
https://github.com/llvm/llvm-project/commit/bbddbe580bca5fa1c0478a3ec6edd0e0c40b9d96
DIFF:
https://github.com/llvm/llvm-project/commit/bbddbe580bca5fa1c0478a3ec6edd0e0c40b9d96.diff
LOG: [clangd] Add semantic token for angle brackets
This is needed for clients that would like to visualize matching
opening and closing angle brackets, which can be valuable in non-trivial
template declarations or instantiations.
It is not possible to do this with simple lexing, as the tokens
could also refer to operators.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D139926
Added:
Modified:
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/SemanticHighlighting.h
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index a516c688efc1..e3022ad131ff 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -369,6 +369,47 @@ class HighlightingsBuilder {
return addToken(*Range, Kind);
}
+ // Most of this function works around
+ // https://github.com/clangd/clangd/issues/871.
+ void addAngleBracketTokens(SourceLocation LLoc, SourceLocation RLoc) {
+if (!LLoc.isValid() || !RLoc.isValid())
+ return;
+
+auto LRange = getRangeForSourceLocation(LLoc);
+if (!LRange)
+ return;
+
+// RLoc might be pointing at a virtual buffer when it's part of a `>>`
+// token.
+RLoc = SourceMgr.getFileLoc(RLoc);
+// Make sure token is part of the main file.
+RLoc = getHighlightableSpellingToken(RLoc, SourceMgr);
+if (!RLoc.isValid())
+ return;
+
+const auto *RTok = TB.spelledTokenAt(RLoc);
+// Handle `>>`. RLoc is always pointing at the right location, just change
+// the end to be offset by 1.
+// We'll either point at the beginning of `>>`, hence get a proper spelled
+// or point in the middle of `>>` hence get no spelled tok.
+if (!RTok || RTok->kind() == tok::greatergreater) {
+ Position Begin = sourceLocToPosition(SourceMgr, RLoc);
+ Position End = sourceLocToPosition(SourceMgr, RLoc.getLocWithOffset(1));
+ addToken(*LRange, HighlightingKind::Bracket);
+ addToken({Begin, End}, HighlightingKind::Bracket);
+ return;
+}
+
+// Easy case, we have the `>` token directly available.
+if (RTok->kind() == tok::greater) {
+ if (auto RRange = getRangeForSourceLocation(RLoc)) {
+addToken(*LRange, HighlightingKind::Bracket);
+addToken(*RRange, HighlightingKind::Bracket);
+ }
+ return;
+}
+ }
+
HighlightingToken (Range R, HighlightingKind Kind) {
HighlightingToken HT;
HT.R = std::move(R);
@@ -559,6 +600,12 @@ class CollectExtraHighlightings
return Base::TraverseConstructorInitializer(Init);
}
+ bool TraverseTypeConstraint(const TypeConstraint *C) {
+if (auto *Args = C->getTemplateArgsAsWritten())
+ H.addAngleBracketTokens(Args->getLAngleLoc(), Args->getRAngleLoc());
+return Base::TraverseTypeConstraint(C);
+ }
+
bool VisitPredefinedExpr(PredefinedExpr *E) {
H.addToken(E->getLocation(), HighlightingKind::LocalVariable)
.addModifier(HighlightingModifier::Static)
@@ -567,6 +614,77 @@ class CollectExtraHighlightings
return true;
}
+ bool VisitConceptSpecializationExpr(ConceptSpecializationExpr *E) {
+if (auto *Args = E->getTemplateArgsAsWritten())
+ H.addAngleBracketTokens(Args->getLAngleLoc(), Args->getRAngleLoc());
+return true;
+ }
+
+ bool VisitTemplateDecl(TemplateDecl *D) {
+if (auto *TPL = D->getTemplateParameters())
+ H.addAngleBracketTokens(TPL->getLAngleLoc(), TPL->getRAngleLoc());
+return true;
+ }
+
+ bool VisitTagDecl(TagDecl *D) {
+for (unsigned i = 0; i < D->getNumTemplateParameterLists(); ++i) {
+ if (auto *TPL = D->getTemplateParameterList(i))
+H.addAngleBracketTokens(TPL->getLAngleLoc(), TPL->getRAngleLoc());
+}
+return true;
+ }
+
+ bool VisitClassTemplatePartialSpecializationDecl(
+ ClassTemplatePartialSpecializationDecl *D) {
+if (auto *TPL = D->getTemplateParameters())
+ H.addAngleBracketTokens(TPL->getLAngleLoc(), TPL->getRAngleLoc());
+if (auto *Args = D->getTemplateArgsAsWritten())
+ H.addAngleBracketTokens(Args->getLAngleLoc(), Args->getRAngleLoc());
+return true;
+ }
+
+ bool VisitVarTemplateSpecializationDecl(VarTemplateSpecializationDecl *D) {
+if (auto *Args = D->getTemplateArgsInfo())
+ H.addAngleBracketTokens(Args->getLAngleLoc(), Args->getRAngleLoc());
+return true;
+ }
+
+ bool VisitVarTemplatePartialSpecializationDecl(
+
[clang-tools-extra] 647d314 - [clangd] Add support for semantic token type "operator"
Author: Christian Kandeler
Date: 2022-12-12T16:17:43+01:00
New Revision: 647d314eb059b6d2e7c00d7eefe6a7afc37dff25
URL:
https://github.com/llvm/llvm-project/commit/647d314eb059b6d2e7c00d7eefe6a7afc37dff25
DIFF:
https://github.com/llvm/llvm-project/commit/647d314eb059b6d2e7c00d7eefe6a7afc37dff25.diff
LOG: [clangd] Add support for semantic token type "operator"
Also add new modifier for differentiating between built-in and user-
provided operators.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D136594
Added:
Modified:
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/SemanticHighlighting.h
clang-tools-extra/clangd/test/initialize-params.test
clang-tools-extra/clangd/test/semantic-tokens.test
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index ccdaab89620b2..6745e2594ead3 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -566,6 +566,71 @@ class CollectExtraHighlightings
return true;
}
+ bool VisitFunctionDecl(FunctionDecl *D) {
+if (D->isOverloadedOperator()) {
+ const auto addOpDeclToken = [&](SourceLocation Loc) {
+auto = H.addToken(Loc, HighlightingKind::Operator)
+ .addModifier(HighlightingModifier::Declaration);
+if (D->isThisDeclarationADefinition())
+ Token.addModifier(HighlightingModifier::Definition);
+ };
+ const auto Range = D->getNameInfo().getCXXOperatorNameRange();
+ addOpDeclToken(Range.getBegin());
+ const auto Kind = D->getOverloadedOperator();
+ if (Kind == OO_Call || Kind == OO_Subscript)
+addOpDeclToken(Range.getEnd());
+}
+return true;
+ }
+
+ bool VisitCXXOperatorCallExpr(CXXOperatorCallExpr *E) {
+const auto addOpToken = [&](SourceLocation Loc) {
+ H.addToken(Loc, HighlightingKind::Operator)
+ .addModifier(HighlightingModifier::UserDefined);
+};
+addOpToken(E->getOperatorLoc());
+const auto Kind = E->getOperator();
+if (Kind == OO_Call || Kind == OO_Subscript) {
+ if (auto *Callee = E->getCallee())
+addOpToken(Callee->getBeginLoc());
+}
+return true;
+ }
+
+ bool VisitUnaryOperator(UnaryOperator *Op) {
+auto = H.addToken(Op->getOperatorLoc(), HighlightingKind::Operator);
+if (Op->getSubExpr()->isTypeDependent())
+ Token.addModifier(HighlightingModifier::UserDefined);
+return true;
+ }
+
+ bool VisitBinaryOperator(BinaryOperator *Op) {
+auto = H.addToken(Op->getOperatorLoc(), HighlightingKind::Operator);
+if (Op->getLHS()->isTypeDependent() || Op->getRHS()->isTypeDependent())
+ Token.addModifier(HighlightingModifier::UserDefined);
+return true;
+ }
+
+ bool VisitConditionalOperator(ConditionalOperator *Op) {
+H.addToken(Op->getQuestionLoc(), HighlightingKind::Operator);
+H.addToken(Op->getColonLoc(), HighlightingKind::Operator);
+return true;
+ }
+
+ bool VisitCXXNewExpr(CXXNewExpr *E) {
+auto = H.addToken(E->getBeginLoc(), HighlightingKind::Operator);
+if (isa(E->getOperatorNew()))
+ Token.addModifier(HighlightingModifier::UserDefined);
+return true;
+ }
+
+ bool VisitCXXDeleteExpr(CXXDeleteExpr *E) {
+auto = H.addToken(E->getBeginLoc(), HighlightingKind::Operator);
+if (isa(E->getOperatorDelete()))
+ Token.addModifier(HighlightingModifier::UserDefined);
+return true;
+ }
+
bool VisitCallExpr(CallExpr *E) {
// Highlighting parameters passed by non-const reference does not really
// make sense for literals...
@@ -671,12 +736,20 @@ class CollectExtraHighlightings
bool VisitCXXMemberCallExpr(CXXMemberCallExpr *CE) {
// getMethodDecl can return nullptr with member pointers, e.g.
// `(foo.*pointer_to_member_fun)(arg);`
-if (isa_and_present(CE->getMethodDecl())) {
- if (auto *ME = dyn_cast(CE->getCallee())) {
-if (auto *TI = ME->getMemberNameInfo().getNamedTypeInfo()) {
- H.addExtraModifier(TI->getTypeLoc().getBeginLoc(),
- HighlightingModifier::ConstructorOrDestructor);
+if (auto *D = CE->getMethodDecl()) {
+ if (isa(D)) {
+if (auto *ME = dyn_cast(CE->getCallee())) {
+ if (auto *TI = ME->getMemberNameInfo().getNamedTypeInfo()) {
+H.addExtraModifier(TI->getTypeLoc().getBeginLoc(),
+ HighlightingModifier::ConstructorOrDestructor);
+ }
}
+ } else if (D->isOverloadedOperator()) {
+if (auto *ME = dyn_cast(CE->getCallee()))
+ H.addToken(
+ ME->getMemberNameInfo().getCXXOperatorNameRange().getBegin(),
+ HighlightingKind::Operator)
+
[clang-tools-extra] 699a59a - [clangd] Mark "override" and "final" as modifiers
Author: Christian Kandeler
Date: 2022-11-21T22:01:12+01:00
New Revision: 699a59aa5865d8b10f42284f68c424a9123cb8b2
URL:
https://github.com/llvm/llvm-project/commit/699a59aa5865d8b10f42284f68c424a9123cb8b2
DIFF:
https://github.com/llvm/llvm-project/commit/699a59aa5865d8b10f42284f68c424a9123cb8b2.diff
LOG: [clangd] Mark "override" and "final" as modifiers
... in semantic highlighting.
These specifiers cannot be identified by simple lexing (since e.g.
variables with these names can legally be declared), which means they
should be semantic tokens.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D137943
Added:
Modified:
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/SemanticHighlighting.h
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index dd9392b029df8..dd25a2f31230c 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -809,6 +809,18 @@ class CollectExtraHighlightings
return true;
}
+ bool VisitAttr(Attr *A) {
+switch (A->getKind()) {
+case attr::Override:
+case attr::Final:
+ H.addToken(A->getLocation(), HighlightingKind::Modifier);
+ break;
+default:
+ break;
+}
+return true;
+ }
+
bool VisitDependentNameTypeLoc(DependentNameTypeLoc L) {
H.addToken(L.getNameLoc(), HighlightingKind::Type)
.addModifier(HighlightingModifier::DependentName)
@@ -985,6 +997,8 @@ llvm::raw_ostream <<(llvm::raw_ostream ,
HighlightingKind K) {
return OS << "Primitive";
case HighlightingKind::Macro:
return OS << "Macro";
+ case HighlightingKind::Modifier:
+return OS << "Modifier";
case HighlightingKind::InactiveCode:
return OS << "InactiveCode";
}
@@ -1119,6 +1133,8 @@ llvm::StringRef toSemanticTokenType(HighlightingKind
Kind) {
return "type";
case HighlightingKind::Macro:
return "macro";
+ case HighlightingKind::Modifier:
+return "modifier";
case HighlightingKind::InactiveCode:
return "comment";
}
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.h
b/clang-tools-extra/clangd/SemanticHighlighting.h
index 64ad431909faa..e8f60c13000e1 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.h
+++ b/clang-tools-extra/clangd/SemanticHighlighting.h
@@ -49,6 +49,7 @@ enum class HighlightingKind {
Concept,
Primitive,
Macro,
+ Modifier,
// This one is
diff erent from the other kinds as it's a line style
// rather than a token style.
diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index 3ea4a58a83a70..70d8f91f129e4 100644
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -864,6 +864,12 @@ sizeof...($TemplateParameter[[Elements]]);
const char *$LocalVariable_def_readonly[[s]] =
$LocalVariable_readonly_static[[__func__]];
}
)cpp",
+ // override and final
+ R"cpp(
+class $Class_def_abstract[[Base]] { virtual void
$Method_decl_abstract_virtual[[m]]() = 0; };
+class $Class_def[[override]] : public $Class_abstract[[Base]] { void
$Method_decl_virtual[[m]]() $Modifier[[override]]; };
+class $Class_def[[final]] : public $Class[[override]] { void
$Method_decl_virtual[[m]]() $Modifier[[override]] $Modifier[[final]]; };
+ )cpp",
// Issue 1222: readonly modifier for generic parameter
R"cpp(
template
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] 2bf960a - [clangd] Add "usedAsMutablePointer" highlighting modifier
Author: Christian Kandeler
Date: 2022-11-07T11:58:33+01:00
New Revision: 2bf960aef08e93d687f21e6d636186561b56cbf3
URL:
https://github.com/llvm/llvm-project/commit/2bf960aef08e93d687f21e6d636186561b56cbf3
DIFF:
https://github.com/llvm/llvm-project/commit/2bf960aef08e93d687f21e6d636186561b56cbf3.diff
LOG: [clangd] Add "usedAsMutablePointer" highlighting modifier
Counterpart to "usedAsMutableReference". Just as for references, there
are const and non-const pointer parameters, and it's valuable to be able
to have different highlighting for the two cases at the call site.
We could have re-used the existing modifier, but having a dedicated one
maximizes client flexibility.
Reviewed By: nridge
Differential Revision: https://reviews.llvm.org/D130015
Added:
Modified:
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/SemanticHighlighting.h
clang-tools-extra/clangd/test/initialize-params.test
clang-tools-extra/clangd/test/semantic-tokens.test
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index af3a3e6f8e941..dd9392b029df8 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -597,19 +597,27 @@ class CollectExtraHighlightings
if (!Arg)
return;
-// Is this parameter passed by non-const reference?
+// Is this parameter passed by non-const pointer or reference?
// FIXME The condition T->idDependentType() could be relaxed a bit,
// e.g. std::vector& is dependent but we would want to highlight it
-if (!T->isLValueReferenceType() ||
-T.getNonReferenceType().isConstQualified() || T->isDependentType()) {
+bool IsRef = T->isLValueReferenceType();
+bool IsPtr = T->isPointerType();
+if ((!IsRef && !IsPtr) || T->getPointeeType().isConstQualified() ||
+T->isDependentType()) {
return;
}
llvm::Optional Location;
-// FIXME Add "unwrapping" for ArraySubscriptExpr and UnaryOperator,
+// FIXME Add "unwrapping" for ArraySubscriptExpr,
// e.g. highlight `a` in `a[i]`
// FIXME Handle dependent expression types
+if (auto *IC = dyn_cast(Arg))
+ Arg = IC->getSubExprAsWritten();
+if (auto *UO = dyn_cast(Arg)) {
+ if (UO->getOpcode() == UO_AddrOf)
+Arg = UO->getSubExpr();
+}
if (auto *DR = dyn_cast(Arg))
Location = DR->getLocation();
else if (auto *M = dyn_cast(Arg))
@@ -617,7 +625,8 @@ class CollectExtraHighlightings
if (Location)
H.addExtraModifier(*Location,
- HighlightingModifier::UsedAsMutableReference);
+ IsRef ? HighlightingModifier::UsedAsMutableReference
+ : HighlightingModifier::UsedAsMutablePointer);
}
void
@@ -1140,6 +1149,8 @@ llvm::StringRef
toSemanticTokenModifier(HighlightingModifier Modifier) {
return "defaultLibrary";
case HighlightingModifier::UsedAsMutableReference:
return "usedAsMutableReference"; // nonstandard
+ case HighlightingModifier::UsedAsMutablePointer:
+return "usedAsMutablePointer"; // nonstandard
case HighlightingModifier::ConstructorOrDestructor:
return "constructorOrDestructor"; // nonstandard
case HighlightingModifier::FunctionScope:
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.h
b/clang-tools-extra/clangd/SemanticHighlighting.h
index 79ecb344275d1..64ad431909faa 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.h
+++ b/clang-tools-extra/clangd/SemanticHighlighting.h
@@ -71,6 +71,7 @@ enum class HighlightingModifier {
DependentName,
DefaultLibrary,
UsedAsMutableReference,
+ UsedAsMutablePointer,
ConstructorOrDestructor,
FunctionScope,
diff --git a/clang-tools-extra/clangd/test/initialize-params.test
b/clang-tools-extra/clangd/test/initialize-params.test
index eb958cac20279..a2df61ca75235 100644
--- a/clang-tools-extra/clangd/test/initialize-params.test
+++ b/clang-tools-extra/clangd/test/initialize-params.test
@@ -68,6 +68,7 @@
# CHECK-NEXT:"dependentName",
# CHECK-NEXT:"defaultLibrary",
# CHECK-NEXT:"usedAsMutableReference",
+# CHECK-NEXT:"usedAsMutablePointer",
# CHECK-NEXT:"constructorOrDestructor",
# CHECK-NEXT:"functionScope",
# CHECK-NEXT:"classScope",
diff --git a/clang-tools-extra/clangd/test/semantic-tokens.test
b/clang-tools-extra/clangd/test/semantic-tokens.test
index 5abe78e9a51e1..b3a92b7cc737b 100644
--- a/clang-tools-extra/clangd/test/semantic-tokens.test
+++ b/clang-tools-extra/clangd/test/semantic-tokens.test
@@ -23,7 +23,7 @@
# CHECK-NEXT: 4,
# CHECK-NEXT: 1,
# CHECK-NEXT: 0,
-# CHECK-NEXT: 32771
[clang-tools-extra] 6ed4a54 - [clangd] Make file limit for textDocument/rename configurable
Author: Christian Kandeler
Date: 2022-10-22T10:17:41+02:00
New Revision: 6ed4a543b8b3ab38ddb30cf4c15b70ed11266388
URL:
https://github.com/llvm/llvm-project/commit/6ed4a543b8b3ab38ddb30cf4c15b70ed11266388
DIFF:
https://github.com/llvm/llvm-project/commit/6ed4a543b8b3ab38ddb30cf4c15b70ed11266388.diff
LOG: [clangd] Make file limit for textDocument/rename configurable
Without this, clients are unable to rename often-used symbols in larger
projects.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D136454
Added:
Modified:
clang-tools-extra/clangd/tool/ClangdMain.cpp
Removed:
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 68c0a51a2d74d..53b6653c51f20 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -327,6 +327,14 @@ opt ReferencesLimit{
init(1000),
};
+opt RenameFileLimit{
+"rename-file-limit",
+cat(Features),
+desc("Limit the number of files to be affected by symbol renaming. "
+ "0 means no limit (default=50)"),
+init(50),
+};
+
list TweakList{
"tweaks",
cat(Features),
@@ -891,6 +899,7 @@ clangd accepts flags on the commandline, and in the
CLANGD_FLAGS environment var
Opts.BackgroundIndex = EnableBackgroundIndex;
Opts.BackgroundIndexPriority = BackgroundIndexPriority;
Opts.ReferencesLimit = ReferencesLimit;
+ Opts.Rename.LimitFiles = RenameFileLimit;
auto PAI = createProjectAwareIndex(loadExternalIndex, Sync);
if (StaticIdx) {
IdxStack.emplace_back(std::move(StaticIdx));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] 8b36687 - [clangd] Add highlighting modifier "constructorOrDestructor"
Author: Christian Kandeler
Date: 2022-10-21T15:14:38+02:00
New Revision: 8b3668754c889a9412a76035235b6fc581ca9863
URL:
https://github.com/llvm/llvm-project/commit/8b3668754c889a9412a76035235b6fc581ca9863
DIFF:
https://github.com/llvm/llvm-project/commit/8b3668754c889a9412a76035235b6fc581ca9863.diff
LOG: [clangd] Add highlighting modifier "constructorOrDestructor"
This is useful for clients that want to highlight constructors and
destructors different from classes, e.g. like functions.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D134728
Added:
Modified:
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/SemanticHighlighting.h
clang-tools-extra/clangd/test/initialize-params.test
clang-tools-extra/clangd/test/semantic-tokens.test
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 1a6fa528acced..08cca2203667b 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -649,6 +649,29 @@ class CollectExtraHighlightings
return true;
}
+ bool VisitCXXDestructorDecl(CXXDestructorDecl *D) {
+if (auto *TI = D->getNameInfo().getNamedTypeInfo()) {
+ SourceLocation Loc = TI->getTypeLoc().getBeginLoc();
+ H.addExtraModifier(Loc, HighlightingModifier::ConstructorOrDestructor);
+ H.addExtraModifier(Loc, HighlightingModifier::Declaration);
+ if (D->isThisDeclarationADefinition())
+H.addExtraModifier(Loc, HighlightingModifier::Definition);
+}
+return true;
+ }
+
+ bool VisitCXXMemberCallExpr(CXXMemberCallExpr *CE) {
+if (isa(CE->getMethodDecl())) {
+ if (auto *ME = dyn_cast(CE->getCallee())) {
+if (auto *TI = ME->getMemberNameInfo().getNamedTypeInfo()) {
+ H.addExtraModifier(TI->getTypeLoc().getBeginLoc(),
+ HighlightingModifier::ConstructorOrDestructor);
+}
+ }
+}
+return true;
+ }
+
bool VisitDeclaratorDecl(DeclaratorDecl *D) {
auto *AT = D->getType()->getContainedAutoType();
if (!AT)
@@ -879,6 +902,8 @@ std::vector
getSemanticHighlightings(ParsedAST ) {
Tok.addModifier(HighlightingModifier::DefaultLibrary);
if (Decl->isDeprecated())
Tok.addModifier(HighlightingModifier::Deprecated);
+ if (isa(Decl))
+Tok.addModifier(HighlightingModifier::ConstructorOrDestructor);
if (R.IsDecl) {
// Do not treat an UnresolvedUsingValueDecl as a declaration.
// It's more common to think of it as a reference to the
@@ -960,6 +985,8 @@ llvm::raw_ostream <<(llvm::raw_ostream ,
HighlightingModifier K) {
return OS << "decl"; // abbreviation for common case
case HighlightingModifier::Definition:
return OS << "def"; // abbrevation for common case
+ case HighlightingModifier::ConstructorOrDestructor:
+return OS << "constrDestr";
default:
return OS << toSemanticTokenModifier(K);
}
@@ -,6 +1138,8 @@ llvm::StringRef
toSemanticTokenModifier(HighlightingModifier Modifier) {
return "defaultLibrary";
case HighlightingModifier::UsedAsMutableReference:
return "usedAsMutableReference"; // nonstandard
+ case HighlightingModifier::ConstructorOrDestructor:
+return "constructorOrDestructor"; // nonstandard
case HighlightingModifier::FunctionScope:
return "functionScope"; // nonstandard
case HighlightingModifier::ClassScope:
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.h
b/clang-tools-extra/clangd/SemanticHighlighting.h
index 4fe2fc7aee54d..79ecb344275d1 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.h
+++ b/clang-tools-extra/clangd/SemanticHighlighting.h
@@ -71,6 +71,7 @@ enum class HighlightingModifier {
DependentName,
DefaultLibrary,
UsedAsMutableReference,
+ ConstructorOrDestructor,
FunctionScope,
ClassScope,
diff --git a/clang-tools-extra/clangd/test/initialize-params.test
b/clang-tools-extra/clangd/test/initialize-params.test
index 031be9e147f74..eb958cac20279 100644
--- a/clang-tools-extra/clangd/test/initialize-params.test
+++ b/clang-tools-extra/clangd/test/initialize-params.test
@@ -68,6 +68,7 @@
# CHECK-NEXT:"dependentName",
# CHECK-NEXT:"defaultLibrary",
# CHECK-NEXT:"usedAsMutableReference",
+# CHECK-NEXT:"constructorOrDestructor",
# CHECK-NEXT:"functionScope",
# CHECK-NEXT:"classScope",
# CHECK-NEXT:"fileScope",
diff --git a/clang-tools-extra/clangd/test/semantic-tokens.test
b/clang-tools-extra/clangd/test/semantic-tokens.test
index b85d720ff5137..5abe78e9a51e1 100644
--- a/clang-tools-extra/clangd/test/semantic-tokens.test
+++
