[clang-tools-extra] r344513 - [clangd] Minimal implementation of automatic static index (not enabled).
Author: sammccall
Date: Mon Oct 15 06:34:10 2018
New Revision: 344513
URL: http://llvm.org/viewvc/llvm-project?rev=344513&view=rev
Log:
[clangd] Minimal implementation of automatic static index (not enabled).
Summary:
See tinyurl.com/clangd-automatic-index for design and goals.
Lots of limitations to keep this patch smallish, TODOs everywhere:
- no serialization to disk
- no changes to dynamic index, which now has a much simpler job
- no partitioning of symbols by file to avoid duplication of header symbols
- no reindexing of edited files
- only a single worker thread
- compilation database is slurped synchronously (doesn't scale)
- uses memindex, rebuilds after every file (should be dex, periodically)
It's not hooked up to ClangdServer/ClangdLSPServer yet: the layering
isn't clear (it should really be in ClangdServer, but ClangdLSPServer
has all the CDB interactions).
Reviewers: ioeric
Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, jfb,
cfe-commits
Differential Revision: https://reviews.llvm.org/D53032
Added:
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/Background.h
clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp
clang-tools-extra/trunk/unittests/clangd/SyncAPI.h
Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=344513&r1=344512&r2=344513&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Mon Oct 15 06:34:10 2018
@@ -38,6 +38,7 @@ add_clang_library(clangDaemon
URI.cpp
XRefs.cpp
+ index/Background.cpp
index/CanonicalIncludes.cpp
index/FileIndex.cpp
index/Index.cpp
Added: clang-tools-extra/trunk/clangd/index/Background.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=344513&view=auto
==
--- clang-tools-extra/trunk/clangd/index/Background.cpp (added)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Mon Oct 15 06:34:10 2018
@@ -0,0 +1,191 @@
+//===-- Background.cpp - Build an index in a background thread
===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "index/Background.h"
+#include "ClangdUnit.h"
+#include "Compiler.h"
+#include "Logger.h"
+#include "Trace.h"
+#include "index/IndexAction.h"
+#include "index/MemIndex.h"
+#include "index/Serialization.h"
+#include "llvm/Support/SHA1.h"
+#include
+
+using namespace llvm;
+namespace clang {
+namespace clangd {
+
+BackgroundIndex::BackgroundIndex(Context BackgroundContext,
+ StringRef ResourceDir,
+ const FileSystemProvider &FSProvider)
+: SwapIndex(llvm::make_unique()), ResourceDir(ResourceDir),
+ FSProvider(FSProvider), BackgroundContext(std::move(BackgroundContext)),
+ Thread([this] { run(); }) {}
+
+BackgroundIndex::~BackgroundIndex() {
+ stop();
+ Thread.join();
+}
+
+void BackgroundIndex::stop() {
+ {
+std::lock_guard Lock(QueueMu);
+ShouldStop = true;
+ }
+ QueueCV.notify_all();
+}
+
+void BackgroundIndex::run() {
+ WithContext Background(std::move(BackgroundContext));
+ while (true) {
+llvm::Optional Task;
+{
+ std::unique_lock Lock(QueueMu);
+ QueueCV.wait(Lock, [&] { return ShouldStop || !Queue.empty(); });
+ if (ShouldStop) {
+Queue.clear();
+QueueCV.notify_all();
+return;
+ }
+ ++NumActiveTasks;
+ Task = std::move(Queue.front());
+ Queue.pop_front();
+}
+(*Task)();
+{
+ std::unique_lock Lock(QueueMu);
+ assert(NumActiveTasks > 0 && "before decrementing");
+ --NumActiveTasks;
+}
+QueueCV.notify_all();
+ }
+}
+
+void BackgroundIndex::blockUntilIdleForTest() {
+ std::unique_lock Lock(QueueMu);
+ QueueCV.wait(Lock, [&] { return Queue.empty() && NumActiveTasks == 0; });
+}
+
+void BackgroundIndex::enqueue(StringRef Directory,
+ tooling::CompileCommand Cmd) {
+ std::lock_guard Lock(QueueMu);
+ enqueueLocked(std::move(Cmd));
+}
+
+void BackgroundIndex::enqueueAll(StringRef Directory,
+ const tooling::CompilationDatabase &CDB) {
+ trace::Span Tracer("BackgroundIndexEnqueueCDB");
+ // FIXME: this function may be slow. Perhaps enqueue a task to re-read the
CDB
+ // fro
[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.
ioeric accepted this revision.
ioeric added a comment.
still lgtm
Comment at: clangd/Compiler.cpp:83
+std::string getStandardResourceDir() {
+ static int Dummy; // Just an address in this process.
Maybe also revert this change?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53032
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.
sammccall added inline comments.
Comment at: unittests/clangd/BackgroundIndexTests.cpp:14
+
+TEST(BackgroundIndexTest, IndexesOneFile) {
+ MockFSProvider FS;
sammccall wrote:
> ioeric wrote:
> > sammccall wrote:
> > > ioeric wrote:
> > > > sammccall wrote:
> > > > > ioeric wrote:
> > > > > > Also add a test for `enqueueAll` with multiple TUs ?
> > > > > Is it important to call `enqueueAll` specifically vs `enqueue`
> > > > > multiple times?
> > > > >
> > > > > We don't have a good test fixture for a compilation database, and
> > > > > `enqueueAll` is trivial...
> > > > I think the randomization code worths a test.
> > > >
> > > > How about adding a test in ClangdServer with the auto index enabled? I
> > > > think we'd also want coverage in ClangdServer anyway.
> > > How would you suggest testing the randomization :-)
> > >
> > > The problem with a ClangdServer test is that it doesn't know anything
> > > about autoindex.
> > > AutoIndex lives in ClangdLSPServer, because that's where the compilation
> > > database lives (because people keep cramming LSP extensions in to
> > > manipulate it, and I haven't been able to remove them).
> > > Nobody has worked out how to test ClangdLSPServer yet. It's a worthy
> > > project, but...
> > > How would you suggest testing the randomization :-)
> > Not suggesting testing the behavior; just test that it works really. I
> > guess a single file would also do it.
> >
> > > The problem with a ClangdServer test is that it doesn't know anything
> > > about autoindex.
> > Ah, I see. That's a bit unfortunate. ClangdServer seems to be a better
> > place for the auto index. I wonder if we could move AutoIndex into
> > ClangdServer? I'm not very familiar with CDB APIs, but intuitively this
> > seems doable if we tweak the interface of CDB a bit e.g. inject the
> > callback into `GlobalCompilationDatabase` without going through
> > `CompilationDB`? Not sure if how well this would play with the CDB design
> > though.
> > ClangdServer seems to be a better place for the auto index
> I fully agree.
>
> > I wonder if we could move AutoIndex into ClangdServer? I'm not very
> > familiar with CDB APIs...
> Currently the global CDB is trying to be all things to all people:
> - by default, it follows Tooling conventions and looks for compile_commands
> above the source files
> - google's hosted option wants it to be completely opaque, no indexing
> - apple wants it to be an in-memory store mutated over LSP
> - ericsson wants it to be a fixed directory set by the user
>
> And we've implemented these in ad-hoc ways, not behind any real abstraction.
> I don't think adding more functionality to the interface without addressing
> this is a great idea, and I don't really know how to untangle all this
> without asking people to rethink their use cases. But I'll think about this
> some more.
After thinking about this, decided to just check in the library with unit tests
for now while we sort out the layering.
I've ripped out all the ClangdMain and ClangdLSPServer changes.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53032
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.
sammccall updated this revision to Diff 169694.
sammccall marked an inline comment as done.
sammccall added a comment.
Address comments, strip out of clangdlspserver for now.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53032
Files:
clangd/CMakeLists.txt
clangd/ClangdServer.cpp
clangd/Compiler.cpp
clangd/Compiler.h
clangd/index/Background.cpp
clangd/index/Background.h
unittests/clangd/BackgroundIndexTests.cpp
unittests/clangd/CMakeLists.txt
unittests/clangd/SyncAPI.cpp
unittests/clangd/SyncAPI.h
Index: unittests/clangd/SyncAPI.h
===
--- unittests/clangd/SyncAPI.h
+++ unittests/clangd/SyncAPI.h
@@ -17,7 +17,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H
#include "ClangdServer.h"
-#include
+#include "index/Index.h"
namespace clang {
namespace clangd {
@@ -50,6 +50,9 @@
llvm::Expected>
runDocumentSymbols(ClangdServer &Server, PathRef File);
+SymbolSlab runFuzzyFind(const SymbolIndex &Index, StringRef Query);
+SymbolSlab runFuzzyFind(const SymbolIndex &Index, const FuzzyFindRequest &Req);
+
} // namespace clangd
} // namespace clang
Index: unittests/clangd/SyncAPI.cpp
===
--- unittests/clangd/SyncAPI.cpp
+++ unittests/clangd/SyncAPI.cpp
@@ -125,5 +125,17 @@
return std::move(*Result);
}
+SymbolSlab runFuzzyFind(const SymbolIndex &Index, StringRef Query) {
+ FuzzyFindRequest Req;
+ Req.Query = Query;
+ return runFuzzyFind(Index, Req);
+}
+
+SymbolSlab runFuzzyFind(const SymbolIndex &Index, const FuzzyFindRequest &Req) {
+ SymbolSlab::Builder Builder;
+ Index.fuzzyFind(Req, [&](const Symbol &Sym) { Builder.insert(Sym); });
+ return std::move(Builder).build();
+}
+
} // namespace clangd
} // namespace clang
Index: unittests/clangd/CMakeLists.txt
===
--- unittests/clangd/CMakeLists.txt
+++ unittests/clangd/CMakeLists.txt
@@ -10,6 +10,7 @@
add_extra_unittest(ClangdTests
Annotations.cpp
+ BackgroundIndexTests.cpp
CancellationTests.cpp
ClangdTests.cpp
ClangdUnitTests.cpp
Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- /dev/null
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -0,0 +1,37 @@
+#include "SyncAPI.h"
+#include "TestFS.h"
+#include "index/Background.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using testing::UnorderedElementsAre;
+
+namespace clang {
+namespace clangd {
+
+MATCHER_P(Named, N, "") { return arg.Name == N; }
+
+TEST(BackgroundIndexTest, IndexTwoFiles) {
+ MockFSProvider FS;
+ // a.h yields different symbols when included by A.cc vs B.cc.
+ // Currently we store symbols for each TU, so we get both.
+ FS.Files[testPath("root/A.h")] = "void a_h(); void NAME(){}";
+ FS.Files[testPath("root/A.cc")] = "#include \"A.h\"";
+ FS.Files[testPath("root/B.cc")] = "#define NAME bar\n#include \"A.h\"";
+ BackgroundIndex Idx(Context::empty(), "", FS);
+
+ tooling::CompileCommand Cmd;
+ Cmd.Filename = testPath("root/A.cc");
+ Cmd.Directory = testPath("root");
+ Cmd.CommandLine = {"clang++", "-DNAME=foo", testPath("root/A.cc")};
+ Idx.enqueue(testPath("root"), Cmd);
+ Cmd.CommandLine.back() = Cmd.Filename = testPath("root/B.cc");
+ Idx.enqueue(testPath("root"), Cmd);
+
+ Idx.blockUntilIdleForTest();
+ EXPECT_THAT(runFuzzyFind(Idx, ""),
+ UnorderedElementsAre(Named("a_h"), Named("foo"), Named("bar")));
+}
+
+} // namespace clangd
+} // namespace clang
Index: clangd/index/Background.h
===
--- /dev/null
+++ clangd/index/Background.h
@@ -0,0 +1,79 @@
+//===--- Background.h - Build an index in a background thread *- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_BACKGROUND_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_BACKGROUND_H
+
+#include "Context.h"
+#include "FSProvider.h"
+#include "index/FileIndex.h"
+#include "index/Index.h"
+#include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/Support/SHA1.h"
+#include
+#include
+#include
+
+namespace clang {
+namespace clangd {
+
+// Builds an in-memory index by by running the static indexer action over
+// all commands in a compilation database. Indexing happens in the background.
+// FIXME: it should also persist its state on disk for fast start.
+// FIXME: it should watch for changes to files on disk.
+class BackgroundIndex : public SwapIndex {
+public:
+ // FIXME: resource-dir injection should be hoisted somewhere common.
+ BackgroundIndex(Context BackgroundContext, StringRef ResourceDir,
+
[PATCH] D53280: [analyzer] Emit a warning for unknown -analyzer-config options
Szelethus updated this revision to Diff 169693.
https://reviews.llvm.org/D53280
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/Frontend/CompilerInvocation.cpp
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -314,6 +314,8 @@
Opts.CheckersControlList.emplace_back(checker, enable);
}
+ const std::vector RegisteredOptions = Opts.getConfigOptionList();
+
// Go through the analyzer configuration options.
for (const auto *A : Args.filtered(OPT_analyzer_config)) {
A->claim();
@@ -338,6 +340,14 @@
Success = false;
break;
}
+ // Check whether this really is a valid -analyzer-condfig option.
+ // TODO: We should check whether all options are valid or not, but for
+ // now, skip checker options.
+ if (key.count(':') == 0) {
+if (llvm::find(RegisteredOptions, key) == RegisteredOptions.end())
+ Diags.Report(diag::warn_analyzer_config_unknown_config) << key;
+ }
+
Opts.Config[key] = val;
}
}
Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===
--- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -319,6 +319,15 @@
template
T getDefaultValForUserMode(T ShallowVal, T DeepVal);
+ std::vector getConfigOptionList() const {
+return {
+#define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC) \
+ CMDFLAG,
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.def"
+#undef ANALYZER_OPTION
+};
+ }
+
#define ANALYZER_OPTION_WITH_FN(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL, \
CREATE_FN) \
TYPE CREATE_FN();
Index: include/clang/Basic/DiagnosticDriverKinds.td
===
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -291,6 +291,9 @@
"analyzer-config option '%0' has a key but no value">;
def err_analyzer_config_multiple_values : Error<
"analyzer-config option '%0' should contain only one '='">;
+def warn_analyzer_config_unknown_config : Warning<
+ "unkown -analyzer-config option '%0'">,
+ InGroup;
def err_drv_invalid_hvx_length : Error<
"-mhvx-length is not supported without a -mhvx/-mhvx= flag">;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -314,6 +314,8 @@
Opts.CheckersControlList.emplace_back(checker, enable);
}
+ const std::vector RegisteredOptions = Opts.getConfigOptionList();
+
// Go through the analyzer configuration options.
for (const auto *A : Args.filtered(OPT_analyzer_config)) {
A->claim();
@@ -338,6 +340,14 @@
Success = false;
break;
}
+ // Check whether this really is a valid -analyzer-condfig option.
+ // TODO: We should check whether all options are valid or not, but for
+ // now, skip checker options.
+ if (key.count(':') == 0) {
+if (llvm::find(RegisteredOptions, key) == RegisteredOptions.end())
+ Diags.Report(diag::warn_analyzer_config_unknown_config) << key;
+ }
+
Opts.Config[key] = val;
}
}
Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===
--- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -319,6 +319,15 @@
template
T getDefaultValForUserMode(T ShallowVal, T DeepVal);
+ std::vector getConfigOptionList() const {
+return {
+#define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC) \
+ CMDFLAG,
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.def"
+#undef ANALYZER_OPTION
+};
+ }
+
#define ANALYZER_OPTION_WITH_FN(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL, \
CREATE_FN) \
TYPE CREATE_FN();
Index: include/clang/Basic/DiagnosticDriverKinds.td
===
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -291,6 +291,9 @@
"analyzer-config option '%0' has a key but no value">;
def err_analyzer_config_multiple_values : Error<
"analyzer-config option '%0' should contain only one '='">;
+def warn_analyzer_config_unknown_config : Warning<
+ "unkown -analyzer-config option '%0'">,
+ InGroup;
def err_drv_invalid_hvx_length : Error<
"-mhvx-length is not supported without a -mhvx/-mhvx= flag">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.o
[PATCH] D53277: [analyzer][NFC][WIP] Collect all -analyzer-config options in a .def file
Szelethus added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:128-135
+/// Describes the kinds for high-level analyzer mode.
+enum UserModeKind {
+ /// Perform shallow but fast analyzes.
+ UMK_Shallow = 1,
+
+ /// Perform deep analyzes.
+ UMK_Deep = 2
Do we actually //ever// use `UMK_SHALLOW`? If not, the whole .def file can be
simplified, because the use of `getDefaultValForUserMode(/* ShallowVal */, /*
DeepVal */)` is clunky. If we do, I'd also be fine with rewriting the function
generating macro from
```
#define ANALYZER_OPTION_WITH_FN(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL, \
CREATE_FN)
```
to
```
#define ANALYZER_OPTION_WITH_FN(TYPE, NAME, CMDFLAG, DESC, SHALLOW_VAL, \
DEEP_VAL, CREATE_FN)
```
because that'd be nicer than the current solution.
https://reviews.llvm.org/D53277
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53286: [clangd] Refactor JSON-over-stdin/stdout code into Transport abstraction.
sammccall added a comment. This patch is big and hard to navigate. I tried to keep it contained, but JSONRPCDispatcher is pretty tangled. The main parts are: - `Transport.h` is the key new abstraction that should be understood first - `JSONTransport.cpp` is its standard implementation. The raw IO stuff in there is just lifted from the old JSONRPCDispatcher, and isn't very interesting. Everything else should be though - it shows how the new interface works. - The `ClangdLSPServer` and `ClangdMain` changes show how the new interface is used - `JSONRPCDispatcher` has had most of its guts ripped out. There's more that can be done there, that file may go away completely. - The test changes just reflect the shutdown getting a little bit stricter (they also pass before this patch) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53286 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker
peter.smith updated this revision to Diff 169689.
peter.smith marked 3 inline comments as done.
peter.smith added a comment.
Updated diff to reflect review comments. Main changes are:
- isArmBigEndian always returns false if the target architecture isn't Arm.
- Added tests to make sure "--be8" doesn't get added by mistake (would have
been in previous patch for aarch64_be arch with -mbig-endian flag.
https://reviews.llvm.org/D52784
Files:
lib/Driver/ToolChains/Arch/ARM.cpp
lib/Driver/ToolChains/Arch/ARM.h
lib/Driver/ToolChains/Gnu.cpp
lib/Driver/ToolChains/NetBSD.cpp
test/Driver/linux-as.c
test/Driver/linux-ld.c
Index: test/Driver/linux-ld.c
===
--- test/Driver/linux-ld.c
+++ test/Driver/linux-ld.c
@@ -1759,6 +1759,7 @@
// RUN: | FileCheck --check-prefix=CHECK-ARMEB %s
// CHECK-ARMEB: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
// CHECK-ARMEB-NOT: "--be8"
+// CHECK-ARMEB: "-EB"
// CHECK-ARMEB: "-m" "armelfb_linux_eabi"
// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -1768,16 +1769,88 @@
// RUN: | FileCheck --check-prefix=CHECK-ARMV7EB %s
// CHECK-ARMV7EB: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
// CHECK-ARMV7EB: "--be8"
+// CHECK-ARMV7EB: "-EB"
// CHECK-ARMV7EB: "-m" "armelfb_linux_eabi"
// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=armv7-unknown-linux \
+// RUN: -mbig-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: | FileCheck --check-prefix=CHECK-ARMV7EB %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=armebv7-unknown-linux \
+// RUN: -mbig-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: | FileCheck --check-prefix=CHECK-ARMV7EB %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=armv7-unknown-linux \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: | FileCheck --check-prefix=CHECK-ARMV7EL %s
+// CHECK-ARMV7EL: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-ARMV7EL-NOT: "--be8"
+// CHECK-ARMV7EL: "-EL"
+// CHECK-ARMV7EL: "-m" "armelf_linux_eabi"
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=armebv7-unknown-linux \
+// RUN: -mlittle-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: | FileCheck --check-prefix=CHECK-ARMV7EL %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=armv7-unknown-linux \
+// RUN: -mlittle-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: | FileCheck --check-prefix=CHECK-ARMV7EL %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
// RUN: --target=aarch64_be-unknown-linux \
// RUN: --gcc-toolchain="" \
// RUN: --sysroot=%S/Inputs/basic_linux_tree \
// RUN: | FileCheck --check-prefix=CHECK-AARCH64BE %s
// CHECK-AARCH64BE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-AARCH64BE-NOT: "--be8"
+// CHECK-AARCH64BE: "-EB"
// CHECK-AARCH64BE: "-m" "aarch64linuxb"
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=aarch64-unknown-linux \
+// RUN: -mbig-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: | FileCheck --check-prefix=CHECK-AARCH64BE %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=aarch64_be-unknown-linux \
+// RUN: -mbig-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: | FileCheck --check-prefix=CHECK-AARCH64BE %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=aarch64-unknown-linux \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: | FileCheck --check-prefix=CHECK-AARCH64LE %s
+// CHECK-AARCH64LE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-AARCH64LE-NOT: "--be8"
+// CHECK-AARCH64LE: "-EL"
+// CHECK-AARCH64LE: "-m" "aarch64linux"
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=aarch64_be-unknown-linux \
+// RUN: -mlittle-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: | FileCheck --check-prefix=CHECK-AARCH64LE %s
+
// Check dynamic-linker for musl-libc
// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=i386-pc-linux-musl \
Index: test/Driver/linux-as.c
===
--- test/Driver/linux-as.c
+++ test/Driver/linux-as.c
@@ -3,129 +3,160 @@
// RUN: %clang -target arm-linux -### \
// RUN: -no-integrated-as -c %s 2>&1 \
// RUN: | FileCheck -check-pre
[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker
peter.smith marked 7 inline comments as done.
peter.smith added a comment.
Thanks very much for the comments. I'll post an update shortly.
Comment at: lib/Driver/ToolChains/Gnu.cpp:357-364
+const char* EndianFlag = "-EL";
+if (isArmBigEndian(Triple, Args)) {
+ EndianFlag = "-EB";
+ arm::appendBE8LinkFlag(Args, CmdArgs, Triple);
+}
+else if (Arch == llvm::Triple::aarch64_be)
+ EndianFlag = "-EB";
nickdesaulniers wrote:
> nickdesaulniers wrote:
> > ```
> > bool IsBigEndian = isArmBigEndian(Triple, Args);
> > if (IsBigEndian)
> > arm::appendBE8LinkFlag(Args, CmdArgs, Triple);
> > IsBigEndian |= Arch == llvm::Triple::aarch64_be;
> > CmdArgs.push_back(IsBigEndian ? "-EB" : "-EL");
> > ```
> `IsBigEndian |= Arch == llvm::Triple::aarch64_be;`
>
> should be:
>
> `IsBigEndian = IsBigEndian || Arch == llvm::Triple::aarch64_be;`
>
> in order to not evaluate `Arch == llvm::Triple::aarch64_b` if `IsBigEndian`
> is already true.
Thanks for the suggestion. One thing it highlighted was that isArmBigEndian
could return true for an aarch64_be arch with -mbig-endian so I've rewritten
isArmBigEndian to always return false if the architecture isn't Arm and have
added some test cases to check that "--be8" doesn't sneak in.
Comment at: lib/Driver/ToolChains/Gnu.cpp:362
+}
+else if (Arch == llvm::Triple::aarch64_be)
+ EndianFlag = "-EB";
nickdesaulniers wrote:
> is having the `else if` on its own line what the formatter chose?
I'd forgot to run clang-format over that part of the code. I've adopted the
snippet below which replaces it.
Comment at: lib/Driver/ToolChains/Gnu.cpp:703
case llvm::Triple::aarch64_be: {
+if (getToolChain().getTriple().isLittleEndian())
+ CmdArgs.push_back("-EL");
nickdesaulniers wrote:
> earlier (L362), you check the endianess of the triple with:
>
> ```
> Arch == llvm::Triple::aarch64_be
> ```
> where `Arch` is `ToolChain.getArch()`.
>
> I don't have a preference, but these two seem inconsistent. Can we either
> check the explicit `llvm::Triple::` or call
> `getToolChain().getTriple().isLittleEndian()` in both, rather than mix?
I originally took that part from the Mips code, I've replaced it with a check
against aarch64_be which is more consistent with the other Arm and AArch64 code.
https://reviews.llvm.org/D52784
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53277: [analyzer][NFC][WIP] Collect all -analyzer-config options in a .def file
Szelethus updated this revision to Diff 169688.
https://reviews.llvm.org/D53277
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Core/CoreEngine.cpp
Index: lib/StaticAnalyzer/Core/CoreEngine.cpp
===
--- lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -56,17 +56,17 @@
static std::unique_ptr generateWorkList(AnalyzerOptions &Opts,
SubEngine &subengine) {
switch (Opts.getExplorationStrategy()) {
-case AnalyzerOptions::ExplorationStrategyKind::DFS:
+case ExplorationStrategyKind::DFS:
return WorkList::makeDFS();
-case AnalyzerOptions::ExplorationStrategyKind::BFS:
+case ExplorationStrategyKind::BFS:
return WorkList::makeBFS();
-case AnalyzerOptions::ExplorationStrategyKind::BFSBlockDFSContents:
+case ExplorationStrategyKind::BFSBlockDFSContents:
return WorkList::makeBFSBlockDFSContents();
-case AnalyzerOptions::ExplorationStrategyKind::UnexploredFirst:
+case ExplorationStrategyKind::UnexploredFirst:
return WorkList::makeUnexploredFirst();
-case AnalyzerOptions::ExplorationStrategyKind::UnexploredFirstQueue:
+case ExplorationStrategyKind::UnexploredFirstQueue:
return WorkList::makeUnexploredFirstPriorityQueue();
-case AnalyzerOptions::ExplorationStrategyKind::UnexploredFirstLocationQueue:
+case ExplorationStrategyKind::UnexploredFirstLocationQueue:
return WorkList::makeUnexploredFirstPriorityLocationQueue();
}
}
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -49,7 +49,7 @@
return Result;
}
-AnalyzerOptions::UserModeKind AnalyzerOptions::getUserMode() {
+UserModeKind AnalyzerOptions::getUserMode() {
if (!UserMode.hasValue()) {
StringRef ModeStr = getOptionAsString("mode", "deep");
UserMode = llvm::StringSwitch>(ModeStr)
@@ -61,7 +61,7 @@
return UserMode.getValue();
}
-AnalyzerOptions::ExplorationStrategyKind
+ExplorationStrategyKind
AnalyzerOptions::getExplorationStrategy() {
if (!ExplorationStrategy.hasValue()) {
StringRef StratStr = getOptionAsString("exploration_strategy",
@@ -182,137 +182,6 @@
return V.getValue();
}
-bool AnalyzerOptions::includeTemporaryDtorsInCFG() {
- return getBooleanOption(IncludeTemporaryDtorsInCFG,
- "cfg-temporary-dtors",
- /* Default = */ true);
-}
-
-bool AnalyzerOptions::includeImplicitDtorsInCFG() {
- return getBooleanOption(IncludeImplicitDtorsInCFG,
- "cfg-implicit-dtors",
- /* Default = */ true);
-}
-
-bool AnalyzerOptions::includeLifetimeInCFG() {
- return getBooleanOption(IncludeLifetimeInCFG, "cfg-lifetime",
- /* Default = */ false);
-}
-
-bool AnalyzerOptions::includeLoopExitInCFG() {
- return getBooleanOption(IncludeLoopExitInCFG, "cfg-loopexit",
- /* Default = */ false);
-}
-
-bool AnalyzerOptions::includeRichConstructorsInCFG() {
- return getBooleanOption(IncludeRichConstructorsInCFG,
- "cfg-rich-constructors",
- /* Default = */ true);
-}
-
-bool AnalyzerOptions::includeScopesInCFG() {
- return getBooleanOption(IncludeScopesInCFG,
- "cfg-scopes",
- /* Default = */ false);
-}
-
-bool AnalyzerOptions::mayInlineCXXStandardLibrary() {
- return getBooleanOption(InlineCXXStandardLibrary,
- "c++-stdlib-inlining",
- /*Default=*/true);
-}
-
-bool AnalyzerOptions::mayInlineTemplateFunctions() {
- return getBooleanOption(InlineTemplateFunctions,
- "c++-template-inlining",
- /*Default=*/true);
-}
-
-bool AnalyzerOptions::mayInlineCXXAllocator() {
- return getBooleanOption(InlineCXXAllocator,
- "c++-allocator-inlining",
- /*Default=*/true);
-}
-
-bool AnalyzerOptions::mayInlineCXXContainerMethods() {
- return getBooleanOption(InlineCXXContainerMethods,
- "c++-container-inlining",
- /*Default=*/false);
-}
-
-bool AnalyzerOptions::mayInlineCXXSharedPtrDtor() {
- return getBooleanOption(InlineCXXSharedPtrDtor,
- "c++-shared_ptr-inlining",
- /*Default=*/false);
-}
-
-bool AnalyzerOptions::mayInlineCXXTemporaryDtors() {
- return getBooleanOption(InlineCXXTemporaryDtors,
- "c++-temp-dtor-inlining",
- /*Default=*/true);
-}
-
-bool AnalyzerOptions::mayI
[PATCH] D53019: [clangd] dump xrefs information in dexp tool.
sammccall added inline comments.
Comment at: clangd/index/dex/dexp/Dexp.cpp:185
clang::clangd::LookupRequest Request;
-Request.IDs = {*SID};
+Request.IDs.insert(IDs.begin(), IDs.end());
bool FoundSymbol = false;
hokein wrote:
> sammccall wrote:
> > (nit: why not just initialize in place above?)
> In fact, Request.IDs and IDs are different types, and DenseSet is missing
> such constructor :(
I meant, why does the local variable "IDs" exist at all? Why not just populate
`Request.IDs` directly?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53019
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53286: [clangd] Refactor JSON-over-stdin/stdout code into Transport abstraction.
sammccall created this revision.
sammccall added reviewers: jkorous, ioeric, hokein.
Herald added subscribers: cfe-commits, kadircet, arphaman, MaskRay,
ilya-biryukov, mgorny.
This paves the way for alternative transports (mac XPC, maybe messagepack?),
and also generally improves layering: testing ClangdLSPServer becomes less of
a pipe dream, we split up the JSONOutput monolith, etc.
This isn't a final state, much of what remains in JSONRPCDispatcher can go away,
handlers can call reply() on the transport directly, JSONOutput can be renamed
to StreamLogger and removed, etc. But this patch is sprawling already.
The main observable change (see tests) is that hitting EOF on input is now an
error: the client should send the 'exit' notification.
This is defensible: the protocol doesn't spell this case out. Reproducing the
current behavior for all combinations of shutdown/exit/EOF clutters interfaces.
We can iterate on this if desired.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53286
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/JSONRPCDispatcher.cpp
clangd/JSONRPCDispatcher.h
clangd/JSONTransport.cpp
clangd/Protocol.cpp
clangd/Protocol.h
clangd/ProtocolHandlers.cpp
clangd/Transport.h
clangd/tool/ClangdMain.cpp
test/clangd/compile-commands-path-in-initialize.test
test/clangd/compile-commands-path.test
test/clangd/completion-snippets.test
test/clangd/completion.test
test/clangd/crash-non-added-files.test
test/clangd/execute-command.test
test/clangd/input-mirror.test
test/clangd/signature-help.test
test/clangd/textdocument-didchange-fail.test
test/clangd/trace.test
test/clangd/xrefs.test
Index: test/clangd/xrefs.test
===
--- test/clangd/xrefs.test
+++ test/clangd/xrefs.test
@@ -55,3 +55,5 @@
# CHECK-NEXT: ]
---
{"jsonrpc":"2.0","id":1,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: test/clangd/trace.test
===
--- test/clangd/trace.test
+++ test/clangd/trace.test
@@ -21,3 +21,5 @@
# CHECK: },
---
{"jsonrpc":"2.0","id":5,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: test/clangd/textdocument-didchange-fail.test
===
--- test/clangd/textdocument-didchange-fail.test
+++ test/clangd/textdocument-didchange-fail.test
@@ -35,3 +35,5 @@
# CHECK-NEXT:}
---
{"jsonrpc":"2.0","id":4,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: test/clangd/signature-help.test
===
--- test/clangd/signature-help.test
+++ test/clangd/signature-help.test
@@ -23,3 +23,5 @@
# CHECK-NEXT: }
---
{"jsonrpc":"2.0","id":10,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: test/clangd/input-mirror.test
===
--- test/clangd/input-mirror.test
+++ test/clangd/input-mirror.test
@@ -12,3 +12,6 @@
Content-Length: 44
{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+Content-Length: 33
+
+{"jsonrpc":"2.0","method":"exit"}
Index: test/clangd/execute-command.test
===
--- test/clangd/execute-command.test
+++ test/clangd/execute-command.test
@@ -62,3 +62,5 @@
{"jsonrpc":"2.0","id":9,"method":"workspace/executeCommand","params":{"arguments":[{"custom":"foo", "changes":{"test:///foo.c":[{"range":{"start":{"line":0,"character":32},"end":{"line":0,"character":32}},"newText":"("},{"range":{"start":{"line":0,"character":37},"end":{"line":0,"character":37}},"newText":")"}]}}],"command":"clangd.applyFix"}}
---
{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: test/clangd/crash-non-added-files.test
===
--- test/clangd/crash-non-added-files.test
+++ test/clangd/crash-non-added-files.test
@@ -32,3 +32,5 @@
{"jsonrpc":"2.0","id":6,"method":"shutdown"}
---
{"jsonrpc":"2.0","method":"exit"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: test/clangd/completion.test
===
--- test/clangd/completion.test
+++ test/clangd/completion.test
@@ -68,3 +68,5 @@
# CHECK-NEXT: ]
---
{"jsonrpc":"2.0","id":4,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: test/clangd/completion-snippets.test
===
--- test/clangd/completion-snippets.test
+++ test/clangd/completion-snippets.test
@@ -52,3 +52,5 @@
# CHECK-NEXT: }
---
{"jsonrpc":"2.0","id":4,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: test/clangd/compile-commands-path.test
===
--- test/clangd/compile-commands-path.tes
[clang-tools-extra] r344510 - [clangd] Remove an unused include header, NFC.
Author: hokein
Date: Mon Oct 15 05:39:45 2018
New Revision: 344510
URL: http://llvm.org/viewvc/llvm-project?rev=344510&view=rev
Log:
[clangd] Remove an unused include header, NFC.
Modified:
clang-tools-extra/trunk/clangd/index/Merge.cpp
Modified: clang-tools-extra/trunk/clangd/index/Merge.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Merge.cpp?rev=344510&r1=344509&r2=344510&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Merge.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Merge.cpp Mon Oct 15 05:39:45 2018
@@ -13,7 +13,6 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/Support/raw_ostream.h"
-#include
namespace clang {
namespace clangd {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53284: [CodeComplete] Make sure keyword 'template' is added even when code pattern is disabled.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC344509: [CodeComplete] Make sure keyword
'template' is added even when code pattern is… (authored by ioeric,
committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53284?vs=169681&id=169685#toc
Repository:
rC Clang
https://reviews.llvm.org/D53284
Files:
lib/Sema/SemaCodeComplete.cpp
test/Index/complete-template-keywords.cpp
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -1731,6 +1731,8 @@
Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
Builder.AddPlaceholderChunk("declaration");
Results.AddResult(Result(Builder.TakeString()));
+ } else {
+Results.AddResult(Result("template",
CodeCompletionResult::RK_Keyword));
}
}
@@ -1805,6 +1807,8 @@
Builder.AddPlaceholderChunk("parameters");
Builder.AddChunk(CodeCompletionString::CK_RightAngle);
Results.AddResult(Result(Builder.TakeString()));
+} else {
+ Results.AddResult(Result("template", CodeCompletionResult::RK_Keyword));
}
AddStorageSpecifiers(CCC, SemaRef.getLangOpts(), Results);
Index: test/Index/complete-template-keywords.cpp
===
--- test/Index/complete-template-keywords.cpp
+++ test/Index/complete-template-keywords.cpp
@@ -0,0 +1,5 @@
+templ
+// RUN: env c-index-test -code-completion-at=%s:1:5 %s | FileCheck
-check-prefix=CHECK-NO-PATTERN %s
+// CHECK-NO-PATTERN: {TypedText template} (1)
+// RUN: env CINDEXTEST_CODE_COMPLETE_PATTERNS=1 c-index-test
-code-completion-at=%s:1:5 %s | FileCheck -check-prefix=CHECK-PATTERN %s
+// CHECK-PATTERN: {TypedText template}{LeftAngle <}
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -1731,6 +1731,8 @@
Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
Builder.AddPlaceholderChunk("declaration");
Results.AddResult(Result(Builder.TakeString()));
+ } else {
+Results.AddResult(Result("template", CodeCompletionResult::RK_Keyword));
}
}
@@ -1805,6 +1807,8 @@
Builder.AddPlaceholderChunk("parameters");
Builder.AddChunk(CodeCompletionString::CK_RightAngle);
Results.AddResult(Result(Builder.TakeString()));
+} else {
+ Results.AddResult(Result("template", CodeCompletionResult::RK_Keyword));
}
AddStorageSpecifiers(CCC, SemaRef.getLangOpts(), Results);
Index: test/Index/complete-template-keywords.cpp
===
--- test/Index/complete-template-keywords.cpp
+++ test/Index/complete-template-keywords.cpp
@@ -0,0 +1,5 @@
+templ
+// RUN: env c-index-test -code-completion-at=%s:1:5 %s | FileCheck -check-prefix=CHECK-NO-PATTERN %s
+// CHECK-NO-PATTERN: {TypedText template} (1)
+// RUN: env CINDEXTEST_CODE_COMPLETE_PATTERNS=1 c-index-test -code-completion-at=%s:1:5 %s | FileCheck -check-prefix=CHECK-PATTERN %s
+// CHECK-PATTERN: {TypedText template}{LeftAngle <}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r344509 - [CodeComplete] Make sure keyword 'template' is added even when code pattern is disabled.
Author: ioeric
Date: Mon Oct 15 05:37:23 2018
New Revision: 344509
URL: http://llvm.org/viewvc/llvm-project?rev=344509&view=rev
Log:
[CodeComplete] Make sure keyword 'template' is added even when code pattern is
disabled.
Reviewers: sammccall, hokein
Subscribers: arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D53284
Added:
cfe/trunk/test/Index/complete-template-keywords.cpp
Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=344509&r1=344508&r2=344509&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Mon Oct 15 05:37:23 2018
@@ -1731,6 +1731,8 @@ static void AddOrdinaryNameResults(Sema:
Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
Builder.AddPlaceholderChunk("declaration");
Results.AddResult(Result(Builder.TakeString()));
+ } else {
+Results.AddResult(Result("template",
CodeCompletionResult::RK_Keyword));
}
}
@@ -1805,6 +1807,8 @@ static void AddOrdinaryNameResults(Sema:
Builder.AddPlaceholderChunk("parameters");
Builder.AddChunk(CodeCompletionString::CK_RightAngle);
Results.AddResult(Result(Builder.TakeString()));
+} else {
+ Results.AddResult(Result("template", CodeCompletionResult::RK_Keyword));
}
AddStorageSpecifiers(CCC, SemaRef.getLangOpts(), Results);
Added: cfe/trunk/test/Index/complete-template-keywords.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/complete-template-keywords.cpp?rev=344509&view=auto
==
--- cfe/trunk/test/Index/complete-template-keywords.cpp (added)
+++ cfe/trunk/test/Index/complete-template-keywords.cpp Mon Oct 15 05:37:23 2018
@@ -0,0 +1,5 @@
+templ
+// RUN: env c-index-test -code-completion-at=%s:1:5 %s | FileCheck
-check-prefix=CHECK-NO-PATTERN %s
+// CHECK-NO-PATTERN: {TypedText template} (1)
+// RUN: env CINDEXTEST_CODE_COMPLETE_PATTERNS=1 c-index-test
-code-completion-at=%s:1:5 %s | FileCheck -check-prefix=CHECK-PATTERN %s
+// CHECK-PATTERN: {TypedText template}{LeftAngle <}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53019: [clangd] dump xrefs information in dexp tool.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE344508: [clangd] dump xrefs information in dexp tool.
(authored by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53019?vs=169683&id=169684#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53019
Files:
clangd/index/dex/dexp/CMakeLists.txt
clangd/index/dex/dexp/Dexp.cpp
Index: clangd/index/dex/dexp/Dexp.cpp
===
--- clangd/index/dex/dexp/Dexp.cpp
+++ clangd/index/dex/dexp/Dexp.cpp
@@ -12,8 +12,9 @@
//
//===--===//
-#include "../../Serialization.h"
-#include "../Dex.h"
+#include "Dex.h"
+#include "Serialization.h"
+#include "SourceCode.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSwitch.h"
@@ -52,6 +53,26 @@
llvm::outs() << llvm::formatv("{0} took {1:ms+n}.\n", Name, Duration);
}
+std::vector
+getSymbolIDsFromIndex(llvm::StringRef QualifiedName, const SymbolIndex *Index) {
+ FuzzyFindRequest Request;
+ // Remove leading "::" qualifier as FuzzyFind doesn't need leading "::"
+ // qualifier for global scope.
+ bool IsGlobalScope = QualifiedName.consume_front("::");
+ auto Names = clang::clangd::splitQualifiedName(QualifiedName);
+ if (IsGlobalScope || !Names.first.empty())
+Request.Scopes = {Names.first};
+
+ Request.Query = Names.second;
+ std::vector SymIDs;
+ Index->fuzzyFind(Request, [&](const Symbol &Sym) {
+std::string SymQualifiedName = (Sym.Scope + Sym.Name).str();
+if (QualifiedName == SymQualifiedName)
+ SymIDs.push_back(Sym.ID);
+ });
+ return SymIDs;
+}
+
// REPL commands inherit from Command and contain their options as members.
// Creating a Command populates parser options, parseAndRun() resets them.
class Command {
@@ -88,7 +109,6 @@
};
// FIXME(kbobyrev): Ideas for more commands:
-// * find symbol references: print set of reference locations
// * load/swap/reload index: this would make it possible to get rid of llvm::cl
// usages in the tool driver and actually use llvm::cl library in the REPL.
// * show posting list density histogram (our dump data somewhere so that user
@@ -139,19 +159,32 @@
cl::opt ID{
"id",
cl::Positional,
- cl::Required,
cl::desc("Symbol ID to look up (hex)"),
};
+ cl::opt Name{
+ "name", cl::desc("Qualified name to look up."),
+ };
void run() override {
-auto SID = clang::clangd::SymbolID::fromStr(ID);
-if (!SID) {
- llvm::outs() << llvm::toString(SID.takeError()) << "\n";
+if (ID.getNumOccurrences() == 0 && Name.getNumOccurrences() == 0) {
+ llvm::outs()
+ << "Missing required argument: please provide id or -name.\n";
return;
}
+std::vector IDs;
+if (ID.getNumOccurrences()) {
+ auto SID = clang::clangd::SymbolID::fromStr(ID);
+ if (!SID) {
+llvm::outs() << llvm::toString(SID.takeError()) << "\n";
+return;
+ }
+ IDs.push_back(*SID);
+} else {
+ IDs = getSymbolIDsFromIndex(Name, Index);
+}
clang::clangd::LookupRequest Request;
-Request.IDs = {*SID};
+Request.IDs.insert(IDs.begin(), IDs.end());
bool FoundSymbol = false;
Index->lookup(Request, [&](const Symbol &Sym) {
FoundSymbol = true;
@@ -162,13 +195,62 @@
}
};
+class Refs : public Command {
+ cl::opt ID{
+ "id", cl::Positional,
+ cl::desc("Symbol ID of the symbol being queried (hex)."),
+ };
+ cl::opt Name{
+ "name", cl::desc("Qualified name of the symbol being queried."),
+ };
+ cl::opt Filter{
+ "filter", cl::init(".*"),
+ cl::desc(
+ "Print all results from files matching this regular expression."),
+ };
+
+ void run() override {
+if (ID.getNumOccurrences() == 0 && Name.getNumOccurrences() == 0) {
+ llvm::outs()
+ << "Missing required argument: please provide id or -name.\n";
+ return;
+}
+std::vector IDs;
+if (ID.getNumOccurrences()) {
+ auto SID = clang::clangd::SymbolID::fromStr(ID);
+ if (!SID) {
+llvm::outs() << llvm::toString(SID.takeError()) << "\n";
+return;
+ }
+ IDs.push_back(*SID);
+} else {
+ IDs = getSymbolIDsFromIndex(Name, Index);
+}
+clang::clangd::RefsRequest RefRequest;
+RefRequest.IDs.insert(IDs.begin(), IDs.end());
+llvm::Regex RegexFilter(Filter);
+Index->refs(RefRequest, [&RegexFilter](const clang::clangd::Ref &R) {
+ auto U = clang::clangd::URI::parse(R.Location.FileURI);
+ if (!U) {
+llvm::outs() << U.takeError();
+return;
+ }
+ if (RegexFilter.match(U->body()))
+llvm::outs() << R << "\n";
+});
+ }
+};
+
struct {
const char *Name;
const char *Description;
std::function()> Implementation;
} CommandInfo[] = {
{"find", "Search for symbols
[clang-tools-extra] r344508 - [clangd] dump xrefs information in dexp tool.
Author: hokein
Date: Mon Oct 15 05:32:49 2018
New Revision: 344508
URL: http://llvm.org/viewvc/llvm-project?rev=344508&view=rev
Log:
[clangd] dump xrefs information in dexp tool.
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet,
cfe-commits
Differential Revision: https://reviews.llvm.org/D53019
Modified:
clang-tools-extra/trunk/clangd/index/dex/dexp/CMakeLists.txt
clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp
Modified: clang-tools-extra/trunk/clangd/index/dex/dexp/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/dexp/CMakeLists.txt?rev=344508&r1=344507&r2=344508&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/dexp/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/index/dex/dexp/CMakeLists.txt Mon Oct 15
05:32:49 2018
@@ -1,3 +1,5 @@
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../..)
include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../../../)
set(LLVM_LINK_COMPONENTS
Modified: clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp?rev=344508&r1=344507&r2=344508&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp Mon Oct 15 05:32:49
2018
@@ -12,8 +12,9 @@
//
//===--===//
-#include "../../Serialization.h"
-#include "../Dex.h"
+#include "Dex.h"
+#include "Serialization.h"
+#include "SourceCode.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSwitch.h"
@@ -52,6 +53,26 @@ void reportTime(StringRef Name, llvm::fu
llvm::outs() << llvm::formatv("{0} took {1:ms+n}.\n", Name, Duration);
}
+std::vector
+getSymbolIDsFromIndex(llvm::StringRef QualifiedName, const SymbolIndex *Index)
{
+ FuzzyFindRequest Request;
+ // Remove leading "::" qualifier as FuzzyFind doesn't need leading "::"
+ // qualifier for global scope.
+ bool IsGlobalScope = QualifiedName.consume_front("::");
+ auto Names = clang::clangd::splitQualifiedName(QualifiedName);
+ if (IsGlobalScope || !Names.first.empty())
+Request.Scopes = {Names.first};
+
+ Request.Query = Names.second;
+ std::vector SymIDs;
+ Index->fuzzyFind(Request, [&](const Symbol &Sym) {
+std::string SymQualifiedName = (Sym.Scope + Sym.Name).str();
+if (QualifiedName == SymQualifiedName)
+ SymIDs.push_back(Sym.ID);
+ });
+ return SymIDs;
+}
+
// REPL commands inherit from Command and contain their options as members.
// Creating a Command populates parser options, parseAndRun() resets them.
class Command {
@@ -88,7 +109,6 @@ public:
};
// FIXME(kbobyrev): Ideas for more commands:
-// * find symbol references: print set of reference locations
// * load/swap/reload index: this would make it possible to get rid of llvm::cl
// usages in the tool driver and actually use llvm::cl library in the REPL.
// * show posting list density histogram (our dump data somewhere so that user
@@ -139,19 +159,32 @@ class Lookup : public Command {
cl::opt ID{
"id",
cl::Positional,
- cl::Required,
cl::desc("Symbol ID to look up (hex)"),
};
+ cl::opt Name{
+ "name", cl::desc("Qualified name to look up."),
+ };
void run() override {
-auto SID = clang::clangd::SymbolID::fromStr(ID);
-if (!SID) {
- llvm::outs() << llvm::toString(SID.takeError()) << "\n";
+if (ID.getNumOccurrences() == 0 && Name.getNumOccurrences() == 0) {
+ llvm::outs()
+ << "Missing required argument: please provide id or -name.\n";
return;
}
+std::vector IDs;
+if (ID.getNumOccurrences()) {
+ auto SID = clang::clangd::SymbolID::fromStr(ID);
+ if (!SID) {
+llvm::outs() << llvm::toString(SID.takeError()) << "\n";
+return;
+ }
+ IDs.push_back(*SID);
+} else {
+ IDs = getSymbolIDsFromIndex(Name, Index);
+}
clang::clangd::LookupRequest Request;
-Request.IDs = {*SID};
+Request.IDs.insert(IDs.begin(), IDs.end());
bool FoundSymbol = false;
Index->lookup(Request, [&](const Symbol &Sym) {
FoundSymbol = true;
@@ -162,13 +195,62 @@ class Lookup : public Command {
}
};
+class Refs : public Command {
+ cl::opt ID{
+ "id", cl::Positional,
+ cl::desc("Symbol ID of the symbol being queried (hex)."),
+ };
+ cl::opt Name{
+ "name", cl::desc("Qualified name of the symbol being queried."),
+ };
+ cl::opt Filter{
+ "filter", cl::init(".*"),
+ cl::desc(
+ "Print all results from files matching this regular expression."),
+ };
+
+ void run() override {
+
[PATCH] D53019: [clangd] dump xrefs information in dexp tool.
hokein updated this revision to Diff 169683.
hokein marked 3 inline comments as done.
hokein added a comment.
Fix global scope, and clang-format.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53019
Files:
clangd/index/dex/dexp/CMakeLists.txt
clangd/index/dex/dexp/Dexp.cpp
Index: clangd/index/dex/dexp/Dexp.cpp
===
--- clangd/index/dex/dexp/Dexp.cpp
+++ clangd/index/dex/dexp/Dexp.cpp
@@ -12,8 +12,9 @@
//
//===--===//
-#include "../../Serialization.h"
-#include "../Dex.h"
+#include "Dex.h"
+#include "Serialization.h"
+#include "SourceCode.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSwitch.h"
@@ -52,6 +53,26 @@
llvm::outs() << llvm::formatv("{0} took {1:ms+n}.\n", Name, Duration);
}
+std::vector
+getSymbolIDsFromIndex(llvm::StringRef QualifiedName, const SymbolIndex *Index) {
+ FuzzyFindRequest Request;
+ // Remove leading "::" qualifier as FuzzyFind doesn't need leading "::"
+ // qualifier for global scope.
+ bool IsGlobalScope = QualifiedName.consume_front("::");
+ auto Names = clang::clangd::splitQualifiedName(QualifiedName);
+ if (IsGlobalScope || !Names.first.empty())
+Request.Scopes = {Names.first};
+
+ Request.Query = Names.second;
+ std::vector SymIDs;
+ Index->fuzzyFind(Request, [&](const Symbol &Sym) {
+std::string SymQualifiedName = (Sym.Scope + Sym.Name).str();
+if (QualifiedName == SymQualifiedName)
+ SymIDs.push_back(Sym.ID);
+ });
+ return SymIDs;
+}
+
// REPL commands inherit from Command and contain their options as members.
// Creating a Command populates parser options, parseAndRun() resets them.
class Command {
@@ -88,7 +109,6 @@
};
// FIXME(kbobyrev): Ideas for more commands:
-// * find symbol references: print set of reference locations
// * load/swap/reload index: this would make it possible to get rid of llvm::cl
// usages in the tool driver and actually use llvm::cl library in the REPL.
// * show posting list density histogram (our dump data somewhere so that user
@@ -139,19 +159,32 @@
cl::opt ID{
"id",
cl::Positional,
- cl::Required,
cl::desc("Symbol ID to look up (hex)"),
};
+ cl::opt Name{
+ "name", cl::desc("Qualified name to look up."),
+ };
void run() override {
-auto SID = clang::clangd::SymbolID::fromStr(ID);
-if (!SID) {
- llvm::outs() << llvm::toString(SID.takeError()) << "\n";
+if (ID.getNumOccurrences() == 0 && Name.getNumOccurrences() == 0) {
+ llvm::outs()
+ << "Missing required argument: please provide id or -name.\n";
return;
}
+std::vector IDs;
+if (ID.getNumOccurrences()) {
+ auto SID = clang::clangd::SymbolID::fromStr(ID);
+ if (!SID) {
+llvm::outs() << llvm::toString(SID.takeError()) << "\n";
+return;
+ }
+ IDs.push_back(*SID);
+} else {
+ IDs = getSymbolIDsFromIndex(Name, Index);
+}
clang::clangd::LookupRequest Request;
-Request.IDs = {*SID};
+Request.IDs.insert(IDs.begin(), IDs.end());
bool FoundSymbol = false;
Index->lookup(Request, [&](const Symbol &Sym) {
FoundSymbol = true;
@@ -162,13 +195,62 @@
}
};
+class Refs : public Command {
+ cl::opt ID{
+ "id", cl::Positional,
+ cl::desc("Symbol ID of the symbol being queried (hex)."),
+ };
+ cl::opt Name{
+ "name", cl::desc("Qualified name of the symbol being queried."),
+ };
+ cl::opt Filter{
+ "filter", cl::init(".*"),
+ cl::desc(
+ "Print all results from files matching this regular expression."),
+ };
+
+ void run() override {
+if (ID.getNumOccurrences() == 0 && Name.getNumOccurrences() == 0) {
+ llvm::outs()
+ << "Missing required argument: please provide id or -name.\n";
+ return;
+}
+std::vector IDs;
+if (ID.getNumOccurrences()) {
+ auto SID = clang::clangd::SymbolID::fromStr(ID);
+ if (!SID) {
+llvm::outs() << llvm::toString(SID.takeError()) << "\n";
+return;
+ }
+ IDs.push_back(*SID);
+} else {
+ IDs = getSymbolIDsFromIndex(Name, Index);
+}
+clang::clangd::RefsRequest RefRequest;
+RefRequest.IDs.insert(IDs.begin(), IDs.end());
+llvm::Regex RegexFilter(Filter);
+Index->refs(RefRequest, [&RegexFilter](const clang::clangd::Ref &R) {
+ auto U = clang::clangd::URI::parse(R.Location.FileURI);
+ if (!U) {
+llvm::outs() << U.takeError();
+return;
+ }
+ if (RegexFilter.match(U->body()))
+llvm::outs() << R << "\n";
+});
+ }
+};
+
struct {
const char *Name;
const char *Description;
std::function()> Implementation;
} CommandInfo[] = {
{"find", "Search for symbols with fuzzyFind", llvm::make_unique},
-{"lookup", "Dump symbol details by ID", llvm::make_unique},
+{"lookup", "Dum
[PATCH] D53019: [clangd] dump xrefs information in dexp tool.
hokein added inline comments.
Comment at: clangd/index/dex/dexp/Dexp.cpp:61
+ Request.Scopes.emplace_back();
+ std::tie(Request.Scopes.back(), Request.Query) =
+ clang::clangd::splitQualifiedName(QualifiedName);
sammccall wrote:
> Are you sure you want both `foo` to mean `::foo` only, rather than accept any
> scope and the user can type `::foo` for explicitly global scope?
Oops, this is not intended. "foo" and "::foo" are different.
Comment at: clangd/index/dex/dexp/Dexp.cpp:185
clang::clangd::LookupRequest Request;
-Request.IDs = {*SID};
+Request.IDs.insert(IDs.begin(), IDs.end());
bool FoundSymbol = false;
sammccall wrote:
> (nit: why not just initialize in place above?)
In fact, Request.IDs and IDs are different types, and DenseSet is missing such
constructor :(
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53019
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53284: [CodeComplete] Make sure keyword 'template' is added even when code pattern is disabled.
ioeric created this revision.
ioeric added reviewers: sammccall, hokein.
Herald added subscribers: cfe-commits, arphaman.
Repository:
rC Clang
https://reviews.llvm.org/D53284
Files:
lib/Sema/SemaCodeComplete.cpp
test/Index/complete-template-keywords.cpp
Index: test/Index/complete-template-keywords.cpp
===
--- /dev/null
+++ test/Index/complete-template-keywords.cpp
@@ -0,0 +1,5 @@
+templ
+// RUN: env c-index-test -code-completion-at=%s:1:5 %s | FileCheck
-check-prefix=CHECK-NO-PATTERN %s
+// CHECK-NO-PATTERN: {TypedText template} (1)
+// RUN: env CINDEXTEST_CODE_COMPLETE_PATTERNS=1 c-index-test
-code-completion-at=%s:1:5 %s | FileCheck -check-prefix=CHECK-PATTERN %s
+// CHECK-PATTERN: {TypedText template}{LeftAngle <}
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -1731,6 +1731,8 @@
Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
Builder.AddPlaceholderChunk("declaration");
Results.AddResult(Result(Builder.TakeString()));
+ } else {
+Results.AddResult(Result("template",
CodeCompletionResult::RK_Keyword));
}
}
@@ -1805,6 +1807,8 @@
Builder.AddPlaceholderChunk("parameters");
Builder.AddChunk(CodeCompletionString::CK_RightAngle);
Results.AddResult(Result(Builder.TakeString()));
+} else {
+ Results.AddResult(Result("template", CodeCompletionResult::RK_Keyword));
}
AddStorageSpecifiers(CCC, SemaRef.getLangOpts(), Results);
Index: test/Index/complete-template-keywords.cpp
===
--- /dev/null
+++ test/Index/complete-template-keywords.cpp
@@ -0,0 +1,5 @@
+templ
+// RUN: env c-index-test -code-completion-at=%s:1:5 %s | FileCheck -check-prefix=CHECK-NO-PATTERN %s
+// CHECK-NO-PATTERN: {TypedText template} (1)
+// RUN: env CINDEXTEST_CODE_COMPLETE_PATTERNS=1 c-index-test -code-completion-at=%s:1:5 %s | FileCheck -check-prefix=CHECK-PATTERN %s
+// CHECK-PATTERN: {TypedText template}{LeftAngle <}
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -1731,6 +1731,8 @@
Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
Builder.AddPlaceholderChunk("declaration");
Results.AddResult(Result(Builder.TakeString()));
+ } else {
+Results.AddResult(Result("template", CodeCompletionResult::RK_Keyword));
}
}
@@ -1805,6 +1807,8 @@
Builder.AddPlaceholderChunk("parameters");
Builder.AddChunk(CodeCompletionString::CK_RightAngle);
Results.AddResult(Result(Builder.TakeString()));
+} else {
+ Results.AddResult(Result("template", CodeCompletionResult::RK_Keyword));
}
AddStorageSpecifiers(CCC, SemaRef.getLangOpts(), Results);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53273: [clangd] Fix some references missing in dynamic index.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344507: [clangd] Fix some references missing in dynamic
index. (authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D53273
Files:
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
@@ -8,13 +8,15 @@
//===--===//
#include "Annotations.h"
+#include "AST.h"
#include "ClangdUnit.h"
#include "TestFS.h"
#include "TestTU.h"
#include "gmock/gmock.h"
#include "index/FileIndex.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Frontend/Utils.h"
#include "clang/Index/IndexSymbol.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/CompilationDatabase.h"
@@ -346,6 +348,55 @@
EXPECT_TRUE(SeenSymbol);
}
+TEST(FileIndexTest, ReferencesInMainFileWithPreamble) {
+ const std::string Header = R"cpp(
+class Foo {};
+ )cpp";
+ Annotations Main(R"cpp(
+#include "foo.h"
+void f() {
+ [[Foo]] foo;
+}
+ )cpp");
+ auto MainFile = testPath("foo.cpp");
+ auto HeaderFile = testPath("foo.h");
+ std::vector Cmd = {"clang", "-xc++", MainFile.c_str()};
+ // Preparse ParseInputs.
+ ParseInputs PI;
+ PI.CompileCommand.Directory = testRoot();
+ PI.CompileCommand.Filename = MainFile;
+ PI.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
+ PI.Contents = Main.code();
+ PI.FS = buildTestFS({{MainFile, Main.code()}, {HeaderFile, Header}});
+
+ // Prepare preamble.
+ auto CI = buildCompilerInvocation(PI);
+ auto PreambleData = buildPreamble(
+ MainFile,
+ *buildCompilerInvocation(PI), /*OldPreamble=*/nullptr,
+ tooling::CompileCommand(), PI,
+ std::make_shared(), /*StoreInMemory=*/true,
+ [&](ASTContext &Ctx, std::shared_ptr PP) {});
+ // Build AST for main file with preamble.
+ auto AST = ParsedAST::build(
+ createInvocationFromCommandLine(Cmd), PreambleData,
+ llvm::MemoryBuffer::getMemBufferCopy(Main.code()),
+ std::make_shared(),
+ PI.FS);
+ ASSERT_TRUE(AST);
+ FileIndex Index;
+ Index.updateMain(MainFile, *AST);
+
+ auto Foo =
+ findSymbol(TestTU::withHeaderCode(Header).headerSymbols(), "Foo");
+ RefsRequest Request;
+ Request.IDs.insert(Foo.ID);
+
+ // Expect to see references in main file, references in headers are excluded
+ // because we only index main AST.
+ EXPECT_THAT(getRefs(Index, Foo.ID), RefsAre({RefRange(Main.range())}));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
@@ -345,16 +345,20 @@
SM.getFileID(SpellingLoc) == SM.getMainFileID())
ReferencedDecls.insert(ND);
- if ((static_cast(Opts.RefFilter) & Roles) &&
- SM.getFileID(SpellingLoc) == SM.getMainFileID())
-DeclRefs[ND].emplace_back(SpellingLoc, Roles);
+ bool CollectRef = static_cast(Opts.RefFilter) & Roles;
+ bool IsOnlyRef =
+ !(Roles & (static_cast(index::SymbolRole::Declaration) |
+ static_cast(index::SymbolRole::Definition)));
- // Don't continue indexing if this is a mere reference.
- if (!(Roles & static_cast(index::SymbolRole::Declaration) ||
-Roles & static_cast(index::SymbolRole::Definition)))
+ if (IsOnlyRef && !CollectRef)
return true;
if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
return true;
+ if (CollectRef && SM.getFileID(SpellingLoc) == SM.getMainFileID())
+DeclRefs[ND].emplace_back(SpellingLoc, Roles);
+ // Don't continue indexing if this is a mere reference.
+ if (IsOnlyRef)
+return true;
auto ID = getSymbolID(ND);
if (!ID)
@@ -476,17 +480,15 @@
std::string MainURI = *MainFileURI;
for (const auto &It : DeclRefs) {
if (auto ID = getSymbolID(It.first)) {
-if (Symbols.find(*ID)) {
- for (const auto &LocAndRole : It.second) {
-Ref R;
-auto Range =
-getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
-R.Location.Start = Range.first;
-R.Location.End = Range.second;
-R.Location.FileURI = MainURI;
-R.Kind = toRefKind(LocAndRole.second);
-Refs.insert(*ID, R);
- }
+for (const auto &LocAndRole : It.second) {
+ Ref R;
+ auto Range =
+ getTokenRange(LocAndRole.first
[clang-tools-extra] r344507 - [clangd] Fix some references missing in dynamic index.
Author: hokein
Date: Mon Oct 15 04:46:26 2018
New Revision: 344507
URL: http://llvm.org/viewvc/llvm-project?rev=344507&view=rev
Log:
[clangd] Fix some references missing in dynamic index.
Summary:
Previously, SymbolCollector postfilters all references at the end to
find all references of interesting symbols.
It was incorrect when indxing main AST where we don't see locations
of symbol declarations and definitions in the main AST (as those are in
preamble AST).
The fix is to do earily check during collecting references.
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet,
cfe-commits
Differential Revision: https://reviews.llvm.org/D53273
Modified:
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=344507&r1=344506&r2=344507&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Mon Oct 15
04:46:26 2018
@@ -345,16 +345,20 @@ bool SymbolCollector::handleDeclOccurenc
SM.getFileID(SpellingLoc) == SM.getMainFileID())
ReferencedDecls.insert(ND);
- if ((static_cast(Opts.RefFilter) & Roles) &&
- SM.getFileID(SpellingLoc) == SM.getMainFileID())
-DeclRefs[ND].emplace_back(SpellingLoc, Roles);
+ bool CollectRef = static_cast(Opts.RefFilter) & Roles;
+ bool IsOnlyRef =
+ !(Roles & (static_cast(index::SymbolRole::Declaration) |
+ static_cast(index::SymbolRole::Definition)));
- // Don't continue indexing if this is a mere reference.
- if (!(Roles & static_cast(index::SymbolRole::Declaration) ||
-Roles & static_cast(index::SymbolRole::Definition)))
+ if (IsOnlyRef && !CollectRef)
return true;
if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
return true;
+ if (CollectRef && SM.getFileID(SpellingLoc) == SM.getMainFileID())
+DeclRefs[ND].emplace_back(SpellingLoc, Roles);
+ // Don't continue indexing if this is a mere reference.
+ if (IsOnlyRef)
+return true;
auto ID = getSymbolID(ND);
if (!ID)
@@ -476,17 +480,15 @@ void SymbolCollector::finish() {
std::string MainURI = *MainFileURI;
for (const auto &It : DeclRefs) {
if (auto ID = getSymbolID(It.first)) {
-if (Symbols.find(*ID)) {
- for (const auto &LocAndRole : It.second) {
-Ref R;
-auto Range =
-getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
-R.Location.Start = Range.first;
-R.Location.End = Range.second;
-R.Location.FileURI = MainURI;
-R.Kind = toRefKind(LocAndRole.second);
-Refs.insert(*ID, R);
- }
+for (const auto &LocAndRole : It.second) {
+ Ref R;
+ auto Range =
+ getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
+ R.Location.Start = Range.first;
+ R.Location.End = Range.second;
+ R.Location.FileURI = MainURI;
+ R.Kind = toRefKind(LocAndRole.second);
+ Refs.insert(*ID, R);
}
}
}
Modified: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp?rev=344507&r1=344506&r2=344507&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp Mon Oct 15
04:46:26 2018
@@ -8,6 +8,7 @@
//===--===//
#include "Annotations.h"
+#include "AST.h"
#include "ClangdUnit.h"
#include "TestFS.h"
#include "TestTU.h"
@@ -15,6 +16,7 @@
#include "index/FileIndex.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Frontend/Utils.h"
#include "clang/Index/IndexSymbol.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/CompilationDatabase.h"
@@ -346,6 +348,55 @@ TEST(FileIndexTest, CollectMacros) {
EXPECT_TRUE(SeenSymbol);
}
+TEST(FileIndexTest, ReferencesInMainFileWithPreamble) {
+ const std::string Header = R"cpp(
+class Foo {};
+ )cpp";
+ Annotations Main(R"cpp(
+#include "foo.h"
+void f() {
+ [[Foo]] foo;
+}
+ )cpp");
+ auto MainFile = testPath("foo.cpp");
+ auto HeaderFile = testPath("foo.h");
+ std::vector Cmd = {"clang", "-xc++", MainFile.c_str()};
+ // Preparse ParseInputs.
+ ParseInputs PI;
+ PI.CompileCommand.Directory = testRoot();
+ PI.CompileCommand.Filename = MainFile;
+ PI.CompileCommand.CommandLine = {Cmd.begin(),
[PATCH] D51633: [ASTImporter] Added error handling for AST import.
balazske marked 9 inline comments as done. balazske added inline comments. Comment at: lib/AST/ASTImporter.cpp:2683 +continue; + } else if (isa(Found)) +continue; a_sidorin wrote: > Same here. I do not know exactly why this was made, it looks like a bugfix for a problem encountered during the tests. Maybe I can undo this change (there are other related problem fixes so this is probably not needed). Comment at: lib/AST/ASTImporter.cpp:4604 + if (Error Err = ImportDeclContext(D)) +// FIXME: Really ignore the error? +consumeError(std::move(Err)); a_sidorin wrote: > I think we can just `return std::move(Err);` as it was done below. Changed to return the error. Comment at: lib/AST/ASTImporter.cpp:5657 + if (!ToSemiLocOrErr) +return nullptr; + return new (Importer.getToContext()) NullStmt( a_sidorin wrote: > Shouldn't we return an error here? Good catch, this looks like the change was missed. Repository: rC Clang https://reviews.llvm.org/D51633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53280: [analyzer] Emit a warning for unknown -analyzer-config options
Szelethus added a comment. @whisperity @xazax.hun A worry of mine is shared libraries, for example, we've got an array of Ericsson-specific checkers that we load run-time. Do we support (or should we) support acquiring non-checker `-analyzer-config` options? Repository: rC Clang https://reviews.llvm.org/D53280 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53280: [analyzer] Emit a warning for unknown -analyzer-config options
Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs, MTC.
Herald added subscribers: cfe-commits, donat.nagy, mikhail.ramalho, a.sidorin,
szepet, whisperity.
I'm in the process of refactoring AnalyzerOptions. The main motivation behind
here is to emit warnings if an invalid -analyzer-config option is given from
the command line, and be able to list them all.
In this patch, I'll implement the warning.
Repository:
rC Clang
https://reviews.llvm.org/D53280
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/Frontend/CompilerInvocation.cpp
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -314,6 +314,8 @@
Opts.CheckersControlList.emplace_back(checker, enable);
}
+ std::vector RegisteredOptions = Opts.getConfigOptionList();
+
// Go through the analyzer configuration options.
for (const auto *A : Args.filtered(OPT_analyzer_config)) {
A->claim();
@@ -338,6 +340,13 @@
Success = false;
break;
}
+ // Check whether this really is a valid -analyzer-condfig option.
+ // TODO: We should check whether all options are valid or not, but for
+ // now, skip checker options.
+ if (key.count(':') == 0) {
+if (llvm::find(RegisteredOptions, key) == RegisteredOptions.end())
+ Diags.Report(diag::warn_analyzer_config_unknown_config) << key;
+ }
Opts.Config[key] = val;
}
}
Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===
--- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -320,6 +320,15 @@
template
T getDefaultValForUserMode(T ShallowVal, T DeepVal);
+ std::vector getConfigOptionList() const {
+return {
+#define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL) \
+ CMDFLAG,
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.def"
+#undef ANALYZER_OPTION
+};
+ }
+
#define ANALYZER_OPTION_WITH_FN(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL, \
CREATE_FN) \
TYPE CREATE_FN();
Index: include/clang/Basic/DiagnosticDriverKinds.td
===
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -291,6 +291,9 @@
"analyzer-config option '%0' has a key but no value">;
def err_analyzer_config_multiple_values : Error<
"analyzer-config option '%0' should contain only one '='">;
+def warn_analyzer_config_unknown_config : Warning<
+ "unkown -analyzer-config option '%0'">,
+ InGroup;
def err_drv_invalid_hvx_length : Error<
"-mhvx-length is not supported without a -mhvx/-mhvx= flag">;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -314,6 +314,8 @@
Opts.CheckersControlList.emplace_back(checker, enable);
}
+ std::vector RegisteredOptions = Opts.getConfigOptionList();
+
// Go through the analyzer configuration options.
for (const auto *A : Args.filtered(OPT_analyzer_config)) {
A->claim();
@@ -338,6 +340,13 @@
Success = false;
break;
}
+ // Check whether this really is a valid -analyzer-condfig option.
+ // TODO: We should check whether all options are valid or not, but for
+ // now, skip checker options.
+ if (key.count(':') == 0) {
+if (llvm::find(RegisteredOptions, key) == RegisteredOptions.end())
+ Diags.Report(diag::warn_analyzer_config_unknown_config) << key;
+ }
Opts.Config[key] = val;
}
}
Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===
--- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -320,6 +320,15 @@
template
T getDefaultValForUserMode(T ShallowVal, T DeepVal);
+ std::vector getConfigOptionList() const {
+return {
+#define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL) \
+ CMDFLAG,
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.def"
+#undef ANALYZER_OPTION
+};
+ }
+
#define ANALYZER_OPTION_WITH_FN(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL, \
CREATE_FN) \
TYPE CREATE_FN();
Index: include/clang/Basic/DiagnosticDriverKinds.td
===
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -291,6 +291,9 @@
"analyzer-config option '%0' has a
r344504 - [TI removal] Make `getTerminator()` return a generic `Instruction`.
Author: chandlerc
Date: Mon Oct 15 03:42:50 2018
New Revision: 344504
URL: http://llvm.org/viewvc/llvm-project?rev=344504&view=rev
Log:
[TI removal] Make `getTerminator()` return a generic `Instruction`.
This removes the primary remaining API producing `TerminatorInst` which
will reduce the rate at which code is introduced trying to use it and
generally make it much easier to remove the remaining APIs across the
codebase.
Also clean up some of the stragglers that the previous mechanical update
of variables missed.
Users of LLVM and out-of-tree code generally will need to update any
explicit variable types to handle this. Replacing `TerminatorInst` with
`Instruction` (or `auto`) almost always works. Most of these edits were
made in prior commits using the perl one-liner:
```
perl -i -ple 's/TerminatorInst(\b.* = .*getTerminator\(\))/Instruction\1/g'
```
This also my break some rare use cases where people overload for both
`Instruction` and `TerminatorInst`, but these should be easily fixed by
removing the `TerminatorInst` overload.
Modified:
cfe/trunk/lib/CodeGen/CGCleanup.cpp
cfe/trunk/lib/CodeGen/CGException.cpp
Modified: cfe/trunk/lib/CodeGen/CGCleanup.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCleanup.cpp?rev=344504&r1=344503&r2=344504&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCleanup.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCleanup.cpp Mon Oct 15 03:42:50 2018
@@ -366,7 +366,7 @@ static llvm::SwitchInst *TransitionToCle
llvm::BasicBlock *Block) {
// If it's a branch, turn it into a switch whose default
// destination is its original target.
- llvm::TerminatorInst *Term = Block->getTerminator();
+ llvm::Instruction *Term = Block->getTerminator();
assert(Term && "can't transition block without terminator");
if (llvm::BranchInst *Br = dyn_cast(Term)) {
@@ -589,7 +589,7 @@ static void ForwardPrebranchedFallthroug
llvm::BasicBlock *To) {
// Exit is the exit block of a cleanup, so it always terminates in
// an unconditional branch or a switch.
- llvm::TerminatorInst *Term = Exit->getTerminator();
+ llvm::Instruction *Term = Exit->getTerminator();
if (llvm::BranchInst *Br = dyn_cast(Term)) {
assert(Br->isUnconditional() && Br->getSuccessor(0) == From);
Modified: cfe/trunk/lib/CodeGen/CGException.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGException.cpp?rev=344504&r1=344503&r2=344504&view=diff
==
--- cfe/trunk/lib/CodeGen/CGException.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGException.cpp Mon Oct 15 03:42:50 2018
@@ -1248,7 +1248,7 @@ void CodeGenFunction::ExitCXXTryStmt(con
// we follow the false destination for each of the cond branches to reach
// the rethrow block.
llvm::BasicBlock *RethrowBlock = WasmCatchStartBlock;
-while (llvm::TerminatorInst *TI = RethrowBlock->getTerminator()) {
+while (llvm::Instruction *TI = RethrowBlock->getTerminator()) {
auto *BI = cast(TI);
assert(BI->isConditional());
RethrowBlock = BI->getSuccessor(1);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53277: [analyzer][NFC][WIP] Collect all -analyzer-config options in a .def file
Szelethus added a comment. Also, this patch does not contain checker options. That I would suspect be a little more invasive, so I'll do that in a followup patch. Repository: rC Clang https://reviews.llvm.org/D53277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part
lebedev.ri added a comment. Ping. The prerequisite "split truncation sanitizer into unsigned and signed cases" has landed. I believe i have replied/addressed all the points previously raised here. Would be awesome to get this going at long last :) Repository: rC Clang https://reviews.llvm.org/D50250 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
ebevhan added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:1019 + assert(!SrcType->isFixedPointType() && !DstType->isFixedPointType() && + "Use the TargetCodeGenInfo::emitFixedPoint family functions for " + "handling conversions involving fixed point types."); It's not in TargetCodeGenInfo any more. Repository: rC Clang https://reviews.llvm.org/D50616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53019: [clangd] dump xrefs information in dexp tool.
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
Comment at: clangd/index/dex/dexp/Dexp.cpp:61
+ Request.Scopes.emplace_back();
+ std::tie(Request.Scopes.back(), Request.Query) =
+ clang::clangd::splitQualifiedName(QualifiedName);
Are you sure you want both `foo` to mean `::foo` only, rather than accept any
scope and the user can type `::foo` for explicitly global scope?
Comment at: clangd/index/dex/dexp/Dexp.cpp:64
+ std::vector SymIDs;
+ // We choose the first one if there are overloaded symbols.
+ Index->fuzzyFind(Request, [&](const Symbol &Sym) {
stale comment
Comment at: clangd/index/dex/dexp/Dexp.cpp:180
+}
+else {
+ IDs = getSymbolIDsFromIndex(Name, Index);
clang-format
Comment at: clangd/index/dex/dexp/Dexp.cpp:185
clang::clangd::LookupRequest Request;
-Request.IDs = {*SID};
+Request.IDs.insert(IDs.begin(), IDs.end());
bool FoundSymbol = false;
(nit: why not just initialize in place above?)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53019
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53273: [clangd] Fix some references missing in dynamic index.
hokein updated this revision to Diff 169671.
hokein marked an inline comment as done.
hokein added a comment.
avoid calling shouldCollectSymbol every time during indexing.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53273
Files:
clangd/index/SymbolCollector.cpp
unittests/clangd/FileIndexTests.cpp
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -8,13 +8,15 @@
//===--===//
#include "Annotations.h"
+#include "AST.h"
#include "ClangdUnit.h"
#include "TestFS.h"
#include "TestTU.h"
#include "gmock/gmock.h"
#include "index/FileIndex.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Frontend/Utils.h"
#include "clang/Index/IndexSymbol.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/CompilationDatabase.h"
@@ -346,6 +348,55 @@
EXPECT_TRUE(SeenSymbol);
}
+TEST(FileIndexTest, ReferencesInMainFileWithPreamble) {
+ const std::string Header = R"cpp(
+class Foo {};
+ )cpp";
+ Annotations Main(R"cpp(
+#include "foo.h"
+void f() {
+ [[Foo]] foo;
+}
+ )cpp");
+ auto MainFile = testPath("foo.cpp");
+ auto HeaderFile = testPath("foo.h");
+ std::vector Cmd = {"clang", "-xc++", MainFile.c_str()};
+ // Preparse ParseInputs.
+ ParseInputs PI;
+ PI.CompileCommand.Directory = testRoot();
+ PI.CompileCommand.Filename = MainFile;
+ PI.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
+ PI.Contents = Main.code();
+ PI.FS = buildTestFS({{MainFile, Main.code()}, {HeaderFile, Header}});
+
+ // Prepare preamble.
+ auto CI = buildCompilerInvocation(PI);
+ auto PreambleData = buildPreamble(
+ MainFile,
+ *buildCompilerInvocation(PI), /*OldPreamble=*/nullptr,
+ tooling::CompileCommand(), PI,
+ std::make_shared(), /*StoreInMemory=*/true,
+ [&](ASTContext &Ctx, std::shared_ptr PP) {});
+ // Build AST for main file with preamble.
+ auto AST = ParsedAST::build(
+ createInvocationFromCommandLine(Cmd), PreambleData,
+ llvm::MemoryBuffer::getMemBufferCopy(Main.code()),
+ std::make_shared(),
+ PI.FS);
+ ASSERT_TRUE(AST);
+ FileIndex Index;
+ Index.updateMain(MainFile, *AST);
+
+ auto Foo =
+ findSymbol(TestTU::withHeaderCode(Header).headerSymbols(), "Foo");
+ RefsRequest Request;
+ Request.IDs.insert(Foo.ID);
+
+ // Expect to see references in main file, references in headers are excluded
+ // because we only index main AST.
+ EXPECT_THAT(getRefs(Index, Foo.ID), RefsAre({RefRange(Main.range())}));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -345,16 +345,20 @@
SM.getFileID(SpellingLoc) == SM.getMainFileID())
ReferencedDecls.insert(ND);
- if ((static_cast(Opts.RefFilter) & Roles) &&
- SM.getFileID(SpellingLoc) == SM.getMainFileID())
-DeclRefs[ND].emplace_back(SpellingLoc, Roles);
+ bool CollectRef = static_cast(Opts.RefFilter) & Roles;
+ bool IsOnlyRef =
+ !(Roles & (static_cast(index::SymbolRole::Declaration) |
+ static_cast(index::SymbolRole::Definition)));
- // Don't continue indexing if this is a mere reference.
- if (!(Roles & static_cast(index::SymbolRole::Declaration) ||
-Roles & static_cast(index::SymbolRole::Definition)))
+ if (IsOnlyRef && !CollectRef)
return true;
if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
return true;
+ if (CollectRef && SM.getFileID(SpellingLoc) == SM.getMainFileID())
+DeclRefs[ND].emplace_back(SpellingLoc, Roles);
+ // Don't continue indexing if this is a mere reference.
+ if (IsOnlyRef)
+return true;
auto ID = getSymbolID(ND);
if (!ID)
@@ -476,17 +480,15 @@
std::string MainURI = *MainFileURI;
for (const auto &It : DeclRefs) {
if (auto ID = getSymbolID(It.first)) {
-if (Symbols.find(*ID)) {
- for (const auto &LocAndRole : It.second) {
-Ref R;
-auto Range =
-getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
-R.Location.Start = Range.first;
-R.Location.End = Range.second;
-R.Location.FileURI = MainURI;
-R.Kind = toRefKind(LocAndRole.second);
-Refs.insert(*ID, R);
- }
+for (const auto &LocAndRole : It.second) {
+ Ref R;
+ auto Range =
+ getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
+ R.Location.Start = Range.first;
+ R.Location.End = Range.second;
+ R.Location.FileURI = MainURI;
+ R.Kind = toRefKind(LocAndRole.second);
+ Refs.insert(*ID, R);
[PATCH] D53273: [clangd] Fix some references missing in dynamic index.
hokein added inline comments. Comment at: clangd/index/SymbolCollector.cpp:348 + if (!shouldCollectSymbol(*ND, *ASTCtx, Opts)) +return true; sammccall wrote: > This seems better for the main-AST case, but substantially more expensive for > indexing preambles: we may not want references at all, so paying for > shouldCollectSymbol for every reference seems wasteful. > > what about > ``` > bool RefMayBeInteresting = RefFilter & Roles; > bool IsOnlyRef = !(Roles & (Declaration | Definition)); > if (IsOnlyRef && !RefMayBeInteresting) > return; > if (!shouldCollectSymbol()) > return; > if (RefMayBeInteresting && File == getMainFileID()) > // add ref > if (IsOnlyRef) // don't continue if mere reference > return; > ``` > I thought `shouldCollectSymbol` is not as expensive as before (since we have removed AST matchers stuff there), but I'm definitely +1 on the idea of avoid calling it for every reference. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53273 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53277: [analyzer][NFC][WIP] Collect all -analyzer-config options in a .def file
Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs, MTC.
Herald added subscribers: cfe-commits, donat.nagy, mikhail.ramalho, a.sidorin,
szepet, whisperity.
I'm in the process of refactoring AnalyzerOptions. The main motivation behind
here is to emit warnings if an invalid -analyzer-config option is given from
the command line, and be able to list them all.
In this patch, I'm moving **//all//** analyzer options to a def file, and move
2 enums to global namespace.
This patch is WIP, because I didn't add descriptions just yet.
Repository:
rC Clang
https://reviews.llvm.org/D53277
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Core/CoreEngine.cpp
Index: lib/StaticAnalyzer/Core/CoreEngine.cpp
===
--- lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -56,17 +56,17 @@
static std::unique_ptr generateWorkList(AnalyzerOptions &Opts,
SubEngine &subengine) {
switch (Opts.getExplorationStrategy()) {
-case AnalyzerOptions::ExplorationStrategyKind::DFS:
+case ExplorationStrategyKind::DFS:
return WorkList::makeDFS();
-case AnalyzerOptions::ExplorationStrategyKind::BFS:
+case ExplorationStrategyKind::BFS:
return WorkList::makeBFS();
-case AnalyzerOptions::ExplorationStrategyKind::BFSBlockDFSContents:
+case ExplorationStrategyKind::BFSBlockDFSContents:
return WorkList::makeBFSBlockDFSContents();
-case AnalyzerOptions::ExplorationStrategyKind::UnexploredFirst:
+case ExplorationStrategyKind::UnexploredFirst:
return WorkList::makeUnexploredFirst();
-case AnalyzerOptions::ExplorationStrategyKind::UnexploredFirstQueue:
+case ExplorationStrategyKind::UnexploredFirstQueue:
return WorkList::makeUnexploredFirstPriorityQueue();
-case AnalyzerOptions::ExplorationStrategyKind::UnexploredFirstLocationQueue:
+case ExplorationStrategyKind::UnexploredFirstLocationQueue:
return WorkList::makeUnexploredFirstPriorityLocationQueue();
}
}
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -49,7 +49,7 @@
return Result;
}
-AnalyzerOptions::UserModeKind AnalyzerOptions::getUserMode() {
+UserModeKind AnalyzerOptions::getUserMode() {
if (!UserMode.hasValue()) {
StringRef ModeStr = getOptionAsString("mode", "deep");
UserMode = llvm::StringSwitch>(ModeStr)
@@ -61,7 +61,7 @@
return UserMode.getValue();
}
-AnalyzerOptions::ExplorationStrategyKind
+ExplorationStrategyKind
AnalyzerOptions::getExplorationStrategy() {
if (!ExplorationStrategy.hasValue()) {
StringRef StratStr = getOptionAsString("exploration_strategy",
@@ -182,137 +182,6 @@
return V.getValue();
}
-bool AnalyzerOptions::includeTemporaryDtorsInCFG() {
- return getBooleanOption(IncludeTemporaryDtorsInCFG,
- "cfg-temporary-dtors",
- /* Default = */ true);
-}
-
-bool AnalyzerOptions::includeImplicitDtorsInCFG() {
- return getBooleanOption(IncludeImplicitDtorsInCFG,
- "cfg-implicit-dtors",
- /* Default = */ true);
-}
-
-bool AnalyzerOptions::includeLifetimeInCFG() {
- return getBooleanOption(IncludeLifetimeInCFG, "cfg-lifetime",
- /* Default = */ false);
-}
-
-bool AnalyzerOptions::includeLoopExitInCFG() {
- return getBooleanOption(IncludeLoopExitInCFG, "cfg-loopexit",
- /* Default = */ false);
-}
-
-bool AnalyzerOptions::includeRichConstructorsInCFG() {
- return getBooleanOption(IncludeRichConstructorsInCFG,
- "cfg-rich-constructors",
- /* Default = */ true);
-}
-
-bool AnalyzerOptions::includeScopesInCFG() {
- return getBooleanOption(IncludeScopesInCFG,
- "cfg-scopes",
- /* Default = */ false);
-}
-
-bool AnalyzerOptions::mayInlineCXXStandardLibrary() {
- return getBooleanOption(InlineCXXStandardLibrary,
- "c++-stdlib-inlining",
- /*Default=*/true);
-}
-
-bool AnalyzerOptions::mayInlineTemplateFunctions() {
- return getBooleanOption(InlineTemplateFunctions,
- "c++-template-inlining",
- /*Default=*/true);
-}
-
-bool AnalyzerOptions::mayInlineCXXAllocator() {
- return getBooleanOption(InlineCXXAllocator,
- "c++-allocator-inlining",
- /*Default=*/true);
-}
-
-bool AnalyzerOptions::mayInlineCXXContainerMethods() {
- return getBooleanOption(InlineCXXCont
[PATCH] D53019: [clangd] dump xrefs information in dexp tool.
hokein updated this revision to Diff 169668.
hokein marked 2 inline comments as done.
hokein added a comment.
Address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53019
Files:
clangd/index/dex/dexp/CMakeLists.txt
clangd/index/dex/dexp/Dexp.cpp
Index: clangd/index/dex/dexp/Dexp.cpp
===
--- clangd/index/dex/dexp/Dexp.cpp
+++ clangd/index/dex/dexp/Dexp.cpp
@@ -12,13 +12,15 @@
//
//===--===//
-#include "../../Serialization.h"
-#include "../Dex.h"
+#include "Dex.h"
+#include "Serialization.h"
+#include "SourceCode.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/LineEditor/LineEditor.h"
#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/Signals.h"
using clang::clangd::FuzzyFindRequest;
@@ -52,6 +54,22 @@
llvm::outs() << llvm::formatv("{0} took {1:ms+n}.\n", Name, Duration);
}
+std::vector
+getSymbolIDsFromIndex(llvm::StringRef QualifiedName, const SymbolIndex *Index) {
+ FuzzyFindRequest Request;
+ Request.Scopes.emplace_back();
+ std::tie(Request.Scopes.back(), Request.Query) =
+ clang::clangd::splitQualifiedName(QualifiedName);
+ std::vector SymIDs;
+ // We choose the first one if there are overloaded symbols.
+ Index->fuzzyFind(Request, [&](const Symbol &Sym) {
+std::string SymQualifiedName = (Sym.Scope + Sym.Name).str();
+if (QualifiedName == SymQualifiedName)
+ SymIDs.push_back(Sym.ID);
+ });
+ return SymIDs;
+}
+
// REPL commands inherit from Command and contain their options as members.
// Creating a Command populates parser options, parseAndRun() resets them.
class Command {
@@ -88,7 +106,6 @@
};
// FIXME(kbobyrev): Ideas for more commands:
-// * find symbol references: print set of reference locations
// * load/swap/reload index: this would make it possible to get rid of llvm::cl
// usages in the tool driver and actually use llvm::cl library in the REPL.
// * show posting list density histogram (our dump data somewhere so that user
@@ -139,19 +156,33 @@
cl::opt ID{
"id",
cl::Positional,
- cl::Required,
cl::desc("Symbol ID to look up (hex)"),
};
+ cl::opt Name{
+ "name", cl::desc("Qualified name to look up."),
+ };
void run() override {
-auto SID = clang::clangd::SymbolID::fromStr(ID);
-if (!SID) {
- llvm::outs() << llvm::toString(SID.takeError()) << "\n";
+if (ID.getNumOccurrences() == 0 && Name.getNumOccurrences() == 0) {
+ llvm::outs()
+ << "Missing required argument: please provide id or -name.\n";
return;
}
+std::vector IDs;
+if (ID.getNumOccurrences()) {
+ auto SID = clang::clangd::SymbolID::fromStr(ID);
+ if (!SID) {
+llvm::outs() << llvm::toString(SID.takeError()) << "\n";
+return;
+ }
+ IDs.push_back(*SID);
+}
+else {
+ IDs = getSymbolIDsFromIndex(Name, Index);
+}
clang::clangd::LookupRequest Request;
-Request.IDs = {*SID};
+Request.IDs.insert(IDs.begin(), IDs.end());
bool FoundSymbol = false;
Index->lookup(Request, [&](const Symbol &Sym) {
FoundSymbol = true;
@@ -162,13 +193,64 @@
}
};
+class Refs : public Command {
+ cl::opt ID{
+ "id", cl::Positional,
+ cl::desc("Symbol ID of the symbol being queried (hex)."),
+ };
+ cl::opt Name{
+ "name", cl::desc("Qualified name of the symbol being queried."),
+ };
+ cl::opt Filter{
+ "filter",
+ cl::init(".*"),
+ cl::desc(
+ "Print all results from files matching this regular expression."),
+ };
+
+ void run() override {
+if (ID.getNumOccurrences() == 0 && Name.getNumOccurrences() == 0) {
+ llvm::outs()
+ << "Missing required argument: please provide id or -name.\n";
+ return;
+}
+std::vector IDs;
+if (ID.getNumOccurrences()) {
+ auto SID = clang::clangd::SymbolID::fromStr(ID);
+ if (!SID) {
+llvm::outs() << llvm::toString(SID.takeError()) << "\n";
+return;
+ }
+ IDs.push_back(*SID);
+}
+else {
+ IDs = getSymbolIDsFromIndex(Name, Index);
+}
+clang::clangd::RefsRequest RefRequest;
+RefRequest.IDs.insert(IDs.begin(), IDs.end());
+llvm::Regex RegexFilter(Filter);
+Index->refs(RefRequest, [&RegexFilter](const clang::clangd::Ref &R) {
+ auto U = clang::clangd::URI::parse(R.Location.FileURI);
+ if (!U) {
+llvm::outs() << U.takeError();
+return;
+ }
+ if (RegexFilter.match(U->body()))
+llvm::outs() << R << "\n";
+});
+ }
+};
+
struct {
const char *Name;
const char *Description;
std::function()> Implementation;
} CommandInfo[] = {
{"find", "Search for symbols with fuzzyFind", llvm::make_unique},
-{"lookup", "Dump symbol deta
[PATCH] D53276: [analyzer][NFC] Fix some incorrect uses of AnalyzerOptions
Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, MTC, rnkovacs.
Herald added subscribers: cfe-commits, donat.nagy, mikhail.ramalho, a.sidorin,
szepet, whisperity.
I'm in the process of refactoring AnalyzerOptions. The main motivation behind
here is to emit warnings if an invalid -analyzer-config option is given from
the command line, and be able to list them all.
In this patch, I found some flags that should've been used as checker options,
or have absolutely no mention of in `AnalyzerOptions`, or are nonexistent.
- `NonLocalizedStringChecker` now uses its `"AggressiveReport"` flag as a
checker option
- `lib/StaticAnalyzer/Frontend/ModelInjector.cpp` now accesses the
`"model-path"` option through a getter in `AnalyzerOptions`
- `-analyzer-config path-diagnostics-alternate=false` is not a thing, I removed
it.
Repository:
rC Clang
https://reviews.llvm.org/D53276
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Frontend/ModelInjector.cpp
test/Analysis/cstring-plist.c
test/Analysis/localization-aggressive.m
Index: test/Analysis/localization-aggressive.m
===
--- test/Analysis/localization-aggressive.m
+++ test/Analysis/localization-aggressive.m
@@ -1,6 +1,10 @@
// RUN: %clang_cc1 -fblocks -x objective-c-header -emit-pch -o %t.pch
%S/Inputs/localization-pch.h
-// RUN: %clang_analyze_cc1 -fblocks -analyzer-store=region
-analyzer-checker=optin.osx.cocoa.localizability.NonLocalizedStringChecker
-analyzer-checker=optin.osx.cocoa.localizability.EmptyLocalizationContextChecker
-include-pch %t.pch -verify -analyzer-config AggressiveReport=true %s
+// RUN: %clang_analyze_cc1 -fblocks -analyzer-store=region \
+// RUN: -analyzer-config
optin.osx.cocoa.localizability.NonLocalizedStringChecker:AggressiveReport=true \
+// RUN:
-analyzer-checker=optin.osx.cocoa.localizability.NonLocalizedStringChecker \
+// RUN:
-analyzer-checker=optin.osx.cocoa.localizability.EmptyLocalizationContextChecker
\
+// RUN: -include-pch %t.pch -verify %s
// These declarations were reduced using Delta-Debugging from Foundation.h
// on Mac OS X.
Index: test/Analysis/cstring-plist.c
===
--- test/Analysis/cstring-plist.c
+++ test/Analysis/cstring-plist.c
@@ -1,5 +1,8 @@
// RUN: rm -f %t
-// RUN: %clang_analyze_cc1 -fblocks
-analyzer-checker=core,unix.Malloc,unix.cstring.NullArg
-analyzer-disable-checker=alpha.unix.cstring.OutOfBounds -analyzer-output=plist
-analyzer-config path-diagnostics-alternate=false -o %t %s
+// RUN: %clang_analyze_cc1 -fblocks \
+// RUN: -analyzer-checker=core,unix.Malloc,unix.cstring.NullArg \
+// RUN: -analyzer-disable-checker=alpha.unix.cstring.OutOfBounds \
+// RUN: -analyzer-output=plist -o %t %s
// RUN: FileCheck -input-file %t %s
typedef __typeof(sizeof(int)) size_t;
Index: lib/StaticAnalyzer/Frontend/ModelInjector.cpp
===
--- lib/StaticAnalyzer/Frontend/ModelInjector.cpp
+++ lib/StaticAnalyzer/Frontend/ModelInjector.cpp
@@ -48,7 +48,7 @@
FileID mainFileID = SM.getMainFileID();
AnalyzerOptionsRef analyzerOpts = CI.getAnalyzerOpts();
- llvm::StringRef modelPath = analyzerOpts->Config["model-path"];
+ llvm::StringRef modelPath = analyzerOpts->getModelPath();
llvm::SmallString<128> fileName;
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -502,3 +502,9 @@
CTUIndexName = getOptionAsString("ctu-index-name", "externalFnMap.txt");
return CTUIndexName.getValue();
}
+
+StringRef AnalyzerOptions::getModelPath() {
+ if (!ModelPath.hasValue())
+ModelPath = getOptionAsString("model-path", "");
+ return ModelPath.getValue();
+}
Index: lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
+++ lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
@@ -1398,7 +1398,8 @@
NonLocalizedStringChecker *checker =
mgr.registerChecker();
checker->IsAggressive =
- mgr.getAnalyzerOptions().getBooleanOption("AggressiveReport", false);
+ mgr.getAnalyzerOptions().getBooleanOption("AggressiveReport", false,
+checker);
}
void ento::registerEmptyLocalizationContextChecker(CheckerManager &mgr) {
Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===
--- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -330,6 +33
[PATCH] D53274: [analyzer][NFC] Tighten up AnalyzerOptions
Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, MTC, rnkovacs.
Herald added subscribers: cfe-commits, donat.nagy, mikhail.ramalho, a.sidorin,
szepet, whisperity.
I was a little unsure about the title, but it is what it is.
I'm in the process of refactoring `AnalyzerOptions`. The main motivation behind
here is to emit warnings if an invalid `-analyzer-config` option is given from
the command line, and be able to list them all.
This first NFC patch contains small modifications to make `AnalyzerOptions.cpp`
a little more consistent.
Repository:
rC Clang
https://reviews.llvm.org/D53274
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Core/CoreEngine.cpp
Index: lib/StaticAnalyzer/Core/CoreEngine.cpp
===
--- lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -68,8 +68,6 @@
return WorkList::makeUnexploredFirstPriorityQueue();
case AnalyzerOptions::ExplorationStrategyKind::UnexploredFirstLocationQueue:
return WorkList::makeUnexploredFirstPriorityLocationQueue();
-default:
- llvm_unreachable("Unexpected case");
}
}
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -50,27 +50,24 @@
}
AnalyzerOptions::UserModeKind AnalyzerOptions::getUserMode() {
- if (UserMode == UMK_NotSet) {
-StringRef ModeStr =
-Config.insert(std::make_pair("mode", "deep")).first->second;
-UserMode = llvm::StringSwitch(ModeStr)
+ if (!UserMode.hasValue()) {
+StringRef ModeStr = getOptionAsString("mode", "deep");
+UserMode = llvm::StringSwitch>(ModeStr)
.Case("shallow", UMK_Shallow)
.Case("deep", UMK_Deep)
- .Default(UMK_NotSet);
-assert(UserMode != UMK_NotSet && "User mode is invalid.");
+ .Default(None);
+assert(UserMode.getValue() && "User mode is invalid.");
}
- return UserMode;
+ return UserMode.getValue();
}
AnalyzerOptions::ExplorationStrategyKind
AnalyzerOptions::getExplorationStrategy() {
- if (ExplorationStrategy == ExplorationStrategyKind::NotSet) {
-StringRef StratStr =
-Config
-.insert(std::make_pair("exploration_strategy", "unexplored_first_queue"))
-.first->second;
+ if (!ExplorationStrategy.hasValue()) {
+StringRef StratStr = getOptionAsString("exploration_strategy",
+ "unexplored_first_queue");
ExplorationStrategy =
-llvm::StringSwitch(StratStr)
+llvm::StringSwitch>(StratStr)
.Case("dfs", ExplorationStrategyKind::DFS)
.Case("bfs", ExplorationStrategyKind::BFS)
.Case("unexplored_first",
@@ -81,15 +78,15 @@
ExplorationStrategyKind::UnexploredFirstLocationQueue)
.Case("bfs_block_dfs_contents",
ExplorationStrategyKind::BFSBlockDFSContents)
-.Default(ExplorationStrategyKind::NotSet);
-assert(ExplorationStrategy != ExplorationStrategyKind::NotSet &&
+.Default(None);
+assert(ExplorationStrategy.hasValue() &&
"User mode is invalid.");
}
- return ExplorationStrategy;
+ return ExplorationStrategy.getValue();
}
IPAKind AnalyzerOptions::getIPAMode() {
- if (IPAMode == IPAK_NotSet) {
+ if (!IPAMode.hasValue()) {
// Use the User Mode to set the default IPA value.
// Note, we have to add the string to the Config map for the ConfigDumper
// checker to function properly.
@@ -102,53 +99,40 @@
assert(DefaultIPA);
// Lookup the ipa configuration option, use the default from User Mode.
-StringRef ModeStr =
-Config.insert(std::make_pair("ipa", DefaultIPA)).first->second;
-IPAKind IPAConfig = llvm::StringSwitch(ModeStr)
+StringRef ModeStr = getOptionAsString("ipa", DefaultIPA);
+IPAMode = llvm::StringSwitch>(ModeStr)
.Case("none", IPAK_None)
.Case("basic-inlining", IPAK_BasicInlining)
.Case("inlining", IPAK_Inlining)
.Case("dynamic", IPAK_DynamicDispatch)
.Case("dynamic-bifurcate", IPAK_DynamicDispatchBifurcate)
-.Default(IPAK_NotSet);
-assert(IPAConfig != IPAK_NotSet && "IPA Mode is invalid.");
-
-// Set the member variable.
-IPAMode = IPAConfig;
+.Default(None);
+assert(IPAMode.hasValue() && "IPA Mode is invalid.");
}
- return IPAMode;
+ return IPAMode.getValue();
}
bool
AnalyzerOptions::mayInlineCXXMemberFunction(CXXInlineableMemberKind K) {
if (getIPAMode() < IPAK_Inlining)
return false;
if (!CXXMemberInliningMode) {
-static const char *ModeKey = "c++-inlining";
-
-StringRef ModeStr =
-Config.insert(std::make_
[PATCH] D53273: [clangd] Fix some references missing in dynamic index.
sammccall added a comment. Nice fix, just performance concerns. (I do think this might be significant, but measuring dynamic index time before/after for a big TU might show this to be unimportant) Comment at: clangd/index/SymbolCollector.cpp:348 + if (!shouldCollectSymbol(*ND, *ASTCtx, Opts)) +return true; This seems better for the main-AST case, but substantially more expensive for indexing preambles: we may not want references at all, so paying for shouldCollectSymbol for every reference seems wasteful. what about ``` bool RefMayBeInteresting = RefFilter & Roles; bool IsOnlyRef = !(Roles & (Declaration | Definition)); if (IsOnlyRef && !RefMayBeInteresting) return; if (!shouldCollectSymbol()) return; if (RefMayBeInteresting && File == getMainFileID()) // add ref if (IsOnlyRef) // don't continue if mere reference return; ``` Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53273 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53273: [clangd] Fix some references missing in dynamic index.
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric,
ilya-biryukov.
Previously, SymbolCollector postfilters all references at the end to
find all references of interesting symbols.
It was incorrect when indxing main AST where we don't see locations
of symbol declarations and definitions in the main AST (as those are in
preamble AST).
The fix is to do earily check during collecting references.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53273
Files:
clangd/index/SymbolCollector.cpp
unittests/clangd/FileIndexTests.cpp
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -8,13 +8,15 @@
//===--===//
#include "Annotations.h"
+#include "AST.h"
#include "ClangdUnit.h"
#include "TestFS.h"
#include "TestTU.h"
#include "gmock/gmock.h"
#include "index/FileIndex.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Frontend/Utils.h"
#include "clang/Index/IndexSymbol.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/CompilationDatabase.h"
@@ -346,6 +348,55 @@
EXPECT_TRUE(SeenSymbol);
}
+TEST(FileIndexTest, ReferencesInMainFileWithPreamble) {
+ const std::string Header = R"cpp(
+class Foo {};
+ )cpp";
+ Annotations Main(R"cpp(
+#include "foo.h"
+void f() {
+ [[Foo]] foo;
+}
+ )cpp");
+ auto MainFile = testPath("foo.cpp");
+ auto HeaderFile = testPath("foo.h");
+ std::vector Cmd = {"clang", "-xc++", MainFile.c_str()};
+ // Preparse ParseInputs.
+ ParseInputs PI;
+ PI.CompileCommand.Directory = testRoot();
+ PI.CompileCommand.Filename = MainFile;
+ PI.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
+ PI.Contents = Main.code();
+ PI.FS = buildTestFS({{MainFile, Main.code()}, {HeaderFile, Header}});
+
+ // Prepare preamble.
+ auto CI = buildCompilerInvocation(PI);
+ auto PreambleData = buildPreamble(
+ MainFile,
+ *buildCompilerInvocation(PI), /*OldPreamble=*/nullptr,
+ tooling::CompileCommand(), PI,
+ std::make_shared(), /*StoreInMemory=*/true,
+ [&](ASTContext &Ctx, std::shared_ptr PP) {});
+ // Build AST for main file with preamble.
+ auto AST = ParsedAST::build(
+ createInvocationFromCommandLine(Cmd), PreambleData,
+ llvm::MemoryBuffer::getMemBufferCopy(Main.code()),
+ std::make_shared(),
+ PI.FS);
+ ASSERT_TRUE(AST);
+ FileIndex Index;
+ Index.updateMain(MainFile, *AST);
+
+ auto Foo =
+ findSymbol(TestTU::withHeaderCode(Header).headerSymbols(), "Foo");
+ RefsRequest Request;
+ Request.IDs.insert(Foo.ID);
+
+ // Expect to see references in main file, references in headers are excluded
+ // because we only index main AST.
+ EXPECT_THAT(getRefs(Index, Foo.ID), RefsAre({RefRange(Main.range())}));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -345,16 +345,17 @@
SM.getFileID(SpellingLoc) == SM.getMainFileID())
ReferencedDecls.insert(ND);
+ if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
+return true;
+
if ((static_cast(Opts.RefFilter) & Roles) &&
SM.getFileID(SpellingLoc) == SM.getMainFileID())
DeclRefs[ND].emplace_back(SpellingLoc, Roles);
// Don't continue indexing if this is a mere reference.
if (!(Roles & static_cast(index::SymbolRole::Declaration) ||
Roles & static_cast(index::SymbolRole::Definition)))
return true;
- if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
-return true;
auto ID = getSymbolID(ND);
if (!ID)
@@ -476,17 +477,15 @@
std::string MainURI = *MainFileURI;
for (const auto &It : DeclRefs) {
if (auto ID = getSymbolID(It.first)) {
-if (Symbols.find(*ID)) {
- for (const auto &LocAndRole : It.second) {
-Ref R;
-auto Range =
-getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
-R.Location.Start = Range.first;
-R.Location.End = Range.second;
-R.Location.FileURI = MainURI;
-R.Kind = toRefKind(LocAndRole.second);
-Refs.insert(*ID, R);
- }
+for (const auto &LocAndRole : It.second) {
+ Ref R;
+ auto Range =
+ getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
+ R.Location.Start = Range.first;
+ R.Location.End = Range.second;
+ R.Location.FileURI = MainURI;
+ R.Kind = toRefKind(LocAndRole.second);
+ Refs.insert(*ID, R);
}
}
}
___
c
[PATCH] D53272: Add target requirement to profile remap test.
yroux created this revision. yroux added a reviewer: rsmith. Herald added a reviewer: javed.absar. Herald added a subscriber: kristof.beyls. Fix bots which don't have x86_64 target built (ARM/AArch64). Repository: rC Clang https://reviews.llvm.org/D53272 Files: test/CodeGenCXX/profile-remap.cpp Index: test/CodeGenCXX/profile-remap.cpp === --- test/CodeGenCXX/profile-remap.cpp +++ test/CodeGenCXX/profile-remap.cpp @@ -1,3 +1,5 @@ +// REQUIRES: x86-registered-target +// // RUN: %clang_cc1 -triple x86_64-linux-gnu -fprofile-sample-use=%S/Inputs/profile-remap.samples -fprofile-remapping-file=%S/Inputs/profile-remap.map -fexperimental-new-pass-manager -O2 %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-SAMPLES // RUN: llvm-profdata merge -output %T.profdata %S/Inputs/profile-remap.proftext // RUN: %clang_cc1 -triple x86_64-linux-gnu -fprofile-instrument-use-path=%T.profdata -fprofile-remapping-file=%S/Inputs/profile-remap.map -fexperimental-new-pass-manager -O2 %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-INSTR Index: test/CodeGenCXX/profile-remap.cpp === --- test/CodeGenCXX/profile-remap.cpp +++ test/CodeGenCXX/profile-remap.cpp @@ -1,3 +1,5 @@ +// REQUIRES: x86-registered-target +// // RUN: %clang_cc1 -triple x86_64-linux-gnu -fprofile-sample-use=%S/Inputs/profile-remap.samples -fprofile-remapping-file=%S/Inputs/profile-remap.map -fexperimental-new-pass-manager -O2 %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-SAMPLES // RUN: llvm-profdata merge -output %T.profdata %S/Inputs/profile-remap.proftext // RUN: %clang_cc1 -triple x86_64-linux-gnu -fprofile-instrument-use-path=%T.profdata -fprofile-remapping-file=%S/Inputs/profile-remap.map -fexperimental-new-pass-manager -O2 %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-INSTR ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53244: [Coverage] Fix PR39258: support coverage regions that start deeper than they end
orivej added a comment.
Thanks! Could you merge this (after a while to let others review)?
Comment at: test/CoverageMapping/macros.c:60
+// CHECK-NEXT: func6
+void func6(unsigned count) { // CHECK-NEXT: File 0, [[@LINE]]:28 ->
[[@LINE+4]]:2 = #0
vsk wrote:
> Please use CHECK-LABEL here.
`CHECK-LABEL` would accept unchecked output between the previous `CHECK-NEXT`
and `func6`. It seems that the intention of this test is to check all lines of
output, so I used `CHECK-NEXT` as in other tests in this file.
Repository:
rC Clang
https://reviews.llvm.org/D53244
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
