[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 &R = 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 &Sel) {
+// 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-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)
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 &TokBuf,
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 &TokBuf,
const HeuristicResolver *Resolver) {
auto &AST = FD->getASTContext();
auto &SM = 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 &LangOpts) {
- 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 &Sel) 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());
+
// Bai
[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)
@@ -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:
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)
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 &TokBuf,
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 &TokBuf,
const HeuristicResolver *Resolver) {
auto &AST = FD->getASTContext();
auto &SM = 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 &LangOpts) {
- 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 &Sel) 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());
+
//
[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] 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 &OuterImplicit = 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 = [
[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)
@@ -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)
@@ -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)
@@ -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 = [[ [&args...]() {} ]];
-}
- )cpp",
- R"cpp(
-template
-void foo(Ts ...args) {
- auto placeholder = [&args...]() {}; 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)
@@ -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)
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] 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 &Params,
ReferenceParams &R,
}
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 &I) {
___
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 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 &CD, StringRef Prefix);
+ llvm::Error scopifyEnumValue(const EnumConstantDecl &CD, StringRef EnumName,
+ bool StripPrefix);
llvm::Expected getContentForFile(StringRef FilePath);
unsigned getOffsetFromPosition(const Position &Pos, StringRef Content) const;
llvm::Error addReplacementForReference(const ReferencesResult::Reference
&Ref,
@@ -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 &CD,
- StringRef Prefix) {
+ StringRef EnumName,
+ bool StripPrefix) {
for (const auto &Ref :
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 = [&EnumName](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 = [&Prefix](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 &CD,
}
return false;
};
+ if (StripPrefix) {
+if (IsAlreadyScoped())
+ return tooling::Replacement(FilePath, O
[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 &CD, StringRef Prefix);
+ llvm::Error scopifyEnumValue(const EnumConstantDecl &CD, StringRef EnumName,
+ bool StripPrefix);
llvm::Expected getContentForFile(StringRef FilePath);
unsigned getOffsetFromPosition(const Position &Pos, StringRef Content) const;
llvm::Error addReplacementForReference(const ReferencesResult::Reference
&Ref,
@@ -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 &CD,
- StringRef Prefix) {
+ StringRef EnumName,
+ bool StripPrefix) {
for (const auto &Ref :
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 = [&EnumName](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 = [&Prefix](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 &CD,
}
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()
+ :
[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] [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: 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] [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] [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] [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] [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 &ND, 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 &ND,
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(&getTemplateOrThis(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] [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-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 &AST) {
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 &operator<<(llvm::raw_ostream &OS,
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
[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] 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] 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 &addToken(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] 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] 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 &operator<<(llvm::raw_ostream &OS,
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] 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
&RenameDecl,
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] 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 &operator<<(llvm::raw_ostream &OS,
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] 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 &Token = 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 &Token = H.addToken(Op->getOperatorLoc(), HighlightingKind::Operator);
+if (Op->getSubExpr()->isTypeDependent())
+ Token.addModifier(HighlightingModifier::UserDefined);
+return true;
+ }
+
+ bool VisitBinaryOperator(BinaryOperator *Op) {
+auto &Token = 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 &Token = H.addToken(E->getBeginLoc(), HighlightingKind::Operator);
+if (isa(E->getOperatorNew()))
+ Token.addModifier(HighlightingModifier::UserDefined);
+return true;
+ }
+
+ bool VisitCXXDeleteExpr(CXXDeleteExpr *E) {
+auto &Token = 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(),
+ Highlight
[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-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:
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)
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] [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 80e15f748b30d4e977b58a42d8fd4403234bb954 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 | 42 +++
.../clangd/index/SymbolCollector.h| 3 +-
.../clangd/unittests/SymbolCollectorTests.cpp | 20 +
clang/lib/AST/ASTContext.cpp | 11 -
5 files changed, 69 insertions(+), 11 deletions(-)
diff --git a/clang-tools-extra/clangd/index/Symbol.h
b/clang-tools-extra/clangd/index/Symbol.h
index 1aa5265299231b..62c47ddfc5758d 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 5c4e2150cf3123..d3df6a26fd9365 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -635,17 +635,20 @@ bool SymbolCollector::handleDeclOccurrence(
return true;
const Symbol *BasicSymbol = Symbols.find(ID);
- if (isPreferredDeclaration(*OriginalDecl, Roles))
+ bool SkipDocCheckInDef = false;
+ if (isPreferredDeclaration(*OriginalDecl, Roles)) {
// If OriginalDecl is preferred, replace/create the existing canonical
// declaration (e.g. a class forward declaration). There should be at most
// one duplicate as we expect to see only one preferred declaration per
// TU, because in practice they are definitions.
BasicSymbol = addDeclaration(*OriginalDecl, std::move(ID), IsMainFileOnly);
- else if (!BasicSymbol || DeclIsCanonical)
+SkipDocCheckInDef = true;
+ } else if (!BasicSymbol || DeclIsCanonical) {
BasicSymbol = addDeclaration(*ND, std::move(ID), IsMainFileOnly);
+ }
if (Roles & static_cast(index::SymbolRole::Definition))
-addDefinition(*OriginalDecl, *BasicSymbol);
+addDefinition(*OriginalDecl, *BasicSymbol, SkipDocCheckInDef);
return true;
}
@@ -1025,15 +1028,17 @@ const Symbol *SymbolCollector::addDeclaration(const
NamedDecl &ND, 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;
@@ -1058,8 +1063,8 @@ const Symbol *SymbolCollector::addDeclaration(const
NamedDecl &ND, SymbolID ID,
return Symbols.find(S.ID);
}
-void SymbolCollector::addDefinition(const NamedDecl &ND,
-const Symbol &DeclSym) {
+void SymbolCollector::addDefinition(const NamedDecl &ND, const Symbol &DeclSym,
+bool SkipDocCheck) {
if (DeclSym.Definition)
return;
const auto &SM = ND.getASTContext().getSourceManager();
@@ -1074,6 +1079,27 @@ void SymbolCollector::addDefinition(const NamedDecl &ND,
Symbol S = DeclSym;
// FIXME: use the result to filter out symbols.
S.Definition = *DefLoc;
+
+ std::string DocComment;
+ std::string Documentation;
+ if (!SkipDocCheck && !(S.Flags & Symbol::HasDocComment) &&
+ (llvm::isa(ND) || llvm::isa(ND))) {
+CodeCompletionResult SymbolCompletion(&getTemplateOrThis(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.emp
[clang] [clang-tools-extra] [clangd] Collect comments from function definitions into the index (PR #67802)
ckandeler wrote: > At this point, my main feedback is that it would be good to include some unit > tests. `SymbolCollectorTests.cpp` is a good > place for them. I've added a new test and verified that it fails without this patch. > What is the purpose of the change to `ASTContext.cpp`? There was some assumption about the order of redeclarations that did not match reality, like that the definition always comes first or something along those lines. I do not know whether the assumption there was wrong or whether some other code messed up by breaking it. > This codepath has consumers other than clangd, so if we're changing the > behaviour, we need to evaluate whether it's > appropriate for other consumers (and maybe add libAST unit tests as well). Hence my original comment. Though it should be noted that no existing tests appear to have been broken. 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)
@@ -1069,6 +1071,27 @@ void SymbolCollector::addDefinition(const NamedDecl &ND, 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) && ckandeler wrote: Done. 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)
ckandeler wrote: > I wrote a libAST unit test to demonstrate the bug, and filed #108145 about > it. Hopefully folks more familiar with that code can suggest an appropriate > fix there. Awesome, thanks. 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)
@@ -1477,6 +1477,26 @@ TEST_F(SymbolCollectorTest, Documentation) {
forCodeCompletion(false;
}
+TEST_F(SymbolCollectorTest, DocumentationInMain) {
ckandeler wrote:
Hm, do we really want to specify a behavior there? My thinking was that the
comment should be at either the declaration or the definition and otherwise one
of them gets chosen at random, depending on the parse order. I suppose one
could also cook up some elaborated merging scheme, but that seems like overkill.
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)
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] 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 &Context, 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
+
[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] 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] 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 &CD, StringRef Prefix);
+ llvm::Error scopifyEnumValue(const EnumConstantDecl &CD, StringRef EnumName,
+ bool StripPrefix);
llvm::Expected getContentForFile(StringRef FilePath);
unsigned getOffsetFromPosition(const Position &Pos, StringRef Content) const;
llvm::Error addReplacementForReference(const ReferencesResult::Reference
&Ref,
@@ -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 &CD,
- StringRef Prefix) {
+ StringRef EnumName,
+ bool StripPrefix) {
for (const auto &Ref :
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 = [&EnumName](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 = [&Prefix](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 &CD,
}
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] 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] 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] 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] 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 &CD, StringRef Prefix);
+ llvm::Error scopifyEnumValue(const EnumConstantDecl &CD, StringRef EnumName,
+ bool StripPrefix);
llvm::Expected getContentForFile(StringRef FilePath);
unsigned getOffsetFromPosition(const Position &Pos, StringRef Content) const;
llvm::Error addReplacementForReference(const ReferencesResult::Reference
&Ref,
@@ -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 &CD,
- StringRef Prefix) {
+ StringRef EnumName,
+ bool StripPrefix) {
for (const auto &Ref :
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 = [&EnumName](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 = [&Prefix](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 &CD,
}
return false;
};
- return IsAlreadyScoped()
- ? tooling::Replacement()
- : tooling::Replacement(FilePath, Offset, 0, Prefix);
+ if (StripPrefix) {
+const int ExtraLength =
+Content[Offset + EnumName.size()] == '_' ? 1 : 0
[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 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] 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] 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 &Params,
+Callback> Reply) {
+ Server->outgoingCalls(Params.item, std::move(Reply));
+}
+
void ClangdLSPServer::onClangdInlayHints(const InlayHintsParams &Params,
Callback Reply) {
// Our extension has a different representation on the wire than the
standard.
@@ -1688,6 +1694,7 @@ void ClangdLSPServer::bindMethods(LSPBinder &Bind,
Bind.method("typeHierarchy/subtypes", this, &ClangdLSPServer::onSubTypes);
Bind.method("textDocument/prepareCallHierarchy", this,
&ClangdLSPServer::onPrepareCallHierarchy);
Bind.method("callHierarchy/incomingCalls", this,
&ClangdLSPServer::onCallHierarchyIncomingCalls);
+ Bind.method("callHierarchy/outgoingCalls", this,
&ClangdLSPServer::onCallHierarchyOutgoingCalls);
Bind.method("textDocument/selectionRange", this,
&ClangdLSPServer::onSelectionRange);
Bind.method("textDocument/documentLink", this,
&ClangdLSPServer::onDocumentLink);
Bind.method("textDocument/semanticTokens/full", this,
&ClangdLSPServer::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 &Item,
+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 &Item,
Callback>);
+ /// Resolve outgoing calls for a given call hierarchy item.
+ void outgoingCalls(const CallHierarchyItem &Item,
+ 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/
[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)
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)
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] Let DefineOutline tweak handle member functions (PR #95235)
@@ -144,8 +144,13 @@ getQualification(ASTContext &Context, 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())
ckandeler wrote:
The function doesn't look expensive, and this would complicate the code, so I'd
rather not.
https://github.com/llvm/llvm-project/pull/95235
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Let DefineOutline tweak handle member functions (PR #95235)
@@ -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()) {
ckandeler wrote:
Done, but note that auto is not generally used in this project the way you
prefer (with the type obvious from the rhs).
https://github.com/llvm/llvm-project/pull/95235
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Let DefineOutline tweak handle member functions (PR #95235)
https://github.com/ckandeler updated
https://github.com/llvm/llvm-project/pull/95235
>From de740c00c4cc3a9f2e8aff14cf8ebd880a240e58 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 | 50 +++--
.../unittests/tweaks/DefineOutlineTests.cpp | 55 ++-
clang-tools-extra/docs/ReleaseNotes.rst | 2 +
4 files changed, 94 insertions(+), 20 deletions(-)
diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp
index fda1e5fdf8d82c..2f2f8437d6ed9a 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -144,8 +144,13 @@ getQualification(ASTContext &Context, 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 f43f2417df8fce..591a8b245260ea 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -128,7 +128,28 @@ 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 (const TemplateParameterList *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 +233,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() &&
+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 +432,23 @@ 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 (const TemplateParameterList *Params =
+ Parent->getDescribedTemplateParams()) {
+
+// Class template member functions must be defined in the
+// same file.
+SameFile = true;
+
+// Bail out if the template parameter is unnamed.
+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 906ff33db87344..6a9e90c3bfa70f 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 with
[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 dec33143e617967dee46ce4123a2e298220d73fc 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 | 56 +++
.../clangd/index/SymbolCollector.h| 3 +-
.../clangd/unittests/SymbolCollectorTests.cpp | 52 +
clang/lib/AST/ASTContext.cpp | 11 +++-
5 files changed, 113 insertions(+), 13 deletions(-)
diff --git a/clang-tools-extra/clangd/index/Symbol.h
b/clang-tools-extra/clangd/index/Symbol.h
index 1aa5265299231b..62c47ddfc5758d 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 5c4e2150cf3123..066c7428ace555 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -635,17 +635,20 @@ bool SymbolCollector::handleDeclOccurrence(
return true;
const Symbol *BasicSymbol = Symbols.find(ID);
- if (isPreferredDeclaration(*OriginalDecl, Roles))
+ bool SkipDocCheckInDef = false;
+ if (isPreferredDeclaration(*OriginalDecl, Roles)) {
// If OriginalDecl is preferred, replace/create the existing canonical
// declaration (e.g. a class forward declaration). There should be at most
// one duplicate as we expect to see only one preferred declaration per
// TU, because in practice they are definitions.
BasicSymbol = addDeclaration(*OriginalDecl, std::move(ID), IsMainFileOnly);
- else if (!BasicSymbol || DeclIsCanonical)
+SkipDocCheckInDef = true;
+ } else if (!BasicSymbol || DeclIsCanonical) {
BasicSymbol = addDeclaration(*ND, std::move(ID), IsMainFileOnly);
+ }
if (Roles & static_cast(index::SymbolRole::Definition))
-addDefinition(*OriginalDecl, *BasicSymbol);
+addDefinition(*OriginalDecl, *BasicSymbol, SkipDocCheckInDef);
return true;
}
@@ -1025,16 +1028,28 @@ const Symbol *SymbolCollector::addDeclaration(const
NamedDecl &ND, SymbolID ID,
*ASTCtx, *PP, CodeCompletionContext::CCC_Symbol, *CompletionAllocator,
*CompletionTUInfo,
/*IncludeBriefComments*/ false);
- std::string Documentation =
- formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion,
- /*CommentsFromHeaders=*/true));
+ std::string DocComment;
+ std::string Documentation;
+ bool AlreadyHasDoc = S.Flags & Symbol::HasDocComment;
+ if (!AlreadyHasDoc) {
+DocComment = getDocComment(Ctx, SymbolCompletion,
+ /*CommentsFromHeaders=*/true);
+Documentation = formatDocumentation(*CCS, DocComment);
+ }
+ const auto UpdateDoc = [&] {
+if (!AlreadyHasDoc) {
+ if (!DocComment.empty())
+S.Flags |= Symbol::HasDocComment;
+ S.Documentation = Documentation;
+}
+ };
if (!(S.Flags & Symbol::IndexedForCodeCompletion)) {
if (Opts.StoreAllDocumentation)
- S.Documentation = Documentation;
+ UpdateDoc();
Symbols.insert(S);
return Symbols.find(S.ID);
}
- S.Documentation = Documentation;
+ UpdateDoc();
std::string Signature;
std::string SnippetSuffix;
getSignature(*CCS, &Signature, &SnippetSuffix, SymbolCompletion.Kind,
@@ -1058,8 +1073,8 @@ const Symbol *SymbolCollector::addDeclaration(const
NamedDecl &ND, SymbolID ID,
return Symbols.find(S.ID);
}
-void SymbolCollector::addDefinition(const NamedDecl &ND,
-const Symbol &DeclSym) {
+void SymbolCollector::addDefinition(const NamedDecl &ND, const Symbol &DeclSym,
+bool SkipDocCheck) {
if (DeclSym.Definition)
return;
const auto &SM = ND.getASTContext().getSourceManager();
@@ -1074,6 +1089,27 @@ void SymbolCollector::addDefinition(const NamedDecl &ND,
Symbol S = DeclSym;
// FIXME: use the result to filter out symbols.
S.Definition = *DefLoc;
+
+ std::string DocComment;
+ std::string Documentation;
+ if (!SkipDocCheck && !(S.Flags & Symbol::HasDocComment) &&
+ (llvm::isa(ND) || llvm::isa(ND))) {
+CodeCompletionResult SymbolCompletion(&getTemplateOrThis(ND), 0);
+const auto *CCS = Symbol
[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 f441c0bd52add3d5b63bc0f19a3d38cb12477359 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 | 57 +++
.../clangd/index/SymbolCollector.h| 3 +-
.../clangd/unittests/SymbolCollectorTests.cpp | 52 +
clang/lib/AST/ASTContext.cpp | 11 +++-
5 files changed, 114 insertions(+), 13 deletions(-)
diff --git a/clang-tools-extra/clangd/index/Symbol.h
b/clang-tools-extra/clangd/index/Symbol.h
index 1aa5265299231b..62c47ddfc5758d 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 5c4e2150cf3123..a76894cf0855f3 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -635,17 +635,21 @@ bool SymbolCollector::handleDeclOccurrence(
return true;
const Symbol *BasicSymbol = Symbols.find(ID);
- if (isPreferredDeclaration(*OriginalDecl, Roles))
+ bool SkipDocCheckInDef = false;
+ if (isPreferredDeclaration(*OriginalDecl, Roles)) {
// If OriginalDecl is preferred, replace/create the existing canonical
// declaration (e.g. a class forward declaration). There should be at most
// one duplicate as we expect to see only one preferred declaration per
// TU, because in practice they are definitions.
BasicSymbol = addDeclaration(*OriginalDecl, std::move(ID), IsMainFileOnly);
- else if (!BasicSymbol || DeclIsCanonical)
+SkipDocCheckInDef = true;
+ } else if (!BasicSymbol || DeclIsCanonical) {
BasicSymbol = addDeclaration(*ND, std::move(ID), IsMainFileOnly);
+SkipDocCheckInDef = true;
+ }
if (Roles & static_cast(index::SymbolRole::Definition))
-addDefinition(*OriginalDecl, *BasicSymbol);
+addDefinition(*OriginalDecl, *BasicSymbol, SkipDocCheckInDef);
return true;
}
@@ -1025,16 +1029,28 @@ const Symbol *SymbolCollector::addDeclaration(const
NamedDecl &ND, SymbolID ID,
*ASTCtx, *PP, CodeCompletionContext::CCC_Symbol, *CompletionAllocator,
*CompletionTUInfo,
/*IncludeBriefComments*/ false);
- std::string Documentation =
- formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion,
- /*CommentsFromHeaders=*/true));
+ std::string DocComment;
+ std::string Documentation;
+ bool AlreadyHasDoc = S.Flags & Symbol::HasDocComment;
+ if (!AlreadyHasDoc) {
+DocComment = getDocComment(Ctx, SymbolCompletion,
+ /*CommentsFromHeaders=*/true);
+Documentation = formatDocumentation(*CCS, DocComment);
+ }
+ const auto UpdateDoc = [&] {
+if (!AlreadyHasDoc) {
+ if (!DocComment.empty())
+S.Flags |= Symbol::HasDocComment;
+ S.Documentation = Documentation;
+}
+ };
if (!(S.Flags & Symbol::IndexedForCodeCompletion)) {
if (Opts.StoreAllDocumentation)
- S.Documentation = Documentation;
+ UpdateDoc();
Symbols.insert(S);
return Symbols.find(S.ID);
}
- S.Documentation = Documentation;
+ UpdateDoc();
std::string Signature;
std::string SnippetSuffix;
getSignature(*CCS, &Signature, &SnippetSuffix, SymbolCompletion.Kind,
@@ -1058,8 +1074,8 @@ const Symbol *SymbolCollector::addDeclaration(const
NamedDecl &ND, SymbolID ID,
return Symbols.find(S.ID);
}
-void SymbolCollector::addDefinition(const NamedDecl &ND,
-const Symbol &DeclSym) {
+void SymbolCollector::addDefinition(const NamedDecl &ND, const Symbol &DeclSym,
+bool SkipDocCheck) {
if (DeclSym.Definition)
return;
const auto &SM = ND.getASTContext().getSourceManager();
@@ -1074,6 +1090,27 @@ void SymbolCollector::addDefinition(const NamedDecl &ND,
Symbol S = DeclSym;
// FIXME: use the result to filter out symbols.
S.Definition = *DefLoc;
+
+ std::string DocComment;
+ std::string Documentation;
+ if (!SkipDocCheck && !(S.Flags & Symbol::HasDocComment) &&
+ (llvm::isa(ND) || llvm::isa(ND))) {
+CodeCompletionResult SymbolCompletion(&getTemplateOrThis(ND), 0)
[clang] [clang-tools-extra] [clangd] Collect comments from function definitions into the index (PR #67802)
ckandeler wrote: > Meanwhile, I thought of a fix for #108145 and submitted it as #108475. > If that's accepted, you should be able to drop the `ASTContext.cpp` changes > from this patch. Nice, will do. 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] Let DefineOutline tweak handle member functions (PR #95235)
https://github.com/ckandeler closed https://github.com/llvm/llvm-project/pull/95235 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Let DefineOutline tweak handle member functions (PR #95235)
ckandeler wrote: Thanks! https://github.com/llvm/llvm-project/pull/95235 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Let DefineOutline tweak handle member function templates (PR #112345)
https://github.com/ckandeler created
https://github.com/llvm/llvm-project/pull/112345
None
>From f2b6d67d882fca9a1dae3b50166541df7f22cb44 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Mon, 14 Oct 2024 15:37:01 +0200
Subject: [PATCH] [clangd] Let DefineOutline tweak handle member function
templates
---
.../clangd/refactor/tweaks/DefineOutline.cpp | 69 ++-
.../unittests/tweaks/DefineOutlineTests.cpp | 51 --
2 files changed, 94 insertions(+), 26 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index 591a8b245260ea..14d9858b702a1d 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -109,7 +109,8 @@ findContextForNS(llvm::StringRef TargetNS, const
DeclContext *CurContext) {
// afterwards it can be shared with define-inline code action.
llvm::Expected
getFunctionSourceAfterReplacements(const FunctionDecl *FD,
- const tooling::Replacements &Replacements) {
+ const tooling::Replacements &Replacements,
+ bool TargetFileIsHeader) {
const auto &SM = FD->getASTContext().getSourceManager();
auto OrigFuncRange = toHalfOpenFileRange(
SM, FD->getASTContext().getLangOpts(), FD->getSourceRange());
@@ -130,23 +131,41 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD,
return QualifiedFunc.takeError();
std::string TemplatePrefix;
+ auto AddToTemplatePrefixIfApplicable = [&](const Decl *D, bool append) {
+const TemplateParameterList *Params = D->getDescribedTemplateParams();
+if (!Params)
+ return;
+for (Decl *P : *Params) {
+ if (auto *TTP = dyn_cast(P)) {
+TTP->removeDefaultArgument();
+ } else if (auto *NTTP = dyn_cast(P)) {
+NTTP->removeDefaultArgument();
+ } else if (auto * TTPD = dyn_cast(P)) {
+TTPD->removeDefaultArgument();
+ }
+}
+std::string S;
+llvm::raw_string_ostream Stream(S);
+Params->print(Stream, FD->getASTContext());
+if (!S.empty())
+ *S.rbegin() = '\n'; // Replace space with newline
+if (append)
+ TemplatePrefix.append(S);
+else
+ TemplatePrefix.insert(0, S);
+ };
if (auto *MD = llvm::dyn_cast(FD)) {
for (const CXXRecordDecl *Parent = MD->getParent(); Parent;
Parent =
llvm::dyn_cast_or_null(Parent->getParent()))
{
- if (const TemplateParameterList *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);
- }
+ AddToTemplatePrefixIfApplicable(Parent, false);
}
}
-
+
+ AddToTemplatePrefixIfApplicable(FD, true);
auto Source = QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
+ if (TargetFileIsHeader)
+Source.insert(0, "inline ");
if (!TemplatePrefix.empty())
Source.insert(0, TemplatePrefix);
return Source;
@@ -202,7 +221,8 @@ deleteTokensWithKind(const syntax::TokenBuffer &TokBuf,
tok::TokenKind Kind,
llvm::Expected
getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext,
const syntax::TokenBuffer &TokBuf,
- const HeuristicResolver *Resolver) {
+ const HeuristicResolver *Resolver,
+ bool targetFileIsHeader) {
auto &AST = FD->getASTContext();
auto &SM = AST.getSourceManager();
@@ -337,7 +357,7 @@ getFunctionSourceCode(const FunctionDecl *FD, const
DeclContext *TargetContext,
if (Errors)
return std::move(Errors);
- return getFunctionSourceAfterReplacements(FD, DeclarationCleanups);
+ return getFunctionSourceAfterReplacements(FD, DeclarationCleanups,
targetFileIsHeader);
}
struct InsertionPoint {
@@ -419,15 +439,15 @@ class DefineOutline : public Tweak {
Source->isOutOfLine())
return false;
-// Bail out if this is a function template or specialization, as their
+// Bail out if this is a function template specialization, as their
// definitions need to be visible in all including translation units.
-if (Source->getDescribedFunctionTemplate())
- return false;
if (Source->getTemplateSpecializationInfo())
return false;
auto *MD = llvm::dyn_cast(Source);
if (!MD) {
+ if (Source->getDescribedFunctionTemplate())
+return false;
// Can't outline free-standing functions in the same file.
return !SameFile;
}
@@ -443,13 +463,24 @@ class DefineOutline : public Tweak {
SameFile = true;
// Bail out if the template parameter is unnamed.
+// FIXME: Is this really ne
[clang-tools-extra] [clangd] Let DefineOutline tweak handle member function templates (PR #112345)
https://github.com/ckandeler updated
https://github.com/llvm/llvm-project/pull/112345
>From 0d847ccd3b26f2e0ef1203c091d6152d6abea58a Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Mon, 14 Oct 2024 15:37:01 +0200
Subject: [PATCH] [clangd] Let DefineOutline tweak handle member function
templates
---
.../clangd/refactor/tweaks/DefineOutline.cpp | 67 ++-
.../unittests/tweaks/DefineOutlineTests.cpp | 49 --
2 files changed, 94 insertions(+), 22 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index 591a8b245260ea..a552114da1054d 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -109,7 +109,8 @@ findContextForNS(llvm::StringRef TargetNS, const
DeclContext *CurContext) {
// afterwards it can be shared with define-inline code action.
llvm::Expected
getFunctionSourceAfterReplacements(const FunctionDecl *FD,
- const tooling::Replacements &Replacements) {
+ const tooling::Replacements &Replacements,
+ bool TargetFileIsHeader) {
const auto &SM = FD->getASTContext().getSourceManager();
auto OrigFuncRange = toHalfOpenFileRange(
SM, FD->getASTContext().getLangOpts(), FD->getSourceRange());
@@ -130,23 +131,41 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD,
return QualifiedFunc.takeError();
std::string TemplatePrefix;
+ auto AddToTemplatePrefixIfApplicable = [&](const Decl *D, bool append) {
+const TemplateParameterList *Params = D->getDescribedTemplateParams();
+if (!Params)
+ return;
+for (Decl *P : *Params) {
+ if (auto *TTP = dyn_cast(P)) {
+TTP->removeDefaultArgument();
+ } else if (auto *NTTP = dyn_cast(P)) {
+NTTP->removeDefaultArgument();
+ } else if (auto *TTPD = dyn_cast(P)) {
+TTPD->removeDefaultArgument();
+ }
+}
+std::string S;
+llvm::raw_string_ostream Stream(S);
+Params->print(Stream, FD->getASTContext());
+if (!S.empty())
+ *S.rbegin() = '\n'; // Replace space with newline
+if (append)
+ TemplatePrefix.append(S);
+else
+ TemplatePrefix.insert(0, S);
+ };
if (auto *MD = llvm::dyn_cast(FD)) {
for (const CXXRecordDecl *Parent = MD->getParent(); Parent;
Parent =
llvm::dyn_cast_or_null(Parent->getParent()))
{
- if (const TemplateParameterList *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);
- }
+ AddToTemplatePrefixIfApplicable(Parent, false);
}
}
+ AddToTemplatePrefixIfApplicable(FD, true);
auto Source = QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
+ if (TargetFileIsHeader)
+Source.insert(0, "inline ");
if (!TemplatePrefix.empty())
Source.insert(0, TemplatePrefix);
return Source;
@@ -202,7 +221,8 @@ deleteTokensWithKind(const syntax::TokenBuffer &TokBuf,
tok::TokenKind Kind,
llvm::Expected
getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext,
const syntax::TokenBuffer &TokBuf,
- const HeuristicResolver *Resolver) {
+ const HeuristicResolver *Resolver,
+ bool targetFileIsHeader) {
auto &AST = FD->getASTContext();
auto &SM = AST.getSourceManager();
@@ -337,7 +357,8 @@ getFunctionSourceCode(const FunctionDecl *FD, const
DeclContext *TargetContext,
if (Errors)
return std::move(Errors);
- return getFunctionSourceAfterReplacements(FD, DeclarationCleanups);
+ return getFunctionSourceAfterReplacements(FD, DeclarationCleanups,
+targetFileIsHeader);
}
struct InsertionPoint {
@@ -419,15 +440,15 @@ class DefineOutline : public Tweak {
Source->isOutOfLine())
return false;
-// Bail out if this is a function template or specialization, as their
+// Bail out if this is a function template specialization, as their
// definitions need to be visible in all including translation units.
-if (Source->getDescribedFunctionTemplate())
- return false;
if (Source->getTemplateSpecializationInfo())
return false;
auto *MD = llvm::dyn_cast(Source);
if (!MD) {
+ if (Source->getDescribedFunctionTemplate())
+return false;
// Can't outline free-standing functions in the same file.
return !SameFile;
}
@@ -443,6 +464,8 @@ class DefineOutline : public Tweak {
SameFile = true;
// Bail out if the template parameter is unnamed.
+
[clang-tools-extra] [clangd] Let DefineOutline tweak handle member function templates (PR #112345)
https://github.com/ckandeler updated
https://github.com/llvm/llvm-project/pull/112345
>From 5b729d0bc7b2d5ed2f0d1645a29e3fb9b0a35033 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Mon, 14 Oct 2024 15:37:01 +0200
Subject: [PATCH] [clangd] Let DefineOutline tweak handle member function
templates
---
.../clangd/refactor/tweaks/DefineOutline.cpp | 66 ++-
.../unittests/tweaks/DefineOutlineTests.cpp | 49 --
2 files changed, 93 insertions(+), 22 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index 591a8b245260ea..c2a517e644c8aa 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -109,7 +109,8 @@ findContextForNS(llvm::StringRef TargetNS, const
DeclContext *CurContext) {
// afterwards it can be shared with define-inline code action.
llvm::Expected
getFunctionSourceAfterReplacements(const FunctionDecl *FD,
- const tooling::Replacements &Replacements) {
+ const tooling::Replacements &Replacements,
+ bool TargetFileIsHeader) {
const auto &SM = FD->getASTContext().getSourceManager();
auto OrigFuncRange = toHalfOpenFileRange(
SM, FD->getASTContext().getLangOpts(), FD->getSourceRange());
@@ -130,23 +131,40 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD,
return QualifiedFunc.takeError();
std::string TemplatePrefix;
+ auto AddToTemplatePrefixIfApplicable = [&](const Decl *D, bool append) {
+const TemplateParameterList *Params = D->getDescribedTemplateParams();
+if (!Params)
+ return;
+for (Decl *P : *Params) {
+ if (auto *TTP = dyn_cast(P))
+TTP->removeDefaultArgument();
+ else if (auto *NTTP = dyn_cast(P))
+NTTP->removeDefaultArgument();
+ else if (auto *TTPD = dyn_cast(P))
+TTPD->removeDefaultArgument();
+}
+std::string S;
+llvm::raw_string_ostream Stream(S);
+Params->print(Stream, FD->getASTContext());
+if (!S.empty())
+ *S.rbegin() = '\n'; // Replace space with newline
+if (append)
+ TemplatePrefix.append(S);
+else
+ TemplatePrefix.insert(0, S);
+ };
if (auto *MD = llvm::dyn_cast(FD)) {
for (const CXXRecordDecl *Parent = MD->getParent(); Parent;
Parent =
llvm::dyn_cast_or_null(Parent->getParent()))
{
- if (const TemplateParameterList *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);
- }
+ AddToTemplatePrefixIfApplicable(Parent, false);
}
}
+ AddToTemplatePrefixIfApplicable(FD, true);
auto Source = QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
+ if (TargetFileIsHeader)
+Source.insert(0, "inline ");
if (!TemplatePrefix.empty())
Source.insert(0, TemplatePrefix);
return Source;
@@ -202,7 +220,8 @@ deleteTokensWithKind(const syntax::TokenBuffer &TokBuf,
tok::TokenKind Kind,
llvm::Expected
getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext,
const syntax::TokenBuffer &TokBuf,
- const HeuristicResolver *Resolver) {
+ const HeuristicResolver *Resolver,
+ bool targetFileIsHeader) {
auto &AST = FD->getASTContext();
auto &SM = AST.getSourceManager();
@@ -337,7 +356,8 @@ getFunctionSourceCode(const FunctionDecl *FD, const
DeclContext *TargetContext,
if (Errors)
return std::move(Errors);
- return getFunctionSourceAfterReplacements(FD, DeclarationCleanups);
+ return getFunctionSourceAfterReplacements(FD, DeclarationCleanups,
+targetFileIsHeader);
}
struct InsertionPoint {
@@ -419,15 +439,15 @@ class DefineOutline : public Tweak {
Source->isOutOfLine())
return false;
-// Bail out if this is a function template or specialization, as their
+// Bail out if this is a function template specialization, as their
// definitions need to be visible in all including translation units.
-if (Source->getDescribedFunctionTemplate())
- return false;
if (Source->getTemplateSpecializationInfo())
return false;
auto *MD = llvm::dyn_cast(Source);
if (!MD) {
+ if (Source->getDescribedFunctionTemplate())
+return false;
// Can't outline free-standing functions in the same file.
return !SameFile;
}
@@ -443,6 +463,8 @@ class DefineOutline : public Tweak {
SameFile = true;
// Bail out if the template parameter is unnamed.
+// FIXME: Is th
[clang-tools-extra] [clangd] Let DefineOutline tweak handle member function templates (PR #112345)
https://github.com/ckandeler updated
https://github.com/llvm/llvm-project/pull/112345
>From 42ea89f38b599796b15d50d078cba1cb0e0f2b02 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Mon, 14 Oct 2024 15:37:01 +0200
Subject: [PATCH] [clangd] Let DefineOutline tweak handle member function
templates
---
.../clangd/refactor/tweaks/DefineOutline.cpp | 68 ++-
.../unittests/tweaks/DefineOutlineTests.cpp | 49 +++--
2 files changed, 93 insertions(+), 24 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index 591a8b245260ea..56f3ccdf493419 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -109,14 +109,13 @@ findContextForNS(llvm::StringRef TargetNS, const
DeclContext *CurContext) {
// afterwards it can be shared with define-inline code action.
llvm::Expected
getFunctionSourceAfterReplacements(const FunctionDecl *FD,
- const tooling::Replacements &Replacements) {
+ const tooling::Replacements &Replacements,
+ bool TargetFileIsHeader) {
const auto &SM = FD->getASTContext().getSourceManager();
auto OrigFuncRange = toHalfOpenFileRange(
SM, FD->getASTContext().getLangOpts(), FD->getSourceRange());
if (!OrigFuncRange)
return error("Couldn't get range for function.");
- assert(!FD->getDescribedFunctionTemplate() &&
- "Define out-of-line doesn't apply to function templates.");
// Get new begin and end positions for the qualified function definition.
unsigned FuncBegin = SM.getFileOffset(OrigFuncRange->getBegin());
@@ -130,23 +129,40 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD,
return QualifiedFunc.takeError();
std::string TemplatePrefix;
+ auto AddToTemplatePrefixIfApplicable = [&](const Decl *D, bool append) {
+const TemplateParameterList *Params = D->getDescribedTemplateParams();
+if (!Params)
+ return;
+for (Decl *P : *Params) {
+ if (auto *TTP = dyn_cast(P))
+TTP->removeDefaultArgument();
+ else if (auto *NTTP = dyn_cast(P))
+NTTP->removeDefaultArgument();
+ else if (auto *TTPD = dyn_cast(P))
+TTPD->removeDefaultArgument();
+}
+std::string S;
+llvm::raw_string_ostream Stream(S);
+Params->print(Stream, FD->getASTContext());
+if (!S.empty())
+ *S.rbegin() = '\n'; // Replace space with newline
+if (append)
+ TemplatePrefix.append(S);
+else
+ TemplatePrefix.insert(0, S);
+ };
if (auto *MD = llvm::dyn_cast(FD)) {
for (const CXXRecordDecl *Parent = MD->getParent(); Parent;
Parent =
llvm::dyn_cast_or_null(Parent->getParent()))
{
- if (const TemplateParameterList *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);
- }
+ AddToTemplatePrefixIfApplicable(Parent, false);
}
}
+ AddToTemplatePrefixIfApplicable(FD, true);
auto Source = QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
+ if (TargetFileIsHeader)
+Source.insert(0, "inline ");
if (!TemplatePrefix.empty())
Source.insert(0, TemplatePrefix);
return Source;
@@ -202,7 +218,8 @@ deleteTokensWithKind(const syntax::TokenBuffer &TokBuf,
tok::TokenKind Kind,
llvm::Expected
getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext,
const syntax::TokenBuffer &TokBuf,
- const HeuristicResolver *Resolver) {
+ const HeuristicResolver *Resolver,
+ bool targetFileIsHeader) {
auto &AST = FD->getASTContext();
auto &SM = AST.getSourceManager();
@@ -337,7 +354,8 @@ getFunctionSourceCode(const FunctionDecl *FD, const
DeclContext *TargetContext,
if (Errors)
return std::move(Errors);
- return getFunctionSourceAfterReplacements(FD, DeclarationCleanups);
+ return getFunctionSourceAfterReplacements(FD, DeclarationCleanups,
+targetFileIsHeader);
}
struct InsertionPoint {
@@ -419,15 +437,15 @@ class DefineOutline : public Tweak {
Source->isOutOfLine())
return false;
-// Bail out if this is a function template or specialization, as their
+// Bail out if this is a function template specialization, as their
// definitions need to be visible in all including translation units.
-if (Source->getDescribedFunctionTemplate())
- return false;
if (Source->getTemplateSpecializationInfo())
return false;
auto *MD = llvm::dyn_cast(Source);
if (!
[clang-tools-extra] [clangd] Let DefineOutline tweak handle member functions (PR #95235)
ckandeler wrote: ping https://github.com/llvm/llvm-project/pull/95235 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] fix extract-to-function for overloaded operators (PR #81640)
ckandeler wrote: Needs rebase and another typo fix. Looks good to me otherwise. https://github.com/llvm/llvm-project/pull/81640 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] fix extract-to-function for overloaded operators (PR #81640)
@@ -105,7 +105,7 @@ bool isRootStmt(const Node *N) {
if (N->Selected == SelectionTree::Partial)
return false;
// A DeclStmt can be an unselected RootStmt since VarDecls claim the entire
- // selection range in selectionTree. Additionally, an CXXOperatorCallExpr of
a
+ // selection range in selectionTree. Additionally, a CXXOperatorCallExpr of a
// binary operation can be unselected because it's children claim the entire
ckandeler wrote:
"it's" should also be changed to "its"
https://github.com/llvm/llvm-project/pull/81640
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Let DefineOutline tweak handle member function templates (PR #112345)
ckandeler wrote:
> ```c++
> template
> inline void Foo::bar(const T& t, const U& u) {}
> ```
>
> this won't compile as `T` isn't defined. you also need to print `template
> ` on top (and keep doing for rest of the outer decls).
The patch does that already.
In the test, the outer template declaration is on the line above.
https://github.com/llvm/llvm-project/pull/112345
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 9dd725113a156a01f1866cfefe181c1b22f7e8d0 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 | 57 +++
.../clangd/index/SymbolCollector.h| 3 +-
.../clangd/unittests/SymbolCollectorTests.cpp | 52 +
4 files changed, 104 insertions(+), 12 deletions(-)
diff --git a/clang-tools-extra/clangd/index/Symbol.h
b/clang-tools-extra/clangd/index/Symbol.h
index 1aa5265299231b..62c47ddfc5758d 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 5c4e2150cf3123..a76894cf0855f3 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -635,17 +635,21 @@ bool SymbolCollector::handleDeclOccurrence(
return true;
const Symbol *BasicSymbol = Symbols.find(ID);
- if (isPreferredDeclaration(*OriginalDecl, Roles))
+ bool SkipDocCheckInDef = false;
+ if (isPreferredDeclaration(*OriginalDecl, Roles)) {
// If OriginalDecl is preferred, replace/create the existing canonical
// declaration (e.g. a class forward declaration). There should be at most
// one duplicate as we expect to see only one preferred declaration per
// TU, because in practice they are definitions.
BasicSymbol = addDeclaration(*OriginalDecl, std::move(ID), IsMainFileOnly);
- else if (!BasicSymbol || DeclIsCanonical)
+SkipDocCheckInDef = true;
+ } else if (!BasicSymbol || DeclIsCanonical) {
BasicSymbol = addDeclaration(*ND, std::move(ID), IsMainFileOnly);
+SkipDocCheckInDef = true;
+ }
if (Roles & static_cast(index::SymbolRole::Definition))
-addDefinition(*OriginalDecl, *BasicSymbol);
+addDefinition(*OriginalDecl, *BasicSymbol, SkipDocCheckInDef);
return true;
}
@@ -1025,16 +1029,28 @@ const Symbol *SymbolCollector::addDeclaration(const
NamedDecl &ND, SymbolID ID,
*ASTCtx, *PP, CodeCompletionContext::CCC_Symbol, *CompletionAllocator,
*CompletionTUInfo,
/*IncludeBriefComments*/ false);
- std::string Documentation =
- formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion,
- /*CommentsFromHeaders=*/true));
+ std::string DocComment;
+ std::string Documentation;
+ bool AlreadyHasDoc = S.Flags & Symbol::HasDocComment;
+ if (!AlreadyHasDoc) {
+DocComment = getDocComment(Ctx, SymbolCompletion,
+ /*CommentsFromHeaders=*/true);
+Documentation = formatDocumentation(*CCS, DocComment);
+ }
+ const auto UpdateDoc = [&] {
+if (!AlreadyHasDoc) {
+ if (!DocComment.empty())
+S.Flags |= Symbol::HasDocComment;
+ S.Documentation = Documentation;
+}
+ };
if (!(S.Flags & Symbol::IndexedForCodeCompletion)) {
if (Opts.StoreAllDocumentation)
- S.Documentation = Documentation;
+ UpdateDoc();
Symbols.insert(S);
return Symbols.find(S.ID);
}
- S.Documentation = Documentation;
+ UpdateDoc();
std::string Signature;
std::string SnippetSuffix;
getSignature(*CCS, &Signature, &SnippetSuffix, SymbolCompletion.Kind,
@@ -1058,8 +1074,8 @@ const Symbol *SymbolCollector::addDeclaration(const
NamedDecl &ND, SymbolID ID,
return Symbols.find(S.ID);
}
-void SymbolCollector::addDefinition(const NamedDecl &ND,
-const Symbol &DeclSym) {
+void SymbolCollector::addDefinition(const NamedDecl &ND, const Symbol &DeclSym,
+bool SkipDocCheck) {
if (DeclSym.Definition)
return;
const auto &SM = ND.getASTContext().getSourceManager();
@@ -1074,6 +1090,27 @@ void SymbolCollector::addDefinition(const NamedDecl &ND,
Symbol S = DeclSym;
// FIXME: use the result to filter out symbols.
S.Definition = *DefLoc;
+
+ std::string DocComment;
+ std::string Documentation;
+ if (!SkipDocCheck && !(S.Flags & Symbol::HasDocComment) &&
+ (llvm::isa(ND) || llvm::isa(ND))) {
+CodeCompletionResult SymbolCompletion(&getTemplateOrThis(ND), 0);
+const auto *CCS = SymbolCompletion.CreateCodeCompl
[clang-tools-extra] [clangd] Collect comments from function definitions into the index (PR #67802)
@@ -451,8 +451,17 @@ const RawComment *ASTContext::getRawCommentForAnyRedecl(
if (LastCheckedRedecl) {
if (LastCheckedRedecl == Redecl) {
LastCheckedRedecl = nullptr;
+continue;
ckandeler wrote:
Done.
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] Support outgoing calls in call hierarchy (PR #77556)
https://github.com/ckandeler approved this pull request. Appears to work fine. Regarding the slight memory increase: If anyone feels strongly about it, they had plenty of time to chime in. https://github.com/llvm/llvm-project/pull/77556 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Store full decl/def range with symbol locations (PR #118102)
https://github.com/ckandeler created
https://github.com/llvm/llvm-project/pull/118102
Apart from fixing the linked issue, this is also necessary for supporting LSP's
LocationLink feature and for finding proper insertion locations in the
DefineOutline tweak. Memory consumption of the background index grows by about
~2.5%.
Closes https://github.com/clangd/clangd/issues/59
>From 99e7189c6f3c19e3aabeee56793f5683251ecb26 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Wed, 27 Nov 2024 13:47:32 +0100
Subject: [PATCH] [clangd] Store full decl/def range with symbol locations
Apart from fixing the linked issue, this is also necessary
for supporting LSP's LocationLink feature and for finding
proper insertion locations in the DefineOutline tweak.
Memory consumption of the background index grows by about ~2.5%.
Closes https://github.com/clangd/clangd/issues/59
---
clang-tools-extra/clangd/CodeComplete.cpp | 4 +-
clang-tools-extra/clangd/FindSymbols.cpp | 34 --
clang-tools-extra/clangd/FindSymbols.h| 8 +-
.../clangd/HeaderSourceSwitch.cpp | 4 +-
clang-tools-extra/clangd/IncludeFixer.cpp | 6 +-
clang-tools-extra/clangd/Quality.cpp | 2 +-
clang-tools-extra/clangd/XRefs.cpp| 65 ++-
clang-tools-extra/clangd/index/FileIndex.cpp | 6 +-
clang-tools-extra/clangd/index/Merge.cpp | 13 ++-
clang-tools-extra/clangd/index/Ref.h | 11 +-
.../clangd/index/Serialization.cpp| 36 --
clang-tools-extra/clangd/index/StdLib.cpp | 12 +-
clang-tools-extra/clangd/index/Symbol.h | 8 +-
.../clangd/index/SymbolCollector.cpp | 89 +++
.../clangd/index/SymbolCollector.h| 3 +-
.../clangd/index/SymbolLocation.cpp | 11 +-
.../clangd/index/SymbolLocation.h | 103 +++---
.../clangd/index/YAMLSerialization.cpp| 37 +--
clang-tools-extra/clangd/index/dex/Dex.cpp| 4 +-
clang-tools-extra/clangd/refactor/Rename.cpp | 4 +-
.../clangd/test/Inputs/symbols.test.yaml | 17 ++-
.../index-serialization/Inputs/sample.idx | Bin 470 -> 496 bytes
.../clangd/test/type-hierarchy-ext.test | 8 +-
.../clangd/test/type-hierarchy.test | 8 +-
.../clangd/unittests/BackgroundIndexTests.cpp | 4 +-
.../clangd/unittests/CodeCompleteTests.cpp| 18 +--
.../clangd/unittests/DexTests.cpp | 5 +-
.../clangd/unittests/DiagnosticsTests.cpp | 13 ++-
.../clangd/unittests/FileIndexTests.cpp | 14 +--
.../clangd/unittests/IndexTests.cpp | 45
.../clangd/unittests/SerializationTests.cpp | 23 +++-
.../clangd/unittests/SymbolCollectorTests.cpp | 12 +-
32 files changed, 388 insertions(+), 239 deletions(-)
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp
b/clang-tools-extra/clangd/CodeComplete.cpp
index 2c2d5f0b5ac924..04e4aa2d2d1ca9 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -418,7 +418,7 @@ struct CodeCompletionBuilder {
auto Inserted = [&](llvm::StringRef Header)
-> llvm::Expected> {
auto ResolvedDeclaring =
- URI::resolve(C.IndexResult->CanonicalDeclaration.FileURI, FileName);
+ URI::resolve(C.IndexResult->CanonicalDeclaration.fileURI(),
FileName);
if (!ResolvedDeclaring)
return ResolvedDeclaring.takeError();
auto ResolvedInserted = toHeaderFile(Header, FileName);
@@ -451,7 +451,7 @@ struct CodeCompletionBuilder {
} else
log("Failed to generate include insertion edits for adding header "
"(FileURI='{0}', IncludeHeader='{1}') into {2}: {3}",
-C.IndexResult->CanonicalDeclaration.FileURI, Inc.Header, FileName,
+C.IndexResult->CanonicalDeclaration.fileURI(), Inc.Header,
FileName,
ToInclude.takeError());
}
// Prefer includes that do not need edits (i.e. already exist).
diff --git a/clang-tools-extra/clangd/FindSymbols.cpp
b/clang-tools-extra/clangd/FindSymbols.cpp
index 84bcbc1f2ddd3f..646cb261309c80 100644
--- a/clang-tools-extra/clangd/FindSymbols.cpp
+++ b/clang-tools-extra/clangd/FindSymbols.cpp
@@ -54,9 +54,19 @@ bool approximateScopeMatch(llvm::StringRef Scope,
llvm::StringRef Query) {
return Query.empty();
}
+Range indexToLSPRange(const SymbolPosition &SrcStart,
+ const SymbolPosition &SrcEnd) {
+ Position Start, End;
+ Start.line = SrcStart.line();
+ Start.character = SrcStart.column();
+ End.line = SrcEnd.line();
+ End.character = SrcEnd.column();
+ return {Start, End};
+}
+
} // namespace
-llvm::Expected indexToLSPLocation(const SymbolLocation &Loc,
+llvm::Expected indexToLSPLocation(const SymbolNameLocation &Loc,
llvm::StringRef TUPath) {
auto Path = URI::resolve(Loc.FileURI, TUPath);
if (!Path)
@@ -64,17 +74,21 @@ llvm::Expected indexToLSPLocation(c
[clang-tools-extra] [clangd] Consolidate two functions converting index to LSP locations (PR #117885)
https://github.com/ckandeler created
https://github.com/llvm/llvm-project/pull/117885
None
>From 7aa28a386a84c4fa8e7e8aa13822fd3e058b076c Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Wed, 27 Nov 2024 14:55:18 +0100
Subject: [PATCH] [clangd] Consolidate two functions converting index to LSP
locations
---
clang-tools-extra/clangd/XRefs.cpp | 22 --
1 file changed, 4 insertions(+), 18 deletions(-)
diff --git a/clang-tools-extra/clangd/XRefs.cpp
b/clang-tools-extra/clangd/XRefs.cpp
index 61fa66180376cd..f1e701f1ad0210 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -121,31 +121,17 @@ void logIfOverflow(const SymbolLocation &Loc) {
// Convert a SymbolLocation to LSP's Location.
// TUPath is used to resolve the path of URI.
-// FIXME: figure out a good home for it, and share the implementation with
-// FindSymbols.
std::optional toLSPLocation(const SymbolLocation &Loc,
llvm::StringRef TUPath) {
if (!Loc)
return std::nullopt;
- auto Uri = URI::parse(Loc.FileURI);
- if (!Uri) {
-elog("Could not parse URI {0}: {1}", Loc.FileURI, Uri.takeError());
+ auto LSPLoc = indexToLSPLocation(Loc, TUPath);
+ if (!LSPLoc) {
+elog("{0}", LSPLoc.takeError());
return std::nullopt;
}
- auto U = URIForFile::fromURI(*Uri, TUPath);
- if (!U) {
-elog("Could not resolve URI {0}: {1}", Loc.FileURI, U.takeError());
-return std::nullopt;
- }
-
- Location LSPLoc;
- LSPLoc.uri = std::move(*U);
- LSPLoc.range.start.line = Loc.Start.line();
- LSPLoc.range.start.character = Loc.Start.column();
- LSPLoc.range.end.line = Loc.End.line();
- LSPLoc.range.end.character = Loc.End.column();
logIfOverflow(Loc);
- return LSPLoc;
+ return *LSPLoc;
}
SymbolLocation toIndexLocation(const Location &Loc, std::string &URIStorage) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Fix erroneous qualification of template type parameters (PR #116821)
https://github.com/ckandeler created
https://github.com/llvm/llvm-project/pull/116821
...in DefineOutline tweak.
E.g. moving the following definition:
template struct S { T f^oo() const { return T(); } };
would result in:
// ...
template S::T S::foo() const { return T(); }
instead of:
template T S::foo() const { return T(); }
>From 00f85ed13e689331adbbc3513dde09023dfc68d4 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Tue, 19 Nov 2024 16:00:29 +0100
Subject: [PATCH] [clangd] Fix erroneous qualification of template type
parameters
...in DefineOutline tweak.
E.g. moving the following definition:
template struct S { T f^oo() const { return T(); } };
would result in:
// ...
template S::T S::foo() const { return T(); }
instead of:
template T S::foo() const { return T(); }
---
.../clangd/refactor/tweaks/DefineOutline.cpp | 2 ++
.../clangd/unittests/tweaks/DefineOutlineTests.cpp | 12 ++--
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index 2a96b305bd7a25..789c10bdd4822a 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -239,6 +239,8 @@ getFunctionSourceCode(const FunctionDecl *FD, const
DeclContext *TargetContext,
return;
for (const NamedDecl *ND : Ref.Targets) {
+ if (ND->getKind() == Decl::TemplateTypeParm)
+return;
if (ND->getDeclContext() != Ref.Targets.front()->getDeclContext()) {
elog("Targets from multiple contexts: {0}, {1}",
printQualifiedName(*Ref.Targets.front()),
diff --git a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
index 1af1bc31bf6486..d2d2ae9e7bb610 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
@@ -411,14 +411,14 @@ inline typename O1::template O2::E O1::template O2
R"cpp(
struct Foo {
template
- void ^bar() {}
+ T ^bar() { return {}; }
};)cpp",
R"cpp(
struct Foo {
template
- void bar() ;
+ T bar() ;
};template
-inline void Foo::bar() {}
+inline T Foo::bar() { return {}; }
)cpp",
""},
@@ -426,14 +426,14 @@ inline void Foo::bar() {}
{
R"cpp(
template struct Foo {
- template void ^bar(const T& t, const U& u) {}
+ template T ^bar(const T& t, const U& u) { return
{}; }
};)cpp",
R"cpp(
template struct Foo {
- template void bar(const T& t, const U& u) ;
+ template T bar(const T& t, const U& u) ;
};template
template
-inline void Foo::bar(const T& t, const U& u) {}
+inline T Foo::bar(const T& t, const U& u) { return {}; }
)cpp",
""},
};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Let DefineOutline tweak handle member function templates (PR #112345)
@@ -443,13 +461,26 @@ class DefineOutline : public Tweak {
SameFile = true;
// Bail out if the template parameter is unnamed.
+// FIXME: Is this really needed? It inhibits application on
ckandeler wrote:
I've moved the comment down to the function template part.
https://github.com/llvm/llvm-project/pull/112345
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Fix erroneous qualification of template type parameters (PR #116821)
https://github.com/ckandeler closed https://github.com/llvm/llvm-project/pull/116821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Support outgoing calls in call hierarchy (PR #77556)
@@ -88,56 +89,6 @@
# CHECK-NEXT: }
# CHECK-NEXT: ]
---
-{"jsonrpc":"2.0","id":2,"method":"typeHierarchy/subtypes","params":{"item":{"uri":"test:///main.cpp","data":{"parents":[{"parents":[{"parents":[],"symbolID":"FE546E7B648D69A7"}],"symbolID":"ECDC0C46D75120F4"}],"symbolID":"8A991335E4E67D08"},"name":"Child2","kind":23,"range":{"end":{"character":13,"line":3},"start":{"character":7,"line":3}},"selectionRange":{"end":{"character":13,"line":3},"start":{"character":7,"line":3}
ckandeler wrote:
What was the reasoning for removing this entirely?
https://github.com/llvm/llvm-project/pull/77556
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Support outgoing calls in call hierarchy (PR #77556)
@@ -2346,11 +2346,11 @@ outgoingCalls(const CallHierarchyItem &Item, const
SymbolIndex *Index) {
// Filter references to only keep function calls
using SK = index::SymbolKind;
auto Kind = Callee.SymInfo.Kind;
-if (Kind != SK::Function && Kind != SK::InstanceMethod &&
-Kind != SK::ClassMethod && Kind != SK::StaticMethod &&
-Kind != SK::Constructor && Kind != SK::Destructor &&
-Kind != SK::ConversionFunction)
- return;
+bool NotCall = (Kind != SK::Function && Kind != SK::InstanceMethod &&
+Kind != SK::ClassMethod && Kind != SK::StaticMethod &&
+Kind != SK::Constructor && Kind != SK::Destructor &&
+Kind != SK::ConversionFunction);
+assert(!NotCall);
ckandeler wrote:
Can we get rid of the double negation? It just seems to obfuscate the logic for
no particular reason.
https://github.com/llvm/llvm-project/pull/77556
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Support outgoing calls in call hierarchy (PR #77556)
@@ -2346,11 +2346,11 @@ outgoingCalls(const CallHierarchyItem &Item, const
SymbolIndex *Index) {
// Filter references to only keep function calls
ckandeler wrote:
This comment doesn't really apply anymore.
https://github.com/llvm/llvm-project/pull/77556
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Fix erroneous qualification of template type parameters (PR #116821)
https://github.com/ckandeler edited https://github.com/llvm/llvm-project/pull/116821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Let DefineOutline tweak handle member function templates (PR #112345)
https://github.com/ckandeler closed https://github.com/llvm/llvm-project/pull/112345 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Let DefineOutline tweak handle member function templates (PR #112345)
https://github.com/ckandeler updated
https://github.com/llvm/llvm-project/pull/112345
>From 5b563ceec2c3462a00101526fa416a835a59e7c0 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Mon, 14 Oct 2024 15:37:01 +0200
Subject: [PATCH] [clangd] Let DefineOutline tweak handle member function
templates
---
.../clangd/refactor/tweaks/DefineOutline.cpp | 67 +--
.../unittests/tweaks/DefineOutlineTests.cpp | 49 --
2 files changed, 91 insertions(+), 25 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index 591a8b245260ea..2a96b305bd7a25 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -109,14 +109,13 @@ findContextForNS(llvm::StringRef TargetNS, const
DeclContext *CurContext) {
// afterwards it can be shared with define-inline code action.
llvm::Expected
getFunctionSourceAfterReplacements(const FunctionDecl *FD,
- const tooling::Replacements &Replacements) {
+ const tooling::Replacements &Replacements,
+ bool TargetFileIsHeader) {
const auto &SM = FD->getASTContext().getSourceManager();
auto OrigFuncRange = toHalfOpenFileRange(
SM, FD->getASTContext().getLangOpts(), FD->getSourceRange());
if (!OrigFuncRange)
return error("Couldn't get range for function.");
- assert(!FD->getDescribedFunctionTemplate() &&
- "Define out-of-line doesn't apply to function templates.");
// Get new begin and end positions for the qualified function definition.
unsigned FuncBegin = SM.getFileOffset(OrigFuncRange->getBegin());
@@ -129,24 +128,38 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD,
if (!QualifiedFunc)
return QualifiedFunc.takeError();
+ auto Source = QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
std::string TemplatePrefix;
+ auto AddToTemplatePrefixIfApplicable = [&](const Decl *D) {
+const TemplateParameterList *Params = D->getDescribedTemplateParams();
+if (!Params)
+ return;
+for (Decl *P : *Params) {
+ if (auto *TTP = dyn_cast(P))
+TTP->removeDefaultArgument();
+ else if (auto *NTTP = dyn_cast(P))
+NTTP->removeDefaultArgument();
+ else if (auto *TTPD = dyn_cast(P))
+TTPD->removeDefaultArgument();
+}
+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);
+ };
+ AddToTemplatePrefixIfApplicable(FD);
if (auto *MD = llvm::dyn_cast(FD)) {
for (const CXXRecordDecl *Parent = MD->getParent(); Parent;
Parent =
llvm::dyn_cast_or_null(Parent->getParent()))
{
- if (const TemplateParameterList *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);
- }
+ AddToTemplatePrefixIfApplicable(Parent);
}
}
- auto Source = QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
+ if (TargetFileIsHeader)
+Source.insert(0, "inline ");
if (!TemplatePrefix.empty())
Source.insert(0, TemplatePrefix);
return Source;
@@ -202,7 +215,8 @@ deleteTokensWithKind(const syntax::TokenBuffer &TokBuf,
tok::TokenKind Kind,
llvm::Expected
getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext,
const syntax::TokenBuffer &TokBuf,
- const HeuristicResolver *Resolver) {
+ const HeuristicResolver *Resolver,
+ bool TargetFileIsHeader) {
auto &AST = FD->getASTContext();
auto &SM = AST.getSourceManager();
@@ -337,7 +351,8 @@ getFunctionSourceCode(const FunctionDecl *FD, const
DeclContext *TargetContext,
if (Errors)
return std::move(Errors);
- return getFunctionSourceAfterReplacements(FD, DeclarationCleanups);
+ return getFunctionSourceAfterReplacements(FD, DeclarationCleanups,
+TargetFileIsHeader);
}
struct InsertionPoint {
@@ -419,15 +434,15 @@ class DefineOutline : public Tweak {
Source->isOutOfLine())
return false;
-// Bail out if this is a function template or specialization, as their
+// Bail out if this is a function template specialization, as their
// definitions need to be visible in all including translation units.
-if (Source->getDescribedFunctionTemplate())
- return false;
if (Source->getTemplateSpecializationInfo())
return false;
auto *MD = llvm::dyn_cast(Source)
[clang-tools-extra] [clangd] Drop requirement for named template parameters (PR #117565)
https://github.com/ckandeler created
https://github.com/llvm/llvm-project/pull/117565
... in DefineOutline tweak for function templates. As opposed to class
templates, the name is not required for writing an out-of-line definition.
>From 064d23753ed1ac673bda33ab815dbb12c6bcc6f6 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Mon, 25 Nov 2024 15:42:54 +0100
Subject: [PATCH] [clangd] Drop requirement for named template parameters
... in DefineOutline tweak for function templates.
As opposed to class templates, the name is not required for writing an
out-of-line definition.
---
.../clangd/refactor/tweaks/DefineOutline.cpp | 13 ++---
.../unittests/tweaks/DefineOutlineTests.cpp| 18 ++
2 files changed, 8 insertions(+), 23 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index 789c10bdd4822a..e4eef228b6b99f 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -467,18 +467,9 @@ class DefineOutline : public Tweak {
}
}
-// For function templates, the same limitations as for class templates
-// apply.
-if (const TemplateParameterList *Params =
-MD->getDescribedTemplateParams()) {
- // FIXME: Is this really needed? It inhibits application on
- //e.g. std::enable_if.
- for (NamedDecl *P : *Params) {
-if (!P->getIdentifier())
- return false;
- }
+// Function templates must be defined in the same file.
+if (MD->getDescribedTemplate())
SameFile = true;
-}
// 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 d2d2ae9e7bb610..b5f09f9b14604f 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
@@ -118,12 +118,6 @@ TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
template <> void fo^o() {}
)cpp");
- // Not available on member function templates with unnamed template
- // parameters.
- EXPECT_UNAVAILABLE(R"cpp(
-struct Foo { template void ba^r() {} };
- )cpp");
-
// Not available on methods of unnamed classes.
EXPECT_UNAVAILABLE(R"cpp(
struct Foo {
@@ -410,14 +404,14 @@ inline typename O1::template O2::E O1::template O2
{
R"cpp(
struct Foo {
- template
+ template
T ^bar() { return {}; }
};)cpp",
R"cpp(
struct Foo {
- template
+ template
T bar() ;
-};template
+};template
inline T Foo::bar() { return {}; }
)cpp",
""},
@@ -426,13 +420,13 @@ inline T Foo::bar() { return {}; }
{
R"cpp(
template struct Foo {
- template T ^bar(const T& t, const U& u) { return
{}; }
+ template T ^bar(const T& t, const U& u) {
return {}; }
};)cpp",
R"cpp(
template struct Foo {
- template T bar(const T& t, const U& u) ;
+ template T bar(const T& t, const U& u) ;
};template
-template
+template
inline T Foo::bar(const T& t, const U& u) { return {}; }
)cpp",
""},
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Drop requirement for named template parameters (PR #117565)
https://github.com/ckandeler closed https://github.com/llvm/llvm-project/pull/117565 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Consolidate two functions converting index to LSP locations (PR #117885)
https://github.com/ckandeler closed https://github.com/llvm/llvm-project/pull/117885 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Store full decl/def range with symbol locations (PR #118102)
https://github.com/ckandeler updated
https://github.com/llvm/llvm-project/pull/118102
>From c44237813fb3357dee3f7d17048a7178e67a2422 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Wed, 27 Nov 2024 13:47:32 +0100
Subject: [PATCH] [clangd] Store full decl/def range with symbol locations
Apart from fixing the linked issue, this is also necessary
for supporting LSP's LocationLink feature and for finding
proper insertion locations in the DefineOutline tweak.
Memory consumption of the background index grows by about ~2.5%.
Closes https://github.com/clangd/clangd/issues/59
---
clang-tools-extra/clangd/CodeComplete.cpp | 4 +-
clang-tools-extra/clangd/FindSymbols.cpp | 34 --
clang-tools-extra/clangd/FindSymbols.h| 8 +-
.../clangd/HeaderSourceSwitch.cpp | 4 +-
clang-tools-extra/clangd/IncludeFixer.cpp | 6 +-
clang-tools-extra/clangd/Quality.cpp | 2 +-
clang-tools-extra/clangd/XRefs.cpp| 65 ++-
clang-tools-extra/clangd/index/FileIndex.cpp | 6 +-
clang-tools-extra/clangd/index/Index.h| 2 +-
clang-tools-extra/clangd/index/Merge.cpp | 13 ++-
clang-tools-extra/clangd/index/Ref.h | 11 +-
.../clangd/index/Serialization.cpp| 34 --
clang-tools-extra/clangd/index/StdLib.cpp | 12 +-
clang-tools-extra/clangd/index/Symbol.h | 8 +-
.../clangd/index/SymbolCollector.cpp | 89 +++
.../clangd/index/SymbolCollector.h| 3 +-
.../clangd/index/SymbolLocation.cpp | 11 +-
.../clangd/index/SymbolLocation.h | 103 +++---
.../clangd/index/YAMLSerialization.cpp| 37 +--
clang-tools-extra/clangd/index/dex/Dex.cpp| 4 +-
clang-tools-extra/clangd/refactor/Rename.cpp | 4 +-
.../clangd/test/Inputs/symbols.test.yaml | 17 ++-
.../index-serialization/Inputs/sample.idx | Bin 470 -> 496 bytes
.../clangd/test/type-hierarchy-ext.test | 8 +-
.../clangd/test/type-hierarchy.test | 8 +-
.../clangd/unittests/BackgroundIndexTests.cpp | 4 +-
.../clangd/unittests/CodeCompleteTests.cpp| 18 +--
.../clangd/unittests/DexTests.cpp | 5 +-
.../clangd/unittests/DiagnosticsTests.cpp | 13 ++-
.../clangd/unittests/FileIndexTests.cpp | 14 +--
.../clangd/unittests/IndexTests.cpp | 45
.../clangd/unittests/SerializationTests.cpp | 23 +++-
.../clangd/unittests/SymbolCollectorTests.cpp | 12 +-
33 files changed, 388 insertions(+), 239 deletions(-)
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp
b/clang-tools-extra/clangd/CodeComplete.cpp
index 2c2d5f0b5ac924..04e4aa2d2d1ca9 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -418,7 +418,7 @@ struct CodeCompletionBuilder {
auto Inserted = [&](llvm::StringRef Header)
-> llvm::Expected> {
auto ResolvedDeclaring =
- URI::resolve(C.IndexResult->CanonicalDeclaration.FileURI, FileName);
+ URI::resolve(C.IndexResult->CanonicalDeclaration.fileURI(),
FileName);
if (!ResolvedDeclaring)
return ResolvedDeclaring.takeError();
auto ResolvedInserted = toHeaderFile(Header, FileName);
@@ -451,7 +451,7 @@ struct CodeCompletionBuilder {
} else
log("Failed to generate include insertion edits for adding header "
"(FileURI='{0}', IncludeHeader='{1}') into {2}: {3}",
-C.IndexResult->CanonicalDeclaration.FileURI, Inc.Header, FileName,
+C.IndexResult->CanonicalDeclaration.fileURI(), Inc.Header,
FileName,
ToInclude.takeError());
}
// Prefer includes that do not need edits (i.e. already exist).
diff --git a/clang-tools-extra/clangd/FindSymbols.cpp
b/clang-tools-extra/clangd/FindSymbols.cpp
index 84bcbc1f2ddd3f..646cb261309c80 100644
--- a/clang-tools-extra/clangd/FindSymbols.cpp
+++ b/clang-tools-extra/clangd/FindSymbols.cpp
@@ -54,9 +54,19 @@ bool approximateScopeMatch(llvm::StringRef Scope,
llvm::StringRef Query) {
return Query.empty();
}
+Range indexToLSPRange(const SymbolPosition &SrcStart,
+ const SymbolPosition &SrcEnd) {
+ Position Start, End;
+ Start.line = SrcStart.line();
+ Start.character = SrcStart.column();
+ End.line = SrcEnd.line();
+ End.character = SrcEnd.column();
+ return {Start, End};
+}
+
} // namespace
-llvm::Expected indexToLSPLocation(const SymbolLocation &Loc,
+llvm::Expected indexToLSPLocation(const SymbolNameLocation &Loc,
llvm::StringRef TUPath) {
auto Path = URI::resolve(Loc.FileURI, TUPath);
if (!Path)
@@ -64,17 +74,21 @@ llvm::Expected indexToLSPLocation(const
SymbolLocation &Loc,
Path.takeError());
Location L;
L.uri = URIForFile::canonicalize(*Path, TUPath);
- Position Start, End;
- Start.line = Loc.Start.line();
- Start.character = Loc.Start.column();
- End
[clang-tools-extra] [clangd] Consider expression statements in ExtractVariable tweak (PR #112525)
ckandeler wrote: > In the expression-statement case, the expression isn't moving to a new > location, and the variable name is only mentioned in > one place. In essence, the refactoring amounts to prepending `auto > placeholder = ` to the expression. Given that you're > going to be (very likely) renaming `placeholder` anyways, does this really > add anything meaningful over just typing `auto > VariableNameYouWant = ` "manually"? My thinking was that because the expression is assignable and it's not clearly pointless to do so, the tweak should be available (plus it still saves some keystrokes). My concrete motivation was due to user requests on the client side, e.g.: https://bugreports.qt.io/browse/QTCREATORBUG-9363 https://bugreports.qt.io/browse/QTCREATORBUG-9578 Maybe some developers have the habit of writing just the rhs of an init statement and then let tooling insert the lhs? It's possible, I suppose. E.g. if the return type is complex and you want to have it spelt out in the declaration (as per the project's coding conventions), then you could apply this tweak and then the ExpandDeducedType one. In general, I'm not a big fan of going out of my way to prevent users from doing something. And once you start deliberating about degrees of usefulness, you quickly run into problems deciding where to draw the line. https://github.com/llvm/llvm-project/pull/112525 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Store full decl/def range with symbol locations (PR #118102)
ckandeler wrote: ping https://github.com/llvm/llvm-project/pull/118102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
