richard9404 created this revision.
richard9404 added reviewers: sammccall, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous.
Herald added a project: clang.
Header completion in clang always apppends endings to the results, i.e. `/` for
folders and `"` (or `>`) for files. But a language-neutral client (like many
vim lsp plugins) does not know a subsequent completion is expected, while the
auto-inserted `/` prevents the user from triggering one by manually typing a
`/`. And `"`/`>` might collide with auto pair insertion in some clients.
#include "f"
^ completion triggered here
=> folder/ <- client is not aware that another completion is expected
=> file.h" <- '"' collides with the ending '"' inserted by the client
This commit adds an option to suppress the endings, and adds `/` as a
completion trigger in clangd, which makes header completion more similar to
object member completion. The downside is that clangd will not be able to
distinguish folders from files when setting the completion item kinds.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D64391
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/CodeComplete.h
clang-tools-extra/clangd/test/initialize-params.test
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Sema/CodeCompleteConsumer.h
clang/include/clang/Sema/CodeCompleteOptions.h
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaCodeComplete.cpp
clang/test/CodeCompletion/included-files-endings.cpp
Index: clang/test/CodeCompletion/included-files-endings.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeCompletion/included-files-endings.cpp
@@ -0,0 +1,24 @@
+// RUN: rm -rf %t && mkdir %t && cp %s %t/main.cc && mkdir %t/foo && mkdir %t/sys
+// RUN: touch %t/foobar.h && touch %t/foo/bar.h && touch %t/sys/foosys.h
+
+// With endings.
+#include "foobar.h"
+// RUN: %clang -fsyntax-only -isystem %t/sys -Xclang -code-completion-at=%t/main.cc:5:13 %t/main.cc | FileCheck -check-prefix=CHECK-1 %s
+// CHECK-1: foo/
+// CHECK-1: foobar.h"
+
+// System headers with endings.
+#include <foosys.h>
+// RUN: %clang -fsyntax-only -isystem %t/sys -Xclang -code-completion-at=%t/main.cc:11:14 %t/main.cc | FileCheck -check-prefix=CHECK-2 %s
+// CHECK-2: sys.h>
+
+// Without endings.
+#include "foobar.h"
+// RUN: %clang -fsyntax-only -isystem %t/sys -Xclang -code-completion-at=%t/main.cc:16:13 -Xclang -no-code-completion-included-file-endings %t/main.cc | FileCheck -check-prefix=CHECK-3 %s
+// CHECK-3: foo
+// CHECK-3: foobar.h
+
+// System headers without endings.
+#include <foosys.h>
+// RUN: %clang -fsyntax-only -isystem %t/sys -Xclang -code-completion-at=%t/main.cc:22:14 -Xclang -no-code-completion-included-file-endings %t/main.cc | FileCheck -check-prefix=CHECK-4 %s
+// CHECK-4: sys.h
Index: clang/lib/Sema/SemaCodeComplete.cpp
===================================================================
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -8655,7 +8655,8 @@
auto AddCompletion = [&](StringRef Filename, bool IsDirectory) {
SmallString<64> TypedChunk = Filename;
// Directory completion is up to the slash, e.g. <sys/
- TypedChunk.push_back(IsDirectory ? '/' : Angled ? '>' : '"');
+ if (CodeCompleter->includedFileEndings())
+ TypedChunk.push_back(IsDirectory ? '/' : Angled ? '>' : '"');
auto R = SeenResults.insert(TypedChunk);
if (R.second) { // New completion
const char *InternedTyped = Results.getAllocator().CopyString(TypedChunk);
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1809,6 +1809,8 @@
= Args.hasArg(OPT_code_completion_brief_comments);
Opts.CodeCompleteOpts.IncludeFixIts
= Args.hasArg(OPT_code_completion_with_fixits);
+ Opts.CodeCompleteOpts.IncludedFileEndings
+ = !Args.hasArg(OPT_no_code_completion_included_file_endings);
Opts.OverrideRecordLayoutsFile
= Args.getLastArgValue(OPT_foverride_record_layout_EQ);
Index: clang/include/clang/Sema/CodeCompleteOptions.h
===================================================================
--- clang/include/clang/Sema/CodeCompleteOptions.h
+++ clang/include/clang/Sema/CodeCompleteOptions.h
@@ -43,10 +43,14 @@
/// on member access, etc.
unsigned IncludeFixIts : 1;
+ /// Whether to include the '/', '"' and '>' endings when completing included
+ /// files.
+ unsigned IncludedFileEndings : 1;
+
CodeCompleteOptions()
: IncludeMacros(0), IncludeCodePatterns(0), IncludeGlobals(1),
- IncludeNamespaceLevelDecls(1), IncludeBriefComments(0),
- LoadExternal(1), IncludeFixIts(0) {}
+ IncludeNamespaceLevelDecls(1), IncludeBriefComments(0), LoadExternal(1),
+ IncludeFixIts(0), IncludedFileEndings(1) {}
};
} // namespace clang
Index: clang/include/clang/Sema/CodeCompleteConsumer.h
===================================================================
--- clang/include/clang/Sema/CodeCompleteConsumer.h
+++ clang/include/clang/Sema/CodeCompleteConsumer.h
@@ -1101,6 +1101,12 @@
return CodeCompleteOpts.LoadExternal;
}
+ /// Whether to include the '/', '"' and '>' endings when completing included
+ /// files.
+ bool includedFileEndings() const {
+ return CodeCompleteOpts.IncludedFileEndings;
+ }
+
/// Deregisters and destroys this code-completion consumer.
virtual ~CodeCompleteConsumer();
Index: clang/include/clang/Driver/CC1Options.td
===================================================================
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -491,6 +491,8 @@
HelpText<"Include brief documentation comments in code-completion results.">;
def code_completion_with_fixits : Flag<["-"], "code-completion-with-fixits">,
HelpText<"Include code completion results which require small fix-its.">;
+def no_code_completion_included_file_endings : Flag<["-"], "no-code-completion-included-file-endings">,
+ HelpText<"Do not include '/', '\"' and '>' endings when completing included files.">;
def disable_free : Flag<["-"], "disable-free">,
HelpText<"Disable freeing of memory on exit">;
def discard_value_names : Flag<["-"], "discard-value-names">,
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2285,6 +2285,26 @@
Has("bar.h\"", CompletionItemKind::File)));
}
+TEST(CompletionTest, IncludedCompletionWithoutEndings) {
+ MockFSProvider FS;
+ MockCompilationDatabase CDB;
+ std::string Subdir = testPath("sub");
+ std::string SearchDirArg = (Twine("-I") + Subdir).str();
+ CDB.ExtraClangFlags = {SearchDirArg.c_str()};
+ std::string BarHeader = testPath("sub/bar.h");
+ FS.Files[BarHeader] = "";
+ IgnoreDiagnostics DiagConsumer;
+ ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+ clangd::CodeCompleteOptions CCOpts;
+ CCOpts.IncludedFileEndings = false;
+ auto Results = completions(Server,
+ R"cpp(
+ #include "^"
+ )cpp",
+ {}, CCOpts);
+ EXPECT_THAT(Results.Completions, AllOf(Has("sub"), Has("bar.h")));
+}
+
TEST(CompletionTest, NoCrashAtNonAlphaIncludeHeader) {
auto Results = completions(
R"cpp(
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -285,6 +285,12 @@
"Specify a list of Tweaks to enable (only for clangd developers)."),
llvm::cl::Hidden, llvm::cl::CommaSeparated);
+static llvm::cl::opt<bool> CompleteIncludedFilesWithEndings(
+ "complete-included-files-with-endings",
+ llvm::cl::desc("Complete included files with endings, which are '/' for "
+ "folders, and '\"' (or '>') for files (default=true)"),
+ llvm::cl::init(true));
+
namespace {
/// \brief Supports a test URI scheme with relaxed constraints for lit tests.
@@ -498,6 +504,7 @@
CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
CCOpts.AllScopes = AllScopesCompletion;
CCOpts.RunParser = CodeCompletionParse;
+ CCOpts.IncludedFileEndings = CompleteIncludedFilesWithEndings;
RealFileSystemProvider FSProvider;
// Initialize and run ClangdLSPServer.
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
@@ -11,7 +11,8 @@
# CHECK-NEXT: "triggerCharacters": [
# CHECK-NEXT: ".",
# CHECK-NEXT: ">",
-# CHECK-NEXT: ":"
+# CHECK-NEXT: ":",
+# CHECK-NEXT: "/"
# CHECK-NEXT: ]
# CHECK-NEXT: },
# CHECK-NEXT: "declarationProvider": true,
Index: clang-tools-extra/clangd/CodeComplete.h
===================================================================
--- clang-tools-extra/clangd/CodeComplete.h
+++ clang-tools-extra/clangd/CodeComplete.h
@@ -115,6 +115,10 @@
/// Such completions can insert scope qualifiers.
bool AllScopes = false;
+ /// Whether to include the '/', '"' and '>' endings when completing included
+ /// files.
+ bool IncludedFileEndings = true;
+
/// Whether to use the clang parser, or fallback to text-based completion
/// (using identifiers in the current file and symbol indexes).
enum CodeCompletionParse {
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1694,6 +1694,7 @@
// Tell Sema not to deserialize the preamble to look for results.
Result.LoadExternal = !Index;
Result.IncludeFixIts = IncludeFixIts;
+ Result.IncludedFileEndings = IncludedFileEndings;
return Result;
}
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -24,6 +24,7 @@
#include "llvm/Support/Error.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
#include "llvm/Support/ScopedPrinter.h"
namespace clang {
@@ -395,9 +396,9 @@
{"completionProvider",
llvm::json::Object{
{"resolveProvider", false},
- // We do extra checks for '>' and ':' in completion to only
- // trigger on '->' and '::'.
- {"triggerCharacters", {".", ">", ":"}},
+ // We do extra checks for '>', ':' and '/' in completion to
+ // only trigger on '->', '::' and '#include .../'.
+ {"triggerCharacters", {".", ">", ":", "/"}},
}},
{"signatureHelpProvider",
llvm::json::Object{
@@ -1056,7 +1057,7 @@
const CompletionParams &Params) const {
llvm::StringRef Trigger = Params.context.triggerCharacter;
if (Params.context.triggerKind != CompletionTriggerKind::TriggerCharacter ||
- (Trigger != ">" && Trigger != ":"))
+ (Trigger != ">" && Trigger != ":" && Trigger != "/"))
return true;
auto Code = DraftMgr.getDraft(Params.textDocument.uri.file());
@@ -1083,6 +1084,16 @@
return (*Code)[*Offset - 2] == '-'; // trigger only on '->'.
if (Trigger == ":")
return (*Code)[*Offset - 2] == ':'; // trigger only on '::'.
+
+ if (Trigger == "/") { // trigger only on '#include.../'
+ auto StartOfLine = Code->rfind('\n', *Offset);
+ if (StartOfLine == llvm::StringRef::npos) {
+ StartOfLine = 0;
+ llvm::Regex IncPattern("^#[[:space:]]*include.*");
+ return IncPattern.match(
+ llvm::StringRef(Code->data() + StartOfLine, *Offset - StartOfLine));
+ }
+ }
assert(false && "unhandled trigger character");
return true;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
