[PATCH] D110823: [clangd] Add code completion of param name on /* inside function calls.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1148
+  std::set<llvm::StringRef> ParamNamesSeen;
+}; // SignatureHelpCollector
+
----------------
change the ending comment (well, I'd actually drop it completely)


================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1927
+      {FileName, Offset, *Preamble,
+       PreamblePatch::createFullPatch(FileName, ParseInput, *Preamble),
+       ParseInput});
----------------
i think it's subtle that we require a fullpatch here. maybe put a comment saying that "we want to see signatures coming from newly introduced includes, hence a full patch".


================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1956
+  auto Content = llvm::StringRef(ParseInput.Contents).take_front(*Offset);
+  if (Content.endswith("/*")) {
+    // We are doing code completion of a comment, where we currently only
----------------
what if the user triggers at `foo(/*  b^`? I think we'd still want to provide `bar`.

I think we should detect whether we're in a comment first, we can run the Lexer for that but it'd probably be an overkill. For now I think we can just check for the string after last `*/`, if it has a `/*` we're in a comment, otherwise we check for the current line only. if it has a `//` we're again in a comment.

Then we can switch to the `codeCompleteComment` mode, inside there one branch can collect completion results for possible parameter names (while filtering using the currently typed text) and we can expand with more logic later on (e.g. suggest identifiers from index?).

WDYT?


================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1963
+    auto OffsetBeforeComment = *Offset - 2;
+    return codeCompleteComment(FileName, OffsetBeforeComment, Preamble,
+                               ParseInput);
----------------
i think we should propagate `Opts` here and then override the pieces accordingly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110823/new/

https://reviews.llvm.org/D110823