[clang-tools-extra] r322387 - [clangd] Code completion uses Sema for NS-level things in the current file.
Mikhail Zolotukhin via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 16 15:23:11 PST 2018
Hi,
ClangdTests/FuzzyMatch.Matches has been failing since this commit on a sanitized clang build on MacOS. I've seen some later commits trying to fix it, but the issue is still there. Could you please take a look?
Steps to reproduce in case you need them:
> cmake -G Ninja -DLIBCXX_SYSROOT=/Path/To/MacOSX.sdk -DCOMPILER_RT_BUILD_BUILTINS=Off -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=Off -DLLVM_USE_SANITIZER="Address;Undefined" -DLLVM_BUILD_RUNTIME=NO /Path/To/llvm
> ninja && ninja tools/clang/tools/extra/unittests/clangd/ClangdTests
> tools/clang/tools/extra/unittests/clangd/ClangdTests
[==========] Running 90 tests from 19 test cases.
[----------] Global test environment set-up.
[----------] 8 tests from ClangdVFSTest
[ RUN ] ClangdVFSTest.Parse
[ OK ] ClangdVFSTest.Parse (84 ms)
[ RUN ] ClangdVFSTest.ParseWithHeader
[ OK ] ClangdVFSTest.ParseWithHeader (98 ms)
[ RUN ] ClangdVFSTest.Reparse
[ OK ] ClangdVFSTest.Reparse (49 ms)
[ RUN ] ClangdVFSTest.ReparseOnHeaderChange
[ OK ] ClangdVFSTest.ReparseOnHeaderChange (65 ms)
[ RUN ] ClangdVFSTest.CheckVersions
.../tools/extra/clangd/FuzzyMatch.cpp:78:27: runtime error: division by zero
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior .../tools/extra/clangd/FuzzyMatch.cpp:78:27 in
Abort trap: 6
Thanks,
Michael
> On Jan 12, 2018, at 10:30 AM, Sam McCall via cfe-commits <cfe-commits at lists.llvm.org> wrote:
>
> Author: sammccall
> Date: Fri Jan 12 10:30:08 2018
> New Revision: 322387
>
> URL: http://llvm.org/viewvc/llvm-project?rev=322387&view=rev
> Log:
> [clangd] Code completion uses Sema for NS-level things in the current file.
>
> Summary:
> To stay fast, it avoids deserializing anything outside the current file, by
> disabling the LoadExternal code completion option added in r322377, when the
> index is enabled.
>
> Reviewers: hokein
>
> Subscribers: klimek, ilya-biryukov, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D41996
>
> Modified:
> clang-tools-extra/trunk/clangd/CodeComplete.cpp
> clang-tools-extra/trunk/clangd/index/FileIndex.cpp
> clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
>
> Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=322387&r1=322386&r2=322387&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
> +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Fri Jan 12 10:30:08 2018
> @@ -642,8 +642,10 @@ clang::CodeCompleteOptions CodeCompleteO
> Result.IncludeGlobals = IncludeGlobals;
> Result.IncludeBriefComments = IncludeBriefComments;
>
> - // Enable index-based code completion when Index is provided.
> - Result.IncludeNamespaceLevelDecls = !Index;
> + // When an is used, Sema is responsible for completing the main file,
> + // the index can provide results from the preamble.
> + // Tell Sema not to deserialize the preamble to look for results.
> + Result.LoadExternal = !Index;
>
> return Result;
> }
>
> Modified: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.cpp?rev=322387&r1=322386&r2=322387&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original)
> +++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Fri Jan 12 10:30:08 2018
> @@ -20,6 +20,8 @@ std::unique_ptr<SymbolSlab> indexAST(AST
> std::shared_ptr<Preprocessor> PP,
> llvm::ArrayRef<const Decl *> Decls) {
> SymbolCollector::Options CollectorOpts;
> + // Code completion gets main-file results from Sema.
> + // But we leave this option on because features like go-to-definition want it.
> CollectorOpts.IndexMainFiles = true;
> auto Collector = std::make_shared<SymbolCollector>(std::move(CollectorOpts));
> Collector->setPreprocessor(std::move(PP));
>
> Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=322387&r1=322386&r2=322387&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
> +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Fri Jan 12 10:30:08 2018
> @@ -57,6 +57,7 @@ using ::testing::Contains;
> using ::testing::Each;
> using ::testing::ElementsAre;
> using ::testing::Not;
> +using ::testing::UnorderedElementsAre;
>
> class IgnoreDiagnostics : public DiagnosticsConsumer {
> void
> @@ -104,7 +105,7 @@ CompletionList completions(StringRef Tex
> /*StorePreamblesInMemory=*/true);
> auto File = getVirtualTestFilePath("foo.cpp");
> Annotations Test(Text);
> - Server.addDocument(Context::empty(), File, Test.code());
> + Server.addDocument(Context::empty(), File, Test.code()).wait();
> auto CompletionList =
> Server.codeComplete(Context::empty(), File, Test.point(), Opts)
> .get()
> @@ -506,11 +507,11 @@ TEST(CompletionTest, NoIndex) {
> Opts.Index = nullptr;
>
> auto Results = completions(R"cpp(
> - namespace ns { class No {}; }
> + namespace ns { class Local {}; }
> void f() { ns::^ }
> )cpp",
> Opts);
> - EXPECT_THAT(Results.items, Has("No"));
> + EXPECT_THAT(Results.items, Has("Local"));
> }
>
> TEST(CompletionTest, StaticAndDynamicIndex) {
> @@ -538,13 +539,13 @@ TEST(CompletionTest, SimpleIndexBased) {
> Opts.Index = I.get();
>
> auto Results = completions(R"cpp(
> - namespace ns { class No {}; }
> + namespace ns { int local; }
> void f() { ns::^ }
> )cpp",
> Opts);
> EXPECT_THAT(Results.items, Has("XYZ", CompletionItemKind::Class));
> EXPECT_THAT(Results.items, Has("foo", CompletionItemKind::Function));
> - EXPECT_THAT(Results.items, Not(Has("No")));
> + EXPECT_THAT(Results.items, Has("local"));
> }
>
> TEST(CompletionTest, IndexBasedWithFilter) {
> @@ -585,6 +586,41 @@ TEST(CompletionTest, FullyQualifiedScope
> EXPECT_THAT(Results.items, Has("XYZ", CompletionItemKind::Class));
> }
>
> +TEST(CompletionTest, IndexSuppressesPreambleCompletions) {
> + MockFSProvider FS;
> + MockCompilationDatabase CDB;
> + IgnoreDiagnostics DiagConsumer;
> + ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
> + /*StorePreamblesInMemory=*/true);
> +
> + FS.Files[getVirtualTestFilePath("bar.h")] =
> + R"cpp(namespace ns { int preamble; })cpp";
> + auto File = getVirtualTestFilePath("foo.cpp");
> + Annotations Test(R"cpp(
> + #include "bar.h"
> + namespace ns { int local; }
> + void f() { ns::^ }
> + )cpp");
> + Server.addDocument(Context::empty(), File, Test.code()).wait();
> + clangd::CodeCompleteOptions Opts = {};
> +
> + auto WithoutIndex =
> + Server.codeComplete(Context::empty(), File, Test.point(), Opts)
> + .get()
> + .second.Value;
> + EXPECT_THAT(WithoutIndex.items,
> + UnorderedElementsAre(Named("local"), Named("preamble")));
> +
> + auto I = simpleIndexFromSymbols({{"ns::index", index::SymbolKind::Variable}});
> + Opts.Index = I.get();
> + auto WithIndex =
> + Server.codeComplete(Context::empty(), File, Test.point(), Opts)
> + .get()
> + .second.Value;
> + EXPECT_THAT(WithIndex.items,
> + UnorderedElementsAre(Named("local"), Named("index")));
> +}
> +
> TEST(CompletionTest, ASTIndexMultiFile) {
> MockFSProvider FS;
> MockCompilationDatabase CDB;
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180116/dc620939/attachment-0001.html>
More information about the cfe-commits
mailing list