adamcz updated this revision to Diff 377891.
adamcz marked an inline comment as done.
adamcz added a comment.
addressed review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110823/new/
https://reviews.llvm.org/D110823
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/test/initialize-params.test
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3279,6 +3279,35 @@
EXPECT_THAT(Result.Completions, Not(testing::IsEmpty()));
}
+TEST(CompletionTest, CommentParamName) {
+ clangd::CodeCompleteOptions Opts;
+ const std::string Code = R"cpp(
+ void fun(int foo, int bar);
+ void overloaded(int param_int);
+ void overloaded(int param_int, int param_other);
+ void overloaded(char param_char);
+ int main() {
+ )cpp";
+
+ EXPECT_THAT(completions(Code + "fun(/*^", {}, Opts).Completions,
+ UnorderedElementsAre(Labeled("foo=")));
+ EXPECT_THAT(completions(Code + "fun(1, /*^", {}, Opts).Completions,
+ UnorderedElementsAre(Labeled("bar=")));
+ EXPECT_THAT(completions(Code + "/*^", {}, Opts).Completions, IsEmpty());
+ // Test de-duplication.
+ EXPECT_THAT(
+ completions(Code + "overloaded(/*^", {}, Opts).Completions,
+ UnorderedElementsAre(Labeled("param_int="), Labeled("param_char=")));
+ // Comment already has some text in it.
+ EXPECT_THAT(completions(Code + "fun(/* ^", {}, Opts).Completions,
+ UnorderedElementsAre(Labeled("foo=")));
+ EXPECT_THAT(completions(Code + "fun(/* f^", {}, Opts).Completions,
+ UnorderedElementsAre(Labeled("foo=")));
+ EXPECT_THAT(completions(Code + "fun(/* x^", {}, Opts).Completions, IsEmpty());
+ EXPECT_THAT(completions(Code + "fun(/* f ^", {}, Opts).Completions,
+ IsEmpty());
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/test/initialize-params.test
===================================================================
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -48,7 +48,8 @@
# CHECK-NEXT: ">",
# CHECK-NEXT: ":",
# CHECK-NEXT: "\"",
-# CHECK-NEXT: "/"
+# CHECK-NEXT: "/",
+# CHECK-NEXT: "*"
# CHECK-NEXT: ]
# CHECK-NEXT: },
# CHECK-NEXT: "declarationProvider": true,
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1098,6 +1098,55 @@
const SymbolIndex *Index;
}; // SignatureHelpCollector
+// Used only for completion of C-style comments in function call (i.e.
+// /*foo=*/7). Similar to SignatureHelpCollector, but needs to do less work.
+class ParamNameCollector final : public CodeCompleteConsumer {
+public:
+ ParamNameCollector(const clang::CodeCompleteOptions &CodeCompleteOpts,
+ std::vector<std::string> &ParamNames)
+ : CodeCompleteConsumer(CodeCompleteOpts),
+ Allocator(std::make_shared<clang::GlobalCodeCompletionAllocator>()),
+ CCTUInfo(Allocator), ParamNames(ParamNames) {}
+
+ void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
+ OverloadCandidate *Candidates,
+ unsigned NumCandidates,
+ SourceLocation OpenParLoc) override {
+ assert(CurrentArg <= (unsigned)std::numeric_limits<int>::max() &&
+ "too many arguments");
+
+ for (unsigned I = 0; I < NumCandidates; ++I) {
+ OverloadCandidate Candidate = Candidates[I];
+ if (auto *Func = Candidate.getFunction()) {
+ if (Func->getNumParams() <= CurrentArg)
+ continue;
+ auto *PVD = Func->getParamDecl(CurrentArg);
+ if (PVD == nullptr)
+ continue;
+ auto *Ident = PVD->getIdentifier();
+ if (Ident == nullptr)
+ continue;
+ auto Name = Ident->getName();
+ if (Name.empty() || ParamNamesSeen.find(Name) != ParamNamesSeen.end())
+ continue;
+ ParamNames.emplace_back(Name);
+ ParamNamesSeen.emplace(ParamNames.back());
+ }
+ }
+ }
+
+private:
+ GlobalCodeCompletionAllocator &getAllocator() override { return *Allocator; }
+
+ CodeCompletionTUInfo &getCodeCompletionTUInfo() override { return CCTUInfo; }
+
+ std::shared_ptr<clang::GlobalCodeCompletionAllocator> Allocator;
+ CodeCompletionTUInfo CCTUInfo;
+ std::vector<std::string> &ParamNames;
+ // For de-duplication only. StringRefs based on strings in ParamNames.
+ std::set<llvm::StringRef> ParamNamesSeen;
+};
+
struct SemaCompleteInput {
PathRef FileName;
size_t Offset;
@@ -1857,6 +1906,91 @@
return Result;
}
+// Code complete the argument name on "/*" inside function call.
+// Offset should be pointing to the start of the comment, i.e.:
+// foo(^/*, rather than foo(/*^) where the cursor probably is.
+CodeCompleteResult codeCompleteComment(PathRef FileName, unsigned Offset,
+ llvm::StringRef Prefix,
+ const PreambleData *Preamble,
+ const ParseInputs &ParseInput) {
+ if (Preamble == nullptr) // Can't run without Sema.
+ return CodeCompleteResult();
+
+ clang::CodeCompleteOptions Options;
+ Options.IncludeGlobals = false;
+ Options.IncludeMacros = false;
+ Options.IncludeCodePatterns = false;
+ Options.IncludeBriefComments = false;
+ std::vector<std::string> ParamNames;
+ // We want to see signatures coming from newly introduced includes, hence a
+ // full patch.
+ semaCodeComplete(
+ std::make_unique<ParamNameCollector>(Options, ParamNames), Options,
+ {FileName, Offset, *Preamble,
+ PreamblePatch::createFullPatch(FileName, ParseInput, *Preamble),
+ ParseInput});
+ if (ParamNames.empty())
+ return CodeCompleteResult();
+
+ CodeCompleteResult Result;
+ Result.Context = CodeCompletionContext::CCC_NaturalLanguage;
+ for (llvm::StringRef Name : ParamNames) {
+ if (!Name.startswith(Prefix))
+ continue;
+ CodeCompletion Item;
+ Item.Name = Name.str() + "=";
+ Item.Kind = CompletionItemKind::Text;
+ Result.Completions.push_back(Item);
+ }
+
+ return Result;
+}
+
+static constexpr StringRef Whitespace = " \t\v\f\r\n";
+
+// Since the only code completion we support inside comments right now is
+// completing parameter name, we can quickly check if we're in this mode (and
+// extract "/*" position and prefix) without invoking Sema or even Lexer.
+//
+// If Content ends in whitespace, the only valid case for comment completion is
+// "/*" followed by whitespace only.
+// Otherwise we consume from the back until we find first non-identifier
+// character. We then strip whitespace to support both "/*foo" and "/* foo" and
+// check for trailing "/*".
+//
+// Content must be the content of the file up to the code completion point.
+// Returns true iff code completion should be performed. In that case, Offset
+// is set to the location of the /* (i.e. place where SemaCodeComplete should
+// run) and Prefix is the already existing prefix inside this comment.
+bool shouldCodeCompleteComment(llvm::StringRef Content, unsigned &Offset,
+ llvm::StringRef &Prefix) {
+ if (Content.empty())
+ return false;
+ if (Content.endswith("/*")) {
+ Offset = Content.size() - 2;
+ return true;
+ }
+
+ llvm::StringRef Trimmed = Content.rtrim(Whitespace);
+ if (Trimmed.size() != Content.size()) {
+ if (Trimmed.endswith("/*")) {
+ Offset = Trimmed.size() - 2;
+ return true;
+ }
+ return false;
+ }
+
+ while (!Trimmed.empty() && isAsciiIdentifierContinue(Trimmed.back()))
+ Trimmed = Trimmed.drop_back();
+ Prefix = Content.substr(Trimmed.size());
+ Trimmed = Trimmed.rtrim(Whitespace);
+ if (Trimmed.endswith("/*")) {
+ Offset = Trimmed.size() - 2;
+ return true;
+ }
+ return false;
+}
+
CodeCompleteResult codeComplete(PathRef FileName, Position Pos,
const PreambleData *Preamble,
const ParseInputs &ParseInput,
@@ -1867,6 +2001,21 @@
elog("Code completion position was invalid {0}", Offset.takeError());
return CodeCompleteResult();
}
+
+ auto Content = llvm::StringRef(ParseInput.Contents).take_front(*Offset);
+ unsigned OffsetBeforeComment = 0;
+ llvm::StringRef CommentPrefix;
+ if (Preamble &&
+ shouldCodeCompleteComment(Content, OffsetBeforeComment, CommentPrefix)) {
+ // We are doing code completion of a comment, where we currently only
+ // support completing param names in function calls. To do this, we require
+ // information from Sema, but Sema's comment completion stops at parsing, so
+ // we must move back the position before running it, extract information we
+ // need and construct completion items ourselves.
+ return codeCompleteComment(FileName, OffsetBeforeComment, CommentPrefix,
+ Preamble, ParseInput);
+ }
+
auto Flow = CodeCompleteFlow(
FileName, Preamble ? Preamble->Includes : IncludeStructure(),
SpecFuzzyFind, Opts);
@@ -2050,7 +2199,8 @@
Content = Content.substr(Pos + 1);
// Complete after scope operators.
- if (Content.endswith(".") || Content.endswith("->") || Content.endswith("::"))
+ if (Content.endswith(".") || Content.endswith("->") ||
+ Content.endswith("::") || Content.endswith("/*"))
return true;
// Complete after `#include <` and #include `<foo/`.
if ((Content.endswith("<") || Content.endswith("\"") ||
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -542,7 +542,7 @@
"^", "&", "#", "?", ".", "=", "\"", "'", "|"}},
{"resolveProvider", false},
// We do extra checks, e.g. that > is part of ->.
- {"triggerCharacters", {".", "<", ">", ":", "\"", "/"}},
+ {"triggerCharacters", {".", "<", ">", ":", "\"", "/", "*"}},
}},
{"semanticTokensProvider",
llvm::json::Object{
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
