sammccall updated this revision to Diff 145478.
sammccall marked 9 inline comments as done.
sammccall added a comment.
Address comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46524
Files:
clang-move/ClangMove.cpp
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyOptions.cpp
clang-tidy/ClangTidyOptions.h
clang-tidy/bugprone/LambdaFunctionNameCheck.cpp
clang-tidy/bugprone/MultipleStatementMacroCheck.cpp
clang-tidy/google/IntegerTypesCheck.cpp
clang-tidy/modernize/RawStringLiteralCheck.cpp
clang-tidy/modernize/RawStringLiteralCheck.h
clang-tidy/modernize/UseNoexceptCheck.cpp
clang-tidy/modernize/UseNullptrCheck.cpp
clang-tidy/objc/PropertyDeclarationCheck.cpp
clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
clang-tidy/tool/ClangTidyMain.cpp
clangd/AST.cpp
clangd/CMakeLists.txt
clangd/ClangdServer.cpp
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/CodeComplete.cpp
clangd/Protocol.h
clangd/Quality.cpp
clangd/Quality.h
clangd/SourceCode.cpp
clangd/SourceCode.h
clangd/XRefs.cpp
clangd/XRefs.h
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/MemIndex.cpp
clangd/index/SymbolCollector.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/index.rst
test/clang-tidy/google-runtime-int.cpp
test/clang-tidy/modernize-raw-string-literal.cpp
test/clang-tidy/objc-property-declaration.m
test/clang-tidy/temporaries.cpp
test/clangd/rename.test
unittests/clang-tidy/ClangTidyOptionsTest.cpp
unittests/clangd/CMakeLists.txt
unittests/clangd/ClangdUnitTests.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/DraftStoreTests.cpp
unittests/clangd/FileIndexTests.cpp
unittests/clangd/QualityTests.cpp
unittests/clangd/SourceCodeTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
unittests/clangd/TestFS.cpp
unittests/clangd/TestTU.cpp
unittests/clangd/TestTU.h
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
===================================================================
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -12,10 +12,11 @@
#include "Matchers.h"
#include "SyncAPI.h"
#include "TestFS.h"
+#include "TestTU.h"
#include "XRefs.h"
-#include "clang/Frontend/CompilerInvocation.h"
-#include "clang/Frontend/PCHContainerOperations.h"
-#include "clang/Frontend/Utils.h"
+#include "index/FileIndex.h"
+#include "index/SymbolCollector.h"
+#include "clang/Index/IndexingAction.h"
#include "llvm/Support/Path.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -36,18 +37,6 @@
std::vector<Diag> Diagnostics) override {}
};
-// FIXME: this is duplicated with FileIndexTests. Share it.
-ParsedAST build(StringRef Code) {
- auto CI = createInvocationFromCommandLine(
- {"clang", "-xc++", testPath("Foo.cpp").c_str()});
- auto Buf = MemoryBuffer::getMemBuffer(Code);
- auto AST = ParsedAST::Build(std::move(CI), nullptr, std::move(Buf),
- std::make_shared<PCHContainerOperations>(),
- vfs::getRealFileSystem());
- assert(AST.hasValue());
- return std::move(*AST);
-}
-
// Extracts ranges from an annotated example, and constructs a matcher for a
// highlight set. Ranges should be named $read/$write as appropriate.
Matcher<const std::vector<DocumentHighlight> &>
@@ -98,14 +87,75 @@
};
for (const char *Test : Tests) {
Annotations T(Test);
- auto AST = build(T.code());
+ auto AST = TestTU::withCode(T.code()).build();
EXPECT_THAT(findDocumentHighlights(AST, T.point()), HighlightsFrom(T))
<< Test;
}
}
MATCHER_P(RangeIs, R, "") { return arg.range == R; }
+TEST(GoToDefinition, WithIndex) {
+ Annotations SymbolHeader(R"cpp(
+ class $forward[[Forward]];
+ class $foo[[Foo]] {};
+
+ void $f1[[f1]]();
+
+ inline void $f2[[f2]]() {}
+ )cpp");
+ Annotations SymbolCpp(R"cpp(
+ class $forward[[forward]] {};
+ void $f1[[f1]]() {}
+ )cpp");
+
+ TestTU TU;
+ TU.Code = SymbolCpp.code();
+ TU.HeaderCode = SymbolHeader.code();
+ auto Index = TU.index();
+ auto runFindDefinitionsWithIndex = [&Index](const Annotations &Main) {
+ auto AST = TestTU::withCode(Main.code()).build();
+ return clangd::findDefinitions(AST, Main.point(), Index.get());
+ };
+
+ Annotations Test(R"cpp(// only declaration in AST.
+ void [[f1]]();
+ int main() {
+ ^f1();
+ }
+ )cpp");
+ EXPECT_THAT(runFindDefinitionsWithIndex(Test),
+ testing::ElementsAreArray(
+ {RangeIs(SymbolCpp.range("f1")), RangeIs(Test.range())}));
+
+ Test = Annotations(R"cpp(// definition in AST.
+ void [[f1]]() {}
+ int main() {
+ ^f1();
+ }
+ )cpp");
+ EXPECT_THAT(runFindDefinitionsWithIndex(Test),
+ testing::ElementsAreArray(
+ {RangeIs(Test.range()), RangeIs(SymbolHeader.range("f1"))}));
+
+ Test = Annotations(R"cpp(// forward declaration in AST.
+ class [[Foo]];
+ F^oo* create();
+ )cpp");
+ EXPECT_THAT(runFindDefinitionsWithIndex(Test),
+ testing::ElementsAreArray(
+ {RangeIs(SymbolHeader.range("foo")), RangeIs(Test.range())}));
+
+ Test = Annotations(R"cpp(// defintion in AST.
+ class [[Forward]] {};
+ F^orward create();
+ )cpp");
+ EXPECT_THAT(runFindDefinitionsWithIndex(Test),
+ testing::ElementsAreArray({
+ RangeIs(Test.range()), RangeIs(SymbolHeader.range("forward")),
+ }));
+}
+
TEST(GoToDefinition, All) {
const char *Tests[] = {
R"cpp(// Local variable
@@ -251,7 +301,7 @@
};
for (const char *Test : Tests) {
Annotations T(Test);
- auto AST = build(T.code());
+ auto AST = TestTU::withCode(T.code()).build();
std::vector<Matcher<Location>> ExpectedLocations;
for (const auto &R : T.ranges())
ExpectedLocations.push_back(RangeIs(R));
@@ -583,7 +633,7 @@
for (const OneTest &Test : Tests) {
Annotations T(Test.Input);
- auto AST = build(T.code());
+ auto AST = TestTU::withCode(T.code()).build();
Hover H = getHover(AST, T.point());
EXPECT_EQ(H.contents.value, Test.ExpectedHover) << Test.Input;
Index: unittests/clangd/TestTU.h
===================================================================
--- /dev/null
+++ unittests/clangd/TestTU.h
@@ -0,0 +1,59 @@
+//===--- TestTU.h - Scratch source files for testing ------------*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===---------------------------------------------------------------------===//
+//
+// Many tests for indexing, code completion etc are most naturally expressed
+// using code examples.
+// TestTU lets test define these examples in a common way without dealing with
+// the mechanics of VFS and compiler interactions, and then easily grab the
+// AST, particular symbols, etc.
+//
+//===---------------------------------------------------------------------===//
+#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
+#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
+#include "ClangdUnit.h"
+#include "index/Index.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+
+struct TestTU {
+ static TestTU withCode(llvm::StringRef Code) {
+ TestTU TU;
+ TU.Code = Code;
+ return TU;
+ }
+
+ static TestTU withHeaderCode(llvm::StringRef HeaderCode) {
+ TestTU TU;
+ TU.HeaderCode = HeaderCode;
+ return TU;
+ }
+
+ // The code to be compiled.
+ std::string Code;
+ std::string Filename = "TestTU.cpp";
+
+ // Define contents of a header to be included by TestTU.cpp.
+ std::string HeaderCode;
+ std::string HeaderFilename = "TestTU.h";
+
+ ParsedAST build() const;
+ SymbolSlab headerSymbols() const;
+ std::unique_ptr<SymbolIndex> index() const;
+};
+
+// Look up an index symbol by qualified name, which must be unique.
+const Symbol &findSymbol(const SymbolSlab &, llvm::StringRef QName);
+// Look up an AST symbol by qualified name, which must be unique and top-level.
+const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName);
+
+} // namespace clangd
+} // namespace clang
+#endif
Index: unittests/clangd/TestTU.cpp
===================================================================
--- /dev/null
+++ unittests/clangd/TestTU.cpp
@@ -0,0 +1,95 @@
+//===--- TestTU.cpp - Scratch source files for testing ------------*-
+//C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===---------------------------------------------------------------------===//
+#include "TestTU.h"
+#include "TestFS.h"
+#include "index/FileIndex.h"
+#include "index/MemIndex.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Frontend/Utils.h"
+
+namespace clang {
+namespace clangd {
+using namespace llvm;
+
+ParsedAST TestTU::build() const {
+ std::string FullFilename = testPath(Filename),
+ FullHeaderName = testPath(HeaderFilename);
+ std::vector<const char *> Cmd = {"clang", FullFilename.c_str()};
+ // FIXME: this shouldn't need to be conditional, but it breaks a
+ // GoToDefinition test for some reason (getMacroArgExpandedLocation fails).
+ if (!HeaderCode.empty()) {
+ Cmd.push_back("-include");
+ Cmd.push_back(FullHeaderName.c_str());
+ }
+ auto AST = ParsedAST::Build(
+ createInvocationFromCommandLine(Cmd), nullptr,
+ MemoryBuffer::getMemBufferCopy(Code),
+ std::make_shared<PCHContainerOperations>(),
+ buildTestFS({{FullFilename, Code}, {FullHeaderName, HeaderCode}}));
+ if (!AST.hasValue()) {
+ ADD_FAILURE() << "Failed to build code:\n" << Code;
+ llvm_unreachable("Failed to build TestTU!");
+ }
+ return std::move(*AST);
+}
+
+SymbolSlab TestTU::headerSymbols() const {
+ auto AST = build();
+ return indexAST(&AST);
+}
+
+std::unique_ptr<SymbolIndex> TestTU::index() const {
+ return MemIndex::build(headerSymbols());
+}
+
+// Look up a symbol by qualified name, which must be unique.
+const Symbol &findSymbol(const SymbolSlab &Slab, llvm::StringRef QName) {
+ const Symbol *Result = nullptr;
+ for (const Symbol &S : Slab) {
+ if (QName != (S.Scope + S.Name).str())
+ continue;
+ if (Result) {
+ ADD_FAILURE() << "Multiple symbols named " << QName << ":\n"
+ << *Result << "\n---\n"
+ << S;
+ assert(false && "QName is not unique");
+ }
+ Result = &S;
+ }
+ if (!Result) {
+ ADD_FAILURE() << "No symbol named " << QName << " in "
+ << ::testing::PrintToString(Slab);
+ assert(false && "No symbol with QName");
+ }
+ return *Result;
+}
+
+const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName) {
+ const NamedDecl *Result = nullptr;
+ for (const Decl *D : AST.getTopLevelDecls()) {
+ auto *ND = dyn_cast<NamedDecl>(D);
+ if (!ND || ND->getNameAsString() != QName)
+ continue;
+ if (Result) {
+ ADD_FAILURE() << "Multiple Decls named " << QName;
+ assert(false && "QName is not unique");
+ }
+ Result = ND;
+ }
+ if (!Result) {
+ ADD_FAILURE() << "No Decl named " << QName << " in AST";
+ assert(false && "No Decl with QName");
+ }
+ return *Result;
+}
+
+} // namespace clangd
+} // namespace clang
Index: unittests/clangd/TestFS.cpp
===================================================================
--- unittests/clangd/TestFS.cpp
+++ unittests/clangd/TestFS.cpp
@@ -20,8 +20,8 @@
new vfs::InMemoryFileSystem);
for (auto &FileAndContents : Files) {
MemFS->addFile(FileAndContents.first(), time_t(),
- MemoryBuffer::getMemBuffer(FileAndContents.second,
- FileAndContents.first()));
+ MemoryBuffer::getMemBufferCopy(FileAndContents.second,
+ FileAndContents.first()));
}
return MemFS;
}
Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -689,6 +689,15 @@
IncludeHeader(TestHeaderURI), DefURI(TestFileURI))));
}
+TEST_F(SymbolCollectorTest, UTF16Character) {
+ // ö is 2-bytes.
+ Annotations Header(/*Header=*/"class [[pörk]] {};");
+ runSymbolCollector(Header.code(), /*Main=*/"");
+ EXPECT_THAT(Symbols, UnorderedElementsAre(
+ AllOf(QName("pörk"), DeclRange(Header.range()))));
+}
+
+
} // namespace
} // namespace clangd
} // namespace clang
Index: unittests/clangd/SourceCodeTests.cpp
===================================================================
--- unittests/clangd/SourceCodeTests.cpp
+++ unittests/clangd/SourceCodeTests.cpp
@@ -24,9 +24,10 @@
return arg.line == Line && arg.character == Col;
}
+// The = → 🡆 below are ASCII (1 byte), BMP (3 bytes), and astral (4 bytes).
const char File[] = R"(0:0 = 0
-1:0 = 8
-2:0 = 16)";
+1:0 → 8
+2:0 🡆 18)";
/// A helper to make tests easier to read.
Position position(int line, int character) {
@@ -66,25 +67,31 @@
EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 3), false),
HasValue(11));
EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 6)),
- HasValue(14)); // last character
+ HasValue(16)); // last character
EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 7)),
- HasValue(15)); // the newline itself
+ HasValue(17)); // the newline itself
EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 8)),
- HasValue(15)); // out of range
+ HasValue(17)); // out of range
EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 8), false),
Failed()); // out of range
// last line
EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, -1)),
Failed()); // out of range
EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 0)),
- HasValue(16)); // first character
+ HasValue(18)); // first character
EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 3)),
- HasValue(19)); // middle character
- EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 7)),
- HasValue(23)); // last character
+ HasValue(21)); // middle character
+ EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 5), false),
+ Failed()); // middle of surrogate pair
+ EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 5)),
+ HasValue(26)); // middle of surrogate pair
+ EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 6), false),
+ HasValue(26)); // end of surrogate pair
EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 8)),
- HasValue(24)); // EOF
- EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 9), false),
+ HasValue(28)); // last character
+ EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 9)),
+ HasValue(29)); // EOF
+ EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 10), false),
Failed()); // out of range
// line out of bounds
EXPECT_THAT_EXPECTED(positionToOffset(File, position(3, 0)), Failed());
@@ -97,14 +104,19 @@
EXPECT_THAT(offsetToPosition(File, 6), Pos(0, 6)) << "end of first line";
EXPECT_THAT(offsetToPosition(File, 7), Pos(0, 7)) << "first newline";
EXPECT_THAT(offsetToPosition(File, 8), Pos(1, 0)) << "start of second line";
- EXPECT_THAT(offsetToPosition(File, 11), Pos(1, 3)) << "in second line";
- EXPECT_THAT(offsetToPosition(File, 14), Pos(1, 6)) << "end of second line";
- EXPECT_THAT(offsetToPosition(File, 15), Pos(1, 7)) << "second newline";
- EXPECT_THAT(offsetToPosition(File, 16), Pos(2, 0)) << "start of last line";
- EXPECT_THAT(offsetToPosition(File, 19), Pos(2, 3)) << "in last line";
- EXPECT_THAT(offsetToPosition(File, 23), Pos(2, 7)) << "end of last line";
- EXPECT_THAT(offsetToPosition(File, 24), Pos(2, 8)) << "EOF";
- EXPECT_THAT(offsetToPosition(File, 25), Pos(2, 8)) << "out of bounds";
+ EXPECT_THAT(offsetToPosition(File, 12), Pos(1, 4)) << "before BMP char";
+ EXPECT_THAT(offsetToPosition(File, 13), Pos(1, 5)) << "in BMP char";
+ EXPECT_THAT(offsetToPosition(File, 15), Pos(1, 5)) << "after BMP char";
+ EXPECT_THAT(offsetToPosition(File, 16), Pos(1, 6)) << "end of second line";
+ EXPECT_THAT(offsetToPosition(File, 17), Pos(1, 7)) << "second newline";
+ EXPECT_THAT(offsetToPosition(File, 18), Pos(2, 0)) << "start of last line";
+ EXPECT_THAT(offsetToPosition(File, 21), Pos(2, 3)) << "in last line";
+ EXPECT_THAT(offsetToPosition(File, 22), Pos(2, 4)) << "before astral char";
+ EXPECT_THAT(offsetToPosition(File, 24), Pos(2, 6)) << "in astral char";
+ EXPECT_THAT(offsetToPosition(File, 26), Pos(2, 6)) << "after astral char";
+ EXPECT_THAT(offsetToPosition(File, 28), Pos(2, 8)) << "end of last line";
+ EXPECT_THAT(offsetToPosition(File, 29), Pos(2, 9)) << "EOF";
+ EXPECT_THAT(offsetToPosition(File, 30), Pos(2, 9)) << "out of bounds";
}
} // namespace
Index: unittests/clangd/QualityTests.cpp
===================================================================
--- /dev/null
+++ unittests/clangd/QualityTests.cpp
@@ -0,0 +1,123 @@
+//===-- SourceCodeTests.cpp ------------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Evaluating scoring functions isn't a great fit for assert-based tests.
+// For interesting cases, both exact scores and "X beats Y" are too brittle to
+// make good hard assertions.
+//
+// Here we test the signal extraction and sanity-check that signals point in
+// the right direction. This should be supplemented by quality metrics which
+// we can compute from a corpus of queries and preferred rankings.
+//
+//===----------------------------------------------------------------------===//
+
+#include "Quality.h"
+#include "TestTU.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(QualityTests, SymbolQualitySignalExtraction) {
+ auto Header = TestTU::withHeaderCode(R"cpp(
+ int x;
+
+ [[deprecated]]
+ int f() { return x; }
+ )cpp");
+ auto Symbols = Header.headerSymbols();
+ auto AST = Header.build();
+
+ SymbolQualitySignals Quality;
+ Quality.merge(findSymbol(Symbols, "x"));
+ EXPECT_FALSE(Quality.Deprecated);
+ EXPECT_EQ(Quality.SemaCCPriority, SymbolQualitySignals().SemaCCPriority);
+ EXPECT_EQ(Quality.References, SymbolQualitySignals().References);
+
+ Symbol F = findSymbol(Symbols, "f");
+ F.References = 24; // TestTU doesn't count references, so fake it.
+ Quality = {};
+ Quality.merge(F);
+ EXPECT_FALSE(Quality.Deprecated); // FIXME: Include deprecated bit in index.
+ EXPECT_EQ(Quality.SemaCCPriority, SymbolQualitySignals().SemaCCPriority);
+ EXPECT_EQ(Quality.References, 24u);
+
+ Quality = {};
+ Quality.merge(CodeCompletionResult(&findDecl(AST, "f"), /*Priority=*/42));
+ EXPECT_TRUE(Quality.Deprecated);
+ EXPECT_EQ(Quality.SemaCCPriority, 42u);
+ EXPECT_EQ(Quality.References, SymbolQualitySignals().References);
+}
+
+TEST(QualityTests, SymbolRelevanceSignalExtraction) {
+ auto AST = TestTU::withHeaderCode(R"cpp(
+ [[deprecated]]
+ int f() { return 0; }
+ )cpp")
+ .build();
+
+ SymbolRelevanceSignals Relevance;
+ Relevance.merge(CodeCompletionResult(&findDecl(AST, "f"), /*Priority=*/42,
+ nullptr, false, /*Accessible=*/false));
+ EXPECT_EQ(Relevance.NameMatch, SymbolRelevanceSignals().NameMatch);
+ EXPECT_TRUE(Relevance.Forbidden);
+}
+
+// Do the signals move the scores in the direction we expect?
+TEST(QualityTests, SymbolQualitySignalsSanity) {
+ SymbolQualitySignals Default;
+ EXPECT_EQ(Default.evaluate(), 1);
+
+ SymbolQualitySignals Deprecated;
+ Deprecated.Deprecated = true;
+ EXPECT_LT(Deprecated.evaluate(), Default.evaluate());
+
+ SymbolQualitySignals WithReferences, ManyReferences;
+ WithReferences.References = 10;
+ ManyReferences.References = 1000;
+ EXPECT_GT(WithReferences.evaluate(), Default.evaluate());
+ EXPECT_GT(ManyReferences.evaluate(), WithReferences.evaluate());
+
+ SymbolQualitySignals LowPriority, HighPriority;
+ LowPriority.SemaCCPriority = 60;
+ HighPriority.SemaCCPriority = 20;
+ EXPECT_GT(HighPriority.evaluate(), Default.evaluate());
+ EXPECT_LT(LowPriority.evaluate(), Default.evaluate());
+}
+
+TEST(QualityTests, SymbolRelevanceSignalsSanity) {
+ SymbolRelevanceSignals Default;
+ EXPECT_EQ(Default.evaluate(), 1);
+
+ SymbolRelevanceSignals Forbidden;
+ Forbidden.Forbidden = true;
+ EXPECT_LT(Forbidden.evaluate(), Default.evaluate());
+
+ SymbolRelevanceSignals PoorNameMatch;
+ PoorNameMatch.NameMatch = 0.2;
+ EXPECT_LT(PoorNameMatch.evaluate(), Default.evaluate());
+}
+
+TEST(QualityTests, SortText) {
+ EXPECT_LT(sortText(std::numeric_limits<float>::infinity()), sortText(1000.2));
+ EXPECT_LT(sortText(1000.2), sortText(1));
+ EXPECT_LT(sortText(1), sortText(0.3));
+ EXPECT_LT(sortText(0.3), sortText(0));
+ EXPECT_LT(sortText(0), sortText(-10));
+ EXPECT_LT(sortText(-10), sortText(-std::numeric_limits<float>::infinity()));
+
+ EXPECT_LT(sortText(1, "z"), sortText(0, "a"));
+ EXPECT_LT(sortText(0, "a"), sortText(0, "z"));
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: unittests/clangd/FileIndexTests.cpp
===================================================================
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -7,11 +7,8 @@
//
//===----------------------------------------------------------------------===//
-#include "TestFS.h"
+#include "TestTU.h"
#include "index/FileIndex.h"
-#include "clang/Frontend/CompilerInvocation.h"
-#include "clang/Frontend/PCHContainerOperations.h"
-#include "clang/Frontend/Utils.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -83,38 +80,19 @@
return Matches;
}
-/// Create an ParsedAST for \p Code. Returns None if \p Code is empty.
-/// \p Code is put into <Path>.h which is included by \p <BasePath>.cpp.
-llvm::Optional<ParsedAST> build(llvm::StringRef BasePath,
- llvm::StringRef Code) {
- if (Code.empty())
- return llvm::None;
-
- assert(llvm::sys::path::extension(BasePath).empty() &&
- "BasePath must be a base file path without extension.");
- llvm::IntrusiveRefCntPtr<vfs::InMemoryFileSystem> VFS(
- new vfs::InMemoryFileSystem);
- std::string Path = testPath((BasePath + ".cpp").str());
- std::string Header = testPath((BasePath + ".h").str());
- VFS->addFile(Path, 0, llvm::MemoryBuffer::getMemBuffer(""));
- VFS->addFile(Header, 0, llvm::MemoryBuffer::getMemBuffer(Code));
- const char *Args[] = {"clang", "-xc++", "-include", Header.c_str(),
- Path.c_str()};
-
- auto CI = createInvocationFromCommandLine(Args);
-
- auto Buf = llvm::MemoryBuffer::getMemBuffer(Code);
- auto AST = ParsedAST::Build(std::move(CI), nullptr, std::move(Buf),
- std::make_shared<PCHContainerOperations>(), VFS);
- assert(AST.hasValue());
- return std::move(*AST);
+// Adds Basename.cpp, which includes Basename.h, which contains Code.
+void update(FileIndex &M, llvm::StringRef Basename, llvm::StringRef Code) {
+ TestTU File;
+ File.Filename = (Basename + ".cpp").str();
+ File.HeaderFilename = (Basename + ".h").str();
+ File.HeaderCode = Code;
+ auto AST = File.build();
+ M.update(File.Filename, &AST);
}
TEST(FileIndexTest, IndexAST) {
FileIndex M;
- M.update(
- "f1",
- build("f1", "namespace ns { void f() {} class X {}; }").getPointer());
+ update(M, "f1", "namespace ns { void f() {} class X {}; }");
FuzzyFindRequest Req;
Req.Query = "";
@@ -124,24 +102,17 @@
TEST(FileIndexTest, NoLocal) {
FileIndex M;
- M.update(
- "f1",
- build("f1", "namespace ns { void f() { int local = 0; } class X {}; }")
- .getPointer());
+ update(M, "f1", "namespace ns { void f() { int local = 0; } class X {}; }");
FuzzyFindRequest Req;
Req.Query = "";
EXPECT_THAT(match(M, Req), UnorderedElementsAre("ns", "ns::f", "ns::X"));
}
TEST(FileIndexTest, IndexMultiASTAndDeduplicate) {
FileIndex M;
- M.update(
- "f1",
- build("f1", "namespace ns { void f() {} class X {}; }").getPointer());
- M.update(
- "f2",
- build("f2", "namespace ns { void ff() {} class X {}; }").getPointer());
+ update(M, "f1", "namespace ns { void f() {} class X {}; }");
+ update(M, "f2", "namespace ns { void ff() {} class X {}; }");
FuzzyFindRequest Req;
Req.Query = "";
@@ -151,39 +122,35 @@
TEST(FileIndexTest, RemoveAST) {
FileIndex M;
- M.update(
- "f1",
- build("f1", "namespace ns { void f() {} class X {}; }").getPointer());
+ update(M, "f1", "namespace ns { void f() {} class X {}; }");
FuzzyFindRequest Req;
Req.Query = "";
Req.Scopes = {"ns::"};
EXPECT_THAT(match(M, Req), UnorderedElementsAre("ns::f", "ns::X"));
- M.update("f1", nullptr);
+ M.update("f1.cpp", nullptr);
EXPECT_THAT(match(M, Req), UnorderedElementsAre());
}
TEST(FileIndexTest, RemoveNonExisting) {
FileIndex M;
- M.update("no", nullptr);
+ M.update("no.cpp", nullptr);
EXPECT_THAT(match(M, FuzzyFindRequest()), UnorderedElementsAre());
}
TEST(FileIndexTest, IgnoreClassMembers) {
FileIndex M;
- M.update("f1",
- build("f1", "class X { static int m1; int m2; static void f(); };")
- .getPointer());
+ update(M, "f1", "class X { static int m1; int m2; static void f(); };");
FuzzyFindRequest Req;
Req.Query = "";
EXPECT_THAT(match(M, Req), UnorderedElementsAre("X"));
}
TEST(FileIndexTest, NoIncludeCollected) {
FileIndex M;
- M.update("f", build("f", "class string {};").getPointer());
+ update(M, "f", "class string {};");
FuzzyFindRequest Req;
Req.Query = "";
@@ -206,7 +173,7 @@
)cpp";
FileIndex M;
- M.update("f", build("f", Source).getPointer());
+ update(M, "f", Source);
FuzzyFindRequest Req;
Req.Query = "";
Index: unittests/clangd/DraftStoreTests.cpp
===================================================================
--- unittests/clangd/DraftStoreTests.cpp
+++ unittests/clangd/DraftStoreTests.cpp
@@ -241,7 +241,7 @@
EXPECT_TRUE(!Result);
EXPECT_EQ(llvm::toString(Result.takeError()),
- "Character value is out of range (100)");
+ "UTF-16 offset 100 is invalid for line 0");
}
TEST(DraftStoreIncrementalUpdateTest, EndCharOutOfRange) {
@@ -262,7 +262,7 @@
EXPECT_TRUE(!Result);
EXPECT_EQ(llvm::toString(Result.takeError()),
- "Character value is out of range (100)");
+ "UTF-16 offset 100 is invalid for line 0");
}
TEST(DraftStoreIncrementalUpdateTest, StartLineOutOfRange) {
@@ -338,7 +338,7 @@
EXPECT_TRUE(!Result);
EXPECT_EQ(llvm::toString(Result.takeError()),
- "Character value is out of range (100)");
+ "UTF-16 offset 100 is invalid for line 0");
llvm::Optional<std::string> Contents = DS.getDraft(File);
EXPECT_TRUE(Contents);
Index: unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -144,6 +144,13 @@
Symbol var(StringRef Name) {
return sym(Name, index::SymbolKind::Variable, "@\\0");
}
+Symbol ns(StringRef Name) {
+ return sym(Name, index::SymbolKind::Namespace, "@N@\\0");
+}
+Symbol withReferences(int N, Symbol S) {
+ S.References = N;
+ return S;
+}
TEST(CompletionTest, Limit) {
clangd::CodeCompleteOptions Opts;
@@ -443,6 +450,14 @@
UnorderedElementsAre(AllOf(Named("XYZ"), Filter("XYZ"))));
}
+TEST(CompletionTest, ReferencesAffectRanking) {
+ auto Results = completions("int main() { abs^ }", {ns("absl"), func("abs")});
+ EXPECT_THAT(Results.items, HasSubsequence(Named("abs"), Named("absl")));
+ Results = completions("int main() { abs^ }",
+ {withReferences(10000, ns("absl")), func("abs")});
+ EXPECT_THAT(Results.items, HasSubsequence(Named("absl"), Named("abs")));
+}
+
TEST(CompletionTest, GlobalQualified) {
auto Results = completions(
R"cpp(
Index: unittests/clangd/ClangdUnitTests.cpp
===================================================================
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -9,10 +9,8 @@
#include "Annotations.h"
#include "ClangdUnit.h"
-#include "TestFS.h"
-#include "clang/Frontend/CompilerInvocation.h"
-#include "clang/Frontend/PCHContainerOperations.h"
-#include "clang/Frontend/Utils.h"
+#include "SourceCode.h"
+#include "TestTU.h"
#include "llvm/Support/ScopedPrinter.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -35,24 +33,6 @@
return Field(&Diag::Notes, ElementsAre(NoteMatcher));
}
-// FIXME: this is duplicated with FileIndexTests. Share it.
-ParsedAST build(StringRef Code, std::vector<const char *> Flags = {}) {
- std::vector<const char *> Cmd = {"clang", "main.cpp"};
- Cmd.insert(Cmd.begin() + 1, Flags.begin(), Flags.end());
- auto CI = createInvocationFromCommandLine(Cmd);
- auto Buf = MemoryBuffer::getMemBuffer(Code);
- auto AST = ParsedAST::Build(std::move(CI), nullptr, std::move(Buf),
- std::make_shared<PCHContainerOperations>(),
- vfs::getRealFileSystem());
- assert(AST.hasValue());
- return std::move(*AST);
-}
-
-std::vector<Diag> buildDiags(llvm::StringRef Code,
- std::vector<const char *> Flags = {}) {
- return build(Code, std::move(Flags)).getDiagnostics();
-}
-
MATCHER_P2(Diag, Range, Message,
"Diag at " + llvm::to_string(Range) + " = [" + Message + "]") {
return arg.Range == Range && arg.Message == Message;
@@ -104,7 +84,7 @@
}
)cpp");
EXPECT_THAT(
- buildDiags(Test.code()),
+ TestTU::withCode(Test.code()).build().getDiagnostics(),
ElementsAre(
// This range spans lines.
AllOf(Diag(Test.range("typo"),
@@ -122,13 +102,15 @@
TEST(DiagnosticsTest, FlagsMatter) {
Annotations Test("[[void]] main() {}");
- EXPECT_THAT(buildDiags(Test.code()),
+ auto TU = TestTU::withCode(Test.code());
+ EXPECT_THAT(TU.build().getDiagnostics(),
ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"),
WithFix(Fix(Test.range(), "int",
"change 'void' to 'int'")))));
// Same code built as C gets different diagnostics.
+ TU.Filename = "Plain.c";
EXPECT_THAT(
- buildDiags(Test.code(), {"-x", "c"}),
+ TU.build().getDiagnostics(),
ElementsAre(AllOf(
Diag(Test.range(), "return type of 'main' is not 'int'"),
WithFix(Fix(Test.range(), "int", "change return type to 'int'")))));
@@ -149,7 +131,7 @@
#endif
)cpp");
EXPECT_THAT(
- buildDiags(Test.code()),
+ TestTU::withCode(Test.code()).build().getDiagnostics(),
ElementsAre(Diag(Test.range(), "use of undeclared identifier 'b'")));
}
@@ -216,6 +198,28 @@
Pair(EqualToLSPDiag(NoteInMainLSP), IsEmpty())));
}
+TEST(ClangdUnitTest, GetBeginningOfIdentifier) {
+ // First ^ is the expected beginning, last is the search position.
+ for (const char *Text : {
+ "int ^f^oo();", // inside identifier
+ "int ^foo();", // beginning of identifier
+ "int ^foo^();", // end of identifier
+ "int foo(^);", // non-identifier
+ "^int foo();", // beginning of file (can't back up)
+ "int ^f0^0();", // after a digit (lexing at N-1 is wrong)
+ "int ^λλ^λ();", // UTF-8 handled properly when backing up
+ }) {
+ Annotations TestCase(Text);
+ auto AST = TestTU::withCode(TestCase.code()).build();
+ const auto &SourceMgr = AST.getASTContext().getSourceManager();
+ SourceLocation Actual = getBeginningOfIdentifier(
+ AST, TestCase.points().back(), SourceMgr.getMainFileID());
+ Position ActualPos =
+ offsetToPosition(TestCase.code(), SourceMgr.getFileOffset(Actual));
+ EXPECT_EQ(TestCase.points().front(), ActualPos) << Text;
+ }
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: unittests/clangd/CMakeLists.txt
===================================================================
--- unittests/clangd/CMakeLists.txt
+++ unittests/clangd/CMakeLists.txt
@@ -23,10 +23,12 @@
HeadersTests.cpp
IndexTests.cpp
JSONExprTests.cpp
+ QualityTests.cpp
SourceCodeTests.cpp
SymbolCollectorTests.cpp
SyncAPI.cpp
TestFS.cpp
+ TestTU.cpp
ThreadingTests.cpp
TraceTests.cpp
TUSchedulerTests.cpp
Index: unittests/clang-tidy/ClangTidyOptionsTest.cpp
===================================================================
--- unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -58,38 +58,33 @@
llvm::ErrorOr<ClangTidyOptions> Options =
parseConfiguration("Checks: \"-*,misc-*\"\n"
"HeaderFilterRegex: \".*\"\n"
- "AnalyzeTemporaryDtors: true\n"
"User: some.user");
EXPECT_TRUE(!!Options);
EXPECT_EQ("-*,misc-*", *Options->Checks);
EXPECT_EQ(".*", *Options->HeaderFilterRegex);
- EXPECT_TRUE(*Options->AnalyzeTemporaryDtors);
EXPECT_EQ("some.user", *Options->User);
}
TEST(ParseConfiguration, MergeConfigurations) {
llvm::ErrorOr<ClangTidyOptions> Options1 = parseConfiguration(R"(
Checks: "check1,check2"
HeaderFilterRegex: "filter1"
- AnalyzeTemporaryDtors: true
User: user1
ExtraArgs: ['arg1', 'arg2']
ExtraArgsBefore: ['arg-before1', 'arg-before2']
)");
ASSERT_TRUE(!!Options1);
llvm::ErrorOr<ClangTidyOptions> Options2 = parseConfiguration(R"(
Checks: "check3,check4"
HeaderFilterRegex: "filter2"
- AnalyzeTemporaryDtors: false
User: user2
ExtraArgs: ['arg3', 'arg4']
ExtraArgsBefore: ['arg-before3', 'arg-before4']
)");
ASSERT_TRUE(!!Options2);
ClangTidyOptions Options = Options1->mergeWith(*Options2);
EXPECT_EQ("check1,check2,check3,check4", *Options.Checks);
EXPECT_EQ("filter2", *Options.HeaderFilterRegex);
- EXPECT_FALSE(*Options.AnalyzeTemporaryDtors);
EXPECT_EQ("user2", *Options.User);
ASSERT_TRUE(Options.ExtraArgs.hasValue());
EXPECT_EQ("arg1,arg2,arg3,arg4", llvm::join(Options.ExtraArgs->begin(),
Index: test/clangd/rename.test
===================================================================
--- test/clangd/rename.test
+++ test/clangd/rename.test
@@ -36,4 +36,4 @@
---
{"jsonrpc":"2.0","id":3,"method":"shutdown"}
---
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: test/clang-tidy/temporaries.cpp
===================================================================
--- test/clang-tidy/temporaries.cpp
+++ test/clang-tidy/temporaries.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-tidy -checks='-*,clang-analyzer-core.NullDereference' -analyze-temporary-dtors %s -- | FileCheck %s
+// RUN: clang-tidy -checks='-*,clang-analyzer-core.NullDereference' %s -- | FileCheck %s
struct NoReturnDtor {
~NoReturnDtor() __attribute__((noreturn));
@@ -17,7 +17,8 @@
void testNullPointerDereference() {
int *value = 0;
if (check(NoReturnDtor())) {
- // This unreachable code causes a warning if we don't run with -analyze-temporary-dtors
+ // This unreachable code causes a warning if analysis of temporary
+ // destructors is not enabled.
*value = 1;
}
}
Index: test/clang-tidy/objc-property-declaration.m
===================================================================
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -19,6 +19,8 @@
@property(strong, nonatomic) NSString *VCsPluralToAdd;
@property(assign, nonatomic) int centerX;
@property(assign, nonatomic) int enable2GBackgroundFetch;
+@property(assign, nonatomic) int shouldUseCFPreferences;
+@property(assign, nonatomic) int enableGLAcceleration;
@end
@interface Foo (Bar)
Index: test/clang-tidy/modernize-raw-string-literal.cpp
===================================================================
--- test/clang-tidy/modernize-raw-string-literal.cpp
+++ test/clang-tidy/modernize-raw-string-literal.cpp
@@ -40,6 +40,8 @@
char const *const Us("goink\\\037");
char const *const HexNonPrintable("\\\x03");
char const *const Delete("\\\177");
+char const *const MultibyteSnowman("\xE2\x98\x83");
+// CHECK-FIXES: {{^}}char const *const MultibyteSnowman("\xE2\x98\x83");{{$}}
char const *const TrailingSpace("A line \\with space. \n");
char const *const TrailingNewLine("A single \\line.\n");
Index: test/clang-tidy/google-runtime-int.cpp
===================================================================
--- test/clang-tidy/google-runtime-int.cpp
+++ test/clang-tidy/google-runtime-int.cpp
@@ -72,3 +72,20 @@
B a, b;
a = b;
}
+
+__attribute__((__format__ (__printf__, 1, 2)))
+void myprintf(const char* s, ...);
+
+void doprint_no_warning() {
+ uint64 foo = 23;
+ myprintf("foo %lu %lu", (unsigned long)42, (unsigned long)foo);
+}
+
+void myprintf_no_attribute(const char* s, ...);
+
+void doprint_warning() {
+ uint64 foo = 23;
+ myprintf_no_attribute("foo %lu %lu", (unsigned long)42, (unsigned long)foo);
+// CHECK-MESSAGES: [[@LINE-1]]:41: warning: consider replacing 'unsigned long'
+// CHECK-MESSAGES: [[@LINE-2]]:60: warning: consider replacing 'unsigned long'
+}
Index: docs/clang-tidy/index.rst
===================================================================
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -112,11 +112,6 @@
clang-tidy options:
- -analyze-temporary-dtors -
- Enable temporary destructor-aware analysis in
- clang-analyzer- checks.
- This option overrides the value read from a
- .clang-tidy file.
-checks=<string> -
Comma-separated list of globs with optional '-'
prefix. Globs are processed in order of
@@ -245,7 +240,6 @@
Checks: '-*,some-check'
WarningsAsErrors: ''
HeaderFilterRegex: ''
- AnalyzeTemporaryDtors: false
FormatStyle: none
User: user
CheckOptions:
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -162,6 +162,9 @@
Flags functions that have more than a specified number of variables declared
in the body.
+- The `AnalyzeTemporaryDtors` option was removed, since the corresponding
+ `cfg-temporary-dtors` option of the Static Analyzer now defaults to `true`.
+
- New alias :doc:`hicpp-avoid-goto
<clang-tidy/checks/hicpp-avoid-goto>` to :doc:`cppcoreguidelines-avoid-goto
<clang-tidy/checks/cppcoreguidelines-avoid-goto>`
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -195,14 +195,10 @@
auto TokenLength = clang::Lexer::MeasureTokenLength(NameLoc, SM, LangOpts);
auto CreatePosition = [&SM](SourceLocation Loc) {
- auto FileIdAndOffset = SM.getDecomposedLoc(Loc);
- auto FileId = FileIdAndOffset.first;
- auto Offset = FileIdAndOffset.second;
+ auto LSPLoc = sourceLocToPosition(SM, Loc);
SymbolLocation::Position Pos;
- // Position is 0-based while SourceManager is 1-based.
- Pos.Line = SM.getLineNumber(FileId, Offset) - 1;
- // FIXME: Use UTF-16 code units, not UTF-8 bytes.
- Pos.Column = SM.getColumnNumber(FileId, Offset) - 1;
+ Pos.Line = LSPLoc.line;
+ Pos.Column = LSPLoc.character;
return Pos;
};
Index: clangd/index/MemIndex.cpp
===================================================================
--- clangd/index/MemIndex.cpp
+++ clangd/index/MemIndex.cpp
@@ -47,7 +47,7 @@
continue;
if (auto Score = Filter.match(Sym->Name)) {
- Top.emplace(-*Score, Sym);
+ Top.emplace(-*Score * quality(*Sym), Sym);
if (Top.size() > Req.MaxCandidateCount) {
More = true;
Top.pop();
Index: clangd/index/Index.h
===================================================================
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -199,6 +199,12 @@
};
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Symbol &S);
+// Computes query-independent quality score for a Symbol.
+// This currently falls in the range [1, ln(#indexed documents)].
+// FIXME: this should probably be split into symbol -> signals
+// and signals -> score, so it can be reused for Sema completions.
+double quality(const Symbol &S);
+
// An immutable symbol container that stores a set of symbols.
// The container will maintain the lifetime of the symbols.
class SymbolSlab {
Index: clangd/index/Index.cpp
===================================================================
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -48,6 +48,14 @@
return OS << S.Scope << S.Name;
}
+double quality(const Symbol &S) {
+ // This avoids a sharp gradient for tail symbols, and also neatly avoids the
+ // question of whether 0 references means a bad symbol or missing data.
+ if (S.References < 3)
+ return 1;
+ return std::log(S.References);
+}
+
SymbolSlab::const_iterator SymbolSlab::find(const SymbolID &ID) const {
auto It = std::lower_bound(Symbols.begin(), Symbols.end(), ID,
[](const Symbol &S, const SymbolID &I) {
Index: clangd/index/FileIndex.h
===================================================================
--- clangd/index/FileIndex.h
+++ clangd/index/FileIndex.h
@@ -71,6 +71,10 @@
MemIndex Index;
};
+/// Retrieves namespace and class level symbols in \p AST.
+/// Exposed to assist in unit tests.
+SymbolSlab indexAST(ParsedAST *AST);
+
} // namespace clangd
} // namespace clang
Index: clangd/index/FileIndex.cpp
===================================================================
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -13,12 +13,9 @@
namespace clang {
namespace clangd {
-namespace {
-/// Retrieves namespace and class level symbols in \p Decls.
-std::unique_ptr<SymbolSlab> indexAST(ASTContext &Ctx,
- std::shared_ptr<Preprocessor> PP,
- llvm::ArrayRef<const Decl *> Decls) {
+SymbolSlab indexAST(ParsedAST *AST) {
+ assert(AST && "AST must not be nullptr!");
SymbolCollector::Options CollectorOpts;
// FIXME(ioeric): we might also want to collect include headers. We would need
// to make sure all includes are canonicalized (with CanonicalIncludes), which
@@ -29,21 +26,18 @@
CollectorOpts.CountReferences = false;
SymbolCollector Collector(std::move(CollectorOpts));
- Collector.setPreprocessor(std::move(PP));
+ Collector.setPreprocessor(AST->getPreprocessorPtr());
index::IndexingOptions IndexOpts;
// We only need declarations, because we don't count references.
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly;
IndexOpts.IndexFunctionLocals = false;
- index::indexTopLevelDecls(Ctx, Decls, Collector, IndexOpts);
- auto Symbols = llvm::make_unique<SymbolSlab>();
- *Symbols = Collector.takeSymbols();
- return Symbols;
+ index::indexTopLevelDecls(AST->getASTContext(), AST->getTopLevelDecls(),
+ Collector, IndexOpts);
+ return Collector.takeSymbols();
}
-} // namespace
-
void FileSymbols::update(PathRef Path, std::unique_ptr<SymbolSlab> Slab) {
std::lock_guard<std::mutex> Lock(Mutex);
if (!Slab)
@@ -79,8 +73,8 @@
if (!AST) {
FSymbols.update(Path, nullptr);
} else {
- auto Slab = indexAST(AST->getASTContext(), AST->getPreprocessorPtr(),
- AST->getTopLevelDecls());
+ auto Slab = llvm::make_unique<SymbolSlab>();
+ *Slab = indexAST(AST);
FSymbols.update(Path, std::move(Slab));
}
auto Symbols = FSymbols.allSymbols();
Index: clangd/XRefs.h
===================================================================
--- clangd/XRefs.h
+++ clangd/XRefs.h
@@ -15,13 +15,15 @@
#include "ClangdUnit.h"
#include "Protocol.h"
+#include "index/Index.h"
#include <vector>
namespace clang {
namespace clangd {
/// Get definition of symbol at a specified \p Pos.
-std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos);
+std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos,
+ const SymbolIndex *Index = nullptr);
/// Returns highlights for all usages of a symbol at \p Pos.
std::vector<DocumentHighlight> findDocumentHighlights(ParsedAST &AST,
Index: clangd/XRefs.cpp
===================================================================
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -14,6 +14,7 @@
#include "clang/AST/DeclTemplate.h"
#include "clang/Index/IndexDataConsumer.h"
#include "clang/Index/IndexingAction.h"
+#include "clang/Index/USRGeneration.h"
#include "llvm/Support/Path.h"
namespace clang {
namespace clangd {
@@ -34,6 +35,33 @@
return nullptr;
}
+// Convert a SymbolLocation to LSP's Location.
+// HintPath is used to resolve the path of URI.
+// FIXME: figure out a good home for it, and share the implementation with
+// FindSymbols.
+llvm::Optional<Location> ToLSPLocation(const SymbolLocation &Loc,
+ llvm::StringRef HintPath) {
+ if (!Loc)
+ return llvm::None;
+ auto Uri = URI::parse(Loc.FileURI);
+ if (!Uri) {
+ log("Could not parse URI: " + Loc.FileURI);
+ return llvm::None;
+ }
+ auto Path = URI::resolve(*Uri, HintPath);
+ if (!Path) {
+ log("Could not resolve URI: " + Loc.FileURI);
+ return llvm::None;
+ }
+ Location LSPLoc;
+ LSPLoc.uri = URIForFile(*Path);
+ 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;
+ return LSPLoc;
+}
+
struct MacroDecl {
StringRef Name;
const MacroInfo *Info;
@@ -128,6 +156,38 @@
}
};
+struct IdentifiedSymbol {
+ std::vector<const Decl *> Decls;
+ std::vector<MacroDecl> Macros;
+};
+
+IdentifiedSymbol getSymbolAtPosition(ParsedAST &AST, SourceLocation Pos) {
+ auto DeclMacrosFinder = DeclarationAndMacrosFinder(
+ llvm::errs(), Pos, AST.getASTContext(), AST.getPreprocessor());
+ index::IndexingOptions IndexOpts;
+ IndexOpts.SystemSymbolFilter =
+ index::IndexingOptions::SystemSymbolFilterKind::All;
+ IndexOpts.IndexFunctionLocals = true;
+ indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
+ DeclMacrosFinder, IndexOpts);
+
+ return {DeclMacrosFinder.takeDecls(), DeclMacrosFinder.takeMacroInfos()};
+}
+
+llvm::Optional<std::string>
+getAbsoluteFilePath(const FileEntry *F, const SourceManager &SourceMgr) {
+ SmallString<64> FilePath = F->tryGetRealPathName();
+ if (FilePath.empty())
+ FilePath = F->getName();
+ if (!llvm::sys::path::is_absolute(FilePath)) {
+ if (!SourceMgr.getFileManager().makeAbsolutePath(FilePath)) {
+ log("Could not turn relative path to absolute: " + FilePath);
+ return llvm::None;
+ }
+ }
+ return FilePath.str().str();
+}
+
llvm::Optional<Location>
makeLocation(ParsedAST &AST, const SourceRange &ValSourceRange) {
const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
@@ -145,30 +205,32 @@
Range R = {Begin, End};
Location L;
- SmallString<64> FilePath = F->tryGetRealPathName();
- if (FilePath.empty())
- FilePath = F->getName();
- if (!llvm::sys::path::is_absolute(FilePath)) {
- if (!SourceMgr.getFileManager().makeAbsolutePath(FilePath)) {
- log("Could not turn relative path to absolute: " + FilePath);
- return llvm::None;
- }
+ auto FilePath = getAbsoluteFilePath(F, SourceMgr);
+ if (!FilePath) {
+ log("failed to get path!");
+ return llvm::None;
}
-
- L.uri = URIForFile(FilePath.str());
+ L.uri = URIForFile(*FilePath);
L.range = R;
return L;
}
+// Get the symbol ID for a declaration, if possible.
+llvm::Optional<SymbolID> getSymbolID(const Decl *D) {
+ llvm::SmallString<128> USR;
+ if (index::generateUSRForDecl(D, USR)) {
+ return None;
+ }
+ return SymbolID(USR);
+}
+
} // namespace
-std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos) {
+std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos,
+ const SymbolIndex *Index) {
const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
- const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
- if (!FE)
- return {};
-
- SourceLocation SourceLocationBeg = getBeginningOfIdentifier(AST, Pos, FE);
+ SourceLocation SourceLocationBeg =
+ getBeginningOfIdentifier(AST, Pos, SourceMgr.getMainFileID());
std::vector<Location> Result;
// Handle goto definition for #include.
@@ -182,32 +244,97 @@
if (!Result.empty())
return Result;
- DeclarationAndMacrosFinder DeclMacrosFinder(llvm::errs(), SourceLocationBeg,
- AST.getASTContext(),
- AST.getPreprocessor());
- index::IndexingOptions IndexOpts;
- IndexOpts.SystemSymbolFilter =
- index::IndexingOptions::SystemSymbolFilterKind::All;
- IndexOpts.IndexFunctionLocals = true;
-
- indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
- DeclMacrosFinder, IndexOpts);
-
- std::vector<const Decl *> Decls = DeclMacrosFinder.takeDecls();
- std::vector<MacroDecl> MacroInfos = DeclMacrosFinder.takeMacroInfos();
+ // Identified symbols at a specific position.
+ auto Symbols = getSymbolAtPosition(AST, SourceLocationBeg);
- for (auto D : Decls) {
- auto Loc = findNameLoc(D);
+ for (auto Item : Symbols.Macros) {
+ auto Loc = Item.Info->getDefinitionLoc();
auto L = makeLocation(AST, SourceRange(Loc, Loc));
if (L)
Result.push_back(*L);
}
- for (auto Item : MacroInfos) {
- auto Loc = Item.Info->getDefinitionLoc();
+ // Declaration and definition are different terms in C-family languages, and
+ // LSP only defines the "GoToDefinition" specification, so we try to perform
+ // the "most sensible" GoTo operation:
+ //
+ // - We use the location from AST and index (if available) to provide the
+ // final results. When there are duplicate results, we prefer AST over
+ // index because AST is more up-to-date.
+ //
+ // - For each symbol, we will return a location of the canonical declaration
+ // (e.g. function declaration in header), and a location of definition if
+ // they are available.
+ //
+ // So the work flow:
+ //
+ // 1. Identify the symbols being search for by traversing the AST.
+ // 2. Populate one of the locations with the AST location.
+ // 3. Use the AST information to query the index, and populate the index
+ // location (if available).
+ // 4. Return all populated locations for all symbols, definition first (
+ // which we think is the users wants most often).
+ struct CandidateLocation {
+ llvm::Optional<Location> Def;
+ llvm::Optional<Location> Decl;
+ };
+ llvm::DenseMap<SymbolID, CandidateLocation> ResultCandidates;
+
+ // Emit all symbol locations (declaration or definition) from AST.
+ for (const auto *D : Symbols.Decls) {
+ // Fake key for symbols don't have USR (no SymbolID).
+ // Ideally, there should be a USR for each identified symbols. Symbols
+ // without USR are rare and unimportant cases, we use the a fake holder to
+ // minimize the invasiveness of these cases.
+ SymbolID Key("");
+ if (auto ID = getSymbolID(D))
+ Key = *ID;
+
+ auto &Candidate = ResultCandidates[Key];
+ auto Loc = findNameLoc(D);
auto L = makeLocation(AST, SourceRange(Loc, Loc));
- if (L)
- Result.push_back(*L);
+ // The declaration in the identified symbols is a definition if possible
+ // otherwise it is declaration.
+ bool IsDef = GetDefinition(D) == D;
+ // Populate one of the slots with location for the AST.
+ if (!IsDef)
+ Candidate.Decl = L;
+ else
+ Candidate.Def = L;
+ }
+
+ if (Index) {
+ LookupRequest QueryRequest;
+ // Build request for index query, using SymbolID.
+ for (auto It : ResultCandidates)
+ QueryRequest.IDs.insert(It.first);
+ std::string HintPath;
+ const FileEntry *FE =
+ SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
+ if (auto Path = getAbsoluteFilePath(FE, SourceMgr))
+ HintPath = *Path;
+ // Query the index and populate the empty slot.
+ Index->lookup(
+ QueryRequest, [&HintPath, &ResultCandidates](const Symbol &Sym) {
+ auto It = ResultCandidates.find(Sym.ID);
+ assert(It != ResultCandidates.end());
+ auto &Value = It->second;
+
+ if (!Value.Def)
+ Value.Def = ToLSPLocation(Sym.Definition, HintPath);
+ if (!Value.Decl)
+ Value.Decl = ToLSPLocation(Sym.CanonicalDeclaration, HintPath);
+ });
+ }
+
+ // Populate the results, definition first.
+ for (auto It : ResultCandidates) {
+ const auto &Candidate = It.second;
+ if (Candidate.Def)
+ Result.push_back(*Candidate.Def);
+ if (Candidate.Decl &&
+ Candidate.Decl != Candidate.Def) // Decl and Def might be the same
+ Result.push_back(*Candidate.Decl);
}
return Result;
@@ -280,29 +407,19 @@
std::vector<DocumentHighlight> findDocumentHighlights(ParsedAST &AST,
Position Pos) {
const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
- const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
- if (!FE)
- return {};
+ SourceLocation SourceLocationBeg =
+ getBeginningOfIdentifier(AST, Pos, SourceMgr.getMainFileID());
- SourceLocation SourceLocationBeg = getBeginningOfIdentifier(AST, Pos, FE);
+ auto Symbols = getSymbolAtPosition(AST, SourceLocationBeg);
+ std::vector<const Decl *> SelectedDecls = Symbols.Decls;
+
+ DocumentHighlightsFinder DocHighlightsFinder(
+ llvm::errs(), AST.getASTContext(), AST.getPreprocessor(), SelectedDecls);
- DeclarationAndMacrosFinder DeclMacrosFinder(llvm::errs(), SourceLocationBeg,
- AST.getASTContext(),
- AST.getPreprocessor());
index::IndexingOptions IndexOpts;
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
IndexOpts.IndexFunctionLocals = true;
-
- // Macro occurences are not currently handled.
- indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
- DeclMacrosFinder, IndexOpts);
-
- std::vector<const Decl *> SelectedDecls = DeclMacrosFinder.takeDecls();
-
- DocumentHighlightsFinder DocHighlightsFinder(
- llvm::errs(), AST.getASTContext(), AST.getPreprocessor(), SelectedDecls);
-
indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
DocHighlightsFinder, IndexOpts);
@@ -413,30 +530,16 @@
Hover getHover(ParsedAST &AST, Position Pos) {
const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
- const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
- if (FE == nullptr)
- return Hover();
-
- SourceLocation SourceLocationBeg = getBeginningOfIdentifier(AST, Pos, FE);
- DeclarationAndMacrosFinder DeclMacrosFinder(llvm::errs(), SourceLocationBeg,
- AST.getASTContext(),
- AST.getPreprocessor());
-
- index::IndexingOptions IndexOpts;
- IndexOpts.SystemSymbolFilter =
- index::IndexingOptions::SystemSymbolFilterKind::All;
- IndexOpts.IndexFunctionLocals = true;
-
- indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
- DeclMacrosFinder, IndexOpts);
+ SourceLocation SourceLocationBeg =
+ getBeginningOfIdentifier(AST, Pos, SourceMgr.getMainFileID());
+ // Identified symbols at a specific position.
+ auto Symbols = getSymbolAtPosition(AST, SourceLocationBeg);
- std::vector<MacroDecl> Macros = DeclMacrosFinder.takeMacroInfos();
- if (!Macros.empty())
- return getHoverContents(Macros[0].Name);
+ if (!Symbols.Macros.empty())
+ return getHoverContents(Symbols.Macros[0].Name);
- std::vector<const Decl *> Decls = DeclMacrosFinder.takeDecls();
- if (!Decls.empty())
- return getHoverContents(Decls[0]);
+ if (!Symbols.Decls.empty())
+ return getHoverContents(Symbols.Decls[0]);
return Hover();
}
Index: clangd/SourceCode.h
===================================================================
--- clangd/SourceCode.h
+++ clangd/SourceCode.h
@@ -23,22 +23,17 @@
/// Turn a [line, column] pair into an offset in Code.
///
-/// If the character value is greater than the line length, the behavior depends
-/// on AllowColumnsBeyondLineLength:
-///
-/// - if true: default back to the end of the line
-/// - if false: return an error
-///
-/// If the line number is greater than the number of lines in the document,
-/// always return an error.
+/// If P.character exceeds the line length, returns the offset at end-of-line.
+/// (If !AllowColumnsBeyondLineLength, then returns an error instead).
+/// If the line number is out of range, returns an error.
///
/// The returned value is in the range [0, Code.size()].
llvm::Expected<size_t>
positionToOffset(llvm::StringRef Code, Position P,
bool AllowColumnsBeyondLineLength = true);
/// Turn an offset in Code into a [line, column] pair.
-/// FIXME: This should return an error if the offset is invalid.
+/// The offset must be in range [0, Code.size()].
Position offsetToPosition(llvm::StringRef Code, size_t Offset);
/// Turn a SourceLocation into a [line, column] pair.
@@ -49,6 +44,12 @@
// Note that clang also uses closed source ranges, which this can't handle!
Range halfOpenToRange(const SourceManager &SM, CharSourceRange R);
+// Converts an offset to a clang line/column (1-based, columns are bytes).
+// The offset must be in range [0, Code.size()].
+// Prefer to use SourceManager if one is available.
+std::pair<size_t, size_t> offsetToClangLineColumn(llvm::StringRef Code,
+ size_t Offset);
+
/// From "a::b::c", return {"a::b::", "c"}. Scope is empty if there's no
/// qualifier.
std::pair<llvm::StringRef, llvm::StringRef>
Index: clangd/SourceCode.cpp
===================================================================
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -16,6 +16,66 @@
namespace clangd {
using namespace llvm;
+// Here be dragons. LSP positions use columns measured in *UTF-16 code units*!
+// Clangd uses UTF-8 and byte-offsets internally, so conversion is nontrivial.
+
+// Iterates over unicode codepoints in the (UTF-8) string. For each,
+// invokes CB(UTF-8 length, UTF-16 length), and breaks if it returns true.
+// Returns true if CB returned true, false if we hit the end of string.
+template <typename Callback>
+static bool iterateCodepoints(StringRef U8, const Callback &CB) {
+ for (size_t I = 0; I < U8.size();) {
+ unsigned char C = static_cast<unsigned char>(U8[I]);
+ if (LLVM_LIKELY(!(C & 0x80))) { // ASCII character.
+ if (CB(1, 1))
+ return true;
+ ++I;
+ continue;
+ }
+ // This convenient property of UTF-8 holds for all non-ASCII characters.
+ size_t UTF8Length = countLeadingOnes(C);
+ // 0xxx is ASCII, handled above. 10xxx is a trailing byte, invalid here.
+ // 11111xxx is not valid UTF-8 at all. Assert because it's probably our bug.
+ assert((UTF8Length >= 2 && UTF8Length <= 4) &&
+ "Invalid UTF-8, or transcoding bug?");
+ I += UTF8Length; // Skip over all trailing bytes.
+ // A codepoint takes two UTF-16 code unit if it's astral (outside BMP).
+ // Astral codepoints are encoded as 4 bytes in UTF-8 (11110xxx ...)
+ if (CB(UTF8Length, UTF8Length == 4 ? 2 : 1))
+ return true;
+ }
+ return false;
+}
+
+// Returns the offset into the string that matches \p Units UTF-16 code units.
+// Conceptually, this converts to UTF-16, truncates to CodeUnits, converts back
+// to UTF-8, and returns the length in bytes.
+static size_t measureUTF16(StringRef U8, int U16Units, bool &Valid) {
+ size_t Result = 0;
+ Valid = U16Units == 0 || iterateCodepoints(U8, [&](int U8Len, int U16Len) {
+ Result += U8Len;
+ U16Units -= U16Len;
+ return U16Units <= 0;
+ });
+ if (U16Units < 0) // Offset was into the middle of a surrogate pair.
+ Valid = false;
+ // Don't return an out-of-range index if we overran.
+ return std::min(Result, U8.size());
+}
+
+// Counts the number of UTF-16 code units needed to represent a string.
+// Like most strings in clangd, the input is UTF-8 encoded.
+static size_t utf16Len(StringRef U8) {
+ // A codepoint takes two UTF-16 code unit if it's astral (outside BMP).
+ // Astral codepoints are encoded as 4 bytes in UTF-8, starting with 11110xxx.
+ size_t Count = 0;
+ iterateCodepoints(U8, [&](int U8Len, int U16Len) {
+ Count += U16Len;
+ return false;
+ });
+ return Count;
+}
+
llvm::Expected<size_t> positionToOffset(StringRef Code, Position P,
bool AllowColumnsBeyondLineLength) {
if (P.line < 0)
@@ -40,31 +100,43 @@
if (NextNL == StringRef::npos)
NextNL = Code.size();
- if (StartOfLine + P.character > NextNL && !AllowColumnsBeyondLineLength)
+ bool Valid;
+ size_t ByteOffsetInLine = measureUTF16(
+ Code.substr(StartOfLine, NextNL - StartOfLine), P.character, Valid);
+ if (!Valid && !AllowColumnsBeyondLineLength)
return llvm::make_error<llvm::StringError>(
- llvm::formatv("Character value is out of range ({0})", P.character),
+ llvm::formatv("UTF-16 offset {0} is invalid for line {1}", P.character,
+ P.line),
llvm::errc::invalid_argument);
- // FIXME: officially P.character counts UTF-16 code units, not UTF-8 bytes!
- return std::min(NextNL, StartOfLine + P.character);
+ return StartOfLine + ByteOffsetInLine;
}
Position offsetToPosition(StringRef Code, size_t Offset) {
Offset = std::min(Code.size(), Offset);
StringRef Before = Code.substr(0, Offset);
int Lines = Before.count('\n');
size_t PrevNL = Before.rfind('\n');
size_t StartOfLine = (PrevNL == StringRef::npos) ? 0 : (PrevNL + 1);
- // FIXME: officially character counts UTF-16 code units, not UTF-8 bytes!
Position Pos;
Pos.line = Lines;
- Pos.character = static_cast<int>(Offset - StartOfLine);
+ Pos.character = utf16Len(Before.substr(StartOfLine));
return Pos;
}
Position sourceLocToPosition(const SourceManager &SM, SourceLocation Loc) {
+ // We use the SourceManager's line tables, but its column number is in bytes.
+ FileID FID;
+ unsigned Offset;
+ std::tie(FID, Offset) = SM.getDecomposedSpellingLoc(Loc);
Position P;
- P.line = static_cast<int>(SM.getSpellingLineNumber(Loc)) - 1;
- P.character = static_cast<int>(SM.getSpellingColumnNumber(Loc)) - 1;
+ P.line = static_cast<int>(SM.getLineNumber(FID, Offset)) - 1;
+ bool Invalid = false;
+ StringRef Code = SM.getBufferData(FID, &Invalid);
+ if (!Invalid) {
+ auto ColumnInBytes = SM.getColumnNumber(FID, Offset) - 1;
+ auto LineSoFar = Code.substr(Offset - ColumnInBytes, ColumnInBytes);
+ P.character = utf16Len(LineSoFar);
+ }
return P;
}
@@ -76,6 +148,16 @@
return {Begin, End};
}
+std::pair<size_t, size_t> offsetToClangLineColumn(StringRef Code,
+ size_t Offset) {
+ Offset = std::min(Code.size(), Offset);
+ StringRef Before = Code.substr(0, Offset);
+ int Lines = Before.count('\n');
+ size_t PrevNL = Before.rfind('\n');
+ size_t StartOfLine = (PrevNL == StringRef::npos) ? 0 : (PrevNL + 1);
+ return {Lines + 1, Offset - StartOfLine + 1};
+}
+
std::pair<llvm::StringRef, llvm::StringRef>
splitQualifiedName(llvm::StringRef QName) {
size_t Pos = QName.rfind("::");
Index: clangd/Quality.h
===================================================================
--- /dev/null
+++ clangd/Quality.h
@@ -0,0 +1,126 @@
+//===--- Quality.h - Ranking alternatives for ambiguous queries -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===---------------------------------------------------------------------===//
+///
+/// Some operations such as code completion produce a set of candidates.
+/// Usually the user can choose between them, but we should put the best options
+/// at the top (they're easier to select, and more likely to be seen).
+///
+/// This file defines building blocks for ranking candidates.
+/// It's used by the features directly and also in the implementation of
+/// indexes, as indexes also need to heuristically limit their results.
+///
+/// The facilities here are:
+/// - retrieving scoring signals from e.g. indexes, AST, CodeCompletionString
+/// These are structured in a way that they can be debugged, and are fairly
+/// consistent regardless of the source.
+/// - compute scores from scoring signals. These are suitable for sorting.
+/// - sorting utilities like the TopN container.
+/// These could be split up further to isolate dependencies if we care.
+///
+//===---------------------------------------------------------------------===//
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_QUALITY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_QUALITY_H
+#include "llvm/ADT/StringRef.h"
+#include <algorithm>
+#include <functional>
+#include <vector>
+namespace llvm {
+class raw_ostream;
+}
+namespace clang {
+class CodeCompletionResult;
+namespace clangd {
+struct Symbol;
+
+// Signals structs are designed to be aggregated from 0 or more sources.
+// A default instance has neutral signals, and sources are merged into it.
+// They can be dumped for debugging, and evaluate()d into a score.
+
+// Attributes of a symbol that affect how much we like it.
+struct SymbolQualitySignals {
+ unsigned SemaCCPriority = 0; // 1-80, 1 is best. 0 means absent.
+ // FIXME: this is actually a mix of symbol
+ // quality and relevance. Untangle this.
+ bool Deprecated = false;
+ unsigned References = 0;
+
+ void merge(const CodeCompletionResult &SemaCCResult);
+ void merge(const Symbol &IndexResult);
+
+ // Condense these signals down to a single number, higher is better.
+ float evaluate() const;
+};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &,
+ const SymbolQualitySignals &);
+
+// Attributes of a symbol-query pair that affect how much we like it.
+struct SymbolRelevanceSignals {
+ // 0-1 fuzzy-match score for unqualified name. Must be explicitly assigned.
+ float NameMatch = 1;
+ bool Forbidden = false; // Unavailable (e.g const) or inaccessible (private).
+
+ void merge(const CodeCompletionResult &SemaResult);
+
+ // Condense these signals down to a single number, higher is better.
+ float evaluate() const;
+};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &,
+ const SymbolRelevanceSignals &);
+
+// Combine symbol quality and relevance into a single score.
+float evaluateSymbolAndRelevance(float SymbolQuality, float SymbolRelevance);
+
+// TopN<T> is a lossy container that preserves only the "best" N elements.
+template <typename T, typename Compare = std::greater<T>> class TopN {
+public:
+ using value_type = T;
+ TopN(size_t N, Compare Greater = Compare())
+ : N(N), Greater(std::move(Greater)) {}
+
+ // Adds a candidate to the set.
+ // Returns true if a candidate was dropped to get back under N.
+ bool push(value_type &&V) {
+ bool Dropped = false;
+ if (Heap.size() >= N) {
+ Dropped = true;
+ if (N > 0 && Greater(V, Heap.front())) {
+ std::pop_heap(Heap.begin(), Heap.end(), Greater);
+ Heap.back() = std::move(V);
+ std::push_heap(Heap.begin(), Heap.end(), Greater);
+ }
+ } else {
+ Heap.push_back(std::move(V));
+ std::push_heap(Heap.begin(), Heap.end(), Greater);
+ }
+ assert(Heap.size() <= N);
+ assert(std::is_heap(Heap.begin(), Heap.end(), Greater));
+ return Dropped;
+ }
+
+ // Returns candidates from best to worst.
+ std::vector<value_type> items() && {
+ std::sort_heap(Heap.begin(), Heap.end(), Greater);
+ assert(Heap.size() <= N);
+ return std::move(Heap);
+ }
+
+private:
+ const size_t N;
+ std::vector<value_type> Heap; // Min-heap, comparator is Greater.
+ Compare Greater;
+};
+
+// Returns a string that sorts in the same order as (-Score, Tiebreak), for LSP.
+// (The highest score compares smallest so it sorts at the top).
+std::string sortText(float Score, llvm::StringRef Tiebreak = "");
+
+} // namespace clangd
+} // namespace clang
+
+#endif
Index: clangd/Quality.cpp
===================================================================
--- /dev/null
+++ clangd/Quality.cpp
@@ -0,0 +1,108 @@
+//===--- Quality.cpp --------------------------------------------*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===---------------------------------------------------------------------===//
+#include "Quality.h"
+#include "index/Index.h"
+#include "clang/Sema/CodeCompleteConsumer.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/MathExtras.h"
+#include "llvm/Support/raw_ostream.h"
+
+namespace clang {
+namespace clangd {
+using namespace llvm;
+
+void SymbolQualitySignals::merge(const CodeCompletionResult &SemaCCResult) {
+ SemaCCPriority = SemaCCResult.Priority;
+
+ if (SemaCCResult.Availability == CXAvailability_Deprecated)
+ Deprecated = true;
+}
+
+void SymbolQualitySignals::merge(const Symbol &IndexResult) {
+ References = std::max(IndexResult.References, References);
+}
+
+float SymbolQualitySignals::evaluate() const {
+ float Score = 1;
+
+ // This avoids a sharp gradient for tail symbols, and also neatly avoids the
+ // question of whether 0 references means a bad symbol or missing data.
+ if (References >= 3)
+ Score *= std::log(References);
+
+ if (SemaCCPriority)
+ // Map onto a 0-2 interval, so we don't reward/penalize non-Sema results.
+ // Priority 80 is a really bad score.
+ Score *= 2 - std::min<float>(80, SemaCCPriority) / 40;
+
+ if (Deprecated)
+ Score *= 0.1;
+
+ return Score;
+}
+
+raw_ostream &operator<<(raw_ostream &OS, const SymbolQualitySignals &S) {
+ OS << formatv("=== Symbol quality: {0}\n", S.evaluate());
+ if (S.SemaCCPriority)
+ OS << formatv("\tSemaCCPriority: {0}\n", S.SemaCCPriority);
+ OS << formatv("\tReferences: {0}\n", S.References);
+ OS << formatv("\tDeprecated: {0}\n", S.Deprecated);
+ return OS;
+}
+
+void SymbolRelevanceSignals::merge(const CodeCompletionResult &SemaCCResult) {
+ if (SemaCCResult.Availability == CXAvailability_NotAvailable ||
+ SemaCCResult.Availability == CXAvailability_NotAccessible)
+ Forbidden = true;
+}
+
+float SymbolRelevanceSignals::evaluate() const {
+ if (Forbidden)
+ return 0;
+ return NameMatch;
+}
+raw_ostream &operator<<(raw_ostream &OS, const SymbolRelevanceSignals &S) {
+ OS << formatv("=== Symbol relevance: {0}\n", S.evaluate());
+ OS << formatv("\tName match: {0}\n", S.NameMatch);
+ OS << formatv("\tForbidden: {0}\n", S.Forbidden);
+ return OS;
+}
+
+float evaluateSymbolAndRelevance(float SymbolQuality, float SymbolRelevance) {
+ return SymbolQuality * SymbolRelevance;
+}
+
+// Produces an integer that sorts in the same order as F.
+// That is: a < b <==> encodeFloat(a) < encodeFloat(b).
+static uint32_t encodeFloat(float F) {
+ static_assert(std::numeric_limits<float>::is_iec559, "");
+ constexpr uint32_t TopBit = ~(~uint32_t{0} >> 1);
+
+ // Get the bits of the float. Endianness is the same as for integers.
+ uint32_t U = FloatToBits(F);
+ // IEEE 754 floats compare like sign-magnitude integers.
+ if (U & TopBit) // Negative float.
+ return 0 - U; // Map onto the low half of integers, order reversed.
+ return U + TopBit; // Positive floats map onto the high half of integers.
+}
+
+std::string sortText(float Score, llvm::StringRef Name) {
+ // We convert -Score to an integer, and hex-encode for readability.
+ // Example: [0.5, "foo"] -> "41000000foo"
+ std::string S;
+ llvm::raw_string_ostream OS(S);
+ write_hex(OS, encodeFloat(-Score), llvm::HexPrintStyle::Lower,
+ /*Width=*/2 * sizeof(Score));
+ OS << Name;
+ OS.flush();
+ return S;
+}
+
+} // namespace clangd
+} // namespace clang
Index: clangd/Protocol.h
===================================================================
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -91,6 +91,8 @@
int line = 0;
/// Character offset on a line in a document (zero-based).
+ /// WARNING: this is in UTF-16 codepoints, not bytes or characters!
+ /// Use the functions in SourceCode.h to construct/interpret Positions.
int character = 0;
friend bool operator==(const Position &LHS, const Position &RHS) {
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -19,6 +19,7 @@
#include "Compiler.h"
#include "FuzzyMatch.h"
#include "Logger.h"
+#include "Quality.h"
#include "SourceCode.h"
#include "Trace.h"
#include "index/Index.h"
@@ -32,6 +33,9 @@
#include "llvm/Support/Format.h"
#include <queue>
+// We log detailed candidate here if you run with -debug-only=codecomplete.
+#define DEBUG_TYPE "codecomplete"
+
namespace clang {
namespace clangd {
namespace {
@@ -190,67 +194,14 @@
return Result;
}
-// Produces an integer that sorts in the same order as F.
-// That is: a < b <==> encodeFloat(a) < encodeFloat(b).
-uint32_t encodeFloat(float F) {
- static_assert(std::numeric_limits<float>::is_iec559, "");
- static_assert(sizeof(float) == sizeof(uint32_t), "");
- constexpr uint32_t TopBit = ~(~uint32_t{0} >> 1);
-
- // Get the bits of the float. Endianness is the same as for integers.
- uint32_t U;
- memcpy(&U, &F, sizeof(float));
- // IEEE 754 floats compare like sign-magnitude integers.
- if (U & TopBit) // Negative float.
- return 0 - U; // Map onto the low half of integers, order reversed.
- return U + TopBit; // Positive floats map onto the high half of integers.
-}
-
-// Returns a string that sorts in the same order as (-Score, Name), for LSP.
-std::string sortText(float Score, llvm::StringRef Name) {
- // We convert -Score to an integer, and hex-encode for readability.
- // Example: [0.5, "foo"] -> "41000000foo"
- std::string S;
- llvm::raw_string_ostream OS(S);
- write_hex(OS, encodeFloat(-Score), llvm::HexPrintStyle::Lower,
- /*Width=*/2 * sizeof(Score));
- OS << Name;
- OS.flush();
- return S;
-}
-
/// A code completion result, in clang-native form.
/// It may be promoted to a CompletionItem if it's among the top-ranked results.
struct CompletionCandidate {
llvm::StringRef Name; // Used for filtering and sorting.
// We may have a result from Sema, from the index, or both.
const CodeCompletionResult *SemaResult = nullptr;
const Symbol *IndexResult = nullptr;
- // Computes the "symbol quality" score for this completion. Higher is better.
- float score() const {
- // For now we just use the Sema priority, mapping it onto a 0-1 interval.
- if (!SemaResult) // FIXME(sammccall): better scoring for index results.
- return 0.3f; // fixed mediocre score for index-only results.
-
- // Priority 80 is a really bad score.
- float Score = 1 - std::min<float>(80, SemaResult->Priority) / 80;
-
- switch (static_cast<CXAvailabilityKind>(SemaResult->Availability)) {
- case CXAvailability_Available:
- // No penalty.
- break;
- case CXAvailability_Deprecated:
- Score *= 0.1f;
- break;
- case CXAvailability_NotAccessible:
- case CXAvailability_NotAvailable:
- Score = 0;
- break;
- }
- return Score;
- }
-
// Builds an LSP completion item.
CompletionItem build(llvm::StringRef FileName,
const CompletionItemScores &Scores,
@@ -316,6 +267,7 @@
return I;
}
};
+using ScoredCandidate = std::pair<CompletionCandidate, CompletionItemScores>;
// Determine the symbol ID for a Sema code completion result, if possible.
llvm::Optional<SymbolID> getSymbolID(const CodeCompletionResult &R) {
@@ -522,50 +474,12 @@
UniqueFunction<void()> ResultsCallback;
};
-// Tracks a bounded number of candidates with the best scores.
-class TopN {
-public:
- using value_type = std::pair<CompletionCandidate, CompletionItemScores>;
- static constexpr size_t Unbounded = std::numeric_limits<size_t>::max();
-
- TopN(size_t N) : N(N) {}
-
- // Adds a candidate to the set.
- // Returns true if a candidate was dropped to get back under N.
- bool push(value_type &&V) {
- bool Dropped = false;
- if (Heap.size() >= N) {
- Dropped = true;
- if (N > 0 && greater(V, Heap.front())) {
- std::pop_heap(Heap.begin(), Heap.end(), greater);
- Heap.back() = std::move(V);
- std::push_heap(Heap.begin(), Heap.end(), greater);
- }
- } else {
- Heap.push_back(std::move(V));
- std::push_heap(Heap.begin(), Heap.end(), greater);
- }
- assert(Heap.size() <= N);
- assert(std::is_heap(Heap.begin(), Heap.end(), greater));
- return Dropped;
- }
-
- // Returns candidates from best to worst.
- std::vector<value_type> items() && {
- std::sort_heap(Heap.begin(), Heap.end(), greater);
- assert(Heap.size() <= N);
- return std::move(Heap);
- }
-
-private:
- static bool greater(const value_type &L, const value_type &R) {
+struct ScoredCandidateGreater {
+ bool operator()(const ScoredCandidate &L, const ScoredCandidate &R) {
if (L.second.finalScore != R.second.finalScore)
return L.second.finalScore > R.second.finalScore;
return L.first.Name < R.first.Name; // Earlier name is better.
}
-
- const size_t N;
- std::vector<value_type> Heap; // Min-heap, comparator is greater().
};
class SignatureHelpCollector final : public CodeCompleteConsumer {
@@ -729,8 +643,15 @@
FrontendOpts.SkipFunctionBodies = true;
FrontendOpts.CodeCompleteOpts = Options;
FrontendOpts.CodeCompletionAt.FileName = Input.FileName;
- FrontendOpts.CodeCompletionAt.Line = Input.Pos.line + 1;
- FrontendOpts.CodeCompletionAt.Column = Input.Pos.character + 1;
+ auto Offset = positionToOffset(Input.Contents, Input.Pos);
+ if (!Offset) {
+ log("Code completion position was invalid " +
+ llvm::toString(Offset.takeError()));
+ return false;
+ }
+ std::tie(FrontendOpts.CodeCompletionAt.Line,
+ FrontendOpts.CodeCompletionAt.Column) =
+ offsetToClangLineColumn(Input.Contents, *Offset);
Clang->setCodeCompletionConsumer(Consumer.release());
@@ -938,7 +859,8 @@
const SymbolSlab &IndexResults) {
trace::Span Tracer("Merge and score results");
// We only keep the best N results at any time, in "native" format.
- TopN Top(Opts.Limit == 0 ? TopN::Unbounded : Opts.Limit);
+ TopN<ScoredCandidate, ScoredCandidateGreater> Top(
+ Opts.Limit == 0 ? std::numeric_limits<size_t>::max() : Opts.Limit);
llvm::DenseSet<const Symbol *> UsedIndexResults;
auto CorrespondingIndexResult =
[&](const CodeCompletionResult &SemaResult) -> const Symbol * {
@@ -964,23 +886,42 @@
}
// Scores a candidate and adds it to the TopN structure.
- void addCandidate(TopN &Candidates, const CodeCompletionResult *SemaResult,
+ void addCandidate(TopN<ScoredCandidate, ScoredCandidateGreater> &Candidates,
+ const CodeCompletionResult *SemaResult,
const Symbol *IndexResult) {
CompletionCandidate C;
C.SemaResult = SemaResult;
C.IndexResult = IndexResult;
C.Name = IndexResult ? IndexResult->Name : Recorder->getName(*SemaResult);
- CompletionItemScores Scores;
+ SymbolQualitySignals Quality;
+ SymbolRelevanceSignals Relevance;
if (auto FuzzyScore = Filter->match(C.Name))
- Scores.filterScore = *FuzzyScore;
+ Relevance.NameMatch = *FuzzyScore;
else
return;
- Scores.symbolScore = C.score();
- // We score candidates by multiplying symbolScore ("quality" of the result)
- // with filterScore (how well it matched the query).
- // This is sensitive to the distribution of both component scores!
- Scores.finalScore = Scores.filterScore * Scores.symbolScore;
+ if (IndexResult)
+ Quality.merge(*IndexResult);
+ if (SemaResult) {
+ Quality.merge(*SemaResult);
+ Relevance.merge(*SemaResult);
+ }
+
+ float QualScore = Quality.evaluate(), RelScore = Relevance.evaluate();
+ CompletionItemScores Scores;
+ Scores.finalScore = evaluateSymbolAndRelevance(QualScore, RelScore);
+ // The purpose of exporting component scores is to allow NameMatch to be
+ // replaced on the client-side. So we export (NameMatch, final/NameMatch)
+ // rather than (RelScore, QualScore).
+ Scores.filterScore = Relevance.NameMatch;
+ Scores.symbolScore =
+ Scores.filterScore ? Scores.finalScore / Scores.filterScore : QualScore;
+
+ DEBUG(llvm::dbgs() << "CodeComplete: " << C.Name
+ << (IndexResult ? " (index)" : "")
+ << (SemaResult ? " (sema)" : "") << " = "
+ << Scores.finalScore << "\n"
+ << Quality << Relevance << "\n");
NSema += bool(SemaResult);
NIndex += bool(IndexResult);
Index: clangd/ClangdUnit.h
===================================================================
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -173,8 +173,9 @@
};
/// Get the beginning SourceLocation at a specified \p Pos.
+/// May be invalid if Pos is, or if there's no identifier.
SourceLocation getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos,
- const FileEntry *FE);
+ const FileID FID);
/// For testing/debugging purposes. Note that this method deserializes all
/// unserialized Decls, so use with care.
Index: clangd/ClangdUnit.cpp
===================================================================
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -215,19 +215,6 @@
std::move(IncLocations));
}
-namespace {
-
-SourceLocation getMacroArgExpandedLocation(const SourceManager &Mgr,
- const FileEntry *FE, Position Pos) {
- // The language server protocol uses zero-based line and column numbers.
- // Clang uses one-based numbers.
- SourceLocation InputLoc =
- Mgr.translateFileLineCol(FE, Pos.line + 1, Pos.character + 1);
- return Mgr.getMacroArgExpandedLocation(InputLoc);
-}
-
-} // namespace
-
void ParsedAST::ensurePreambleDeclsDeserialized() {
if (PreambleDeclsDeserialized || !Preamble)
return;
@@ -470,40 +457,34 @@
SourceLocation clangd::getBeginningOfIdentifier(ParsedAST &Unit,
const Position &Pos,
- const FileEntry *FE) {
+ const FileID FID) {
const ASTContext &AST = Unit.getASTContext();
const SourceManager &SourceMgr = AST.getSourceManager();
-
- SourceLocation InputLocation =
- getMacroArgExpandedLocation(SourceMgr, FE, Pos);
- if (Pos.character == 0) {
- return InputLocation;
- }
-
- // This handle cases where the position is in the middle of a token or right
- // after the end of a token. In theory we could just use GetBeginningOfToken
- // to find the start of the token at the input position, but this doesn't
- // work when right after the end, i.e. foo|.
- // So try to go back by one and see if we're still inside an identifier
- // token. If so, Take the beginning of this token.
- // (It should be the same identifier because you can't have two adjacent
- // identifiers without another token in between.)
- Position PosCharBehind = Pos;
- --PosCharBehind.character;
-
- SourceLocation PeekBeforeLocation =
- getMacroArgExpandedLocation(SourceMgr, FE, PosCharBehind);
- Token Result;
- if (Lexer::getRawToken(PeekBeforeLocation, Result, SourceMgr,
- AST.getLangOpts(), false)) {
- // getRawToken failed, just use InputLocation.
- return InputLocation;
+ auto Offset = positionToOffset(SourceMgr.getBufferData(FID), Pos);
+ if (!Offset) {
+ log("getBeginningOfIdentifier: " + toString(Offset.takeError()));
+ return SourceLocation();
}
-
- if (Result.is(tok::raw_identifier)) {
- return Lexer::GetBeginningOfToken(PeekBeforeLocation, SourceMgr,
- AST.getLangOpts());
- }
-
- return InputLocation;
+ SourceLocation InputLoc = SourceMgr.getComposedLoc(FID, *Offset);
+
+ // GetBeginningOfToken(pos) is almost what we want, but does the wrong thing
+ // if the cursor is at the end of the identifier.
+ // Instead, we lex at GetBeginningOfToken(pos - 1). The cases are:
+ // 1) at the beginning of an identifier, we'll be looking at something
+ // that isn't an identifier.
+ // 2) at the middle or end of an identifier, we get the identifier.
+ // 3) anywhere outside an identifier, we'll get some non-identifier thing.
+ // We can't actually distinguish cases 1 and 3, but returning the original
+ // location is correct for both!
+ if (*Offset == 0) // Case 1 or 3.
+ return SourceMgr.getMacroArgExpandedLocation(InputLoc);
+ SourceLocation Before =
+ SourceMgr.getMacroArgExpandedLocation(InputLoc.getLocWithOffset(-1));
+ Before = Lexer::GetBeginningOfToken(Before, SourceMgr, AST.getLangOpts());
+ Token Tok;
+ if (Before.isValid() &&
+ !Lexer::getRawToken(Before, Tok, SourceMgr, AST.getLangOpts(), false) &&
+ Tok.is(tok::raw_identifier))
+ return Before; // Case 2.
+ return SourceMgr.getMacroArgExpandedLocation(InputLoc); // Case 1 or 3.
}
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -232,14 +232,8 @@
RefactoringResultCollector ResultCollector;
const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
- const FileEntry *FE =
- SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
- if (!FE)
- return CB(llvm::make_error<llvm::StringError>(
- "rename called for non-added document",
- llvm::errc::invalid_argument));
SourceLocation SourceLocationBeg =
- clangd::getBeginningOfIdentifier(AST, Pos, FE);
+ clangd::getBeginningOfIdentifier(AST, Pos, SourceMgr.getMainFileID());
tooling::RefactoringRuleContext Context(
AST.getASTContext().getSourceManager());
Context.setASTContext(AST.getASTContext());
@@ -361,11 +355,11 @@
void ClangdServer::findDefinitions(PathRef File, Position Pos,
Callback<std::vector<Location>> CB) {
auto FS = FSProvider.getFileSystem();
- auto Action = [Pos, FS](Callback<std::vector<Location>> CB,
- llvm::Expected<InputsAndAST> InpAST) {
+ auto Action = [Pos, FS, this](Callback<std::vector<Location>> CB,
+ llvm::Expected<InputsAndAST> InpAST) {
if (!InpAST)
return CB(InpAST.takeError());
- CB(clangd::findDefinitions(InpAST->AST, Pos));
+ CB(clangd::findDefinitions(InpAST->AST, Pos, this->FileIdx.get()));
};
WorkScheduler.runWithAST("Definitions", File, Bind(Action, std::move(CB)));
Index: clangd/CMakeLists.txt
===================================================================
--- clangd/CMakeLists.txt
+++ clangd/CMakeLists.txt
@@ -28,6 +28,7 @@
Logger.cpp
Protocol.cpp
ProtocolHandlers.cpp
+ Quality.cpp
SourceCode.cpp
Threading.cpp
Trace.cpp
Index: clangd/AST.cpp
===================================================================
--- clangd/AST.cpp
+++ clangd/AST.cpp
@@ -32,7 +32,7 @@
// be "<scratch space>"
// * symbols controlled and defined by a compile command-line option
// `-DName=foo`, the spelling location will be "<command line>".
- SpellingLoc = SM.getExpansionRange(D->getLocation()).first;
+ SpellingLoc = SM.getExpansionRange(D->getLocation()).getBegin();
}
}
return SpellingLoc;
Index: clang-tidy/tool/ClangTidyMain.cpp
===================================================================
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -41,7 +41,6 @@
Checks: '-*,some-check'
WarningsAsErrors: ''
HeaderFilterRegex: ''
- AnalyzeTemporaryDtors: false
FormatStyle: none
User: user
CheckOptions:
@@ -182,16 +181,6 @@
cl::init(false),
cl::cat(ClangTidyCategory));
-static cl::opt<bool> AnalyzeTemporaryDtors("analyze-temporary-dtors",
- cl::desc(R"(
-Enable temporary destructor-aware analysis in
-clang-analyzer- checks.
-This option overrides the value read from a
-.clang-tidy file.
-)"),
- cl::init(false),
- cl::cat(ClangTidyCategory));
-
static cl::opt<std::string> ExportFixes("export-fixes", cl::desc(R"(
YAML file to store suggested fixes in. The
stored fixes can be applied to the input source
@@ -300,7 +289,6 @@
DefaultOptions.WarningsAsErrors = "";
DefaultOptions.HeaderFilterRegex = HeaderFilter;
DefaultOptions.SystemHeaders = SystemHeaders;
- DefaultOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
DefaultOptions.FormatStyle = FormatStyle;
DefaultOptions.User = llvm::sys::Process::GetEnv("USER");
// USERNAME is used on Windows.
@@ -316,8 +304,6 @@
OverrideOptions.HeaderFilterRegex = HeaderFilter;
if (SystemHeaders.getNumOccurrences() > 0)
OverrideOptions.SystemHeaders = SystemHeaders;
- if (AnalyzeTemporaryDtors.getNumOccurrences() > 0)
- OverrideOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
if (FormatStyle.getNumOccurrences() > 0)
OverrideOptions.FormatStyle = FormatStyle;
Index: clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
===================================================================
--- clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
+++ clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
@@ -47,27 +47,22 @@
if (isUnresolvedExceptionSpec(ProtoType->getExceptionSpecType()))
return;
- switch (ProtoType->getNoexceptSpec(*Result.Context)) {
- case FunctionProtoType::NR_NoNoexcept:
+ if (!isNoexceptExceptionSpec(ProtoType->getExceptionSpecType())) {
diag(Decl->getLocation(), "move %0s should be marked noexcept")
<< MethodType;
// FIXME: Add a fixit.
- break;
- case FunctionProtoType::NR_Throw:
- // Don't complain about nothrow(false), but complain on nothrow(expr)
- // where expr evaluates to false.
- if (const Expr *E = ProtoType->getNoexceptExpr()) {
- if (isa<CXXBoolLiteralExpr>(E))
- break;
+ return;
+ }
+
+ // Don't complain about nothrow(false), but complain on nothrow(expr)
+ // where expr evaluates to false.
+ if (ProtoType->canThrow() == CT_Can) {
+ Expr *E = ProtoType->getNoexceptExpr();
+ if (!isa<CXXBoolLiteralExpr>(ProtoType->getNoexceptExpr())) {
diag(E->getExprLoc(),
"noexcept specifier on the move %0 evaluates to 'false'")
<< MethodType;
}
- break;
- case FunctionProtoType::NR_Nothrow:
- case FunctionProtoType::NR_Dependent:
- case FunctionProtoType::NR_BadNoexcept:
- break;
}
}
}
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===================================================================
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -45,11 +45,17 @@
"ARGB",
"ASCII",
"BGRA",
+ "CA",
+ "CF",
+ "CG",
+ "CI",
+ "CV",
"CMYK",
"DNS",
"FPS",
"FTP",
"GIF",
+ "GL",
"GPS",
"GUID",
"HD",
@@ -65,6 +71,7 @@
"LZW",
"MDNS",
"MIDI",
+ "NS",
"OS",
"PDF",
"PIN",
@@ -81,6 +88,7 @@
"RPC",
"RTF",
"RTL",
+ "SC",
"SDK",
"SSO",
"TCP",
Index: clang-tidy/modernize/UseNullptrCheck.cpp
===================================================================
--- clang-tidy/modernize/UseNullptrCheck.cpp
+++ clang-tidy/modernize/UseNullptrCheck.cpp
@@ -332,7 +332,7 @@
NullMacros.end();
}
- MacroLoc = SM.getExpansionRange(ArgLoc).first;
+ MacroLoc = SM.getExpansionRange(ArgLoc).getBegin();
ArgLoc = Expansion.getSpellingLoc().getLocWithOffset(LocInfo.second);
if (ArgLoc.isFileID())
@@ -387,7 +387,7 @@
continue;
}
- MacroLoc = SM.getImmediateExpansionRange(Loc).first;
+ MacroLoc = SM.getImmediateExpansionRange(Loc).getBegin();
if (MacroLoc.isFileID() && MacroLoc == TestMacroLoc) {
// Match made.
return true;
Index: clang-tidy/modernize/UseNoexceptCheck.cpp
===================================================================
--- clang-tidy/modernize/UseNoexceptCheck.cpp
+++ clang-tidy/modernize/UseNoexceptCheck.cpp
@@ -89,7 +89,7 @@
Result.Context->getLangOpts());
assert(FnTy && "FunctionProtoType is null.");
- bool IsNoThrow = FnTy->isNothrow(*Result.Context);
+ bool IsNoThrow = FnTy->isNothrow();
StringRef ReplacementStr =
IsNoThrow
? NoexceptMacro.empty() ? "noexcept" : NoexceptMacro.c_str()
Index: clang-tidy/modernize/RawStringLiteralCheck.h
===================================================================
--- clang-tidy/modernize/RawStringLiteralCheck.h
+++ clang-tidy/modernize/RawStringLiteralCheck.h
@@ -11,11 +11,14 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_RAW_STRING_LITERAL_H
#include "../ClangTidy.h"
+#include <bitset>
namespace clang {
namespace tidy {
namespace modernize {
+using CharsBitSet = std::bitset<1 << CHAR_BIT>;
+
/// This check replaces string literals with escaped characters to
/// raw string literals.
///
@@ -35,6 +38,7 @@
const StringLiteral *Literal, StringRef Replacement);
std::string DelimiterStem;
+ CharsBitSet DisallowedChars;
const bool ReplaceShorterLiterals;
};
Index: clang-tidy/modernize/RawStringLiteralCheck.cpp
===================================================================
--- clang-tidy/modernize/RawStringLiteralCheck.cpp
+++ clang-tidy/modernize/RawStringLiteralCheck.cpp
@@ -42,28 +42,15 @@
}
bool containsEscapedCharacters(const MatchFinder::MatchResult &Result,
- const StringLiteral *Literal) {
+ const StringLiteral *Literal,
+ const CharsBitSet &DisallowedChars) {
// FIXME: Handle L"", u8"", u"" and U"" literals.
if (!Literal->isAscii())
return false;
- StringRef Bytes = Literal->getBytes();
- // Non-printing characters disqualify this literal:
- // \007 = \a bell
- // \010 = \b backspace
- // \011 = \t horizontal tab
- // \012 = \n new line
- // \013 = \v vertical tab
- // \014 = \f form feed
- // \015 = \r carriage return
- // \177 = delete
- if (Bytes.find_first_of(StringRef("\000\001\002\003\004\005\006\a"
- "\b\t\n\v\f\r\016\017"
- "\020\021\022\023\024\025\026\027"
- "\030\031\032\033\034\035\036\037"
- "\177",
- 33)) != StringRef::npos)
- return false;
+ for (const unsigned char C : Literal->getBytes())
+ if (DisallowedChars.test(C))
+ return false;
CharSourceRange CharRange = Lexer::makeFileCharRange(
CharSourceRange::getTokenRange(Literal->getSourceRange()),
@@ -102,7 +89,28 @@
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
DelimiterStem(Options.get("DelimiterStem", "lit")),
- ReplaceShorterLiterals(Options.get("ReplaceShorterLiterals", false)) {}
+ ReplaceShorterLiterals(Options.get("ReplaceShorterLiterals", false)) {
+ // Non-printing characters are disallowed:
+ // \007 = \a bell
+ // \010 = \b backspace
+ // \011 = \t horizontal tab
+ // \012 = \n new line
+ // \013 = \v vertical tab
+ // \014 = \f form feed
+ // \015 = \r carriage return
+ // \177 = delete
+ for (const unsigned char C : StringRef("\000\001\002\003\004\005\006\a"
+ "\b\t\n\v\f\r\016\017"
+ "\020\021\022\023\024\025\026\027"
+ "\030\031\032\033\034\035\036\037"
+ "\177",
+ 33))
+ DisallowedChars.set(C);
+
+ // Non-ASCII are disallowed too.
+ for (unsigned int C = 0x80u; C <= 0xFFu; ++C)
+ DisallowedChars.set(static_cast<unsigned char>(C));
+}
void RawStringLiteralCheck::storeOptions(ClangTidyOptions::OptionMap &Options) {
ClangTidyCheck::storeOptions(Options);
@@ -124,7 +132,7 @@
if (Literal->getLocStart().isMacroID())
return;
- if (containsEscapedCharacters(Result, Literal)) {
+ if (containsEscapedCharacters(Result, Literal, DisallowedChars)) {
std::string Replacement = asRawStringLiteral(Literal, DelimiterStem);
if (ReplaceShorterLiterals ||
Replacement.length() <=
Index: clang-tidy/google/IntegerTypesCheck.cpp
===================================================================
--- clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tidy/google/IntegerTypesCheck.cpp
@@ -11,6 +11,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/AttrKinds.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/TargetInfo.h"
@@ -56,7 +57,16 @@
// Find all TypeLocs. The relevant Style Guide rule only applies to C++.
if (!getLangOpts().CPlusPlus)
return;
- Finder->addMatcher(typeLoc(loc(isInteger())).bind("tl"), this);
+ // Match any integer types, unless they are passed to a printf-based API:
+ //
+ // http://google.github.io/styleguide/cppguide.html#64-bit_Portability
+ // "Where possible, avoid passing arguments of types specified by
+ // bitwidth typedefs to printf-based APIs."
+ Finder->addMatcher(typeLoc(loc(isInteger()),
+ unless(hasAncestor(callExpr(
+ callee(functionDecl(hasAttr(attr::Format)))))))
+ .bind("tl"),
+ this);
IdentTable = llvm::make_unique<IdentifierTable>(getLangOpts());
}
Index: clang-tidy/bugprone/MultipleStatementMacroCheck.cpp
===================================================================
--- clang-tidy/bugprone/MultipleStatementMacroCheck.cpp
+++ clang-tidy/bugprone/MultipleStatementMacroCheck.cpp
@@ -38,17 +38,18 @@
return nextStmt(Result, Parent);
}
-using ExpansionRanges = std::vector<std::pair<SourceLocation, SourceLocation>>;
+using ExpansionRanges = std::vector<SourceRange>;
/// \bried Get all the macro expansion ranges related to `Loc`.
///
/// The result is ordered from most inner to most outer.
ExpansionRanges getExpansionRanges(SourceLocation Loc,
const MatchFinder::MatchResult &Result) {
ExpansionRanges Locs;
while (Loc.isMacroID()) {
- Locs.push_back(Result.SourceManager->getImmediateExpansionRange(Loc));
- Loc = Locs.back().first;
+ Locs.push_back(
+ Result.SourceManager->getImmediateExpansionRange(Loc).getAsRange());
+ Loc = Locs.back().getBegin();
}
return Locs;
}
@@ -96,9 +97,9 @@
InnerRanges.back() != NextRanges.back())
return;
- diag(InnerRanges.back().first, "multiple statement macro used without "
- "braces; some statements will be "
- "unconditionally executed");
+ diag(InnerRanges.back().getBegin(), "multiple statement macro used without "
+ "braces; some statements will be "
+ "unconditionally executed");
}
} // namespace bugprone
Index: clang-tidy/bugprone/LambdaFunctionNameCheck.cpp
===================================================================
--- clang-tidy/bugprone/LambdaFunctionNameCheck.cpp
+++ clang-tidy/bugprone/LambdaFunctionNameCheck.cpp
@@ -81,7 +81,7 @@
if (E->getLocation().isMacroID()) {
auto ER =
Result.SourceManager->getImmediateExpansionRange(E->getLocation());
- if (SuppressMacroExpansions.find(SourceRange(ER.first, ER.second)) !=
+ if (SuppressMacroExpansions.find(ER.getAsRange()) !=
SuppressMacroExpansions.end()) {
// This is a macro expansion for which we should not warn.
return;
Index: clang-tidy/ClangTidyOptions.h
===================================================================
--- clang-tidy/ClangTidyOptions.h
+++ clang-tidy/ClangTidyOptions.h
@@ -74,9 +74,6 @@
/// \brief Output warnings from system headers matching \c HeaderFilterRegex.
llvm::Optional<bool> SystemHeaders;
- /// \brief Turns on temporary destructor-based analysis.
- llvm::Optional<bool> AnalyzeTemporaryDtors;
-
/// \brief Format code around applied fixes with clang-format using this
/// style.
///
Index: clang-tidy/ClangTidyOptions.cpp
===================================================================
--- clang-tidy/ClangTidyOptions.cpp
+++ clang-tidy/ClangTidyOptions.cpp
@@ -86,7 +86,6 @@
IO.mapOptional("Checks", Options.Checks);
IO.mapOptional("WarningsAsErrors", Options.WarningsAsErrors);
IO.mapOptional("HeaderFilterRegex", Options.HeaderFilterRegex);
- IO.mapOptional("AnalyzeTemporaryDtors", Options.AnalyzeTemporaryDtors);
IO.mapOptional("FormatStyle", Options.FormatStyle);
IO.mapOptional("User", Options.User);
IO.mapOptional("CheckOptions", NOpts->Options);
@@ -107,7 +106,6 @@
Options.WarningsAsErrors = "";
Options.HeaderFilterRegex = "";
Options.SystemHeaders = false;
- Options.AnalyzeTemporaryDtors = false;
Options.FormatStyle = "none";
Options.User = llvm::None;
for (ClangTidyModuleRegistry::iterator I = ClangTidyModuleRegistry::begin(),
@@ -147,7 +145,6 @@
mergeCommaSeparatedLists(Result.WarningsAsErrors, Other.WarningsAsErrors);
overrideValue(Result.HeaderFilterRegex, Other.HeaderFilterRegex);
overrideValue(Result.SystemHeaders, Other.SystemHeaders);
- overrideValue(Result.AnalyzeTemporaryDtors, Other.AnalyzeTemporaryDtors);
overrideValue(Result.FormatStyle, Other.FormatStyle);
overrideValue(Result.User, Other.User);
mergeVectors(Result.ExtraArgs, Other.ExtraArgs);
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -374,7 +374,7 @@
return true;
if (!Loc.isMacroID())
return false;
- Loc = SM.getImmediateExpansionRange(Loc).first;
+ Loc = SM.getImmediateExpansionRange(Loc).getBegin();
}
return false;
}
Index: clang-tidy/ClangTidy.cpp
===================================================================
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -369,11 +369,6 @@
Consumers.push_back(Finder->newASTConsumer());
AnalyzerOptionsRef AnalyzerOptions = Compiler.getAnalyzerOpts();
- // FIXME: Remove this option once clang's cfg-temporary-dtors option defaults
- // to true.
- AnalyzerOptions->Config["cfg-temporary-dtors"] =
- Context.getOptions().AnalyzeTemporaryDtors ? "true" : "false";
-
AnalyzerOptions->CheckersControlList = getCheckersControlList(Context);
if (!AnalyzerOptions->CheckersControlList.empty()) {
setStaticAnalyzerCheckerOpts(Context.getOptions(), AnalyzerOptions);
Index: clang-move/ClangMove.cpp
===================================================================
--- clang-move/ClangMove.cpp
+++ clang-move/ClangMove.cpp
@@ -280,7 +280,10 @@
getLocForEndOfDecl(const clang::Decl *D,
const LangOptions &LangOpts = clang::LangOptions()) {
const auto &SM = D->getASTContext().getSourceManager();
- auto EndExpansionLoc = SM.getExpansionRange(D->getLocEnd()).second;
+ // If the expansion range is a character range, this is the location of
+ // the first character past the end. Otherwise it's the location of the
+ // first character in the final token in the range.
+ auto EndExpansionLoc = SM.getExpansionRange(D->getLocEnd()).getEnd();
std::pair<FileID, unsigned> LocInfo = SM.getDecomposedLoc(EndExpansionLoc);
// Try to load the file buffer.
bool InvalidTemp = false;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
