[PATCH] D40481: [libclang] Fix cursors for arguments of Subscript and Call operators

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan accepted this revision.
yvvan added a comment.
This revision is now accepted and ready to land.

Let's proceed with this one. I really see that it's going to be useful.


https://reviews.llvm.org/D40481



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

Thanks, LGTM.




Comment at: clangd/CodeComplete.cpp:210
+const std::string Name = Method->getNameAsString();
+Overrides[Name].push_back(Method);
+  }

nit: here as well, use `Overrides[Method->getName()].push_back...`


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50898



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51154: [clangd] Log memory usage of DexIndex and MemIndex

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov, sammccall.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.

This patch prints information about built index size estimation to verbose 
logs. This is useful for optimizing memory usage of DexIndex and comparisons 
with MemIndex.


https://reviews.llvm.org/D51154

Files:
  clang-tools-extra/clangd/index/MemIndex.cpp
  clang-tools-extra/clangd/index/MemIndex.h
  clang-tools-extra/clangd/index/dex/DexIndex.cpp
  clang-tools-extra/clangd/index/dex/DexIndex.h


Index: clang-tools-extra/clangd/index/dex/DexIndex.h
===
--- clang-tools-extra/clangd/index/dex/DexIndex.h
+++ clang-tools-extra/clangd/index/dex/DexIndex.h
@@ -58,6 +58,9 @@
Callback) const override;
 
 private:
+  /// Returns estimate size of the index in megabytes.
+  size_t estimateMemoryUsage();
+
   mutable std::mutex Mutex;
 
   std::shared_ptr> Symbols /*GUARDED_BY(Mutex)*/;
Index: clang-tools-extra/clangd/index/dex/DexIndex.cpp
===
--- clang-tools-extra/clangd/index/dex/DexIndex.cpp
+++ clang-tools-extra/clangd/index/dex/DexIndex.cpp
@@ -67,6 +67,9 @@
 InvertedIndex = std::move(TempInvertedIndex);
 SymbolQuality = std::move(TempSymbolQuality);
   }
+
+  vlog("Built DexIndex with estimated memory usage {0} MB.",
+   estimateMemoryUsage());
 }
 
 std::unique_ptr DexIndex::build(SymbolSlab Slab) {
@@ -171,6 +174,20 @@
   log("findOccurrences is not implemented.");
 }
 
+size_t DexIndex::estimateMemoryUsage() {
+  size_t Bytes = LookupTable.size() * sizeof(std::pair);
+  Bytes += SymbolQuality.size() * sizeof(std::pair);
+  Bytes += InvertedIndex.size() * sizeof(Token);
+  {
+std::lock_guard Lock(Mutex);
+
+for (const auto &P : InvertedIndex) {
+  Bytes += P.second.size() * sizeof(DocID);
+}
+  }
+  return Bytes / (1000 * 1000);
+}
+
 } // namespace dex
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/index/MemIndex.h
===
--- clang-tools-extra/clangd/index/MemIndex.h
+++ clang-tools-extra/clangd/index/MemIndex.h
@@ -40,6 +40,9 @@
Callback) const override;
 
 private:
+  /// Returns estimate size of the index in megabytes.
+  size_t estimateMemoryUsage();
+
   std::shared_ptr> Symbols;
   // Index is a set of symbols that are deduplicated by symbol IDs.
   // FIXME: build smarter index structure.
Index: clang-tools-extra/clangd/index/MemIndex.cpp
===
--- clang-tools-extra/clangd/index/MemIndex.cpp
+++ clang-tools-extra/clangd/index/MemIndex.cpp
@@ -26,6 +26,9 @@
 Index = std::move(TempIndex);
 Symbols = std::move(Syms); // Relase old symbols.
   }
+
+  vlog("Built MemIndex with estimated memory usage {0} MB.",
+   estimateMemoryUsage());
 }
 
 std::unique_ptr MemIndex::build(SymbolSlab Slab) {
@@ -98,5 +101,10 @@
   &Snap->Pointers);
 }
 
+size_t MemIndex::estimateMemoryUsage() {
+  size_t Bytes = Index.size() * sizeof(std::pair);
+  return Bytes / (1000 * 1000);
+}
+
 } // namespace clangd
 } // namespace clang


Index: clang-tools-extra/clangd/index/dex/DexIndex.h
===
--- clang-tools-extra/clangd/index/dex/DexIndex.h
+++ clang-tools-extra/clangd/index/dex/DexIndex.h
@@ -58,6 +58,9 @@
Callback) const override;
 
 private:
+  /// Returns estimate size of the index in megabytes.
+  size_t estimateMemoryUsage();
+
   mutable std::mutex Mutex;
 
   std::shared_ptr> Symbols /*GUARDED_BY(Mutex)*/;
Index: clang-tools-extra/clangd/index/dex/DexIndex.cpp
===
--- clang-tools-extra/clangd/index/dex/DexIndex.cpp
+++ clang-tools-extra/clangd/index/dex/DexIndex.cpp
@@ -67,6 +67,9 @@
 InvertedIndex = std::move(TempInvertedIndex);
 SymbolQuality = std::move(TempSymbolQuality);
   }
+
+  vlog("Built DexIndex with estimated memory usage {0} MB.",
+   estimateMemoryUsage());
 }
 
 std::unique_ptr DexIndex::build(SymbolSlab Slab) {
@@ -171,6 +174,20 @@
   log("findOccurrences is not implemented.");
 }
 
+size_t DexIndex::estimateMemoryUsage() {
+  size_t Bytes = LookupTable.size() * sizeof(std::pair);
+  Bytes += SymbolQuality.size() * sizeof(std::pair);
+  Bytes += InvertedIndex.size() * sizeof(Token);
+  {
+std::lock_guard Lock(Mutex);
+
+for (const auto &P : InvertedIndex) {
+  Bytes += P.second.size() * sizeof(DocID);
+}
+  }
+  return Bytes / (1000 * 1000);
+}
+
 } // namespace dex
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/index/MemIndex.h
===

[PATCH] D50580: [clang-tidy] Abseil: no namespace check

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

Looks good.




Comment at: test/clang-tidy/abseil-no-namespace.cpp:10
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:1:11: warning: namespace 'absl' is reserved
+// for implementation of the Abseil library and should not be opened in user

Does the test get passed on the first command `%check_clang_tidy %s 
abseil-no-namespace %t -- -- -I %S/Inputs`? The first command will suppress all 
warning in headers, I think it will fail?


https://reviews.llvm.org/D50580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51154: [clangd] Log memory usage of DexIndex and MemIndex

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/index/MemIndex.cpp:105
+size_t MemIndex::estimateMemoryUsage() {
+  size_t Bytes = Index.size() * sizeof(std::pair);
+  return Bytes / (1000 * 1000);

access to Index needs to be guarded by mutex



Comment at: clang-tools-extra/clangd/index/MemIndex.cpp:105
+size_t MemIndex::estimateMemoryUsage() {
+  size_t Bytes = Index.size() * sizeof(std::pair);
+  return Bytes / (1000 * 1000);

sammccall wrote:
> access to Index needs to be guarded by mutex
`Index.getMemorySize()` is a better estimate.



Comment at: clang-tools-extra/clangd/index/MemIndex.h:43
 private:
+  /// Returns estimate size of the index in megabytes.
+  size_t estimateMemoryUsage();

Can you return bytes here, and do lossy conversions at display time instead?

(Not sure the conversions are even worth it. e.g. it's may be interesting to 
compare even <1mb indexes)



Comment at: clang-tools-extra/clangd/index/MemIndex.h:44
+  /// Returns estimate size of the index in megabytes.
+  size_t estimateMemoryUsage();
+

I'd suggest making this part of the public interface, and implementing it for 
all (the implementations should be trivial).

This is useful e.g. for monitoring. We may want vim's :YcmDebugInfo to return 
this, etc.



Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:178
+size_t DexIndex::estimateMemoryUsage() {
+  size_t Bytes = LookupTable.size() * sizeof(std::pair);
+  Bytes += SymbolQuality.size() * sizeof(std::pair);

again, LookupTable.getMemorySize() and others.



Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:180
+  Bytes += SymbolQuality.size() * sizeof(std::pair);
+  Bytes += InvertedIndex.size() * sizeof(Token);
+  {

I think you're not counting the size of the actual symbols.
This is difficult to do precisely, but avoiding it seems misleading (we have 
"shared ownership" but it's basically exclusive). What's the plan here?



Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:182
+  {
+std::lock_guard Lock(Mutex);
+

why is only the InvertedIndex access guarded by the mutex?


https://reviews.llvm.org/D51154



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50993: [clangd] Increase stack size of the new threads on macOS

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

This part looks good (I think everything controversial got moved into the other 
patch). Thanks!




Comment at: clangd/Threading.cpp:75
+std::string ThreadName;
+decltype(Action) ThreadFunc;
+decltype(CleanupTask) Cleanup;

nit: maybe call these CallAction and CallCleanupTask or something to make the 
parallels completely explicit. Up to you


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50993



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: hokein, ioeric, kbobyrev, sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.

The new mode avoids serializing and deserializing YAML.
This results in better performance and less memory usage. Reduce phase
is now almost instant.

The default is to use the old mode going through YAML serialization to
allow migrating MapReduce clients that require the old mode to operate
properly. After we migrate the clients, we can switch the default to
the new mode.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51155

Files:
  clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp

Index: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
===
--- clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
+++ clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
@@ -49,9 +49,34 @@
"not given, such headers will have relative paths."),
 llvm::cl::init(""));
 
+static llvm::cl::opt MergeOnTheFly(
+"merge-on-the-fly",
+llvm::cl::desc(
+"Merges symbols for each processed translation unit as soon "
+"they become available. This results in a smaller memory "
+"usage and an almost instant reduce stage. Optimal for running as a "
+"standalone tool, but cannot be used inside MapReduce."),
+/// FIXME(ibiryukov): change the default to true after updating usages that
+/// rely on MR use-case to work properly.
+llvm::cl::init(false));
+
+/// Responsible for aggregating symbols from each processed file and producing
+/// the final results. All methods in this class must be thread-safe,
+/// 'consumeSymbols' may be called from multiple threads.
+class SymbolsConsumer {
+public:
+  virtual ~SymbolsConsumer() = default;
+
+  /// Consume a SymbolSlab build for a file.
+  virtual void consumeSymbols(SymbolSlab Symbols) = 0;
+  /// Produce a resulting symbol slab, by combining  occurrences of the same
+  /// symbols across translation units.
+  virtual SymbolSlab mergeResults() = 0;
+};
+
 class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
 public:
-  SymbolIndexActionFactory(tooling::ExecutionContext *Ctx) : Ctx(Ctx) {}
+  SymbolIndexActionFactory(SymbolsConsumer &Consumer) : Consumer(Consumer) {}
 
   clang::FrontendAction *create() override {
 // Wraps the index action and reports collected symbols to the execution
@@ -61,10 +86,10 @@
   WrappedIndexAction(std::shared_ptr C,
  std::unique_ptr Includes,
  const index::IndexingOptions &Opts,
- tooling::ExecutionContext *Ctx)
+ SymbolsConsumer &Consumer)
   : WrapperFrontendAction(
 index::createIndexingAction(C, Opts, nullptr)),
-Ctx(Ctx), Collector(C), Includes(std::move(Includes)),
+Consumer(Consumer), Collector(C), Includes(std::move(Includes)),
 PragmaHandler(collectIWYUHeaderMaps(this->Includes.get())) {}
 
   std::unique_ptr
@@ -91,14 +116,11 @@
   return;
 }
 
-auto Symbols = Collector->takeSymbols();
-for (const auto &Sym : Symbols) {
-  Ctx->reportResult(Sym.ID.str(), SymbolToYAML(Sym));
-}
+Consumer.consumeSymbols(Collector->takeSymbols());
   }
 
 private:
-  tooling::ExecutionContext *Ctx;
+  SymbolsConsumer &Consumer;
   std::shared_ptr Collector;
   std::unique_ptr Includes;
   std::unique_ptr PragmaHandler;
@@ -118,30 +140,72 @@
 CollectorOpts.Includes = Includes.get();
 return new WrappedIndexAction(
 std::make_shared(std::move(CollectorOpts)),
-std::move(Includes), IndexOpts, Ctx);
+std::move(Includes), IndexOpts, Consumer);
   }
 
-  tooling::ExecutionContext *Ctx;
+  SymbolsConsumer &Consumer;
 };
 
-// Combine occurrences of the same symbol across translation units.
-SymbolSlab mergeSymbols(tooling::ToolResults *Results) {
-  SymbolSlab::Builder UniqueSymbols;
-  llvm::BumpPtrAllocator Arena;
-  Symbol::Details Scratch;
-  Results->forEachResult([&](llvm::StringRef Key, llvm::StringRef Value) {
-Arena.Reset();
-llvm::yaml::Input Yin(Value, &Arena);
-auto Sym = clang::clangd::SymbolFromYAML(Yin, Arena);
-clang::clangd::SymbolID ID;
-Key >> ID;
-if (const auto *Existing = UniqueSymbols.find(ID))
-  UniqueSymbols.insert(mergeSymbol(*Existing, Sym, &Scratch));
-else
-  UniqueSymbols.insert(Sym);
-  });
-  return std::move(UniqueSymbols).build();
-}
+/// Stashes per-file results inside ExecutionContext, merges all of them at the
+/// end. Useful for running on MapReduce infrastructure to avoid keeping symbols
+/// from multiple files in memory.
+class ToolExecutorConsumer : public SymbolsConsumer {
+public:
+  ToolExecutorConsumer(ToolExecutor &Executor) : Executor(Executor) {}
+
+  void

[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Collecting the timings to build the index for LLVM now. Will publish when 
they're ready


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51155



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r340521 - [libclang] Fix cursors for arguments of Subscript and Call operators

[Ivan Donchevskii via cfe-commits]

Author: yvvan
Date: Thu Aug 23 02:48:11 2018
New Revision: 340521

URL: http://llvm.org/viewvc/llvm-project?rev=340521&view=rev
Log:
[libclang] Fix cursors for arguments of Subscript and Call operators

The DeclRefExpr of CXXOperatorCallExpr refering to the custom operator
is visited before the arguments to the operator call. For the Call and
Subscript operator the range of this DeclRefExpr includes the whole call
expression, so that all tokens in that range were mapped to the operator
function, even the tokens of the arguments.

Fix this by ensuring that this particular DeclRefExpr is visited last.

Fixes PR25775.

Fix by Nikolai Kosjar.

Differential Revision: https://reviews.llvm.org/D40481

Added:
cfe/trunk/test/Index/annotate-operator-call-expr.cpp
Modified:
cfe/trunk/tools/libclang/CIndex.cpp

Added: cfe/trunk/test/Index/annotate-operator-call-expr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/annotate-operator-call-expr.cpp?rev=340521&view=auto
==
--- cfe/trunk/test/Index/annotate-operator-call-expr.cpp (added)
+++ cfe/trunk/test/Index/annotate-operator-call-expr.cpp Thu Aug 23 02:48:11 
2018
@@ -0,0 +1,84 @@
+struct Foo {
+  int operator[](int key);
+  int operator()(int key = 2);
+};
+
+void testFoo(Foo foo, int index) {
+  foo();
+  foo(index);
+
+  foo[index];
+  foo[index + index];
+
+  foo[foo[index]];
+  foo[foo() + foo[index]];
+  foo[foo(index) + foo[index]];
+}
+
+// RUN: c-index-test -test-annotate-tokens=%s:7:1:7:100 %s -std=c++11 
-Wno-unused-value | FileCheck %s -check-prefix=CHECK1
+// CHECK1: Identifier: "foo" [7:3 - 7:6] DeclRefExpr=foo:6:18
+// CHECK1: Punctuation: "(" [7:6 - 7:7] DeclRefExpr=operator():3:7 
RefName=[7:6 - 7:7] RefName=[7:7 - 7:8]
+// CHECK1: Punctuation: ")" [7:7 - 7:8] DeclRefExpr=operator():3:7 
RefName=[7:6 - 7:7] RefName=[7:7 - 7:8]
+// CHECK1: Punctuation: ";" [7:8 - 7:9] CompoundStmt=
+
+// RUN: c-index-test -test-annotate-tokens=%s:8:1:8:100 %s -std=c++11 
-Wno-unused-value | FileCheck %s -check-prefix=CHECK2
+// CHECK2: Punctuation: "(" [8:6 - 8:7] DeclRefExpr=operator():3:7 
RefName=[8:6 - 8:7] RefName=[8:12 - 8:13]
+// CHECK2: Identifier: "index" [8:7 - 8:12] DeclRefExpr=index:6:27
+// CHECK2: Punctuation: ")" [8:12 - 8:13] DeclRefExpr=operator():3:7 
RefName=[8:6 - 8:7] RefName=[8:12 - 8:13]
+// CHECK2: Punctuation: ";" [8:13 - 8:14] CompoundStmt=
+
+// RUN: c-index-test -test-annotate-tokens=%s:10:1:10:100 %s -std=c++11 
-Wno-unused-value | FileCheck %s -check-prefix=CHECK3
+// CHECK3: Identifier: "foo" [10:3 - 10:6] DeclRefExpr=foo:6:18
+// CHECK3: Punctuation: "[" [10:6 - 10:7] DeclRefExpr=operator[]:2:7 
RefName=[10:6 - 10:7] RefName=[10:12 - 10:13]
+// CHECK3: Identifier: "index" [10:7 - 10:12] DeclRefExpr=index:6:27
+// CHECK3: Punctuation: "]" [10:12 - 10:13] DeclRefExpr=operator[]:2:7 
RefName=[10:6 - 10:7] RefName=[10:12 - 10:13]
+// CHECK3: Punctuation: ";" [10:13 - 10:14] CompoundStmt=
+
+// RUN: c-index-test -test-annotate-tokens=%s:11:1:11:100 %s -std=c++11 
-Wno-unused-value | FileCheck %s -check-prefix=CHECK4
+// CHECK4: Identifier: "foo" [11:3 - 11:6] DeclRefExpr=foo:6:18
+// CHECK4: Punctuation: "[" [11:6 - 11:7] DeclRefExpr=operator[]:2:7 
RefName=[11:6 - 11:7] RefName=[11:20 - 11:21]
+// CHECK4: Identifier: "index" [11:7 - 11:12] DeclRefExpr=index:6:27
+// CHECK4: Punctuation: "+" [11:13 - 11:14] BinaryOperator=
+// CHECK4: Identifier: "index" [11:15 - 11:20] DeclRefExpr=index:6:27
+// CHECK4: Punctuation: "]" [11:20 - 11:21] DeclRefExpr=operator[]:2:7 
RefName=[11:6 - 11:7] RefName=[11:20 - 11:21]
+// CHECK4: Punctuation: ";" [11:21 - 11:22] CompoundStmt=
+
+// RUN: c-index-test -test-annotate-tokens=%s:13:1:13:100 %s -std=c++11 
-Wno-unused-value | FileCheck %s -check-prefix=CHECK5
+// CHECK5: Identifier: "foo" [13:3 - 13:6] DeclRefExpr=foo:6:18
+// CHECK5: Punctuation: "[" [13:6 - 13:7] DeclRefExpr=operator[]:2:7 
RefName=[13:6 - 13:7] RefName=[13:17 - 13:18]
+// CHECK5: Identifier: "foo" [13:7 - 13:10] DeclRefExpr=foo:6:18
+// CHECK5: Punctuation: "[" [13:10 - 13:11] DeclRefExpr=operator[]:2:7 
RefName=[13:10 - 13:11] RefName=[13:16 - 13:17]
+// CHECK5: Identifier: "index" [13:11 - 13:16] DeclRefExpr=index:6:27
+// CHECK5: Punctuation: "]" [13:16 - 13:17] DeclRefExpr=operator[]:2:7 
RefName=[13:10 - 13:11] RefName=[13:16 - 13:17]
+// CHECK5: Punctuation: "]" [13:17 - 13:18] DeclRefExpr=operator[]:2:7 
RefName=[13:6 - 13:7] RefName=[13:17 - 13:18]
+// CHECK5: Punctuation: ";" [13:18 - 13:19] CompoundStmt=
+
+// RUN: c-index-test -test-annotate-tokens=%s:14:1:14:100 %s -std=c++11 
-Wno-unused-value | FileCheck %s -check-prefix=CHECK6
+// CHECK6: Identifier: "foo" [14:3 - 14:6] DeclRefExpr=foo:6:18
+// CHECK6: Punctuation: "[" [14:6 - 14:7] DeclRefExpr=operator[]:2:7 
RefName=[14:6 - 14:7] RefName=[14:25 - 14:26]
+// CHECK6: Identifier: "foo" [14:7 - 14:10] DeclRefExpr=foo:6:18
+// CHECK6: Punctuation: "(" [14:10 - 14:11] DeclRefE

[PATCH] D40481: [libclang] Fix cursors for arguments of Subscript and Call operators

[Ivan Donchevskii via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340521: [libclang] Fix cursors for arguments of Subscript 
and Call operators (authored by yvvan, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D40481?vs=124341&id=162145#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40481

Files:
  cfe/trunk/test/Index/annotate-operator-call-expr.cpp
  cfe/trunk/tools/libclang/CIndex.cpp

Index: cfe/trunk/test/Index/annotate-operator-call-expr.cpp
===
--- cfe/trunk/test/Index/annotate-operator-call-expr.cpp
+++ cfe/trunk/test/Index/annotate-operator-call-expr.cpp
@@ -0,0 +1,84 @@
+struct Foo {
+  int operator[](int key);
+  int operator()(int key = 2);
+};
+
+void testFoo(Foo foo, int index) {
+  foo();
+  foo(index);
+
+  foo[index];
+  foo[index + index];
+
+  foo[foo[index]];
+  foo[foo() + foo[index]];
+  foo[foo(index) + foo[index]];
+}
+
+// RUN: c-index-test -test-annotate-tokens=%s:7:1:7:100 %s -std=c++11 -Wno-unused-value | FileCheck %s -check-prefix=CHECK1
+// CHECK1: Identifier: "foo" [7:3 - 7:6] DeclRefExpr=foo:6:18
+// CHECK1: Punctuation: "(" [7:6 - 7:7] DeclRefExpr=operator():3:7 RefName=[7:6 - 7:7] RefName=[7:7 - 7:8]
+// CHECK1: Punctuation: ")" [7:7 - 7:8] DeclRefExpr=operator():3:7 RefName=[7:6 - 7:7] RefName=[7:7 - 7:8]
+// CHECK1: Punctuation: ";" [7:8 - 7:9] CompoundStmt=
+
+// RUN: c-index-test -test-annotate-tokens=%s:8:1:8:100 %s -std=c++11 -Wno-unused-value | FileCheck %s -check-prefix=CHECK2
+// CHECK2: Punctuation: "(" [8:6 - 8:7] DeclRefExpr=operator():3:7 RefName=[8:6 - 8:7] RefName=[8:12 - 8:13]
+// CHECK2: Identifier: "index" [8:7 - 8:12] DeclRefExpr=index:6:27
+// CHECK2: Punctuation: ")" [8:12 - 8:13] DeclRefExpr=operator():3:7 RefName=[8:6 - 8:7] RefName=[8:12 - 8:13]
+// CHECK2: Punctuation: ";" [8:13 - 8:14] CompoundStmt=
+
+// RUN: c-index-test -test-annotate-tokens=%s:10:1:10:100 %s -std=c++11 -Wno-unused-value | FileCheck %s -check-prefix=CHECK3
+// CHECK3: Identifier: "foo" [10:3 - 10:6] DeclRefExpr=foo:6:18
+// CHECK3: Punctuation: "[" [10:6 - 10:7] DeclRefExpr=operator[]:2:7 RefName=[10:6 - 10:7] RefName=[10:12 - 10:13]
+// CHECK3: Identifier: "index" [10:7 - 10:12] DeclRefExpr=index:6:27
+// CHECK3: Punctuation: "]" [10:12 - 10:13] DeclRefExpr=operator[]:2:7 RefName=[10:6 - 10:7] RefName=[10:12 - 10:13]
+// CHECK3: Punctuation: ";" [10:13 - 10:14] CompoundStmt=
+
+// RUN: c-index-test -test-annotate-tokens=%s:11:1:11:100 %s -std=c++11 -Wno-unused-value | FileCheck %s -check-prefix=CHECK4
+// CHECK4: Identifier: "foo" [11:3 - 11:6] DeclRefExpr=foo:6:18
+// CHECK4: Punctuation: "[" [11:6 - 11:7] DeclRefExpr=operator[]:2:7 RefName=[11:6 - 11:7] RefName=[11:20 - 11:21]
+// CHECK4: Identifier: "index" [11:7 - 11:12] DeclRefExpr=index:6:27
+// CHECK4: Punctuation: "+" [11:13 - 11:14] BinaryOperator=
+// CHECK4: Identifier: "index" [11:15 - 11:20] DeclRefExpr=index:6:27
+// CHECK4: Punctuation: "]" [11:20 - 11:21] DeclRefExpr=operator[]:2:7 RefName=[11:6 - 11:7] RefName=[11:20 - 11:21]
+// CHECK4: Punctuation: ";" [11:21 - 11:22] CompoundStmt=
+
+// RUN: c-index-test -test-annotate-tokens=%s:13:1:13:100 %s -std=c++11 -Wno-unused-value | FileCheck %s -check-prefix=CHECK5
+// CHECK5: Identifier: "foo" [13:3 - 13:6] DeclRefExpr=foo:6:18
+// CHECK5: Punctuation: "[" [13:6 - 13:7] DeclRefExpr=operator[]:2:7 RefName=[13:6 - 13:7] RefName=[13:17 - 13:18]
+// CHECK5: Identifier: "foo" [13:7 - 13:10] DeclRefExpr=foo:6:18
+// CHECK5: Punctuation: "[" [13:10 - 13:11] DeclRefExpr=operator[]:2:7 RefName=[13:10 - 13:11] RefName=[13:16 - 13:17]
+// CHECK5: Identifier: "index" [13:11 - 13:16] DeclRefExpr=index:6:27
+// CHECK5: Punctuation: "]" [13:16 - 13:17] DeclRefExpr=operator[]:2:7 RefName=[13:10 - 13:11] RefName=[13:16 - 13:17]
+// CHECK5: Punctuation: "]" [13:17 - 13:18] DeclRefExpr=operator[]:2:7 RefName=[13:6 - 13:7] RefName=[13:17 - 13:18]
+// CHECK5: Punctuation: ";" [13:18 - 13:19] CompoundStmt=
+
+// RUN: c-index-test -test-annotate-tokens=%s:14:1:14:100 %s -std=c++11 -Wno-unused-value | FileCheck %s -check-prefix=CHECK6
+// CHECK6: Identifier: "foo" [14:3 - 14:6] DeclRefExpr=foo:6:18
+// CHECK6: Punctuation: "[" [14:6 - 14:7] DeclRefExpr=operator[]:2:7 RefName=[14:6 - 14:7] RefName=[14:25 - 14:26]
+// CHECK6: Identifier: "foo" [14:7 - 14:10] DeclRefExpr=foo:6:18
+// CHECK6: Punctuation: "(" [14:10 - 14:11] DeclRefExpr=operator():3:7 RefName=[14:10 - 14:11] RefName=[14:11 - 14:12]
+// CHECK6: Punctuation: ")" [14:11 - 14:12] DeclRefExpr=operator():3:7 RefName=[14:10 - 14:11] RefName=[14:11 - 14:12]
+// CHECK6: Punctuation: "+" [14:13 - 14:14] BinaryOperator=
+// CHECK6: Identifier: "foo" [14:15 - 14:18] DeclRefExpr=foo:6:18
+// CHECK6: Punctuation: "[" [14:18 - 14:19] DeclRefExpr=operator[]:2:7 RefName=[14:18 - 14:19] RefName=[14:24 - 14:25]
+// CHECK6: Identifier: "index" [14:19 - 14:24] DeclRefExpr=operator[]:2:7

Re: r314872 - We allow implicit function declarations as an extension in all C dialects. Remove OpenCL special case.

[Anastasia Stulova via cfe-commits]

Hi Richard,


There was a change in the spec to disallow unprototyped functions, which was 
made this year. Unfortunately it seems it didn't make into the Khronos registry 
yet to appear publicly. I will chase it up with Khronos.


I would like to highlight that OpenCL C was based on C99 originally and 
therefore in contrast to C doesn't have backwards compatibility with the old C 
standards. I don't think it's common to either write or port old C code to 
OpenCL simply because it's not practical to run regular C code on OpenCL 
targets efficiently but also in many cases it won't even compile due to many 
restrictions.


Anastasia



From: Richard Smith 
Sent: 22 August 2018 21:16
To: Anastasia Stulova
Cc: nd; cfe-commits
Subject: Re: r314872 - We allow implicit function declarations as an extension 
in all C dialects. Remove OpenCL special case.

On Wed, 22 Aug 2018 at 06:55, Anastasia Stulova via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Hi Richard,

> This is incorrect. Implicit function declarations declare unprototyped 
> functions, which are *not* variadic, and are in fact supported by Clang's 
> OpenCL language mode.

We have removed the support for the unprototyped functions from the OpenCL as 
well. See commit: https://reviews.llvm.org/D33681. This is the reason why in 
the OpenCL mode we now generated empty parameter list instead of unprototyped 
function like for C in the examples I provided before (that is not governed now 
by any standard or any compiler extension).

That's interesting. Do I understand from that review thread that we're 
diverging from the OpenCL specification in treating () as (void) rather than as 
an unprototyped function declaration? If so, that seems like a surprising and 
concerning decision, unless we're confident that the OpenCL language really did 
mean to change this aspect of the C semantics and omitted the wording change by 
oversight. (And I've checked, and can find nothing in the OpenCL specification 
that justifies this: it looks like a Clang bug that we reject

  int f();
  void g() { f(1, 2, 3); }
  int f(int a, int b, int c) { return 0; }

... for example, unless Khronos really did mean to use the C++ rule.)

If it is indeed the intent of the OpenCL specification to treat () as (void) 
like in C++ and not have unprototyped functions, then I think it does make 
sense to also disable our implicitly declared functions extension in OpenCL. 
But, conversely, if the OpenCL specification instead intends to follow C here 
and allow unprototyped functions, then it is a bug in our OpenCL support that 
we don't follow that intent, and when that bug is fixed it makes sense to 
continue to accept implicit function declarations as an extension.

> I would have sympathy for your position if we did not produce an extension 
> warning on this construct by default. But we do, and it says the construct is 
> invalid in OpenCL; moreover, in our strict conformance mode 
> (-pedantic-errors), we reject the code.

I understand the motivation for C to maintain the compatibility with the 
previous standards and other compilers (like gcc) to be able to support the 
legacy code. However, for OpenCL we don't have this requirement wrt older C 
standards. And therefore it is desirable to take advantage of this and remove 
problematic features that are generally confusing for developers or that can't 
be efficiently supported by the targets (especially if there is a little cost 
to that).

For this "can't be efficiently supported by the target" claim, I think you're 
conflating the target and the language mode. If some target can't reasonably 
support variadic functions, we should disable variadic functions for that 
target. That has *nothing* to do with the language mode; targets and language 
modes can be combined arbitrarily. [If I want to use Clang to compile OpenCL 
code for my PDP-11, then I should be allowed to do that (assuming I have a 
suitable LLVM backend), and that target presumably would support variadic 
functions just fine.] Likewise, if the target doesn't support variadic 
functions, we should not be generating variadic function types when producing 
IR (particularly in calls to non-variadic functions like in your example 
elsewhere in this thread). This is true regardless of whether the source 
language is OpenCL or C89 or C++ or anything else.

It is a goal of Clang to allow its various features to be used together, 
including combining them in ways that we didn't think of. The particular case 
of implicit function declarations is not especially important in and of itself, 
but the underlying principle is: the OpenCL language mode of Clang should not 
disable other Clang extensions unless there's some fundamental reason why they 
are incompatible.

Consider this: we allow implicit function declarations in languages based on C 
in order to allow C89 code (or code that started as C89 code) to be built 
unchanged in those langua

r340522 - Update avr attributes test for output change in r340519

[Alexander Richardson via cfe-commits]

Author: arichardson
Date: Thu Aug 23 03:21:36 2018
New Revision: 340522

URL: http://llvm.org/viewvc/llvm-project?rev=340522&view=rev
Log:
Update avr attributes test for output change in r340519

After this commit there is an addrspace(1) before the attribute #. Since
these tests are only checking the value of the attribute add a {{.*}} to
make the test resilient to future output changes.

Modified:
cfe/trunk/test/CodeGen/avr/attributes/interrupt.c
cfe/trunk/test/CodeGen/avr/attributes/signal.c

Modified: cfe/trunk/test/CodeGen/avr/attributes/interrupt.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avr/attributes/interrupt.c?rev=340522&r1=340521&r2=340522&view=diff
==
--- cfe/trunk/test/CodeGen/avr/attributes/interrupt.c (original)
+++ cfe/trunk/test/CodeGen/avr/attributes/interrupt.c Thu Aug 23 03:21:36 2018
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -triple avr-unknown-unknown -emit-llvm %s -o - | FileCheck 
%s
 
-// CHECK: define void @foo() #0
+// CHECK: define void @foo(){{.*}}#0
 __attribute__((interrupt)) void foo(void) { }
 
 // CHECK: attributes #0 = {{{.*interrupt.*}}}

Modified: cfe/trunk/test/CodeGen/avr/attributes/signal.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avr/attributes/signal.c?rev=340522&r1=340521&r2=340522&view=diff
==
--- cfe/trunk/test/CodeGen/avr/attributes/signal.c (original)
+++ cfe/trunk/test/CodeGen/avr/attributes/signal.c Thu Aug 23 03:21:36 2018
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -triple avr-unknown-unknown -emit-llvm %s -o - | FileCheck 
%s
 
-// CHECK: define void @foo() #0
+// CHECK: define void @foo(){{.*}}#0
 __attribute__((signal)) void foo(void) { }
 
 // CHECK: attributes #0 = {{{.*signal.*}}}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r340523 - [clangd] Increase the timeouts in TUScheduler tests to 10 seconds.

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu Aug 23 03:25:07 2018
New Revision: 340523

URL: http://llvm.org/viewvc/llvm-project?rev=340523&view=rev
Log:
[clangd] Increase the timeouts in TUScheduler tests to 10 seconds.

Some of them timeout on our buildbots in certain configurations.
The timeouts are there to avoid hanging indefinitely on deadlocks, so
the exact number we put there does not matter.

Modified:
clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp?rev=340523&r1=340522&r2=340523&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp Thu Aug 23 
03:25:07 2018
@@ -278,7 +278,7 @@ TEST_F(TUSchedulerTests, EvictedAST) {
   // one that the cache will evict.
   S.update(Foo, getInputs(Foo, SourceContents), WantDiagnostics::Yes,
[&BuiltASTCounter](std::vector Diags) { ++BuiltASTCounter; });
-  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1)));
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   ASSERT_EQ(BuiltASTCounter.load(), 1);
 
   // Build two more files. Since we can retain only 2 ASTs, these should be the
@@ -287,7 +287,7 @@ TEST_F(TUSchedulerTests, EvictedAST) {
[&BuiltASTCounter](std::vector Diags) { ++BuiltASTCounter; });
   S.update(Baz, getInputs(Baz, SourceContents), WantDiagnostics::Yes,
[&BuiltASTCounter](std::vector Diags) { ++BuiltASTCounter; });
-  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1)));
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   ASSERT_EQ(BuiltASTCounter.load(), 3);
 
   // Check only the last two ASTs are retained.
@@ -296,7 +296,7 @@ TEST_F(TUSchedulerTests, EvictedAST) {
   // Access the old file again.
   S.update(Foo, getInputs(Foo, OtherSourceContents), WantDiagnostics::Yes,
[&BuiltASTCounter](std::vector Diags) { ++BuiltASTCounter; });
-  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1)));
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   ASSERT_EQ(BuiltASTCounter.load(), 4);
 
   // Check the AST for foo.cpp is retained now and one of the others got
@@ -333,13 +333,13 @@ TEST_F(TUSchedulerTests, EmptyPreamble)
 0u);
 });
   // Wait for the preamble is being built.
-  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1)));
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
 
   // Update the file which results in an empty preamble.
   S.update(Foo, getInputs(Foo, WithEmptyPreamble), WantDiagnostics::Auto,
[](std::vector) {});
   // Wait for the preamble is being built.
-  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1)));
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   S.runWithPreamble("getEmptyPreamble", Foo,
 [&](llvm::Expected Preamble) {
   // We expect to get an empty preamble.
@@ -409,7 +409,7 @@ TEST_F(TUSchedulerTests, NoopOnEmptyChan
 Updated = false;
 S.update(Source, std::move(Inputs), WantDiagnostics::Yes,
  [&Updated](std::vector) { Updated = true; });
-bool UpdateFinished = S.blockUntilIdle(timeoutSeconds(1));
+bool UpdateFinished = S.blockUntilIdle(timeoutSeconds(10));
 if (!UpdateFinished)
   ADD_FAILURE() << "Updated has not finished in one second. Threading 
bug?";
 return Updated;
@@ -454,14 +454,14 @@ TEST_F(TUSchedulerTests, NoChangeDiags)
 // Make sure the AST was actually built.
 cantFail(std::move(IA));
   });
-  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1)));
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
 
   // Even though the inputs didn't change and AST can be reused, we need to
   // report the diagnostics, as they were not reported previously.
   std::atomic SeenDiags(false);
   S.update(FooCpp, getInputs(FooCpp, Contents), WantDiagnostics::Auto,
[&](std::vector) { SeenDiags = true; });
-  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1)));
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   ASSERT_TRUE(SeenDiags);
 
   // Subsequent request does not get any diagnostics callback because the same
@@ -469,7 +469,7 @@ TEST_F(TUSchedulerTests, NoChangeDiags)
   S.update(
   FooCpp, getInputs(FooCpp, Contents), WantDiagnostics::Auto,
   [&](std::vector) { ADD_FAILURE() << "Should not be called."; });
-  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1)));
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
 }
 
 } // namespace


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51029: [clangd] Implement LIMIT iterator

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

first few comments, sorry I'll need to come back to this.




Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:90
   virtual DocID peek() const = 0;
   /// Retrieves boosting score. Query tree root should pass Root->peek() to 
this
   /// function, the parameter is needed to propagate through the tree. Given ID

Update documentation.

```Informs the iterator that the current document was consumed, and returns its 
boost.```

The rest of the doc seems a bit to far in the details, it's hard to follow.
Maybe just
```If this iterator has any child iterators that contain the document, consume()
should be called on those and their boosts incorporated.
consume() must *not* be called on children that don't contain the current 
doc.```



Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:96
   /// shouldn't apply any boosting to the consumed item.
   virtual float consume(DocID ID) = 0;
 

There's two possible formulations here:
 - the DocID parameter is used by the Iterator to determine whether to actually 
decrement its limit (do we actually have that doc?)
 - the DocID parameter is redundant, as the parent should only call consume() 
on children that have the doc.
We need to be explicit about which. It seems you want the second (based on 
offline discussion). In this case we should either assert that the DocID 
matches peek(), or just drop the DocID parameter and use peek() instead. (If 
this later means we need to optimize peek, so be it).



Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:168
 
+/// Returns LIMIT iterator, which is exhausted as soon requested number of 
items
+/// is consumed from the root of query tree.

This comment is accurate, but maybe a bit too concise as what's going on here 
is a little subtle.
Maybe expand a little like

```
Returns LIMIT iterator, which yields up to N elements of its child iterator.
Elements only count towards the limit if they are part of the final result set.
Therefore the following iterator (AND (2) (LIMIT (1 2) 1)) yields (2), not ().
```


https://reviews.llvm.org/D51029



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51154: [clangd] Log memory usage of DexIndex and MemIndex

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 162146.
kbobyrev marked 6 inline comments as done.
kbobyrev added a comment.

Address a bunch of comments.


https://reviews.llvm.org/D51154

Files:
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/FileIndex.h
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/MemIndex.cpp
  clang-tools-extra/clangd/index/MemIndex.h
  clang-tools-extra/clangd/index/Merge.cpp
  clang-tools-extra/clangd/index/dex/DexIndex.cpp
  clang-tools-extra/clangd/index/dex/DexIndex.h
  clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp

Index: clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
@@ -923,6 +923,10 @@
llvm::function_ref
Callback) const override {}
 
+  // This is incorrect, but IndexRequestCollector is not an actual index and it
+  // isn't used in production code.
+  size_t estimateMemoryUsage() const override { return 0; }
+
   const std::vector allRequests() const { return Requests; }
 
 private:
Index: clang-tools-extra/clangd/index/dex/DexIndex.h
===
--- clang-tools-extra/clangd/index/dex/DexIndex.h
+++ clang-tools-extra/clangd/index/dex/DexIndex.h
@@ -57,7 +57,10 @@
llvm::function_ref
Callback) const override;
 
+  size_t estimateMemoryUsage() const override;
+
 private:
+
   mutable std::mutex Mutex;
 
   std::shared_ptr> Symbols /*GUARDED_BY(Mutex)*/;
Index: clang-tools-extra/clangd/index/dex/DexIndex.cpp
===
--- clang-tools-extra/clangd/index/dex/DexIndex.cpp
+++ clang-tools-extra/clangd/index/dex/DexIndex.cpp
@@ -67,6 +67,9 @@
 InvertedIndex = std::move(TempInvertedIndex);
 SymbolQuality = std::move(TempSymbolQuality);
   }
+
+  vlog("Built DexIndex with estimated memory usage {0} bytes.",
+   estimateMemoryUsage());
 }
 
 std::unique_ptr DexIndex::build(SymbolSlab Slab) {
@@ -171,6 +174,22 @@
   log("findOccurrences is not implemented.");
 }
 
+size_t DexIndex::estimateMemoryUsage() const {
+  size_t Bytes = 0;
+  {
+std::lock_guard Lock(Mutex);
+
+Bytes += LookupTable.size() * sizeof(std::pair);
+Bytes += SymbolQuality.size() * sizeof(std::pair);
+Bytes += InvertedIndex.size() * sizeof(Token);
+
+for (const auto &P : InvertedIndex) {
+  Bytes += P.second.size() * sizeof(DocID);
+}
+  }
+  return Bytes;
+}
+
 } // namespace dex
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/index/Merge.cpp
===
--- clang-tools-extra/clangd/index/Merge.cpp
+++ clang-tools-extra/clangd/index/Merge.cpp
@@ -84,6 +84,10 @@
 log("findOccurrences is not implemented.");
   }
 
+  size_t estimateMemoryUsage() const override {
+return Dynamic->estimateMemoryUsage() + Static->estimateMemoryUsage();
+  }
+
 private:
   const SymbolIndex *Dynamic, *Static;
 };
Index: clang-tools-extra/clangd/index/MemIndex.h
===
--- clang-tools-extra/clangd/index/MemIndex.h
+++ clang-tools-extra/clangd/index/MemIndex.h
@@ -39,7 +39,10 @@
llvm::function_ref
Callback) const override;
 
+  size_t estimateMemoryUsage() const override;
+
 private:
+
   std::shared_ptr> Symbols;
   // Index is a set of symbols that are deduplicated by symbol IDs.
   // FIXME: build smarter index structure.
Index: clang-tools-extra/clangd/index/MemIndex.cpp
===
--- clang-tools-extra/clangd/index/MemIndex.cpp
+++ clang-tools-extra/clangd/index/MemIndex.cpp
@@ -26,6 +26,9 @@
 Index = std::move(TempIndex);
 Symbols = std::move(Syms); // Relase old symbols.
   }
+
+  vlog("Built MemIndex with estimated memory usage {0} bytes.",
+   estimateMemoryUsage());
 }
 
 std::unique_ptr MemIndex::build(SymbolSlab Slab) {
@@ -98,5 +101,18 @@
   &Snap->Pointers);
 }
 
+size_t MemIndex::estimateMemoryUsage() const {
+  size_t Bytes = 0;
+
+  {
+std::lock_guard Lock(Mutex);
+
+Bytes += Index.getMemorySize();
+return Bytes;
+  }
+
+}
+
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/index/Index.h
===
--- clang-tools-extra/clangd/index/Index.h
+++ clang-tools-extra/clangd/index/Index.h
@@ -385,6 +385,12 @@
   virtual void findOccurrences(
   const OccurrencesRequest &Req,
   llvm::function_ref Callback) const = 0;
+
+  /// Returns estimated size of index (in bytes).
+  // FIXME(kbobyrev): Curre

[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Overall looks good to me.




Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:61
+/// rely on MR use-case to work properly.
+llvm::cl::init(false));
+

AFAIK, this flag would basically depend on the executor being used. If executor 
can provide information like whether all tasks share memory, we can make the 
decision totally based on the selected executor (i.e. one fewer option).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51155



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50962: [clangd] Speculative code completion index request before Sema is run.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Really excited about this one, this should give us decent latency improvements 
when using the static index.
My main suggestion would be to put almost all of the speculating code into 
`CodeComplete.cpp`.

We could merely return the request that should be used for speculation to 
`ClangdServer`, that would in turn stash it in the map.
This should keep the code more localized in code completion and keep changes to 
other parts trivial. WDYT?




Comment at: clangd/ClangdServer.cpp:159
+  }
+  if (SpecFuzzyReq) {
+if (auto Filter = speculateCompletionFilter(Content, Pos)) {

NIT: maybe invert condition to reduce nesting?



Comment at: clangd/ClangdServer.cpp:160
+  if (SpecFuzzyReq) {
+if (auto Filter = speculateCompletionFilter(Content, Pos)) {
+  SpecFuzzyReq->Query = *Filter;

Maybe move this logic into code completion, keeping only the storage for the 
old requests?
The code is domain-specific to code completion and we should aim to keep as 
little of domain logic in `ClangdServer`.



Comment at: clangd/CodeComplete.cpp:1053
   IncludeStructure Includes; // Complete once the compiler runs.
+  std::function UpdateCachedFuzzyFindReq;
+  AsyncFuzzyFind *SpeculativeFuzzyFind; // Can be nullptr.

Maybe return the new callback from completion instead of passing a callback to 
update here?



Comment at: clangd/CodeComplete.cpp:1388
+  if (Len > 0)
+St++; // Shift to the first valid character.
+  return Content.substr(St, Len);

This looks incorrect in case of the identifiers starting at offset 0. A corner 
case, but still.



Comment at: clangd/index/Index.cpp:147
+  Result =
+  std::async(std::launch::async, QueryIndex, Context::current().clone());
+}

I think we should just have a generic 'std::async'-like helper that does the 
same things, properly piping our context to the new threads.
Async requests to the index and other code potentially doing network requests 
are good candidates to use use it.

Maybe include it in this change? It shouldn't be more than a few lines, but 
would already make the code cleaner.



Comment at: clangd/index/Index.h:404
+/// any, finishes.
+class AsyncFuzzyFind {
+public:

Could this be an implementation detail of code completion if we move the 
spawning of the task into `codeComplete` function?



Comment at: clangd/index/Index.h:409
+
+  const FuzzyFindRequest &getRequest() const { return Req; }
+

Can this be a simple struct with two fields `std::future` and the 
original `FuzzyFindRequest`?
And having a function that fires the async request instead of doing that in 
constructor should be more straight-forward



Comment at: clangd/tool/ClangdMain.cpp:179
+should be effective for a number of code completions.)"),
+llvm::cl::init(true));
+

Maybe remove this option?
Doing a speculative request seems like the right thing to do in all cases. It 
never hurts completion latency. Any reasons to avoid it?



Comment at: unittests/clangd/CodeCompleteTests.cpp:926
 
-  const std::vector allRequests() const { return Requests; }
+  const std::vector consumeRequests() const {
+auto Reqs = std::move(Requests);

Why do we want to change this? To avoid tests getting requests from the 
previous tests?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50962



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

- New mode

./bin/global-symbol-builder -merge-on-the-fly -executor=all-TUs  > /dev/null

  40079.41s user 1874.40s system 5340% cpu 13:05.56 total

- Old mode

Still running, expect it to be more than 40minutes... Will update when I have 
the numbers.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51155



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:61
+/// rely on MR use-case to work properly.
+llvm::cl::init(false));
+

ioeric wrote:
> AFAIK, this flag would basically depend on the executor being used. If 
> executor can provide information like whether all tasks share memory, we can 
> make the decision totally based on the selected executor (i.e. one fewer 
> option).
That SG. So we could add a virtual method to executor that would provide us 
with this information and then we can get rid of that option, does that LG?
However, do we have non-single-binary executor implementations upstream? We 
need to leave a way to run the code that actually uses YAML serialization to 
make sure we can experiment when things break.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51155



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

In https://reviews.llvm.org/D50144#1209758, @smeenai wrote:

> I'm confused by the funclet-based unwinding support. Do you intend GNUStep to 
> be used with windows-msvc triples? If so, how is the runtime throwing 
> exceptions? We took the approach of having the Obj-C runtime wrap Obj-C 
> exceptions in C++ exceptions and then have vcruntime handle the rest (see 
> https://reviews.llvm.org/D47233 and https://reviews.llvm.org/D47988), but I 
> don't see any of the associated RTTI emission changes in this patch.


I'm assuming 'we' here is Apple?  Microsoft has a friendly fork of the GNUstep 
runtime in the WinObjC project and I am slowly working on pulling in the 
WinObjC downstream changes, incorporating them with the v2 ABI, and making it 
easy for WinObjC to use an unmodified version of the runtime.  The v2 ABI still 
has a few things that don't work with the Windows linkage model, which I'm 
working out how best to address.  The code for throwing an exception is here:

https://github.com/gnustep/libobjc2/blob/master/eh_win32_msvc.cc

The runtime constructs an SEH exception on the stack.  The low-level NT 
exception type knows all of the possible types that might be allowed in a 
catch, so the runtime dynamically constructs a list of all of these, one per 
superclass in the hierarchy, on the stack.  The funclet-based unwinding 
mechanism makes it safe to do this, because the throwing stack frame remains 
valid until after the exception is caught.  The Windows C++ personality 
function then checks the catches the exceptions.  Clang/CX (Clang with the 
Visual Studio compiler back end) has supported this for a while, but it never 
made it upstream to Clang/LLVM and I'm trying to rectify this.

On 64-bit Windows, we must provide 32-bit offsets to some arbitrary base.  For 
C++, this is the base load address of the DLL.  That's fine for C++, where the 
throw types are static (and the entire class hierarchy is available at compile 
time), but our stack may be more than 4GB away from any library.  Instead, we 
use an offset from some arbitrary on-stack location (and if we end up being 
more than 4GB away from the base of our stack frame, then something's gone very 
badly wrong!).




Comment at: lib/AST/MicrosoftMangle.cpp:1886
   case BuiltinType::ObjCId:
-mangleArtificalTagType(TTK_Struct, "objc_object");
+mangleArtificalTagType(TTK_Struct, ".objc_object");
 break;

smeenai wrote:
> theraven wrote:
> > smeenai wrote:
> > > theraven wrote:
> > > > compnerd wrote:
> > > > > DHowett-MSFT wrote:
> > > > > > I'm interested in @smeenai's take on this, as well.
> > > > > Hmm, doesn't this break ObjC/ObjC++ interoperability?  I don't think 
> > > > > that we should be changing the decoration here for the MS ABI case.
> > > > No, this fixes ObjC++ interop.  Throwing an exception from a `@throw` 
> > > > and catching it with `catch` now works and we avoid the problem that a 
> > > > C++ compilation unit declares a `struct` that happens to have the same 
> > > > name as an Objective-C class (which is very common for wrapper code) 
> > > > may catch things of the wrong type.
> > > I've seen a lot of code which does something like this in a header
> > > 
> > > ```lang=cpp
> > > #ifdef __OBJC__
> > > @class I;
> > > #else
> > > struct I;
> > > #endif
> > > 
> > > void f(I *);
> > > ```
> > > 
> > > You want `f` to be mangled consistently across C++ and Objective-C++, 
> > > otherwise you'll get link errors if `f` is e.g. defined in a C++ TU and 
> > > referenced in an Objective-C++ TU. This was the case before your change 
> > > and isn't the case after your change, which is what @compnerd was 
> > > pointing out. They are mangled consistently in the Itanium ABI, and we 
> > > want them to be mangled consistently in the MS ABI as well.
> > > 
> > > Not wanting the catch mix-up is completely reasonable, of course, but it 
> > > can be done in a more targeted way. I'll put up a patch that undoes this 
> > > part of the change while still preventing incorrect catching.
> > With a non-fragile ABI, there is not guarantee that `struct I` and `@class 
> > I` have the same layout.  This is why `@defs` has gone away.  Any code that 
> > does this and treats `struct I` as anything other than an opaque type is 
> > broken.  No other platform supports throwing an Objective-C object and 
> > catching it as a struct, and this is an intentional design decision.  I 
> > would be strongly opposed to a change that intentionally supported this, 
> > because it is breaking correct code in order to make incorrect code do 
> > something that may or may not work.
> To be clear, I'm aware of the layout differences. My plan was to revert the 
> mangling here to what it was before (which would also be consistent with 
> Itanium), and then adjust the RTTI emission for Obj-C exception types (which 
> I'm handling in https://reviews.llvm.org/D47233) to differentiate between C++ 
>

r340524 - Removed unused variable [NFC]

[Mikael Holmen via cfe-commits]

Author: uabelho
Date: Thu Aug 23 03:59:49 2018
New Revision: 340524

URL: http://llvm.org/viewvc/llvm-project?rev=340524&view=rev
Log:
Removed unused variable [NFC]

The compiler warned:
../tools/clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp:329:19: error: 
unused variable 'MD' [-Werror,-Wunused-variable]
  if (const auto *MD = dyn_cast(FD)) {
  ^
1 error generated.

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp?rev=340524&r1=340523&r2=340524&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp Thu Aug 23 
03:59:49 2018
@@ -326,7 +326,7 @@ RetainSummaryManager::generateSummary(co
 }
   }
 
-  if (const auto *MD = dyn_cast(FD)) {
+  if (dyn_cast(FD)) {
 
 // Stop tracking arguments passed to C++ methods, as those might be
 // wrapping smart pointers.


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

The numbers are finally there
New mode

  ./bin/global-symbol-builder -merge-on-the-fly -executor=all-TUs > /dev/null
  40079.41s user 1874.40s system 5340% cpu 13:05.56 total

Old mode

  ./bin/global-symbol-builder -merge-on-the-fly=false -executor=all-TUs  > 
/dev/null
  39789.70s user 2014.45s system 1385% cpu 50:17.37 total

So it's almost 5 times faster now: 50 min to 13 min.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51155



___
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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: lib/CodeGen/CGExprScalar.cpp:1050-1051
+// NOTE: if it is unsigned, then the comparison is naturally always 
'false'.
+llvm::ICmpInst::Predicate Pred =
+VSigned ? llvm::ICmpInst::ICMP_SLT : llvm::ICmpInst::ICMP_ULT;
+// Get the zero of the same type with which we will be comparing.

lebedev.ri wrote:
> rsmith wrote:
> > lebedev.ri wrote:
> > > rsmith wrote:
> > > > If `!VSigned`, the result is a constant `false`; you don't need to emit 
> > > > an `icmp` to work that out.
> > > Ok, if you insist.
> > > I didn't do that in the first place because we will now have an `icmp`
> > > where one operand being a constant, so we can simplify it further.
> > > And i don't want to complicate this logic if middle-end already handles 
> > > it :)
> > This becomes a lot simpler with the approach I described in the other 
> > comment thread, because you don't need a second `icmp eq` at all.
> Humm. So i have initially did this. It is probably broken for non-scalars, 
> but we don't care probably.
> 
> But then i thought more.
> 
> If we do not emit truncation check, we get `icmp eq (icmp ...), false`, which 
> is tautological.
> We can't just drop the outer `icmp eq` since we'd get [[ 
> https://rise4fun.com/Alive/4slv | the opposite value ]].
> We could emit `xor %icmp, -1` to invert it.  Or simply invert the predicate, 
> and avoid the second `icmp`.
> By itself, either of these options doesn't sound that bad.
> 
> But if both are signed, we can't do that. So we have to have two different 
> code paths...
> 
> If we do emit the `icmp ult %x, 0`, [it naturally works with vectors], we 
> avoid complicating the front-end,
> and the middle-end playfully simplifies this IR with no sweat.
> 
> So why do we want to complicate the front-end //in this case//, and not let 
> the middle-end do it's job?
> I'm unconvinced, and i have kept this as is. :/
> If !VSigned, the result is a constant false; you don't need to emit an icmp 
> to work that out.

Even at `-O0`, dagcombine constant-folds (unsurprizingly) this case, 
https://godbolt.org/z/D5ueOq


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] D50043: [RISCV] RISC-V using -fuse-init-array by default

[Alex Bradbury via Phabricator via cfe-commits]

asb accepted this revision.
asb added a comment.
This revision is now accepted and ready to land.

Thanks, looks good to me.


https://reviews.llvm.org/D50043



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50564: Add support for SEH unwinding on Windows.

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: src/Unwind-seh.cpp:163
+#ifdef __x86_64__
+unw_get_reg(&cursor, UNW_X86_64_RAX, &retval);
+unw_get_reg(&cursor, UNW_X86_64_RDX, &exc->private_[3]);

Without understanding the code flow completely - is there a risk that this 
tries to use an uninitialized cursor, in case we hit `ctx = (struct 
_Unwind_Context *)ms_exc->ExceptionInformation[1];` above and never initialized 
the local cursor?



Comment at: src/Unwind-seh.cpp:174
+CONTEXT new_ctx;
+RtlUnwindEx(frame, (PVOID)target, ms_exc, (PVOID)retval, &new_ctx, 
disp->HistoryTable);
+_LIBUNWIND_ABORT("RtlUnwindEx() failed");

Who will get this new uninitialized CONTEXT here, and will they try to use it 
for something?



Comment at: src/UnwindCursor.hpp:31
+struct UNWIND_INFO {
+  uint8_t Version : 3;
+  uint8_t Flags : 5;

Hmm, I think this one is supposed to be arch independent. Although it's easy to 
remove the ifdef once we want to use it for anything else than x86_64...



Comment at: src/UnwindCursor.hpp:54
 #include "EHHeaderParser.hpp"
-#include "libunwind.h"
+#include "libunwind_ext.h"
 #include "Registers.hpp"

This looks like another leftover; there's no `libunwind_ext.h` any longer here.



Comment at: src/UnwindCursor.hpp:482
+  int stepWithSEHData() {
+#if !defined(_LIBUNWIND_TARGET_I386)
+_dispContext.LanguageHandler = RtlVirtualUnwind(UNW_FLAG_UHANDLER,

Maybe skip the i386 ifdefs here, to keep it concise, as we don't expect to 
build this with `__SEH__` defined for i386.



Comment at: src/UnwindCursor.hpp:625
+  memset(&_histTable, 0, sizeof(_histTable));
+  // FIXME
+  // fill in _registers from thread arg

If this constructor is unused and unimplemented, I'd omit it altogether.



Comment at: src/UnwindCursor.hpp:1157
 
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)
+  pint_t getLastPC() const { /* FIXME: Implement */ return 0; }

What does this whole block do here? Isn't this within an !SEH ifdef block? The 
same methods are declared for the SEH version of this struct further above.



Comment at: src/UnwindCursor.hpp:1801
+  if (pc != getLastPC()) {
+UNWIND_INFO *xdata = reinterpret_cast(base + 
unwindEntry->UnwindData);
+if (xdata->Flags & (UNW_FLAG_EHANDLER|UNW_FLAG_UHANDLER)) {

I can't say I understand all of this yet, but I'm slowly getting there, and I'm 
trying to compare this to what libgcc does.

libgcc doesn't use any definition of UNWIND_INFO and doesn't need to do the 
equivalent of `getInfoFromSEH`, used by `step()`, anywhere. `unw_step()` is 
used in `_Unwind_ForcedUnwind`, which in libgcc is implemented using 
`RaiseException (STATUS_GCC_FORCED, ...`.

I guess if you happen to have all of the `unw_step` API available, it's easier 
to just do it like this, in custom code without relying on the NTDLL functions 
for it, while the libgcc version relies more on the NTDLL API.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50738: Remove vestiges of configure buildsystem

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added inline comments.



Comment at: CMakeLists.txt:288
 
-if( CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR AND NOT MSVC_IDE )
-  message(FATAL_ERROR "In-source builds are not allowed. CMake would overwrite 
"
-"the makefiles distributed with LLVM. Please create a directory and run cmake "
+if(CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR)
+  message(FATAL_ERROR "In-source builds are not allowed. "

The `NOT MSVC_IDE` condition is still present in llvm's `CMakeLists.txt`, at 
least in r340526 (line 230), so not sure if we should remove it here?


Repository:
  rC Clang

https://reviews.llvm.org/D50738



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51154: [clangd] Log memory usage of DexIndex and MemIndex

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:180
+  Bytes += SymbolQuality.size() * sizeof(std::pair);
+  Bytes += InvertedIndex.size() * sizeof(Token);
+  {

sammccall wrote:
> I think you're not counting the size of the actual symbols.
> This is difficult to do precisely, but avoiding it seems misleading (we have 
> "shared ownership" but it's basically exclusive). What's the plan here?
As discussed offline: I should put a FIXME and leave it for later.


https://reviews.llvm.org/D51154



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51102: [clangd] Move function argument snippet disable mechanism from LSP rendering to internal clangd reprensentation.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51102



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51157: [x86/SLH] Add a real Clang flag and LLVM IR attribute for Speculative Load Hardening.

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc created this revision.
chandlerc added reviewers: rsmith, rnk, echristo, craig.topper, kristof.beyls.
Herald added subscribers: jfb, dexonsmith, steven_wu, hiraditya, eraman, 
mcrosier, mehdi_amini, sanjoy.

Wires up the existing pass to work with a proper IR attribute rather
than just a hidden/internal flag. The internal flag continues to work
for now, but I'll likely remove it soon.

Most of the churn here is adding the IR attribute. I talked about this
Kristof Beyls and he seemed at least initially OK with this direction.
The idea of using a full attribute here is that we *do* expect at least
some forms of this for other architectures. There isn't anything
*inherently* x86-specific about this technique, just that we only have
an implementation for x86 at the moment.

While we could potentially expose this as a Clang-level attribute as
well, that seems like a good question to defer for the moment as it
isn't 100% clear whether that or some other programmer interface (or
both?) would be best. We'll defer the programmer interface side of this
for now, but at least get to the point where the feature can be enabled
without relying on implementation details.

This also allows us to do something that was really hard before: we can
enable *just* the indirect call retpolines when using SLH. For x86, we
don't have any other way to mitigate indirect calls. Other architectures
may take a different approach of course, and none of this is surfaced to
user-level flags.

I can split this patch up into an LLVM-side and a Clang-side, but it
seemed honestly harder to review there, as this way it is clear how
these pieces are intended to be used. That said, if reviewers would like
them split, I'm happy to oblige.


Repository:
  rL LLVM

https://reviews.llvm.org/D51157

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CodeGenOptions.def
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Driver/ToolChains/Arch/X86.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/x86-target-features.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/O3-pipeline.ll
  llvm/test/CodeGen/X86/speculative-load-hardening-gather.ll
  llvm/test/CodeGen/X86/speculative-load-hardening-indirect.ll
  llvm/test/CodeGen/X86/speculative-load-hardening.ll
  llvm/test/Transforms/Inline/attributes.ll

Index: llvm/test/Transforms/Inline/attributes.ll
===
--- llvm/test/Transforms/Inline/attributes.ll
+++ llvm/test/Transforms/Inline/attributes.ll
@@ -26,6 +26,10 @@
   ret i32 %i
 }
 
+define i32 @slh_callee(i32 %i) speculative_load_hardening {
+  ret i32 %i
+}
+
 define i32 @alwaysinline_callee(i32 %i) alwaysinline {
   ret i32 %i
 }
@@ -161,6 +165,27 @@
 ; CHECK-NEXT: ret i32
 }
 
+; Can inline a normal function into an SLH'ed function.
+define i32 @test_caller_slh(i32 %i) speculative_load_hardening {
+; CHECK-LABEL: @test_caller_slh(
+; CHECK-SAME: ) [[SLH:.*]] {
+; CHECK-NOT: call
+; CHECK: ret i32
+entry:
+  %callee = call i32 @noattr_callee(i32 %i)
+  ret i32 %callee
+}
+
+; Can inline a SLH'ed function into a normal one, dropping the SLH.
+define i32 @test_callee_slh(i32 %i) {
+; CHECK-LABEL: @test_callee_slh(i32 %i) {
+; CHECK-NOT: call
+; CHECK: ret i32
+entry:
+  %callee = call i32 @slh_callee(i32 %i)
+  ret i32 %callee
+}
+
 ; Check that a function doesn't get inlined if target-cpu strings don't match
 ; exactly.
 define i32 @test_target_cpu_callee0(i32 %i) "target-cpu"="corei7" {
@@ -384,6 +409,7 @@
 ; CHECK-NEXT: ret i32
 }
 
+; CHECK: attributes [[SLH]] = { speculative_load_hardening }
 ; CHECK: attributes [[FPMAD_FALSE]] = { "less-precise-fpmad"="false" }
 ; CHECK: attributes [[FPMAD_TRUE]] = { "less-precise-fpmad"="true" }
 ; CHECK: attributes [[NOIMPLICITFLOAT]] = { noimplicitfloat }
Index: llvm/test/CodeGen/X86/speculative-load-hardening.ll
===
--- llvm/test/CodeGen/X86/speculative-load-hardening.ll
+++ llvm/test/CodeGen/X86/speculative-load-hardening.ll
@@ -1,14 +1,14 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -x86-speculative-load-hardening | FileCheck %s --check-prefix=X64
-; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -x86-speculative-load-hardening -x86-slh-lfence | FileCheck %s --check-prefix=X64-LFENCE
+; RUN: llc < %s -mtriple=x86_64-un

[clang-tools-extra] r340527 - [clangd] Move function argument snippet disable mechanism from LSP rendering to internal clangd reprensentation.

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Thu Aug 23 05:19:39 2018
New Revision: 340527

URL: http://llvm.org/viewvc/llvm-project?rev=340527&view=rev
Log:
[clangd] Move function argument snippet disable mechanism from LSP rendering to 
internal clangd reprensentation.

Summary:
We were handling the EnableFunctionArgSnippets only when we are producing LSP
response. Move that code into CompletionItem generation so that internal clients
can benefit from that as well.

Reviewers: ilya-biryukov, ioeric, hokein

Reviewed By: ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, cfe-commits

Differential Revision: https://reviews.llvm.org/D51102

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=340527&r1=340526&r2=340527&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Thu Aug 23 05:19:39 2018
@@ -269,7 +269,8 @@ struct CodeCompletionBuilder {
 CodeCompletionString *SemaCCS,
 const IncludeInserter &Includes, StringRef FileName,
 const CodeCompleteOptions &Opts)
-  : ASTCtx(ASTCtx), ExtractDocumentation(Opts.IncludeComments) {
+  : ASTCtx(ASTCtx), ExtractDocumentation(Opts.IncludeComments),
+EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets) {
 add(C, SemaCCS);
 if (C.SemaResult) {
   Completion.Origin |= SymbolOrigin::AST;
@@ -385,10 +386,17 @@ private:
   }
 
   std::string summarizeSnippet() const {
-if (auto *Snippet = onlyValue<&BundledEntry::SnippetSuffix>())
-  return *Snippet;
-// All bundles are function calls.
-return "(${0})";
+auto *Snippet = onlyValue<&BundledEntry::SnippetSuffix>();
+if (!Snippet)
+  // All bundles are function calls.
+  return "($0)";
+if (!Snippet->empty() && !EnableFunctionArgSnippets &&
+((Completion.Kind == CompletionItemKind::Function) ||
+ (Completion.Kind == CompletionItemKind::Method)) &&
+(Snippet->front() == '(') && (Snippet->back() == ')'))
+  // Check whether function has any parameters or not.
+  return Snippet->size() > 2 ? "($0)" : "()";
+return *Snippet;
   }
 
   std::string summarizeSignature() const {
@@ -402,6 +410,7 @@ private:
   CodeCompletion Completion;
   SmallVector Bundled;
   bool ExtractDocumentation;
+  bool EnableFunctionArgSnippets;
 };
 
 // Determine the symbol ID for a Sema code completion result, if possible.
@@ -1413,16 +1422,8 @@ CompletionItem CodeCompletion::render(co
   LSP.additionalTextEdits.push_back(FixIt);
 }
   }
-  if (Opts.EnableSnippets && !SnippetSuffix.empty()) {
-if (!Opts.EnableFunctionArgSnippets &&
-((Kind == CompletionItemKind::Function) ||
- (Kind == CompletionItemKind::Method)) &&
-(SnippetSuffix.front() == '(') && (SnippetSuffix.back() == ')'))
-  // Check whether function has any parameters or not.
-  LSP.textEdit->newText += SnippetSuffix.size() > 2 ? "(${0})" : "()";
-else
-  LSP.textEdit->newText += SnippetSuffix;
-  }
+  if (Opts.EnableSnippets)
+LSP.textEdit->newText += SnippetSuffix;
 
   // FIXME(kadircet): Do not even fill insertText after making sure textEdit is
   // compatible with most of the editors.

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=340527&r1=340526&r2=340527&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Thu Aug 23 
05:19:39 2018
@@ -1140,7 +1140,7 @@ TEST(CompletionTest, OverloadBundling) {
   // For now we just return one of the doc strings arbitrarily.
   EXPECT_THAT(A.Documentation, AnyOf(HasSubstr("Overload with int"),
  HasSubstr("Overload with bool")));
-  EXPECT_EQ(A.SnippetSuffix, "(${0})");
+  EXPECT_EQ(A.SnippetSuffix, "($0)");
 }
 
 TEST(CompletionTest, DocumentationFromChangedFileCrash) {
@@ -1648,55 +1648,35 @@ TEST(SignatureHelpTest, IndexDocumentati
 SigDoc("Doc from sema";
 }
 
-TEST(CompletionTest, RenderWithSnippetsForFunctionArgsDisabled) {
+TEST(CompletionTest, CompletionFunctionArgsDisabled) {
   CodeCompleteOptions Opts;
-  Opts.EnableFunctionArgSnippets = true;
-  {
-CodeCompletion C;
-C.RequiredQualifier = "Foo::";
-C.Name = "x";
-C.SnippetSuffix = "()";
-
-auto R = C.render(Opts);
-EXPECT_EQ(R.textEdit->newText, "Foo::x");
-EXPECT_EQ(R.insertTextFormat, InsertTextForm

[PATCH] D51102: [clangd] Move function argument snippet disable mechanism from LSP rendering to internal clangd reprensentation.

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340527: [clangd] Move function argument snippet disable 
mechanism from LSP rendering to… (authored by kadircet, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D51102

Files:
  clang-tools-extra/trunk/clangd/CodeComplete.cpp
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -1140,7 +1140,7 @@
   // For now we just return one of the doc strings arbitrarily.
   EXPECT_THAT(A.Documentation, AnyOf(HasSubstr("Overload with int"),
  HasSubstr("Overload with bool")));
-  EXPECT_EQ(A.SnippetSuffix, "(${0})");
+  EXPECT_EQ(A.SnippetSuffix, "($0)");
 }
 
 TEST(CompletionTest, DocumentationFromChangedFileCrash) {
@@ -1648,55 +1648,35 @@
 SigDoc("Doc from sema";
 }
 
-TEST(CompletionTest, RenderWithSnippetsForFunctionArgsDisabled) {
+TEST(CompletionTest, CompletionFunctionArgsDisabled) {
   CodeCompleteOptions Opts;
-  Opts.EnableFunctionArgSnippets = true;
-  {
-CodeCompletion C;
-C.RequiredQualifier = "Foo::";
-C.Name = "x";
-C.SnippetSuffix = "()";
-
-auto R = C.render(Opts);
-EXPECT_EQ(R.textEdit->newText, "Foo::x");
-EXPECT_EQ(R.insertTextFormat, InsertTextFormat::PlainText);
-  }
-
   Opts.EnableSnippets = true;
   Opts.EnableFunctionArgSnippets = false;
+  const std::string Header =
+  R"cpp(
+  void xfoo();
+  void xfoo(int x, int y);
+  void xbar();
+  void f() {
+)cpp";
   {
-CodeCompletion C;
-C.RequiredQualifier = "Foo::";
-C.Name = "x";
-C.SnippetSuffix = "";
-
-auto R = C.render(Opts);
-EXPECT_EQ(R.textEdit->newText, "Foo::x");
-EXPECT_EQ(R.insertTextFormat, InsertTextFormat::Snippet);
+auto Results = completions(Header + "\nxfo^", {}, Opts);
+EXPECT_THAT(
+Results.Completions,
+UnorderedElementsAre(AllOf(Named("xfoo"), SnippetSuffix("()")),
+ AllOf(Named("xfoo"), SnippetSuffix("($0)";
   }
-
   {
-CodeCompletion C;
-C.RequiredQualifier = "Foo::";
-C.Name = "x";
-C.SnippetSuffix = "()";
-C.Kind = CompletionItemKind::Method;
-
-auto R = C.render(Opts);
-EXPECT_EQ(R.textEdit->newText, "Foo::x()");
-EXPECT_EQ(R.insertTextFormat, InsertTextFormat::Snippet);
+auto Results = completions(Header + "\nxba^", {}, Opts);
+EXPECT_THAT(Results.Completions, UnorderedElementsAre(AllOf(
+ Named("xbar"), SnippetSuffix("()";
   }
-
   {
-CodeCompletion C;
-C.RequiredQualifier = "Foo::";
-C.Name = "x";
-C.SnippetSuffix = "(${0:bool})";
-C.Kind = CompletionItemKind::Function;
-
-auto R = C.render(Opts);
-EXPECT_EQ(R.textEdit->newText, "Foo::x(${0})");
-EXPECT_EQ(R.insertTextFormat, InsertTextFormat::Snippet);
+Opts.BundleOverloads = true;
+auto Results = completions(Header + "\nxfo^", {}, Opts);
+EXPECT_THAT(
+Results.Completions,
+UnorderedElementsAre(AllOf(Named("xfoo"), SnippetSuffix("($0)";
   }
 }
 
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -269,7 +269,8 @@
 CodeCompletionString *SemaCCS,
 const IncludeInserter &Includes, StringRef FileName,
 const CodeCompleteOptions &Opts)
-  : ASTCtx(ASTCtx), ExtractDocumentation(Opts.IncludeComments) {
+  : ASTCtx(ASTCtx), ExtractDocumentation(Opts.IncludeComments),
+EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets) {
 add(C, SemaCCS);
 if (C.SemaResult) {
   Completion.Origin |= SymbolOrigin::AST;
@@ -385,10 +386,17 @@
   }
 
   std::string summarizeSnippet() const {
-if (auto *Snippet = onlyValue<&BundledEntry::SnippetSuffix>())
-  return *Snippet;
-// All bundles are function calls.
-return "(${0})";
+auto *Snippet = onlyValue<&BundledEntry::SnippetSuffix>();
+if (!Snippet)
+  // All bundles are function calls.
+  return "($0)";
+if (!Snippet->empty() && !EnableFunctionArgSnippets &&
+((Completion.Kind == CompletionItemKind::Function) ||
+ (Completion.Kind == CompletionItemKind::Method)) &&
+(Snippet->front() == '(') && (Snippet->back() == ')'))
+  // Check whether function has any parameters or not.
+  return Snippet->size() > 2 ? "($0)" : "()";
+return *Snippet;
   }
 
   std::string summarizeSignature() const {
@@ -402,6 +410,7 @@
   CodeC

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

One major limitation of the current workaround is that IIRC clang can actually 
call code completion callbacks, including this method, multiple times (e.g. 
when completing inside a macro definition that gets expanded later in the file 
or during parser recovery).
The workaround will work at the first call, but won't trigger at the repeated 
calls. We could find a way around that by resetting the flag to false at the 
right points (probably at every top-level function call?) WDYT?




Comment at: include/clang/Parse/Parser.h:217
 
+  /// Gets set to true after calling CodeCompleteCall, it is for a hack to make
+  /// signature help working even when it is triggered inside a token.

NIT: would go with something less harsh, like "**workaround** to make sure we 
only call CodeCompleteCall on the deepest call expression"



Comment at: lib/Parse/ParseExpr.cpp:1661
+  auto Completer = [&] {
+Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs);
+CalledOverloadCompletion = true;

Shouldn't we ignore `CodeCompleteCall` if `CalledOverloadCompletion` is set to 
true?



Comment at: lib/Parse/ParseExpr.cpp:1672
+// let the parser do it instead.
+if (!getLangOpts().ObjC1 && PP.isCodeCompletionReached() &&
+!CalledOverloadCompletion)

What's the special handling in ObjC? Can we unify with our workaround?



Comment at: lib/Parse/ParseExpr.cpp:1674
+!CalledOverloadCompletion)
+  Completer();
 LHS = ExprError();

Why don't we handle the `CalledOverloadCompletion` flag inside the 
`Completer()` itself?
We're currently special-casing the middle identifier and error case, why not 
have the same code path for `CodeCompleteCall()` calls at the start of the 
identifier?


Repository:
  rC Clang

https://reviews.llvm.org/D51038



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 162151.
kadircet added a comment.

- Resolve discussions.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50898

Files:
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -61,6 +61,7 @@
 MATCHER(InsertInclude, "") { return bool(arg.HeaderInsertion); }
 MATCHER_P(SnippetSuffix, Text, "") { return arg.SnippetSuffix == Text; }
 MATCHER_P(Origin, OriginSet, "") { return arg.Origin == OriginSet; }
+MATCHER(IsOverride, "") { return bool(arg.IsOverride); }
 
 // Shorthand for Contains(Named(Name)).
 Matcher &> Has(std::string Name) {
@@ -1700,6 +1701,58 @@
   }
 }
 
+TEST(CompletionTest, SuggestOverrides) {
+  constexpr const char *const Text(R"cpp(
+  class A {
+   public:
+virtual void vfunc(bool param);
+virtual void vfunc(bool param, int p);
+void func(bool param);
+  };
+  class B : public A {
+  virtual void ttt(bool param);
+  void vfunc(bool param, int p) override;
+  };
+  class C : public B {
+   public:
+void vfunc(bool param) override;
+^
+  };
+  )cpp");
+  const auto Results = completions(Text);
+  EXPECT_THAT(Results.Completions,
+  AllOf(Contains(AllOf(Named("vfunc"), IsOverride(),
+   Labeled("vfunc(bool param, int p)"))),
+Contains(AllOf(Named("ttt"), IsOverride(),
+   Labeled("ttt(bool param)"))),
+Not(Contains(AllOf(Named("vfunc"), IsOverride(),
+   Labeled("vfunc(bool param)"));
+}
+
+TEST(CompletionTest, RenderOverride) {
+  CodeCompletion C;
+  C.Name = "x";
+  C.Signature = "(bool) const";
+  C.SnippetSuffix = "(${0:bool})";
+  C.ReturnType = "int";
+  C.Documentation = "This is x().";
+  C.Kind = CompletionItemKind::Method;
+  C.Score.Total = 1.0;
+  C.Origin = SymbolOrigin::AST;
+  C.IsOverride = true;
+
+  CodeCompleteOptions Opts;
+  Opts.IncludeIndicator.NoInsert = "";
+  auto R = C.render(Opts);
+  EXPECT_EQ(R.label, "int x(bool) const override");
+  EXPECT_EQ(R.insertText, "int x(bool) const override");
+  EXPECT_EQ(R.insertTextFormat, InsertTextFormat::PlainText);
+  EXPECT_EQ(R.filterText, "x");
+  EXPECT_EQ(R.detail, "int");
+  EXPECT_EQ(R.documentation, "This is x().");
+  EXPECT_THAT(R.additionalTextEdits, IsEmpty());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.h
===
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -131,6 +131,9 @@
   /// Holds the range of the token we are going to replace with this completion.
   Range CompletionTokenRange;
 
+  /// Whether this completion is for overriding a virtual method.
+  bool IsOverride = false;
+
   // Scores are used to rank completion items.
   struct Scores {
 // The score that items are ranked by.
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -188,14 +188,66 @@
   return HeaderFile{std::move(*Resolved), /*Verbatim=*/false};
 }
 
+// First traverses all method definitions inside current class/struct/union
+// definition. Than traverses base classes to find virtual methods that haven't
+// been overriden within current context.
+// FIXME(kadircet): Currently we cannot see declarations below completion point.
+// It is because Sema gets run only upto completion point. Need to find a
+// solution to run it for the whole class/struct/union definition.
+static std::vector
+getNonOverridenMethodCompletionResults(const DeclContext *DC, Sema *S) {
+  const auto *CR = llvm::dyn_cast(DC);
+  // If not inside a class/struct/union return empty.
+  if (!CR)
+return {};
+  // First store overrides within current class.
+  // These are stored by name to make querying fast in the later step.
+  llvm::StringMap> Overrides;
+  for (auto *Method : CR->methods()) {
+if (!Method->isVirtual())
+  continue;
+Overrides[Method->getName()].push_back(Method);
+  }
+
+  std::vector Results;
+  for (const auto &Base : CR->bases()) {
+const auto *BR = Base.getType().getTypePtr()->getAsCXXRecordDecl();
+if (!BR)
+  continue;
+for (auto *Method : BR->methods()) {
+  if (!Method->isVirtual())
+continue;
+  const auto it = Overrides.find(Method->getName());
+  bool IsOverriden = false;
+  if (it != Overrides.end()) {
+for (auto *MD : it->second) {
+  // If the method in current body is not an overload of this virtual
+  // function, that it overrides this one.
+  if (!S->IsOverload(MD, Method, false)) {
+IsOverriden = true;
+break;
+  }
+}
+

[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:61
+/// rely on MR use-case to work properly.
+llvm::cl::init(false));
+

ilya-biryukov wrote:
> ioeric wrote:
> > AFAIK, this flag would basically depend on the executor being used. If 
> > executor can provide information like whether all tasks share memory, we 
> > can make the decision totally based on the selected executor (i.e. one 
> > fewer option).
> That SG. So we could add a virtual method to executor that would provide us 
> with this information and then we can get rid of that option, does that LG?
> However, do we have non-single-binary executor implementations upstream? We 
> need to leave a way to run the code that actually uses YAML serialization to 
> make sure we can experiment when things break.
Sounds good.

> However, do we have non-single-binary executor implementations upstream? We 
> need to leave a way to run the code that actually uses YAML serialization to 
> make sure we can experiment when things break.
The framework allows this, so I'm not sure if there is other downstream 
implementations that do this (probably not?). But I don't think we can get rid 
of the YAML serialization at this point because we still dump everything to 
yaml in the end? 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51155



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50385: [clangd] Collect symbol occurrences from AST.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/index/Index.h:46
+
+inline bool operator==(const SymbolLocation::Position &L,
+   const SymbolLocation::Position &R) {

hokein wrote:
> ilya-biryukov wrote:
> > NIT: having friend decls inside the classes themselves might prove to be 
> > more readable.
> > Not opposed to the current one too, feel free to ignore.
> These operator implementations seem not as much interesting as members in the 
> structure, putting them to the structure probably adds some noise to readers.
> 
> 
Ok, LG outside too



Comment at: clangd/index/Index.h:350
+
+llvm::DenseMap> SymbolOccurrences;
   };

hokein wrote:
> ilya-biryukov wrote:
> > Any store occurences in a file-centric manner?
> > E.g. 
> > ```
> > /// Occurences inside a single file.
> > class FileOccurences {
> >   StringRef File;
> >   vector> Locations;
> > };
> > 
> > // 
> > DenseMap>  SymbolOccurences;
> > ```
> > 
> > As discussed previously, this representation is better suited for both 
> > merging and serialization.
> The file-centric manner doesn't seem to suite our current model:  whenever we 
> update the index for the main AST, we just replace the symbol slab with the 
> new one; and for index merging, we only use the index `findOccurrences` 
> interfaces.
> 
> It would save some memory usage of `StringRef` File, but AFAIK, the memory 
> usage of current model is relatively small (comparing with the SymbolSlab for 
> code completion) since we only store occurrences in main file (~50KB for 
> `CodeComplete.cpp`).
> 
> I'd leave it as it is now, and we could revisit it later.
Isn't the merging model different for the occurrences?
We would actually have to drop **all** references from the older index when 
merging if the new one contains locations in the same file.
If the merge if file-centric, the file-based representation makes more sense in 
the first place. 

Apart from simpler merging the code, the file-based representation also buys us 
more efficient serialization for the static index, arguably efficient enough to 
stash all the occurrences even into our YAML index.

Postponing till later is also fine, but I'm not sure it buys us much now. These 
arguments only apply if we think the file-centric approach is a the right final 
design, though.



Comment at: clangd/index/SymbolCollector.h:59
+  // If none, all symbol occurrences will be collected.
+  llvm::Optional> IDs = llvm::None;
+};

hokein wrote:
> ilya-biryukov wrote:
> > Could you elaborate on what this option will be used for? How do we know in 
> > advance which symbols we're interested in?
> This is used for finding references in the AST as a part of the xref 
> implementation, basically the workflow would be:
> 
> 1. find SymbolIDs of the symbol under the cursor, using 
> `DeclarationAndMacrosFinder`
> 2. run symbol collector to find all occurrences in the main AST with all 
> SymbolIDs in #1
> 3. query the index, to get more occurrences
> 4. merge them  
Can we instead find all the occurences in  `DeclarationAndMacrosFinder` 
directly?
Extra run of `SymbolCollector` means another AST traversal, which is slow by 
itself, and SymbolCollector s designed for a much more hairy problem, its 
interface is just not nicely suited for things like only occurrences. The 
latter seems to be a simpler problem, and we can have a simpler interface to 
solve it (possibly shared between SymbolCollector and 
DeclarationAndMacrosFinder). WDYT?




Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50385



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 162157.
kadircet added a comment.

- Resolve discussions.
- Move override generation logic from render to item generation so that 
internal clients can use it as well, also use a boolean for checking override 
status of a bundle to be more efficient.
- Change unittests accordingly, get rid of unused IsOverride field.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50898

Files:
  clangd/CodeComplete.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1680,6 +1680,31 @@
   }
 }
 
+TEST(CompletionTest, SuggestOverrides) {
+  constexpr const char *const Text(R"cpp(
+  class A {
+   public:
+virtual void vfunc(bool param);
+virtual void vfunc(bool param, int p);
+void func(bool param);
+  };
+  class B : public A {
+  virtual void ttt(bool param) const;
+  void vfunc(bool param, int p) override;
+  };
+  class C : public B {
+   public:
+void vfunc(bool param) override;
+^
+  };
+  )cpp");
+  const auto Results = completions(Text);
+  EXPECT_THAT(Results.Completions,
+  AllOf(Contains(Labeled("void vfunc(bool param, int p) override")),
+Contains(Labeled("void ttt(bool param) const override")),
+Not(Contains(Labeled("void vfunc(bool param) override");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -188,14 +188,66 @@
   return HeaderFile{std::move(*Resolved), /*Verbatim=*/false};
 }
 
+// First traverses all method definitions inside current class/struct/union
+// definition. Than traverses base classes to find virtual methods that haven't
+// been overriden within current context.
+// FIXME(kadircet): Currently we cannot see declarations below completion point.
+// It is because Sema gets run only upto completion point. Need to find a
+// solution to run it for the whole class/struct/union definition.
+static std::vector
+getNonOverridenMethodCompletionResults(const DeclContext *DC, Sema *S) {
+  const auto *CR = llvm::dyn_cast(DC);
+  // If not inside a class/struct/union return empty.
+  if (!CR)
+return {};
+  // First store overrides within current class.
+  // These are stored by name to make querying fast in the later step.
+  llvm::StringMap> Overrides;
+  for (auto *Method : CR->methods()) {
+if (!Method->isVirtual())
+  continue;
+Overrides[Method->getName()].push_back(Method);
+  }
+
+  std::vector Results;
+  for (const auto &Base : CR->bases()) {
+const auto *BR = Base.getType().getTypePtr()->getAsCXXRecordDecl();
+if (!BR)
+  continue;
+for (auto *Method : BR->methods()) {
+  if (!Method->isVirtual())
+continue;
+  const auto it = Overrides.find(Method->getName());
+  bool IsOverriden = false;
+  if (it != Overrides.end()) {
+for (auto *MD : it->second) {
+  // If the method in current body is not an overload of this virtual
+  // function, that it overrides this one.
+  if (!S->IsOverload(MD, Method, false)) {
+IsOverriden = true;
+break;
+  }
+}
+  }
+  if (!IsOverriden)
+Results.emplace_back(Method, 0);
+}
+  }
+
+  return Results;
+}
+
 /// A code completion result, in clang-native form.
 /// It may be promoted to a CompletionItem if it's among the top-ranked results.
 struct CompletionCandidate {
   llvm::StringRef Name; // Used for filtering and sorting.
   // We may have a result from Sema, from the index, or both.
   const CodeCompletionResult *SemaResult = nullptr;
   const Symbol *IndexResult = nullptr;
 
+  // States whether this item is an override suggestion.
+  bool IsOverride = false;
+
   // Returns a token identifying the overload set this is part of.
   // 0 indicates it's not part of any overload set.
   size_t overloadSet() const {
@@ -352,21 +404,30 @@
 Completion.Documentation = getDocComment(ASTCtx, *C.SemaResult,
  /*CommentsFromHeader=*/false);
 }
+if (C.IsOverride)
+  S.OverrideSuffix = true;
   }
 
   CodeCompletion build() {
 Completion.ReturnType = summarizeReturnType();
 Completion.Signature = summarizeSignature();
 Completion.SnippetSuffix = summarizeSnippet();
 Completion.BundleSize = Bundled.size();
+if (summarizeOverride()) {
+  Completion.Name = Completion.ReturnType + ' ' +
+std::move(Completion.Name) +
+std::move(Completion.Signature) + " override";
+  Completion.Signature.clear();
+}
 return std::move(Completion);
   }
 
 private:
   struct BundledEntry {
 std::str

[PATCH] D50993: [clangd] Increase stack size of the new threads on macOS

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.

LGTM. Many thanks!
See the NIT about avoiding the helper class too.




Comment at: clangd/Threading.cpp:88
+  llvm::llvm_execute_on_thread_async(
+  Callable{Name.str(), std::move(Action), std::move(CleanupTask)},
+  clang::DesiredStackSize);

NIT: we usually do the following to avoid writing those classes (not great, but 
arguably a bit less boilerplate):
```

// 'Bind' is in 'Function.h'
Bind([](decltype(ThreadFunc) ThreadFunc, decltype(CleanupTask), std::string 
ThreadName) {
// ... code
},
std::move(Action), std::move(CelanupTask), std::move(ThreadName));
```


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50993



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51154: [clangd] Log memory usage of DexIndex and MemIndex

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Some drive-by NITS.




Comment at: clang-tools-extra/clangd/index/MemIndex.cpp:109
+std::lock_guard Lock(Mutex);
+
+Bytes += Index.getMemorySize();

Why not simply `return Index.getMemorySize()` under a lock?



Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:180
+  {
+std::lock_guard Lock(Mutex);
+

Maybe put a lock over all of the function and remove the unneeded compound 
statement?


https://reviews.llvm.org/D51154



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50246: [RISCV] Add support for computing sysroot for riscv32-unknown-elf

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.
Herald added a subscriber: jocewei.

Thanks for the patch and sorry for the delay. Once someone else marked it 
accepted it moves to a separate list in Phabricator - obviously I need to check 
the 'waiting on authors' list better.

As I see it are two changes here:

1. Calculating libcxx include paths based on specified sysroot
2. Computing the sysroot when an explicit path isn't provided

Could you please add a test that covers 2) above?


Repository:
  rC Clang

https://reviews.llvm.org/D50246



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.

still LG.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50898



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:61
+/// rely on MR use-case to work properly.
+llvm::cl::init(false));
+

ioeric wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > AFAIK, this flag would basically depend on the executor being used. If 
> > > executor can provide information like whether all tasks share memory, we 
> > > can make the decision totally based on the selected executor (i.e. one 
> > > fewer option).
> > That SG. So we could add a virtual method to executor that would provide us 
> > with this information and then we can get rid of that option, does that LG?
> > However, do we have non-single-binary executor implementations upstream? We 
> > need to leave a way to run the code that actually uses YAML serialization 
> > to make sure we can experiment when things break.
> Sounds good.
> 
> > However, do we have non-single-binary executor implementations upstream? We 
> > need to leave a way to run the code that actually uses YAML serialization 
> > to make sure we can experiment when things break.
> The framework allows this, so I'm not sure if there is other downstream 
> implementations that do this (probably not?). But I don't think we can get 
> rid of the YAML serialization at this point because we still dump everything 
> to yaml in the end? 
I meant the intermediate YAML serialization-deserialization parts.

I'd keep an option to force using the consumer that accumulates YAML symbols, 
and only allow to use on-the-fly merge when the tool executor reports that it 
is single-process. Would allow to:
- Avoid running on-the-fly merge when the executor does not support it 
(multi-binary MRs, etc)
- Allow to use on-the-fly by default for executors that run in the single 
binary.
- Allow to run with intermediate serialization even in a single binary if 
`-merge-on-the-fly` is specified.

Does that LG?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51155



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r340530 - [clangd] Suggest code-completions for overriding base class virtual methods.

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Thu Aug 23 06:14:50 2018
New Revision: 340530

URL: http://llvm.org/viewvc/llvm-project?rev=340530&view=rev
Log:
[clangd] Suggest code-completions for overriding base class virtual methods.

Summary:
Whenever a code-completion is triggered within a class/struct/union looks at
base classes and figures out non-overriden virtual functions. Than suggests
completions for those.

Reviewers: ilya-biryukov, hokein, ioeric

Reviewed By: hokein

Subscribers: MaskRay, jkorous, arphaman, cfe-commits

Differential Revision: https://reviews.llvm.org/D50898

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=340530&r1=340529&r2=340530&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Thu Aug 23 06:14:50 2018
@@ -188,6 +188,55 @@ static llvm::Expected toHead
   return HeaderFile{std::move(*Resolved), /*Verbatim=*/false};
 }
 
+// First traverses all method definitions inside current class/struct/union
+// definition. Than traverses base classes to find virtual methods that haven't
+// been overriden within current context.
+// FIXME(kadircet): Currently we cannot see declarations below completion 
point.
+// It is because Sema gets run only upto completion point. Need to find a
+// solution to run it for the whole class/struct/union definition.
+static std::vector
+getNonOverridenMethodCompletionResults(const DeclContext *DC, Sema *S) {
+  const auto *CR = llvm::dyn_cast(DC);
+  // If not inside a class/struct/union return empty.
+  if (!CR)
+return {};
+  // First store overrides within current class.
+  // These are stored by name to make querying fast in the later step.
+  llvm::StringMap> Overrides;
+  for (auto *Method : CR->methods()) {
+if (!Method->isVirtual())
+  continue;
+Overrides[Method->getName()].push_back(Method);
+  }
+
+  std::vector Results;
+  for (const auto &Base : CR->bases()) {
+const auto *BR = Base.getType().getTypePtr()->getAsCXXRecordDecl();
+if (!BR)
+  continue;
+for (auto *Method : BR->methods()) {
+  if (!Method->isVirtual())
+continue;
+  const auto it = Overrides.find(Method->getName());
+  bool IsOverriden = false;
+  if (it != Overrides.end()) {
+for (auto *MD : it->second) {
+  // If the method in current body is not an overload of this virtual
+  // function, that it overrides this one.
+  if (!S->IsOverload(MD, Method, false)) {
+IsOverriden = true;
+break;
+  }
+}
+  }
+  if (!IsOverriden)
+Results.emplace_back(Method, 0);
+}
+  }
+
+  return Results;
+}
+
 /// A code completion result, in clang-native form.
 /// It may be promoted to a CompletionItem if it's among the top-ranked 
results.
 struct CompletionCandidate {
@@ -196,6 +245,9 @@ struct CompletionCandidate {
   const CodeCompletionResult *SemaResult = nullptr;
   const Symbol *IndexResult = nullptr;
 
+  // States whether this item is an override suggestion.
+  bool IsOverride = false;
+
   // Returns a token identifying the overload set this is part of.
   // 0 indicates it's not part of any overload set.
   size_t overloadSet() const {
@@ -352,6 +404,8 @@ struct CodeCompletionBuilder {
 Completion.Documentation = getDocComment(ASTCtx, *C.SemaResult,
  /*CommentsFromHeader=*/false);
 }
+if (C.IsOverride)
+  S.OverrideSuffix = true;
   }
 
   CodeCompletion build() {
@@ -359,6 +413,12 @@ struct CodeCompletionBuilder {
 Completion.Signature = summarizeSignature();
 Completion.SnippetSuffix = summarizeSnippet();
 Completion.BundleSize = Bundled.size();
+if (summarizeOverride()) {
+  Completion.Name = Completion.ReturnType + ' ' +
+std::move(Completion.Name) +
+std::move(Completion.Signature) + " override";
+  Completion.Signature.clear();
+}
 return std::move(Completion);
   }
 
@@ -367,6 +427,7 @@ private:
 std::string SnippetSuffix;
 std::string Signature;
 std::string ReturnType;
+bool OverrideSuffix;
   };
 
   // If all BundledEntrys have the same value for a property, return it.
@@ -379,6 +440,14 @@ private:
 return &(B->*Member);
   }
 
+  template  const bool *onlyValue() const {
+auto B = Bundled.begin(), E = Bundled.end();
+for (auto I = B + 1; I != E; ++I)
+  if (I->*Member != B->*Member)
+return nullptr;
+return &(B->*Member);
+  }
+
   std::string summarizeReturnType() const {
 if (auto *RT = onlyValue<&BundledEntry::ReturnType>())
   return *RT;
@@ -406,6 +475,12 @@ priv

[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE340530: [clangd] Suggest code-completions for overriding 
base class virtual methods. (authored by kadircet, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50898?vs=162157&id=162158#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50898

Files:
  clangd/CodeComplete.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1680,6 +1680,31 @@
   }
 }
 
+TEST(CompletionTest, SuggestOverrides) {
+  constexpr const char *const Text(R"cpp(
+  class A {
+   public:
+virtual void vfunc(bool param);
+virtual void vfunc(bool param, int p);
+void func(bool param);
+  };
+  class B : public A {
+  virtual void ttt(bool param) const;
+  void vfunc(bool param, int p) override;
+  };
+  class C : public B {
+   public:
+void vfunc(bool param) override;
+^
+  };
+  )cpp");
+  const auto Results = completions(Text);
+  EXPECT_THAT(Results.Completions,
+  AllOf(Contains(Labeled("void vfunc(bool param, int p) override")),
+Contains(Labeled("void ttt(bool param) const override")),
+Not(Contains(Labeled("void vfunc(bool param) override");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -188,14 +188,66 @@
   return HeaderFile{std::move(*Resolved), /*Verbatim=*/false};
 }
 
+// First traverses all method definitions inside current class/struct/union
+// definition. Than traverses base classes to find virtual methods that haven't
+// been overriden within current context.
+// FIXME(kadircet): Currently we cannot see declarations below completion point.
+// It is because Sema gets run only upto completion point. Need to find a
+// solution to run it for the whole class/struct/union definition.
+static std::vector
+getNonOverridenMethodCompletionResults(const DeclContext *DC, Sema *S) {
+  const auto *CR = llvm::dyn_cast(DC);
+  // If not inside a class/struct/union return empty.
+  if (!CR)
+return {};
+  // First store overrides within current class.
+  // These are stored by name to make querying fast in the later step.
+  llvm::StringMap> Overrides;
+  for (auto *Method : CR->methods()) {
+if (!Method->isVirtual())
+  continue;
+Overrides[Method->getName()].push_back(Method);
+  }
+
+  std::vector Results;
+  for (const auto &Base : CR->bases()) {
+const auto *BR = Base.getType().getTypePtr()->getAsCXXRecordDecl();
+if (!BR)
+  continue;
+for (auto *Method : BR->methods()) {
+  if (!Method->isVirtual())
+continue;
+  const auto it = Overrides.find(Method->getName());
+  bool IsOverriden = false;
+  if (it != Overrides.end()) {
+for (auto *MD : it->second) {
+  // If the method in current body is not an overload of this virtual
+  // function, that it overrides this one.
+  if (!S->IsOverload(MD, Method, false)) {
+IsOverriden = true;
+break;
+  }
+}
+  }
+  if (!IsOverriden)
+Results.emplace_back(Method, 0);
+}
+  }
+
+  return Results;
+}
+
 /// A code completion result, in clang-native form.
 /// It may be promoted to a CompletionItem if it's among the top-ranked results.
 struct CompletionCandidate {
   llvm::StringRef Name; // Used for filtering and sorting.
   // We may have a result from Sema, from the index, or both.
   const CodeCompletionResult *SemaResult = nullptr;
   const Symbol *IndexResult = nullptr;
 
+  // States whether this item is an override suggestion.
+  bool IsOverride = false;
+
   // Returns a token identifying the overload set this is part of.
   // 0 indicates it's not part of any overload set.
   size_t overloadSet() const {
@@ -352,21 +404,30 @@
 Completion.Documentation = getDocComment(ASTCtx, *C.SemaResult,
  /*CommentsFromHeader=*/false);
 }
+if (C.IsOverride)
+  S.OverrideSuffix = true;
   }
 
   CodeCompletion build() {
 Completion.ReturnType = summarizeReturnType();
 Completion.Signature = summarizeSignature();
 Completion.SnippetSuffix = summarizeSnippet();
 Completion.BundleSize = Bundled.size();
+if (summarizeOverride()) {
+  Completion.Name = Completion.ReturnType + ' ' +
+std::move(Completion.Name) +
+std::move(Completion.Signature) + " override";
+  Completion.Signature.clear();
+}
 return std::move(Completion);
   }
 
 private:
   struct BundledEntry {
 std::string SnippetSuffix;
 std::string Signature;
 std

r340531 - Change dyn_cast(FD) to isa(FD) [NFC]

[Mikael Holmen via cfe-commits]

Author: uabelho
Date: Thu Aug 23 06:18:27 2018
New Revision: 340531

URL: http://llvm.org/viewvc/llvm-project?rev=340531&view=rev
Log:
Change dyn_cast(FD) to isa(FD) [NFC]

The result of the dyn_cast wasn't used to we can just check isa.

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp?rev=340531&r1=340530&r2=340531&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp Thu Aug 23 
06:18:27 2018
@@ -326,7 +326,7 @@ RetainSummaryManager::generateSummary(co
 }
   }
 
-  if (dyn_cast(FD)) {
+  if (isa(FD)) {
 
 // Stop tracking arguments passed to C++ methods, as those might be
 // wrapping smart pointers.


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r340532 - [analyzer] Delete SMTContext. NFC.

[Mikhail R. Gadelha via cfe-commits]

Author: mramalho
Date: Thu Aug 23 06:20:18 2018
New Revision: 340532

URL: http://llvm.org/viewvc/llvm-project?rev=340532&view=rev
Log:
[analyzer] Delete SMTContext. NFC.

Summary: There is no reason to have a base class for a context anymore as each 
SMT object carries a reference to the specific solver context.

Reviewers: NoQ, george.karpenkov, hiraditya

Reviewed By: hiraditya

Subscribers: hiraditya, xazax.hun, szepet, a.sidorin, Szelethus

Differential Revision: https://reviews.llvm.org/D50768

Removed:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTContext.h
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp

Removed: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTContext.h?rev=340531&view=auto
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTContext.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTContext.h 
(removed)
@@ -1,31 +0,0 @@
-//== SMTContext.h ---*- C++ 
-*--==//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-//
-//  This file defines a SMT generic Context API, which will be the base class
-//  for every SMT solver context specific class.
-//
-//===--===//
-
-#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SMTCONTEXT_H
-#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SMTCONTEXT_H
-
-namespace clang {
-namespace ento {
-
-/// Generic base class for SMT contexts
-class SMTContext {
-public:
-  SMTContext() = default;
-  virtual ~SMTContext() = default;
-};
-
-} // namespace ento
-} // namespace clang
-
-#endif

Modified: cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp?rev=340532&r1=340531&r2=340532&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp Thu Aug 23 
06:20:18 2018
@@ -11,7 +11,6 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/SMTContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SMTExpr.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SMTSolver.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SMTSort.h"
@@ -53,11 +52,11 @@ void Z3ErrorHandler(Z3_context Context,
 }
 
 /// Wrapper for Z3 context
-class Z3Context : public SMTContext {
+class Z3Context {
 public:
   Z3_context Context;
 
-  Z3Context() : SMTContext() {
+  Z3Context() {
 Context = Z3_mk_context_rc(Z3Config().Config);
 // The error function is set here because the context is the first object
 // created by the backend


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50768: [analyzer] Delete SMTContext. NFC.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340532: [analyzer] Delete SMTContext. NFC. (authored by 
mramalho, committed by ).
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D50768

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/SMTContext.h
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp


Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -11,7 +11,6 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/SMTContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SMTExpr.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SMTSolver.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SMTSort.h"
@@ -53,11 +52,11 @@
 }
 
 /// Wrapper for Z3 context
-class Z3Context : public SMTContext {
+class Z3Context {
 public:
   Z3_context Context;
 
-  Z3Context() : SMTContext() {
+  Z3Context() {
 Context = Z3_mk_context_rc(Z3Config().Config);
 // The error function is set here because the context is the first object
 // created by the backend
Index: include/clang/StaticAnalyzer/Core/PathSensitive/SMTContext.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/SMTContext.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SMTContext.h
@@ -1,31 +0,0 @@
-//== SMTContext.h ---*- C++ 
-*--==//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-//
-//  This file defines a SMT generic Context API, which will be the base class
-//  for every SMT solver context specific class.
-//
-//===--===//
-
-#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SMTCONTEXT_H
-#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SMTCONTEXT_H
-
-namespace clang {
-namespace ento {
-
-/// Generic base class for SMT contexts
-class SMTContext {
-public:
-  SMTContext() = default;
-  virtual ~SMTContext() = default;
-};
-
-} // namespace ento
-} // namespace clang
-
-#endif


Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -11,7 +11,6 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/SMTContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SMTExpr.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SMTSolver.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SMTSort.h"
@@ -53,11 +52,11 @@
 }
 
 /// Wrapper for Z3 context
-class Z3Context : public SMTContext {
+class Z3Context {
 public:
   Z3_context Context;
 
-  Z3Context() : SMTContext() {
+  Z3Context() {
 Context = Z3_mk_context_rc(Z3Config().Config);
 // The error function is set here because the context is the first object
 // created by the backend
Index: include/clang/StaticAnalyzer/Core/PathSensitive/SMTContext.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/SMTContext.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SMTContext.h
@@ -1,31 +0,0 @@
-//== SMTContext.h ---*- C++ -*--==//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-//
-//  This file defines a SMT generic Context API, which will be the base class
-//  for every SMT solver context specific class.
-//
-//===--===//
-
-#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SMTCONTEXT_H
-#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SMTCONTEXT_H
-
-namespace clang {
-namespace ento {
-
-/// Generic base class for SMT contexts
-class SMTContext {
-public:
-  SMTContext() = default;
-  virtual ~SMTContext() = default;
-};
-
-} // namespace ento
-} // namespace clang
-
-#endif
__

r340533 - [analyzer] Templatefy SMTConstraintManager so more generic code can be moved from solver specific implementations. NFC.

[Mikhail R. Gadelha via cfe-commits]

Author: mramalho
Date: Thu Aug 23 06:21:00 2018
New Revision: 340533

URL: http://llvm.org/viewvc/llvm-project?rev=340533&view=rev
Log:
[analyzer] Templatefy SMTConstraintManager so more generic code can be moved 
from solver specific implementations. NFC.

Summary:
By making SMTConstraintManager a template and passing the SMT constraint type 
and expr, we can further move code from the Z3ConstraintManager class to the 
generic SMT constraint Manager.

Now, each SMT specific constraint manager only needs to implement the method 
`bool canReasonAbout(SVal X) const`.

Reviewers: NoQ, george.karpenkov

Reviewed By: george.karpenkov

Subscribers: mgorny, xazax.hun, szepet, a.sidorin, Szelethus

Differential Revision: https://reviews.llvm.org/D50770

Removed:
cfe/trunk/lib/StaticAnalyzer/Core/SMTConstraintManager.cpp
Modified:

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h?rev=340533&r1=340532&r2=340533&view=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
 (original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
 Thu Aug 23 06:21:00 2018
@@ -21,6 +21,7 @@
 namespace clang {
 namespace ento {
 
+template 
 class SMTConstraintManager : public clang::ento::SimpleConstraintManager {
   SMTSolverRef &Solver;
 
@@ -34,25 +35,191 @@ public:
   // Implementation for interface from SimpleConstraintManager.
   //===--===//
 
-  ProgramStateRef assumeSym(ProgramStateRef state, SymbolRef Sym,
-bool Assumption) override;
+  ProgramStateRef assumeSym(ProgramStateRef State, SymbolRef Sym,
+bool Assumption) override {
+ASTContext &Ctx = getBasicVals().getContext();
+
+QualType RetTy;
+bool hasComparison;
+
+SMTExprRef Exp = Solver->getExpr(Ctx, Sym, &RetTy, &hasComparison);
+
+// Create zero comparison for implicit boolean cast, with reversed
+// assumption
+if (!hasComparison && !RetTy->isBooleanType())
+  return assumeExpr(State, Sym,
+Solver->getZeroExpr(Ctx, Exp, RetTy, !Assumption));
+
+return assumeExpr(State, Sym, Assumption ? Exp : Solver->mkNot(Exp));
+  }
 
   ProgramStateRef assumeSymInclusiveRange(ProgramStateRef State, SymbolRef Sym,
   const llvm::APSInt &From,
   const llvm::APSInt &To,
-  bool InRange) override;
+  bool InRange) override {
+ASTContext &Ctx = getBasicVals().getContext();
+return assumeExpr(State, Sym,
+  Solver->getRangeExpr(Ctx, Sym, From, To, InRange));
+  }
 
   ProgramStateRef assumeSymUnsupported(ProgramStateRef State, SymbolRef Sym,
-   bool Assumption) override;
+   bool Assumption) override {
+// Skip anything that is unsupported
+return State;
+  }
 
   //===--===//
   // Implementation for interface from ConstraintManager.
   //===--===//
 
-  ConditionTruthVal checkNull(ProgramStateRef State, SymbolRef Sym) override;
+  ConditionTruthVal checkNull(ProgramStateRef State, SymbolRef Sym) override {
+ASTContext &Ctx = getBasicVals().getContext();
+
+QualType RetTy;
+// The expression may be casted, so we cannot call getZ3DataExpr() directly
+SMTExprRef VarExp = Solver->getExpr(Ctx, Sym, &RetTy);
+SMTExprRef Exp =
+Solver->getZeroExpr(Ctx, VarExp, RetTy, /*Assumption=*/true);
+
+// Negate the constraint
+SMTExprRef NotExp =
+Solver->getZeroExpr(Ctx, VarExp, RetTy, /*Assumption=*/false);
+
+Solver->reset();
+addStateConstraints(State);
+
+Solver->push();
+Solver->addConstraint(Exp);
+ConditionTruthVal isSat = Solver->check();
+
+Solver->pop();
+Solver->addConstraint(NotExp);
+ConditionTruthVal isNotSat = Solver->check();
+
+// Zero is the only possible solution
+if (isSat.isConstrainedTrue() && isNotSat.isConstrainedFalse())
+  return true;
+
+// Zero is not a solution
+if (isSat.isConstrainedFalse() && isNotSat.isConstrainedTrue())
+  return false;
+
+// Zero may be a solution
+return ConditionTruthVal();
+  }
 
   const llvm::APSInt *getSymVal(ProgramStateRef State,
- 

[PATCH] D50773: [analyzer] added cache for SMT queries in the SMTConstraintManager

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340535: [analyzer] added cache for SMT queries in the 
SMTConstraintManager (authored by mramalho, committed by ).
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D50773

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h


Index: include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
@@ -86,29 +86,15 @@
 SMTExprRef NotExp =
 SMTConv::getZeroExpr(Solver, Ctx, VarExp, RetTy, /*Assumption=*/false);
 
-Solver->reset();
-addStateConstraints(State);
-
-Solver->push();
-Solver->addConstraint(Exp);
-
-Optional isSat = Solver->check();
-if (!isSat.hasValue())
-  return ConditionTruthVal();
-
-Solver->pop();
-Solver->addConstraint(NotExp);
-
-Optional isNotSat = Solver->check();
-if (!isNotSat.hasValue())
-  return ConditionTruthVal();
+ConditionTruthVal isSat = checkModel(State, Sym, Exp);
+ConditionTruthVal isNotSat = checkModel(State, Sym, NotExp);
 
 // Zero is the only possible solution
-if (isSat.getValue() && !isNotSat.getValue())
+if (isSat.isConstrainedTrue() && isNotSat.isConstrainedFalse())
   return true;
 
 // Zero is not a solution
-if (!isSat.getValue() && isNotSat.getValue())
+if (isSat.isConstrainedFalse() && isNotSat.isConstrainedTrue())
   return false;
 
 // Zero may be a solution
@@ -126,6 +112,10 @@
   llvm::APSInt Value(Ctx.getTypeSize(Ty),
  !Ty->isSignedIntegerOrEnumerationType());
 
+  // TODO: this should call checkModel so we can use the cache, however,
+  // this method tries to get the interpretation (the actual value) from
+  // the solver, which is currently not cached.
+
   SMTExprRef Exp =
   SMTConv::fromData(Solver, SD->getSymbolID(), Ty, 
Ctx.getTypeSize(Ty));
 
@@ -236,7 +226,7 @@
   virtual ProgramStateRef assumeExpr(ProgramStateRef State, SymbolRef Sym,
  const SMTExprRef &Exp) {
 // Check the model, avoid simplifying AST to save time
-if (checkModel(State, Exp).isConstrainedTrue())
+if (checkModel(State, Sym, Exp).isConstrainedTrue())
   return State->add(
   std::make_pair(Sym, static_cast(*Exp)));
 
@@ -264,18 +254,34 @@
   }
 
   // Generate and check a Z3 model, using the given constraint.
-  ConditionTruthVal checkModel(ProgramStateRef State,
+  ConditionTruthVal checkModel(ProgramStateRef State, SymbolRef Sym,
const SMTExprRef &Exp) const {
+ProgramStateRef NewState = State->add(
+std::make_pair(Sym, static_cast(*Exp)));
+
+llvm::FoldingSetNodeID ID;
+NewState->get().Profile(ID);
+
+unsigned hash = ID.ComputeHash();
+auto I = Cached.find(hash);
+if (I != Cached.end())
+  return I->second;
+
 Solver->reset();
-Solver->addConstraint(Exp);
-addStateConstraints(State);
+addStateConstraints(NewState);
 
 Optional res = Solver->check();
 if (!res.hasValue())
-  return ConditionTruthVal();
+  Cached[hash] = ConditionTruthVal();
+else
+  Cached[hash] = ConditionTruthVal(res.getValue());
 
-return ConditionTruthVal(res.getValue());
+return Cached[hash];
   }
+
+  // Cache the result of an SMT query (true, false, unknown). The key is the
+  // hash of the constraints in a state
+  mutable llvm::DenseMap Cached;
 }; // end class SMTConstraintManager
 
 } // namespace ento


Index: include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
@@ -86,29 +86,15 @@
 SMTExprRef NotExp =
 SMTConv::getZeroExpr(Solver, Ctx, VarExp, RetTy, /*Assumption=*/false);
 
-Solver->reset();
-addStateConstraints(State);
-
-Solver->push();
-Solver->addConstraint(Exp);
-
-Optional isSat = Solver->check();
-if (!isSat.hasValue())
-  return ConditionTruthVal();
-
-Solver->pop();
-Solver->addConstraint(NotExp);
-
-Optional isNotSat = Solver->check();
-if (!isNotSat.hasValue())
-  return ConditionTruthVal();
+ConditionTruthVal isSat = checkModel(State, Sym, Exp);
+ConditionTruthVal isNotSat = checkModel(State, Sym, NotExp);
 
 // Zero is the only possible solution
-if (isSat.getValue() && !isNotSat.getValue())
+if (isSat.isConstrainedTrue() && isNotSat.isConstrainedFalse())
   return true;
 
 // Zero is not a solution
-if (!isSat.getValue() && isNotSat.getValue())
+if (is

r340534 - [analyzer] Moved all CSA code from the SMT API to a new header, `SMTConv.h`. NFC.

[Mikhail R. Gadelha via cfe-commits]

Author: mramalho
Date: Thu Aug 23 06:21:31 2018
New Revision: 340534

URL: http://llvm.org/viewvc/llvm-project?rev=340534&view=rev
Log:
[analyzer] Moved all CSA code from the SMT API to a new header, `SMTConv.h`. 
NFC.

Summary:
With this patch, the SMT backend is almost completely detached from the CSA.

Unfortunate consequence is that we missed the `ConditionTruthVal` from the CSA 
and had to use `Optional`.

The Z3 solver implementation is still in the same file as the 
`Z3ConstraintManager`, in `lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp` 
though, but except for that, the SMT API can be moved to anywhere in the 
codebase.

Reviewers: NoQ, george.karpenkov

Reviewed By: george.karpenkov

Subscribers: xazax.hun, szepet, a.sidorin, Szelethus

Differential Revision: https://reviews.llvm.org/D50772

Added:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h
Modified:

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTSolver.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h?rev=340534&r1=340533&r2=340534&view=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
 (original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
 Thu Aug 23 06:21:31 2018
@@ -16,7 +16,7 @@
 #define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SMTCONSTRAINTMANAGER_H
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/SMTSolver.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h"
 
 namespace clang {
 namespace ento {
@@ -42,13 +42,14 @@ public:
 QualType RetTy;
 bool hasComparison;
 
-SMTExprRef Exp = Solver->getExpr(Ctx, Sym, &RetTy, &hasComparison);
+SMTExprRef Exp = SMTConv::getExpr(Solver, Ctx, Sym, &RetTy, 
&hasComparison);
 
 // Create zero comparison for implicit boolean cast, with reversed
 // assumption
 if (!hasComparison && !RetTy->isBooleanType())
-  return assumeExpr(State, Sym,
-Solver->getZeroExpr(Ctx, Exp, RetTy, !Assumption));
+  return assumeExpr(
+  State, Sym,
+  SMTConv::getZeroExpr(Solver, Ctx, Exp, RetTy, !Assumption));
 
 return assumeExpr(State, Sym, Assumption ? Exp : Solver->mkNot(Exp));
   }
@@ -58,8 +59,8 @@ public:
   const llvm::APSInt &To,
   bool InRange) override {
 ASTContext &Ctx = getBasicVals().getContext();
-return assumeExpr(State, Sym,
-  Solver->getRangeExpr(Ctx, Sym, From, To, InRange));
+return assumeExpr(
+State, Sym, SMTConv::getRangeExpr(Solver, Ctx, Sym, From, To, 
InRange));
   }
 
   ProgramStateRef assumeSymUnsupported(ProgramStateRef State, SymbolRef Sym,
@@ -77,31 +78,37 @@ public:
 
 QualType RetTy;
 // The expression may be casted, so we cannot call getZ3DataExpr() directly
-SMTExprRef VarExp = Solver->getExpr(Ctx, Sym, &RetTy);
+SMTExprRef VarExp = SMTConv::getExpr(Solver, Ctx, Sym, &RetTy);
 SMTExprRef Exp =
-Solver->getZeroExpr(Ctx, VarExp, RetTy, /*Assumption=*/true);
+SMTConv::getZeroExpr(Solver, Ctx, VarExp, RetTy, /*Assumption=*/true);
 
 // Negate the constraint
 SMTExprRef NotExp =
-Solver->getZeroExpr(Ctx, VarExp, RetTy, /*Assumption=*/false);
+SMTConv::getZeroExpr(Solver, Ctx, VarExp, RetTy, /*Assumption=*/false);
 
 Solver->reset();
 addStateConstraints(State);
 
 Solver->push();
 Solver->addConstraint(Exp);
-ConditionTruthVal isSat = Solver->check();
+
+Optional isSat = Solver->check();
+if (!isSat.hasValue())
+  return ConditionTruthVal();
 
 Solver->pop();
 Solver->addConstraint(NotExp);
-ConditionTruthVal isNotSat = Solver->check();
+
+Optional isNotSat = Solver->check();
+if (!isNotSat.hasValue())
+  return ConditionTruthVal();
 
 // Zero is the only possible solution
-if (isSat.isConstrainedTrue() && isNotSat.isConstrainedFalse())
+if (isSat.getValue() && !isNotSat.getValue())
   return true;
 
 // Zero is not a solution
-if (isSat.isConstrainedFalse() && isNotSat.isConstrainedTrue())
+if (!isSat.getValue() && isNotSat.getValue())
   return false;
 
 // Zero may be a solution
@@ -120,14 +127,14 @@ public:
  !Ty->isSignedIntegerOrEnumerationType());
 
   SMTExprRef Exp =
-  Solver->fromData(SD->getSymbolID(), Ty, Ctx.getTypeSize(Ty));

r340535 - [analyzer] added cache for SMT queries in the SMTConstraintManager

[Mikhail R. Gadelha via cfe-commits]

Author: mramalho
Date: Thu Aug 23 06:21:35 2018
New Revision: 340535

URL: http://llvm.org/viewvc/llvm-project?rev=340535&view=rev
Log:
[analyzer] added cache for SMT queries in the SMTConstraintManager

Summary:
This patch implements a new cache for the result of SMT queries; with this 
patch the regression tests are 25% faster.

It's implemented as a `llvm::DenseMap` where the key is the hash of the set of 
the constraints in a state.

There is still one method that does not use the cache, `getSymVal`, because it 
needs to get a symbol interpretation from the SMT, which is not cached yet.

Reviewers: NoQ, george.karpenkov

Reviewed By: george.karpenkov

Subscribers: xazax.hun, szepet, a.sidorin, Szelethus

Differential Revision: https://reviews.llvm.org/D50773

Modified:

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h?rev=340535&r1=340534&r2=340535&view=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
 (original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
 Thu Aug 23 06:21:35 2018
@@ -86,29 +86,15 @@ public:
 SMTExprRef NotExp =
 SMTConv::getZeroExpr(Solver, Ctx, VarExp, RetTy, /*Assumption=*/false);
 
-Solver->reset();
-addStateConstraints(State);
-
-Solver->push();
-Solver->addConstraint(Exp);
-
-Optional isSat = Solver->check();
-if (!isSat.hasValue())
-  return ConditionTruthVal();
-
-Solver->pop();
-Solver->addConstraint(NotExp);
-
-Optional isNotSat = Solver->check();
-if (!isNotSat.hasValue())
-  return ConditionTruthVal();
+ConditionTruthVal isSat = checkModel(State, Sym, Exp);
+ConditionTruthVal isNotSat = checkModel(State, Sym, NotExp);
 
 // Zero is the only possible solution
-if (isSat.getValue() && !isNotSat.getValue())
+if (isSat.isConstrainedTrue() && isNotSat.isConstrainedFalse())
   return true;
 
 // Zero is not a solution
-if (!isSat.getValue() && isNotSat.getValue())
+if (isSat.isConstrainedFalse() && isNotSat.isConstrainedTrue())
   return false;
 
 // Zero may be a solution
@@ -126,6 +112,10 @@ public:
   llvm::APSInt Value(Ctx.getTypeSize(Ty),
  !Ty->isSignedIntegerOrEnumerationType());
 
+  // TODO: this should call checkModel so we can use the cache, however,
+  // this method tries to get the interpretation (the actual value) from
+  // the solver, which is currently not cached.
+
   SMTExprRef Exp =
   SMTConv::fromData(Solver, SD->getSymbolID(), Ty, 
Ctx.getTypeSize(Ty));
 
@@ -236,7 +226,7 @@ protected:
   virtual ProgramStateRef assumeExpr(ProgramStateRef State, SymbolRef Sym,
  const SMTExprRef &Exp) {
 // Check the model, avoid simplifying AST to save time
-if (checkModel(State, Exp).isConstrainedTrue())
+if (checkModel(State, Sym, Exp).isConstrainedTrue())
   return State->add(
   std::make_pair(Sym, static_cast(*Exp)));
 
@@ -264,18 +254,34 @@ protected:
   }
 
   // Generate and check a Z3 model, using the given constraint.
-  ConditionTruthVal checkModel(ProgramStateRef State,
+  ConditionTruthVal checkModel(ProgramStateRef State, SymbolRef Sym,
const SMTExprRef &Exp) const {
+ProgramStateRef NewState = State->add(
+std::make_pair(Sym, static_cast(*Exp)));
+
+llvm::FoldingSetNodeID ID;
+NewState->get().Profile(ID);
+
+unsigned hash = ID.ComputeHash();
+auto I = Cached.find(hash);
+if (I != Cached.end())
+  return I->second;
+
 Solver->reset();
-Solver->addConstraint(Exp);
-addStateConstraints(State);
+addStateConstraints(NewState);
 
 Optional res = Solver->check();
 if (!res.hasValue())
-  return ConditionTruthVal();
+  Cached[hash] = ConditionTruthVal();
+else
+  Cached[hash] = ConditionTruthVal(res.getValue());
 
-return ConditionTruthVal(res.getValue());
+return Cached[hash];
   }
+
+  // Cache the result of an SMT query (true, false, unknown). The key is the
+  // hash of the constraints in a state
+  mutable llvm::DenseMap Cached;
 }; // end class SMTConstraintManager
 
 } // namespace ento


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51159: [FileManager] Do not call 'real_path' in getFile().

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added reviewers: ilya-biryukov, simark.
Herald added a subscriber: cfe-commits.

This partially rolls back the change in https://reviews.llvm.org/D48903:
https://github.com/llvm-mirror/clang/commit/89aa7f45a1f728144935289d4ce69d8522999de0#diff-0025af005307891b5429b6a834823d5eR318

`real_path` can be very expensive on real file systems, and calling it on each
opened file can slow down the compilation. This also slows down deserialized
ASTs for which real paths need to be recalculated for each input files again.


Repository:
  rC Clang

https://reviews.llvm.org/D51159

Files:
  lib/Basic/FileManager.cpp


Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -316,9 +316,14 @@
   UFE.File = std::move(F);
   UFE.IsValid = true;
 
-  SmallString<128> RealPathName;
-  if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
+  if (UFE.File) {
+if (auto Path = UFE.File->getName()) {
+  llvm::SmallString<128> AbsPath(*Path);
+  makeAbsolutePath(AbsPath);
+  llvm::sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true);
+  UFE.RealPathName = AbsPath.str();
+}
+  }
 
   return &UFE;
 }


Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -316,9 +316,14 @@
   UFE.File = std::move(F);
   UFE.IsValid = true;
 
-  SmallString<128> RealPathName;
-  if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
+  if (UFE.File) {
+if (auto Path = UFE.File->getName()) {
+  llvm::SmallString<128> AbsPath(*Path);
+  makeAbsolutePath(AbsPath);
+  llvm::sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true);
+  UFE.RealPathName = AbsPath.str();
+}
+  }
 
   return &UFE;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51159: [FileManager] Do not call 'real_path' in getFile().

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 162166.
ioeric added a comment.

- Add comment


Repository:
  rC Clang

https://reviews.llvm.org/D51159

Files:
  lib/Basic/FileManager.cpp


Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -316,9 +316,16 @@
   UFE.File = std::move(F);
   UFE.IsValid = true;
 
-  SmallString<128> RealPathName;
-  if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
+  if (UFE.File) {
+if (auto Path = UFE.File->getName()) {
+  llvm::SmallString<128> AbsPath(*Path);
+  // This is not the same as `real_path()` system call, which also resolves
+  // symlinks.
+  makeAbsolutePath(AbsPath);
+  llvm::sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true);
+  UFE.RealPathName = AbsPath.str();
+}
+  }
 
   return &UFE;
 }


Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -316,9 +316,16 @@
   UFE.File = std::move(F);
   UFE.IsValid = true;
 
-  SmallString<128> RealPathName;
-  if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
+  if (UFE.File) {
+if (auto Path = UFE.File->getName()) {
+  llvm::SmallString<128> AbsPath(*Path);
+  // This is not the same as `real_path()` system call, which also resolves
+  // symlinks.
+  makeAbsolutePath(AbsPath);
+  llvm::sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true);
+  UFE.RealPathName = AbsPath.str();
+}
+  }
 
   return &UFE;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:61
+/// rely on MR use-case to work properly.
+llvm::cl::init(false));
+

ilya-biryukov wrote:
> ioeric wrote:
> > ilya-biryukov wrote:
> > > ioeric wrote:
> > > > AFAIK, this flag would basically depend on the executor being used. If 
> > > > executor can provide information like whether all tasks share memory, 
> > > > we can make the decision totally based on the selected executor (i.e. 
> > > > one fewer option).
> > > That SG. So we could add a virtual method to executor that would provide 
> > > us with this information and then we can get rid of that option, does 
> > > that LG?
> > > However, do we have non-single-binary executor implementations upstream? 
> > > We need to leave a way to run the code that actually uses YAML 
> > > serialization to make sure we can experiment when things break.
> > Sounds good.
> > 
> > > However, do we have non-single-binary executor implementations upstream? 
> > > We need to leave a way to run the code that actually uses YAML 
> > > serialization to make sure we can experiment when things break.
> > The framework allows this, so I'm not sure if there is other downstream 
> > implementations that do this (probably not?). But I don't think we can get 
> > rid of the YAML serialization at this point because we still dump 
> > everything to yaml in the end? 
> I meant the intermediate YAML serialization-deserialization parts.
> 
> I'd keep an option to force using the consumer that accumulates YAML symbols, 
> and only allow to use on-the-fly merge when the tool executor reports that it 
> is single-process. Would allow to:
> - Avoid running on-the-fly merge when the executor does not support it 
> (multi-binary MRs, etc)
> - Allow to use on-the-fly by default for executors that run in the single 
> binary.
> - Allow to run with intermediate serialization even in a single binary if 
> `-merge-on-the-fly` is specified.
> 
> Does that LG?
IIUC, there will basically be a flag that allows us to force using yaml 
intermediate serialization, even when the executor supports on-the-fly merging. 
By default, we use on-the-fly merging whenever possible. If that's the case, 
SGTM.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51155



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg marked 7 inline comments as done.
hugoeg added a comment.

If the fix-it can be implemented, I certainly do not know how. As mentioned 
above, I am not the original author for this check.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51132



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50617: [ASTMatchers] Let hasObjectExpression also support UnresolvedMemberExpr, CXXDependentScopeMemberExpr

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D50617



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162174.
hugoeg added a comment.

corrections applied


https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char &operator[](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view a, string_view b);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string &str);
+  AlphaNum(const string_view &pc);
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum &operator=(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum &a);
+string StrCat(const AlphaNum &a, const AlphaNum &b);
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c);
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c,
+  const AlphaNum &d);
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c,
+  const AlphaNum &d, const AlphaNum &e, const AV &... args);
+
+void StrAppend(string *dest, const AlphaNum &a);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c, const AlphaNum &d);
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c, const AlphaNum &d, const AlphaNum &e,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, StrCat("A", StrCat(1.1)));
+  // CHECK-MESSAGES: [[@LINE-1

Re: [PATCH] D49885: Thread safety analysis: Allow relockable scopes

[Aaron Ballman via cfe-commits]

On Wed, Aug 22, 2018 at 6:35 PM, Aaron Puchert via Phabricator
 wrote:
> aaronpuchert added inline comments.
>
>
> 
> Comment at: lib/Analysis/ThreadSafety.cpp:932
> +  // We're relocking the underlying mutexes. Warn on double locking.
> +  if (FSet.findLock(FactMan, UnderCp))
> +Handler.handleDoubleLock(DiagKind, UnderCp.toString(), entry.loc());
> 
> delesley wrote:
>> Minor nit.  Use curly braces on the if, to match the else.
> Removing the braces [was 
> suggested](https://reviews.llvm.org/D49885?id=157599#inline-439256) by 
> @aaron.ballman in an earlier patch set, but I can just add them in again. I 
> slightly favor having them, but I don't feel strongly either way.

My own opinions aren't strong here either, but I'm fine adding them
back in. We don't have clear guidance on what to do with braces for
if/else where one has braces and the other doesn't require them, but
I'm now starting to favor leaving the braces in rather than removing
them.

~Aaron

>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D49885
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

Excellent!


Repository:
  rL LLVM

https://reviews.llvm.org/D51049



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50564: Add support for SEH unwinding on Windows.

[Kristina Brooks via Phabricator via cfe-commits]

kristina added inline comments.



Comment at: src/UnwindCursor.hpp:1801
+  if (pc != getLastPC()) {
+UNWIND_INFO *xdata = reinterpret_cast(base + 
unwindEntry->UnwindData);
+if (xdata->Flags & (UNW_FLAG_EHANDLER|UNW_FLAG_UHANDLER)) {

mstorsjo wrote:
> I can't say I understand all of this yet, but I'm slowly getting there, and 
> I'm trying to compare this to what libgcc does.
> 
> libgcc doesn't use any definition of UNWIND_INFO and doesn't need to do the 
> equivalent of `getInfoFromSEH`, used by `step()`, anywhere. `unw_step()` is 
> used in `_Unwind_ForcedUnwind`, which in libgcc is implemented using 
> `RaiseException (STATUS_GCC_FORCED, ...`.
> 
> I guess if you happen to have all of the `unw_step` API available, it's 
> easier to just do it like this, in custom code without relying on the NTDLL 
> functions for it, while the libgcc version relies more on the NTDLL API.
This primarily deals with the SEH exceptions re-purposed as a C++ exception 
mechanism on x86_64 (if I understood this right), it's possible to set a custom 
filter using a runtime call so I suspect GCC does that or defines a translation 
function (also via a runtime call) which acts as a filter for "true" SEH 
exceptions behind the scenes deep within the runtime. Typially "true" SEH 
exceptions don't, outside of runtime magic, play nicely with C++ exceptions, 
with the `__C_specific_handler` ones being a completely different paradigm that 
falls far outside the scope of libunwind (those ones being the "true"/explicit 
SEH exceptions).

(Don't take my word for it, it's been a while and I only implemented the "true" 
variation for 64-bit Linux by reserving some RT signals and using that to 
invoke additional runtime glue that would then do the unwinding, completely 
ignoring DWARF since CFI exceptions and SEH exceptions really don't mix 
especially on platforms that are not Windows-like)


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51163: [clangd] Check for include overlapping looks for only the line now.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added reviewers: ilya-biryukov, hokein, ioeric.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay.

Currently we match an include only if we are inside filename, with this patch we
will match whenever we are on the starting line of the include.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51163

Files:
  clangd/XRefs.cpp
  unittests/clangd/XRefsTests.cpp


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -1014,11 +1014,13 @@
 
   Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("5"));
   ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations, IsEmpty());
+  EXPECT_THAT(*Locations,
+  ElementsAre(Location{FooHUri, HeaderAnnotations.range()}));
 
   Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("7"));
   ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations, IsEmpty());
+  EXPECT_THAT(*Locations,
+  ElementsAre(Location{FooHUri, HeaderAnnotations.range()}));
 }
 
 TEST(GoToDefinition, WithPreamble) {
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -211,7 +211,7 @@
   std::vector Result;
   // Handle goto definition for #include.
   for (auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
-if (!Inc.Resolved.empty() && Inc.R.contains(Pos))
+if (!Inc.Resolved.empty() && Inc.R.start.line == Pos.line)
   Result.push_back(Location{URIForFile{Inc.Resolved}, {}});
   }
   if (!Result.empty())


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -1014,11 +1014,13 @@
 
   Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("5"));
   ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations, IsEmpty());
+  EXPECT_THAT(*Locations,
+  ElementsAre(Location{FooHUri, HeaderAnnotations.range()}));
 
   Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("7"));
   ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations, IsEmpty());
+  EXPECT_THAT(*Locations,
+  ElementsAre(Location{FooHUri, HeaderAnnotations.range()}));
 }
 
 TEST(GoToDefinition, WithPreamble) {
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -211,7 +211,7 @@
   std::vector Result;
   // Handle goto definition for #include.
   for (auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
-if (!Inc.Resolved.empty() && Inc.R.contains(Pos))
+if (!Inc.Resolved.empty() && Inc.R.start.line == Pos.line)
   Result.push_back(Location{URIForFile{Inc.Resolved}, {}});
   }
   if (!Result.empty())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50246: [RISCV] Add support for computing sysroot for riscv32-unknown-elf

[Lewis Revill via Phabricator via cfe-commits]

lewis-revill updated this revision to Diff 162179.
lewis-revill added a comment.

Added tests for the case where `--sysroot` is not provided.


Repository:
  rC Clang

https://reviews.llvm.org/D50246

Files:
  lib/Driver/ToolChains/RISCV.cpp
  lib/Driver/ToolChains/RISCV.h
  test/Driver/riscv32-toolchain.c

Index: test/Driver/riscv32-toolchain.c
===
--- test/Driver/riscv32-toolchain.c
+++ test/Driver/riscv32-toolchain.c
@@ -18,13 +18,26 @@
 // C-RV32-BAREMETAL-ILP32: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
 // C-RV32-BAREMETAL-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
 
+// RUN: %clang %s -### -no-canonical-prefixes \
+// RUN:   -target riscv32-unknown-elf \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree 2>&1 \
+// RUN:   | FileCheck -check-prefix=C-RV32-BAREMETAL-NOSYSROOT-ILP32 %s
+
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: "{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../bin{{/|}}riscv32-unknown-elf-ld"
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf/lib{{/|}}crt0.o"
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtbegin.o"
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: "-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf{{/|}}lib"
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: "-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1"
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
+
 // RUN: %clangxx %s -### -no-canonical-prefixes \
 // RUN:   -target riscv32-unknown-elf -stdlib=libstdc++ \
 // RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree \
 // RUN:   --sysroot=%S/Inputs/basic_riscv32_tree/riscv32-unknown-elf 2>&1 \
 // RUN:   | FileCheck -check-prefix=CXX-RV32-BAREMETAL-ILP32 %s
 
-// CXX-RV32-BAREMETAL-ILP32: "-internal-isystem" "{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf/include/c++{{/|}}8.0.1"
+// CXX-RV32-BAREMETAL-ILP32: "-internal-isystem" "{{.*}}Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/c++{{/|}}8.0.1"
 // CXX-RV32-BAREMETAL-ILP32: "{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../bin{{/|}}riscv32-unknown-elf-ld"
 // CXX-RV32-BAREMETAL-ILP32: "--sysroot={{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf"
 // CXX-RV32-BAREMETAL-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib{{/|}}crt0.o"
@@ -34,6 +47,20 @@
 // CXX-RV32-BAREMETAL-ILP32: "-lstdc++" "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
 // CXX-RV32-BAREMETAL-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
 
+// RUN: %clangxx %s -### -no-canonical-prefixes \
+// RUN:   -target riscv32-unknown-elf -stdlib=libstdc++ \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree 2>&1 \
+// RUN:   | FileCheck -check-prefix=CXX-RV32-BAREMETAL-NOSYSROOT-ILP32 %s
+
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-internal-isystem" "{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf/include/c++{{/|}}8.0.1"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../bin{{/|}}riscv32-unknown-elf-ld"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf/lib{{/|}}crt0.o"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtbegin.o"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf/lib"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-lstdc++" "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
+
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
 // RUN:   -target riscv32-linux-unknown-elf \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
Index: lib/Driver/ToolChains/RISCV.h
===
--- lib/Driver/ToolChains/RISCV.h
+++ lib/Driver/ToolChains/RISCV.h
@@ -23,6 +23,9 @@
  const llvm::opt::ArgList &Args);
 
   bool IsIntegratedAssemblerDefault() const override { return true; }
+  void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+

[PATCH] D51164: [Tooling] Add a isSingleProcess() helper to ToolExecutor

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: ioeric.
Herald added a subscriber: kadircet.

Used in clangd's symbol builder to optimize for the common
shared-memory executor case.


Repository:
  rC Clang

https://reviews.llvm.org/D51164

Files:
  include/clang/Tooling/AllTUsExecution.h
  include/clang/Tooling/Execution.h
  include/clang/Tooling/StandaloneExecution.h
  unittests/Tooling/ExecutionTest.cpp


Index: unittests/Tooling/ExecutionTest.cpp
===
--- unittests/Tooling/ExecutionTest.cpp
+++ unittests/Tooling/ExecutionTest.cpp
@@ -96,6 +96,8 @@
 
   StringRef getExecutorName() const override { return ExecutorName; }
 
+  bool isSingleProcess() const override { return true; }
+
   llvm::Error
   execute(llvm::ArrayRef,
ArgumentsAdjuster>>) override {
Index: include/clang/Tooling/StandaloneExecution.h
===
--- include/clang/Tooling/StandaloneExecution.h
+++ include/clang/Tooling/StandaloneExecution.h
@@ -52,6 +52,8 @@
 
   StringRef getExecutorName() const override { return ExecutorName; }
 
+  bool isSingleProcess() const override { return true; }
+
   using ToolExecutor::execute;
 
   llvm::Error
Index: include/clang/Tooling/Execution.h
===
--- include/clang/Tooling/Execution.h
+++ include/clang/Tooling/Execution.h
@@ -114,6 +114,13 @@
   /// Returns the name of a specific executor.
   virtual StringRef getExecutorName() const = 0;
 
+  /// Should return true iff executor runs all actions in a single process.
+  /// Clients can use this signal to find out if they can collect results
+  /// in-memory (e.g. to avoid serialization costs of using ToolResults).
+  /// The single-process executors can still run multiple threads, but all
+  /// executions are guaranteed to share the same memory.
+  virtual bool isSingleProcess() const = 0;
+
   /// Executes each action with a corresponding arguments adjuster.
   virtual llvm::Error
   execute(llvm::ArrayRef<
Index: include/clang/Tooling/AllTUsExecution.h
===
--- include/clang/Tooling/AllTUsExecution.h
+++ include/clang/Tooling/AllTUsExecution.h
@@ -45,6 +45,8 @@
 
   StringRef getExecutorName() const override { return ExecutorName; }
 
+  bool isSingleProcess() const override { return true; }
+
   using ToolExecutor::execute;
 
   llvm::Error


Index: unittests/Tooling/ExecutionTest.cpp
===
--- unittests/Tooling/ExecutionTest.cpp
+++ unittests/Tooling/ExecutionTest.cpp
@@ -96,6 +96,8 @@
 
   StringRef getExecutorName() const override { return ExecutorName; }
 
+  bool isSingleProcess() const override { return true; }
+
   llvm::Error
   execute(llvm::ArrayRef,
ArgumentsAdjuster>>) override {
Index: include/clang/Tooling/StandaloneExecution.h
===
--- include/clang/Tooling/StandaloneExecution.h
+++ include/clang/Tooling/StandaloneExecution.h
@@ -52,6 +52,8 @@
 
   StringRef getExecutorName() const override { return ExecutorName; }
 
+  bool isSingleProcess() const override { return true; }
+
   using ToolExecutor::execute;
 
   llvm::Error
Index: include/clang/Tooling/Execution.h
===
--- include/clang/Tooling/Execution.h
+++ include/clang/Tooling/Execution.h
@@ -114,6 +114,13 @@
   /// Returns the name of a specific executor.
   virtual StringRef getExecutorName() const = 0;
 
+  /// Should return true iff executor runs all actions in a single process.
+  /// Clients can use this signal to find out if they can collect results
+  /// in-memory (e.g. to avoid serialization costs of using ToolResults).
+  /// The single-process executors can still run multiple threads, but all
+  /// executions are guaranteed to share the same memory.
+  virtual bool isSingleProcess() const = 0;
+
   /// Executes each action with a corresponding arguments adjuster.
   virtual llvm::Error
   execute(llvm::ArrayRef<
Index: include/clang/Tooling/AllTUsExecution.h
===
--- include/clang/Tooling/AllTUsExecution.h
+++ include/clang/Tooling/AllTUsExecution.h
@@ -45,6 +45,8 @@
 
   StringRef getExecutorName() const override { return ExecutorName; }
 
+  bool isSingleProcess() const override { return true; }
+
   using ToolExecutor::execute;
 
   llvm::Error
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51029: [clangd] Implement LIMIT iterator

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 162183.
kbobyrev marked 3 inline comments as done.
kbobyrev added a comment.

Address a round comments from Sam.


https://reviews.llvm.org/D51029

Files:
  clang-tools-extra/clangd/index/dex/Iterator.cpp
  clang-tools-extra/clangd/index/dex/Iterator.h
  clang-tools-extra/unittests/clangd/DexIndexTests.cpp

Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -234,13 +234,13 @@
   Root->advanceTo(1);
   Root->advanceTo(0);
   EXPECT_EQ(Root->peek(), 1U);
-  auto ElementBoost = Root->consume(Root->peek());
+  auto ElementBoost = Root->consume();
   EXPECT_THAT(ElementBoost, 6);
   Root->advance();
   EXPECT_EQ(Root->peek(), 5U);
   Root->advanceTo(5);
   EXPECT_EQ(Root->peek(), 5U);
-  ElementBoost = Root->consume(Root->peek());
+  ElementBoost = Root->consume();
   EXPECT_THAT(ElementBoost, 8);
   Root->advanceTo(9000);
   EXPECT_TRUE(Root->reachedEnd());
@@ -265,24 +265,26 @@
 }
 
 TEST(DexIndexIterators, Limit) {
-  const PostingList L0 = {4, 7, 8, 20, 42, 100};
-  const PostingList L1 = {1, 3, 5, 8, 9};
-  const PostingList L2 = {1, 5, 7, 9};
-  const PostingList L3 = {0, 5};
-  const PostingList L4 = {0, 1, 5};
-  const PostingList L5;
+  const PostingList L0 = {3, 6, 7, 20, 42, 100};
+  const PostingList L1 = {1, 3, 5, 6, 7, 30, 100};
+  const PostingList L2 = {0, 3, 5, 7, 8, 100};
 
   auto DocIterator = create(L0);
-  EXPECT_THAT(consumeIDs(*DocIterator, 42), ElementsAre(4, 7, 8, 20, 42, 100));
+  EXPECT_THAT(consumeIDs(*DocIterator, 42), ElementsAre(3, 6, 7, 20, 42, 100));
 
   DocIterator = create(L0);
-  EXPECT_THAT(consumeIDs(*DocIterator), ElementsAre(4, 7, 8, 20, 42, 100));
+  EXPECT_THAT(consumeIDs(*DocIterator), ElementsAre(3, 6, 7, 20, 42, 100));
 
   DocIterator = create(L0);
-  EXPECT_THAT(consumeIDs(*DocIterator, 3), ElementsAre(4, 7, 8));
+  EXPECT_THAT(consumeIDs(*DocIterator, 3), ElementsAre(3, 6, 7));
 
   DocIterator = create(L0);
   EXPECT_THAT(consumeIDs(*DocIterator, 0), ElementsAre());
+
+  auto AndIterator =
+  createAnd(createLimit(createTrue(9000), 343), createLimit(create(L0), 2),
+createLimit(create(L1), 3), createLimit(create(L2), 42));
+  EXPECT_THAT(consumeIDs(*AndIterator), ElementsAre(3, 7));
 }
 
 TEST(DexIndexIterators, True) {
@@ -301,28 +303,28 @@
 TEST(DexIndexIterators, Boost) {
   auto BoostIterator = createBoost(createTrue(5U), 42U);
   EXPECT_FALSE(BoostIterator->reachedEnd());
-  auto ElementBoost = BoostIterator->consume(BoostIterator->peek());
+  auto ElementBoost = BoostIterator->consume();
   EXPECT_THAT(ElementBoost, 42U);
 
   const PostingList L0 = {2, 4};
   const PostingList L1 = {1, 4};
   auto Root = createOr(createTrue(5U), createBoost(create(L0), 2U),
createBoost(create(L1), 3U));
 
-  ElementBoost = Root->consume(Root->peek());
+  ElementBoost = Root->consume();
   EXPECT_THAT(ElementBoost, Iterator::DEFAULT_BOOST_SCORE);
   Root->advance();
   EXPECT_THAT(Root->peek(), 1U);
-  ElementBoost = Root->consume(Root->peek());
+  ElementBoost = Root->consume();
   EXPECT_THAT(ElementBoost, 3);
 
   Root->advance();
   EXPECT_THAT(Root->peek(), 2U);
-  ElementBoost = Root->consume(Root->peek());
+  ElementBoost = Root->consume();
   EXPECT_THAT(ElementBoost, 2);
 
   Root->advanceTo(4);
-  ElementBoost = Root->consume(Root->peek());
+  ElementBoost = Root->consume();
   EXPECT_THAT(ElementBoost, 3);
 }
 
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -87,13 +87,14 @@
   ///
   /// Note: reachedEnd() must be false.
   virtual DocID peek() const = 0;
-  /// Retrieves boosting score. Query tree root should pass Root->peek() to this
-  /// function, the parameter is needed to propagate through the tree. Given ID
-  /// should be compared against BOOST iterator peek()s: some of the iterators
-  /// would not point to the item which was propagated to the top of the query
-  /// tree (e.g. if these iterators are branches of OR iterator) and hence
-  /// shouldn't apply any boosting to the consumed item.
-  virtual float consume(DocID ID) = 0;
+  /// Informs the iterator that the current document was consumed, and returns
+  /// its boost.
+  ///
+  /// Note: If this iterator has any child iterators that contain the document,
+  /// consume() should be called on those and their boosts incorporated.
+  /// consume() must *not* be called on children that don't contain the current
+  /// doc.
+  virtual float consume() = 0;
 
   virtual ~Iterator() {}
 
@@ -165,6 +166,13 @@
 std::unique_ptr createBoost(std::unique_ptr Child,
   float Factor);
 
+/// Returns LIMIT iterator, which yield

[PATCH] D51164: [Tooling] Add a isSingleProcess() helper to ToolExecutor

[Eric Liu via Phabricator via cfe-commits]

ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

https://reviews.llvm.org/D51164



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51154: [clangd] Log memory usage of DexIndex and MemIndex

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 162184.
kbobyrev marked 2 inline comments as done.
kbobyrev added a comment.

Slightly simplify the code.


https://reviews.llvm.org/D51154

Files:
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/FileIndex.h
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/MemIndex.cpp
  clang-tools-extra/clangd/index/MemIndex.h
  clang-tools-extra/clangd/index/Merge.cpp
  clang-tools-extra/clangd/index/dex/DexIndex.cpp
  clang-tools-extra/clangd/index/dex/DexIndex.h
  clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp

Index: clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
@@ -923,6 +923,10 @@
llvm::function_ref
Callback) const override {}
 
+  // This is incorrect, but IndexRequestCollector is not an actual index and it
+  // isn't used in production code.
+  size_t estimateMemoryUsage() const override { return 0; }
+
   const std::vector allRequests() const { return Requests; }
 
 private:
Index: clang-tools-extra/clangd/index/dex/DexIndex.h
===
--- clang-tools-extra/clangd/index/dex/DexIndex.h
+++ clang-tools-extra/clangd/index/dex/DexIndex.h
@@ -57,7 +57,10 @@
llvm::function_ref
Callback) const override;
 
+  size_t estimateMemoryUsage() const override;
+
 private:
+
   mutable std::mutex Mutex;
 
   std::shared_ptr> Symbols /*GUARDED_BY(Mutex)*/;
Index: clang-tools-extra/clangd/index/dex/DexIndex.cpp
===
--- clang-tools-extra/clangd/index/dex/DexIndex.cpp
+++ clang-tools-extra/clangd/index/dex/DexIndex.cpp
@@ -67,6 +67,9 @@
 InvertedIndex = std::move(TempInvertedIndex);
 SymbolQuality = std::move(TempSymbolQuality);
   }
+
+  vlog("Built DexIndex with estimated memory usage {0} bytes.",
+   estimateMemoryUsage());
 }
 
 std::unique_ptr DexIndex::build(SymbolSlab Slab) {
@@ -171,6 +174,20 @@
   log("findOccurrences is not implemented.");
 }
 
+size_t DexIndex::estimateMemoryUsage() const {
+  std::lock_guard Lock(Mutex);
+
+  size_t Bytes =
+  LookupTable.size() * sizeof(std::pair);
+  Bytes += SymbolQuality.size() * sizeof(std::pair);
+  Bytes += InvertedIndex.size() * sizeof(Token);
+
+  for (const auto &P : InvertedIndex) {
+Bytes += P.second.size() * sizeof(DocID);
+  }
+  return Bytes;
+}
+
 } // namespace dex
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/index/Merge.cpp
===
--- clang-tools-extra/clangd/index/Merge.cpp
+++ clang-tools-extra/clangd/index/Merge.cpp
@@ -84,6 +84,10 @@
 log("findOccurrences is not implemented.");
   }
 
+  size_t estimateMemoryUsage() const override {
+return Dynamic->estimateMemoryUsage() + Static->estimateMemoryUsage();
+  }
+
 private:
   const SymbolIndex *Dynamic, *Static;
 };
Index: clang-tools-extra/clangd/index/MemIndex.h
===
--- clang-tools-extra/clangd/index/MemIndex.h
+++ clang-tools-extra/clangd/index/MemIndex.h
@@ -39,7 +39,10 @@
llvm::function_ref
Callback) const override;
 
+  size_t estimateMemoryUsage() const override;
+
 private:
+
   std::shared_ptr> Symbols;
   // Index is a set of symbols that are deduplicated by symbol IDs.
   // FIXME: build smarter index structure.
Index: clang-tools-extra/clangd/index/MemIndex.cpp
===
--- clang-tools-extra/clangd/index/MemIndex.cpp
+++ clang-tools-extra/clangd/index/MemIndex.cpp
@@ -26,6 +26,9 @@
 Index = std::move(TempIndex);
 Symbols = std::move(Syms); // Relase old symbols.
   }
+
+  vlog("Built MemIndex with estimated memory usage {0} bytes.",
+   estimateMemoryUsage());
 }
 
 std::unique_ptr MemIndex::build(SymbolSlab Slab) {
@@ -98,5 +101,10 @@
   &Snap->Pointers);
 }
 
+size_t MemIndex::estimateMemoryUsage() const {
+  std::lock_guard Lock(Mutex);
+  return Index.getMemorySize();
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/index/Index.h
===
--- clang-tools-extra/clangd/index/Index.h
+++ clang-tools-extra/clangd/index/Index.h
@@ -385,6 +385,12 @@
   virtual void findOccurrences(
   const OccurrencesRequest &Req,
   llvm::function_ref Callback) const = 0;
+
+  /// Returns estimated size of index (in bytes).
+  // FIXME(kbobyrev): Currently, this only returns the size of index itself
+  // excluding the size of actual symbol slab i

[PATCH] D47849: [OpenMP][Clang][NVPTX] Enable math functions called in an OpenMP NVPTX target device region to be resolved as device-native function calls

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea added inline comments.



Comment at: lib/Basic/Targets/NVPTX.cpp:232
+  // getting inlined on the device.
+  Builder.defineMacro("__NO_MATH_INLINES");
 }

tra wrote:
> This relies on implementation detail of particular variant of the header file 
> you're assuming all compilations will include. This is a workaround of the 
> real problem (attempting to use headers from machine X while targeting Y) at 
> best.
> 
> D50845 is dealing with the issue of headers for target code. Hopefully, 
> they'll find a way to provide device-specific headers, so you don't rely on 
> host headers being parseable during device-side compilation.
I agree. The proper fix would be what the other patch is attempting to do.



Comment at: lib/Driver/ToolChains/Clang.cpp:4758
+// toolchain.
+CmdArgs.push_back("-fno-math-builtin");
   }

tra wrote:
> Could you elaborate on why you don't want the builtins?
> Builtins are enabled and are useful for CUDA. What makes their use different 
> for OpenMP?
> Are you doing it to guarantee that math functions remain unresolved in IR so 
> you could link them in from external bitcode?
> 
That's right. I don't particularly like this approach as this leads to 
OpenMP-NVPTX toolchain missing out on optimizations such as replacing math 
function call with basic operations ( pow(a,2) -> a*a for example).
I am trying to fix this in a future patch by allowing intrinsics/builtins to 
propagate.


Repository:
  rC Clang

https://reviews.llvm.org/D47849



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51093: [ARM] Set __ARM_FEATURE_SIMD32 for +dsp cores

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

Looks reasonable to me.


https://reviews.llvm.org/D51093



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51163: [clangd] Check for include overlapping looks for only the line now.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51163



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 162194.
ilya-biryukov added a comment.

- Don't merge on-the-fly for multi-process executors


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51155

Files:
  clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp

Index: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
===
--- clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
+++ clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
@@ -49,9 +49,33 @@
"not given, such headers will have relative paths."),
 llvm::cl::init(""));
 
+static llvm::cl::opt MergeOnTheFly(
+"merge-on-the-fly",
+llvm::cl::desc(
+"Merges symbols for each processed translation unit as soon "
+"they become available. This results in a smaller memory "
+"usage and an almost instant reduce stage. Optimal for running as a "
+"standalone tool, but cannot be used with multi-process executors like "
+"MapReduce."),
+llvm::cl::init(true));
+
+/// Responsible for aggregating symbols from each processed file and producing
+/// the final results. All methods in this class must be thread-safe,
+/// 'consumeSymbols' may be called from multiple threads.
+class SymbolsConsumer {
+public:
+  virtual ~SymbolsConsumer() = default;
+
+  /// Consume a SymbolSlab build for a file.
+  virtual void consumeSymbols(SymbolSlab Symbols) = 0;
+  /// Produce a resulting symbol slab, by combining  occurrences of the same
+  /// symbols across translation units.
+  virtual SymbolSlab mergeResults() = 0;
+};
+
 class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
 public:
-  SymbolIndexActionFactory(tooling::ExecutionContext *Ctx) : Ctx(Ctx) {}
+  SymbolIndexActionFactory(SymbolsConsumer &Consumer) : Consumer(Consumer) {}
 
   clang::FrontendAction *create() override {
 // Wraps the index action and reports collected symbols to the execution
@@ -61,10 +85,10 @@
   WrappedIndexAction(std::shared_ptr C,
  std::unique_ptr Includes,
  const index::IndexingOptions &Opts,
- tooling::ExecutionContext *Ctx)
+ SymbolsConsumer &Consumer)
   : WrapperFrontendAction(
 index::createIndexingAction(C, Opts, nullptr)),
-Ctx(Ctx), Collector(C), Includes(std::move(Includes)),
+Consumer(Consumer), Collector(C), Includes(std::move(Includes)),
 PragmaHandler(collectIWYUHeaderMaps(this->Includes.get())) {}
 
   std::unique_ptr
@@ -91,14 +115,11 @@
   return;
 }
 
-auto Symbols = Collector->takeSymbols();
-for (const auto &Sym : Symbols) {
-  Ctx->reportResult(Sym.ID.str(), SymbolToYAML(Sym));
-}
+Consumer.consumeSymbols(Collector->takeSymbols());
   }
 
 private:
-  tooling::ExecutionContext *Ctx;
+  SymbolsConsumer &Consumer;
   std::shared_ptr Collector;
   std::unique_ptr Includes;
   std::unique_ptr PragmaHandler;
@@ -118,30 +139,72 @@
 CollectorOpts.Includes = Includes.get();
 return new WrappedIndexAction(
 std::make_shared(std::move(CollectorOpts)),
-std::move(Includes), IndexOpts, Ctx);
+std::move(Includes), IndexOpts, Consumer);
   }
 
-  tooling::ExecutionContext *Ctx;
+  SymbolsConsumer &Consumer;
 };
 
-// Combine occurrences of the same symbol across translation units.
-SymbolSlab mergeSymbols(tooling::ToolResults *Results) {
-  SymbolSlab::Builder UniqueSymbols;
-  llvm::BumpPtrAllocator Arena;
-  Symbol::Details Scratch;
-  Results->forEachResult([&](llvm::StringRef Key, llvm::StringRef Value) {
-Arena.Reset();
-llvm::yaml::Input Yin(Value, &Arena);
-auto Sym = clang::clangd::SymbolFromYAML(Yin, Arena);
-clang::clangd::SymbolID ID;
-Key >> ID;
-if (const auto *Existing = UniqueSymbols.find(ID))
-  UniqueSymbols.insert(mergeSymbol(*Existing, Sym, &Scratch));
-else
-  UniqueSymbols.insert(Sym);
-  });
-  return std::move(UniqueSymbols).build();
-}
+/// Stashes per-file results inside ExecutionContext, merges all of them at the
+/// end. Useful for running on MapReduce infrastructure to avoid keeping symbols
+/// from multiple files in memory.
+class ToolExecutorConsumer : public SymbolsConsumer {
+public:
+  ToolExecutorConsumer(ToolExecutor &Executor) : Executor(Executor) {}
+
+  void consumeSymbols(SymbolSlab Symbols) override {
+for (const auto &Sym : Symbols)
+  Executor.getExecutionContext()->reportResult(Sym.ID.str(),
+   SymbolToYAML(Sym));
+  }
+
+  SymbolSlab mergeResults() override {
+SymbolSlab::Builder UniqueSymbols;
+llvm::BumpPtrAllocator Arena;
+Symbol::Details Scratch;
+Executor.getToolResults()->forEachResult(
+[&](llvm::StringRef Key, llvm::StringRef Value) {
+  Arena.Re

[clang-tools-extra] r340539 - [clangd] Check for include overlapping looks for only the line now.

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Thu Aug 23 08:55:27 2018
New Revision: 340539

URL: http://llvm.org/viewvc/llvm-project?rev=340539&view=rev
Log:
[clangd] Check for include overlapping looks for only the line now.

Summary:
Currently we match an include only if we are inside filename, with this patch we
will match whenever we are on the starting line of the include.

Reviewers: ilya-biryukov, hokein, ioeric

Reviewed By: ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, cfe-commits

Differential Revision: https://reviews.llvm.org/D51163

Modified:
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=340539&r1=340538&r2=340539&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Thu Aug 23 08:55:27 2018
@@ -211,7 +211,7 @@ std::vector findDefinitions(Pa
   std::vector Result;
   // Handle goto definition for #include.
   for (auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
-if (!Inc.Resolved.empty() && Inc.R.contains(Pos))
+if (!Inc.Resolved.empty() && Inc.R.start.line == Pos.line)
   Result.push_back(Location{URIForFile{Inc.Resolved}, {}});
   }
   if (!Result.empty())

Modified: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp?rev=340539&r1=340538&r2=340539&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp Thu Aug 23 08:55:27 
2018
@@ -1014,11 +1014,13 @@ TEST(GoToInclude, All) {
 
   Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("5"));
   ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations, IsEmpty());
+  EXPECT_THAT(*Locations,
+  ElementsAre(Location{FooHUri, HeaderAnnotations.range()}));
 
   Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("7"));
   ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations, IsEmpty());
+  EXPECT_THAT(*Locations,
+  ElementsAre(Location{FooHUri, HeaderAnnotations.range()}));
 }
 
 TEST(GoToDefinition, WithPreamble) {


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51163: [clangd] Check for include overlapping looks for only the line now.

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340539: [clangd] Check for include overlapping looks for 
only the line now. (authored by kadircet, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D51163

Files:
  clang-tools-extra/trunk/clangd/XRefs.cpp
  clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp


Index: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
@@ -1014,11 +1014,13 @@
 
   Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("5"));
   ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations, IsEmpty());
+  EXPECT_THAT(*Locations,
+  ElementsAre(Location{FooHUri, HeaderAnnotations.range()}));
 
   Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("7"));
   ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations, IsEmpty());
+  EXPECT_THAT(*Locations,
+  ElementsAre(Location{FooHUri, HeaderAnnotations.range()}));
 }
 
 TEST(GoToDefinition, WithPreamble) {
Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -211,7 +211,7 @@
   std::vector Result;
   // Handle goto definition for #include.
   for (auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
-if (!Inc.Resolved.empty() && Inc.R.contains(Pos))
+if (!Inc.Resolved.empty() && Inc.R.start.line == Pos.line)
   Result.push_back(Location{URIForFile{Inc.Resolved}, {}});
   }
   if (!Result.empty())


Index: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
@@ -1014,11 +1014,13 @@
 
   Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("5"));
   ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations, IsEmpty());
+  EXPECT_THAT(*Locations,
+  ElementsAre(Location{FooHUri, HeaderAnnotations.range()}));
 
   Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("7"));
   ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations, IsEmpty());
+  EXPECT_THAT(*Locations,
+  ElementsAre(Location{FooHUri, HeaderAnnotations.range()}));
 }
 
 TEST(GoToDefinition, WithPreamble) {
Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -211,7 +211,7 @@
   std::vector Result;
   // Handle goto definition for #include.
   for (auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
-if (!Inc.Resolved.empty() && Inc.R.contains(Pos))
+if (!Inc.Resolved.empty() && Inc.R.start.line == Pos.line)
   Result.push_back(Location{URIForFile{Inc.Resolved}, {}});
   }
   if (!Result.empty())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r340541 - Re-land [ASTImporter] Add test for ObjCAtTryStmt/ObjCAtCatchStmt/ObjCAtThrowStmt

[Raphael Isemann via cfe-commits]

Author: teemperor
Date: Thu Aug 23 09:06:30 2018
New Revision: 340541

URL: http://llvm.org/viewvc/llvm-project?rev=340541&view=rev
Log:
Re-land [ASTImporter] Add test for ObjCAtTryStmt/ObjCAtCatchStmt/ObjCAtThrowStmt

Lands r340468 again, but this time we mark the test as unsupported on Windows
because it seems that try/catch crashes CodeGen at the moment.

Added:
cfe/trunk/test/Import/objc-try-catch/Inputs/F.m
cfe/trunk/test/Import/objc-try-catch/test.m

Added: cfe/trunk/test/Import/objc-try-catch/Inputs/F.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/objc-try-catch/Inputs/F.m?rev=340541&view=auto
==
--- cfe/trunk/test/Import/objc-try-catch/Inputs/F.m (added)
+++ cfe/trunk/test/Import/objc-try-catch/Inputs/F.m Thu Aug 23 09:06:30 2018
@@ -0,0 +1,28 @@
+@interface Exception
+@end
+@interface OtherException
+@end
+
+void f() {
+  @try {
+Exception *e;
+@throw e;
+  }
+  @catch (Exception *varname) {
+  }
+  @finally {
+  }
+
+  @try {
+  }
+  @catch (Exception *varname1) {
+@throw;
+  }
+  @catch (OtherException *varname2) {
+  }
+
+  @try {
+  }
+  @finally {
+  }
+}

Added: cfe/trunk/test/Import/objc-try-catch/test.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/objc-try-catch/test.m?rev=340541&view=auto
==
--- cfe/trunk/test/Import/objc-try-catch/test.m (added)
+++ cfe/trunk/test/Import/objc-try-catch/test.m Thu Aug 23 09:06:30 2018
@@ -0,0 +1,43 @@
+// RUN: clang-import-test -x objective-c++ -Xcc -fobjc-exceptions -dump-ast 
-import %S/Inputs/F.m -expression %s | FileCheck %s
+
+// FIXME: Seems that Objective-C try/catch crash codegen on Windows. Reenable 
once this is fixed.
+// UNSUPPORTED: system-windows
+
+// CHECK: ObjCAtTryStmt
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl
+// CHECK-NEXT: ObjCAtThrowStmt
+// CHECK-NEXT: ImplicitCastExpr
+// CHECK-NEXT: DeclRefExpr
+// CHECK-NEXT: ObjCAtCatchStmt
+// CHECK-NEXT: VarDecl
+// CHECK-SAME: varname
+// CHECK-SAME: 'Exception *'
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: ObjCAtFinallyStmt
+// CHECK-NEXT: CompoundStmt
+
+// CHECK-NEXT: ObjCAtTryStmt
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: ObjCAtCatchStmt
+// CHECK-NEXT: VarDecl
+// CHECK-SAME: varname1
+// CHECK-SAME: 'Exception *'
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: ObjCAtThrowStmt
+// CHECK-NEXT: <>
+// CHECK-NEXT: ObjCAtCatchStmt
+// CHECK-NEXT: VarDecl
+// CHECK-SAME: varname2
+// CHECK-SAME: 'OtherException *'
+// CHECK-NEXT: CompoundStmt
+
+// CHECK-NEXT: ObjCAtTryStmt
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: ObjCAtFinallyStmt
+// CHECK-NEXT: CompoundStmt
+
+void expr() {
+  f();
+}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r340483 - Revert "[ASTImporter] Add test for ObjCAtTryStmt/ObjCAtCatchStmt/ObjCAtThrowStmt"

[Raphael Isemann via cfe-commits]

Thanks! Looking at the list of failing bots that seems indeed to be
the most likely explanation.

- Raphael
Am Mi., 22. Aug. 2018 um 16:59 Uhr schrieb Shoaib Meenai :
>
> Might be because the constructed AST for a @finally on Windows will contain a 
> CapturedStmt: https://reviews.llvm.org/D47564. You probably want to 
> explicitly specify a non windows-msvc triple in the test.
>
>
>
> From: cfe-commits  on behalf of Raphael 
> Isemann via cfe-commits 
> Reply-To: Raphael Isemann 
> Date: Wednesday, August 22, 2018 at 4:51 PM
> To: "cfe-commits@lists.llvm.org" 
> Subject: r340483 - Revert "[ASTImporter] Add test for 
> ObjCAtTryStmt/ObjCAtCatchStmt/ObjCAtThrowStmt"
>
>
>
> Author: teemperor
>
> Date: Wed Aug 22 16:50:30 2018
>
> New Revision: 340483
>
>
>
> URL: 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D340483-26view-3Drev&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=SoW99711ISLe9RQ23xIpwyvcxXTd-Rxawo9VIXlpjuo&s=nc9gqViWXTB0N3ed5Zzdk_7AzYUFDgZpoxqOgBLI4nU&e=
>
> Log:
>
> Revert "[ASTImporter] Add test for 
> ObjCAtTryStmt/ObjCAtCatchStmt/ObjCAtThrowStmt"
>
>
>
> This test breaks llvm-clang-x86_64-expensive-checks-win.
>
>
>
> Removed:
>
> cfe/trunk/test/Import/objc-try-catch/Inputs/F.m
>
> cfe/trunk/test/Import/objc-try-catch/test.m
>
>
>
> Removed: cfe/trunk/test/Import/objc-try-catch/Inputs/F.m
>
> URL: 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_test_Import_objc-2Dtry-2Dcatch_Inputs_F.m-3Frev-3D340482-26view-3Dauto&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=SoW99711ISLe9RQ23xIpwyvcxXTd-Rxawo9VIXlpjuo&s=kFKOp3N2QmGe252ijaqC1sM6nD0j9xQCdVvuBjevcko&e=
>
> ==
>
> --- cfe/trunk/test/Import/objc-try-catch/Inputs/F.m (original)
>
> +++ cfe/trunk/test/Import/objc-try-catch/Inputs/F.m (removed)
>
> @@ -1,28 +0,0 @@
>
> -@interface Exception
>
> -@end
>
> -@interface OtherException
>
> -@end
>
> -
>
> -void f() {
>
> -  @try {
>
> -Exception *e;
>
> -@throw e;
>
> -  }
>
> -  @catch (Exception *varname) {
>
> -  }
>
> -  @finally {
>
> -  }
>
> -
>
> -  @try {
>
> -  }
>
> -  @catch (Exception *varname1) {
>
> -@throw;
>
> -  }
>
> -  @catch (OtherException *varname2) {
>
> -  }
>
> -
>
> -  @try {
>
> -  }
>
> -  @finally {
>
> -  }
>
> -}
>
>
>
> Removed: cfe/trunk/test/Import/objc-try-catch/test.m
>
> URL: 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_test_Import_objc-2Dtry-2Dcatch_test.m-3Frev-3D340482-26view-3Dauto&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=SoW99711ISLe9RQ23xIpwyvcxXTd-Rxawo9VIXlpjuo&s=q6vS8avd9ajINxEtZW7uL9FWfjCwl4TGy4e6NfUNkZI&e=
>
> ==
>
> --- cfe/trunk/test/Import/objc-try-catch/test.m (original)
>
> +++ cfe/trunk/test/Import/objc-try-catch/test.m (removed)
>
> @@ -1,40 +0,0 @@
>
> -// RUN: clang-import-test -x objective-c++ -Xcc -fobjc-exceptions -dump-ast 
> -import %S/Inputs/F.m -expression %s | FileCheck %s
>
> -
>
> -// CHECK: ObjCAtTryStmt
>
> -// CHECK-NEXT: CompoundStmt
>
> -// CHECK-NEXT: DeclStmt
>
> -// CHECK-NEXT: VarDecl
>
> -// CHECK-NEXT: ObjCAtThrowStmt
>
> -// CHECK-NEXT: ImplicitCastExpr
>
> -// CHECK-NEXT: DeclRefExpr
>
> -// CHECK-NEXT: ObjCAtCatchStmt
>
> -// CHECK-NEXT: VarDecl
>
> -// CHECK-SAME: varname
>
> -// CHECK-SAME: 'Exception *'
>
> -// CHECK-NEXT: CompoundStmt
>
> -// CHECK-NEXT: ObjCAtFinallyStmt
>
> -// CHECK-NEXT: CompoundStmt
>
> -
>
> -// CHECK-NEXT: ObjCAtTryStmt
>
> -// CHECK-NEXT: CompoundStmt
>
> -// CHECK-NEXT: ObjCAtCatchStmt
>
> -// CHECK-NEXT: VarDecl
>
> -// CHECK-SAME: varname1
>
> -// CHECK-SAME: 'Exception *'
>
> -// CHECK-NEXT: CompoundStmt
>
> -// CHECK-NEXT: ObjCAtThrowStmt
>
> -// CHECK-NEXT: <>
>
> -// CHECK-NEXT: ObjCAtCatchStmt
>
> -// CHECK-NEXT: VarDecl
>
> -// CHECK-SAME: varname2
>
> -// CHECK-SAME: 'OtherException *'
>
> -// CHECK-NEXT: CompoundStmt
>
> -
>
> -// CHECK-NEXT: ObjCAtTryStmt
>
> -// CHECK-NEXT: CompoundStmt
>
> -// CHECK-NEXT: ObjCAtFinallyStmt
>
> -// CHECK-NEXT: CompoundStmt
>
> -
>
> -void expr() {
>
> -  f();
>
> -}
>
>
>
>
>
> ___
>
> cfe-commits mailing list
>
> cfe-commits@lists.llvm.org
>
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=SoW99711ISLe9RQ23xIpwyvcxXTd-Rxawo9VIXlpjuo&s=7OwU_LMC-xQGUKoTBrJZVj4QNfVYAIM5ZZDXF_RS9oc&e=
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51057: [analyzer][UninitializedObjectChecker] Fixed dereferencing

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 162198.
Szelethus added a comment.

Addressed the inline comments.

- Added a `llvm::SmallSet` to further ensure the termination of the loop.
- Changed `isLocType` to `isDereferencableType`, block pointers are regarded as 
primitive objects.
- Added new tests


https://reviews.llvm.org/D51057

Files:
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
  test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
  test/Analysis/objcpp-uninitialized-object.mm

Index: test/Analysis/objcpp-uninitialized-object.mm
===
--- test/Analysis/objcpp-uninitialized-object.mm
+++ test/Analysis/objcpp-uninitialized-object.mm
@@ -4,7 +4,7 @@
 
 struct StructWithBlock {
   int a;
-  myBlock z; // expected-note{{uninitialized pointer 'this->z'}}
+  myBlock z; // expected-note{{uninitialized field 'this->z'}}
 
   StructWithBlock() : a(0), z(^{}) {}
 
Index: test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
===
--- test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
+++ test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
@@ -46,6 +46,50 @@
 }
 
 //===--===//
+// Alloca tests.
+//===--===//
+
+struct UntypedAllocaTest {
+  void *allocaPtr;
+  int dontGetFilteredByNonPedanticMode = 0;
+
+  UntypedAllocaTest() : allocaPtr(__builtin_alloca(sizeof(int))) {
+// All good!
+  }
+};
+
+void fUntypedAllocaTest() {
+  UntypedAllocaTest();
+}
+
+struct TypedAllocaTest1 {
+  int *allocaPtr; // expected-note{{uninitialized pointee 'this->allocaPtr'}}
+  int dontGetFilteredByNonPedanticMode = 0;
+
+  TypedAllocaTest1() // expected-warning{{1 uninitialized field}}
+  : allocaPtr(static_cast(__builtin_alloca(sizeof(int {}
+};
+
+void fTypedAllocaTest1() {
+  TypedAllocaTest1();
+}
+
+struct TypedAllocaTest2 {
+  int *allocaPtr;
+  int dontGetFilteredByNonPedanticMode = 0;
+
+  TypedAllocaTest2()
+  : allocaPtr(static_cast(__builtin_alloca(sizeof(int {
+*allocaPtr = 5;
+// All good!
+  }
+};
+
+void fTypedAllocaTest2() {
+  TypedAllocaTest2();
+}
+
+//===--===//
 // Heap pointer tests.
 //===--===//
 
@@ -203,18 +247,14 @@
   CyclicPointerTest1();
 }
 
-// TODO: Currently, the checker ends up in an infinite loop for the following
-// test case.
-/*
 struct CyclicPointerTest2 {
-  int **pptr;
+  int **pptr; // no-crash
   CyclicPointerTest2() : pptr(reinterpret_cast(&pptr)) {}
 };
 
 void fCyclicPointerTest2() {
   CyclicPointerTest2();
 }
-*/
 
 //===--===//
 // Void pointer tests.
@@ -645,6 +685,15 @@
   CyclicList(&n1, int());
 }
 
+struct RingListTest {
+  RingListTest *next; // no-crash
+  RingListTest() : next(this) {}
+};
+
+void fRingListTest() {
+  RingListTest();
+}
+
 //===--===//
 // Tests for classes containing references.
 //===--===//
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
@@ -95,11 +95,13 @@
 /// known, and thus FD can not be analyzed.
 static bool isVoidPointer(QualType T);
 
-/// Dereferences \p V and returns the value and dynamic type of the pointee, as
-/// well as wether \p FR needs to be casted back to that type. If for whatever
-/// reason dereferencing fails, returns with None.
-static llvm::Optional>
-dereference(ProgramStateRef State, const FieldRegion *FR);
+using DereferenceInfo = std::pair;
+
+/// Dereferences \p FR and returns with the pointee's region, and whether it
+/// needs to be casted back to it's location type. If for whatever reason
+/// dereferencing fails, returns with None.
+static llvm::Optional dereference(ProgramStateRef State,
+   const FieldRegion *FR);
 
 //===--===//
 //   Methods for FindUninitializedFields.
@@ -110,10 +112,8 @@
 bool FindUninitializedFields::isPointerOrReferenceUninit(
 const FieldRegion *FR, FieldChainInfo LocalChain) {
 
-  assert((FR->getDecl()->getType()->isAnyPointerType() ||
-  FR->getDecl()->getType()->isReferenceType() ||
-  FR->getDecl

[PATCH] D51170: [libc++] Remove race condition in std::async

[Louis Dionne via Phabricator via cfe-commits]

ldionne created this revision.
ldionne added a reviewer: mclow.lists.
Herald added a reviewer: EricWF.
Herald added subscribers: cfe-commits, dexonsmith, christof.

The state associated to the future was set in one thread (with synchronization)
but read in another thread without synchronization, which led to a data race.

https://bugs.llvm.org/show_bug.cgi?id=38682
rdar://problem/42548261


Repository:
  rCXX libc++

https://reviews.llvm.org/D51170

Files:
  libcxx/include/future
  libcxx/src/future.cpp
  libcxx/test/std/thread/futures/futures.async/async_race.38682.pass.cpp

Index: libcxx/test/std/thread/futures/futures.async/async_race.38682.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/thread/futures/futures.async/async_race.38682.pass.cpp
@@ -0,0 +1,58 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// UNSUPPORTED: libcpp-has-no-threads
+// UNSUPPORTED: c++98, c++03
+
+// This test is designed to cause and allow TSAN to detect a race condition
+// in std::async, as reported in https://bugs.llvm.org/show_bug.cgi?id=38682.
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+
+static int worker(std::vector const& data) {
+  return std::accumulate(data.begin(), data.end(), 0);
+}
+
+static int& worker_ref(int& i) { return i; }
+
+static void worker_void() { }
+
+int main() {
+  // future
+  {
+std::vector const v{1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
+for (int i = 0; i != 20; ++i) {
+  std::future fut = std::async(std::launch::async, worker, v);
+  int answer = fut.get();
+  assert(answer == 55);
+}
+  }
+
+  // future
+  {
+for (int i = 0; i != 20; ++i) {
+  std::future fut = std::async(std::launch::async, worker_ref, std::ref(i));
+  int& answer = fut.get();
+  assert(answer == i);
+}
+  }
+
+  // future
+  {
+for (int i = 0; i != 20; ++i) {
+  std::future fut = std::async(std::launch::async, worker_void);
+  fut.get();
+}
+  }
+}
Index: libcxx/src/future.cpp
===
--- libcxx/src/future.cpp
+++ libcxx/src/future.cpp
@@ -179,10 +179,7 @@
 future::future(__assoc_sub_state* __state)
 : __state_(__state)
 {
-if (__state_->__has_future_attached())
-__throw_future_error(future_errc::future_already_retrieved);
-__state_->__add_shared();
-__state_->__set_future_attached();
+__state_->__attach_future();
 }
 
 future::~future()
Index: libcxx/include/future
===
--- libcxx/include/future
+++ libcxx/include/future
@@ -556,13 +556,14 @@
 {return (__state_ & __constructed) || (__exception_ != nullptr);}
 
 _LIBCPP_INLINE_VISIBILITY
-void __set_future_attached()
-{
+void __attach_future() {
 lock_guard __lk(__mut_);
+bool __has_future_attached = (__state_ & __future_attached) != 0;
+if (__has_future_attached)
+__throw_future_error(future_errc::future_already_retrieved);
+this->__add_shared();
 __state_ |= __future_attached;
 }
-_LIBCPP_INLINE_VISIBILITY
-bool __has_future_attached() const {return (__state_ & __future_attached) != 0;}
 
 _LIBCPP_INLINE_VISIBILITY
 void __set_deferred() {__state_ |= deferred;}
@@ -1154,10 +1155,7 @@
 future<_Rp>::future(__assoc_state<_Rp>* __state)
 : __state_(__state)
 {
-if (__state_->__has_future_attached())
-__throw_future_error(future_errc::future_already_retrieved);
-__state_->__add_shared();
-__state_->__set_future_attached();
+__state_->__attach_future();
 }
 
 struct __release_shared_count
@@ -1257,10 +1255,7 @@
 future<_Rp&>::future(__assoc_state<_Rp&>* __state)
 : __state_(__state)
 {
-if (__state_->__has_future_attached())
-__throw_future_error(future_errc::future_already_retrieved);
-__state_->__add_shared();
-__state_->__set_future_attached();
+__state_->__attach_future();
 }
 
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:61
+/// rely on MR use-case to work properly.
+llvm::cl::init(false));
+

ioeric wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > ilya-biryukov wrote:
> > > > ioeric wrote:
> > > > > AFAIK, this flag would basically depend on the executor being used. 
> > > > > If executor can provide information like whether all tasks share 
> > > > > memory, we can make the decision totally based on the selected 
> > > > > executor (i.e. one fewer option).
> > > > That SG. So we could add a virtual method to executor that would 
> > > > provide us with this information and then we can get rid of that 
> > > > option, does that LG?
> > > > However, do we have non-single-binary executor implementations 
> > > > upstream? We need to leave a way to run the code that actually uses 
> > > > YAML serialization to make sure we can experiment when things break.
> > > Sounds good.
> > > 
> > > > However, do we have non-single-binary executor implementations 
> > > > upstream? We need to leave a way to run the code that actually uses 
> > > > YAML serialization to make sure we can experiment when things break.
> > > The framework allows this, so I'm not sure if there is other downstream 
> > > implementations that do this (probably not?). But I don't think we can 
> > > get rid of the YAML serialization at this point because we still dump 
> > > everything to yaml in the end? 
> > I meant the intermediate YAML serialization-deserialization parts.
> > 
> > I'd keep an option to force using the consumer that accumulates YAML 
> > symbols, and only allow to use on-the-fly merge when the tool executor 
> > reports that it is single-process. Would allow to:
> > - Avoid running on-the-fly merge when the executor does not support it 
> > (multi-binary MRs, etc)
> > - Allow to use on-the-fly by default for executors that run in the single 
> > binary.
> > - Allow to run with intermediate serialization even in a single binary if 
> > `-merge-on-the-fly` is specified.
> > 
> > Does that LG?
> IIUC, there will basically be a flag that allows us to force using yaml 
> intermediate serialization, even when the executor supports on-the-fly 
> merging. By default, we use on-the-fly merging whenever possible. If that's 
> the case, SGTM.
The flag is still called `-merge-on-the-fly`, but it's true by default and can 
be set to false to reenable intermediate YAML serialization


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51155



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51057: [analyzer][UninitializedObjectChecker] Fixed dereferencing

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus marked 5 inline comments as done.
Szelethus added inline comments.



Comment at: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h:259
+  return T->isBuiltinType() || T->isEnumeralType() ||
+ T->isMemberPointerType() || T->isBlockPointerType();
+}

I'm not sure this is correct -- do block pointers belong here? Since their 
region is not `TypedValueRegion`, I though they better fit here.



Comment at: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h:261-265
+inline bool isLocType(const QualType &T) {
+  return T->isAnyPointerType() || T->isReferenceType() ||
+ T->isBlockPointerType();
+}
+

NoQ wrote:
> We have a fancy static `Loc::isLocType()`.
Oh, good to know! However, it also returns true for `nullptr_t`, which also 
happens to be a `BuiltinType`. I'd like to keep `isPrimitiveType` and (the now 
renamed) `isDereferencableType` categories disjunctive. Primitive types require 
no further analysis other then checking whether they are initialized or not, 
which is true for `nullptr_t` objects.



Comment at: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:240-244
+if (Tmp->getRegion()->getSymbolicBase())
   return None;
-}
 
-V = State->getSVal(*Tmp, DynT);
+DynT = DynT->getPointeeType();
+R = Tmp->getRegionAs();

NoQ wrote:
> This code seems to be duplicated with the "0th iteration" before the loop. I 
> guess you can put everything into the loop.
I moved some code into the loop, but I think that I really need a 0th iteration 
to make the code readable.


https://reviews.llvm.org/D51057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

[Eric Liu via Phabricator via cfe-commits]

ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.

lgtm




Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:60
+"MapReduce."),
+llvm::cl::init(true));
+

Maybe make this hidden?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51155



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50892: [analyzer][UninitializedObjectChecker] Correct dynamic type is acquired for record pointees

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 162205.
Szelethus added a comment.

Rebased to https://reviews.llvm.org/D51057.


https://reviews.llvm.org/D50892

Files:
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
  test/Analysis/cxx-uninitialized-object-inheritance.cpp


Index: test/Analysis/cxx-uninitialized-object-inheritance.cpp
===
--- test/Analysis/cxx-uninitialized-object-inheritance.cpp
+++ test/Analysis/cxx-uninitialized-object-inheritance.cpp
@@ -781,21 +781,38 @@
 // Dynamic type test.
 
//===--===//
 
-struct DynTBase {};
-struct DynTDerived : DynTBase {
-  // TODO: we'd expect the note: {{uninitialized field 'this->x'}}
-  int x; // no-note
+struct DynTBase1 {};
+struct DynTDerived1 : DynTBase1 {
+  int y; // expected-note{{uninitialized field 'this->bptr->y'}}
 };
 
-struct DynamicTypeTest {
-  DynTBase *bptr;
+struct DynamicTypeTest1 {
+  DynTBase1 *bptr;
   int i = 0;
 
-  // TODO: we'd expect the warning: {{1 uninitialized field}}
-  DynamicTypeTest(DynTBase *bptr) : bptr(bptr) {} // no-warning
+  DynamicTypeTest1(DynTBase1 *bptr) : bptr(bptr) {} // expected-warning{{1 
uninitialized field}}
 };
 
-void f() {
-  DynTDerived d;
-  DynamicTypeTest t(&d);
+void fDynamicTypeTest1() {
+  DynTDerived1 d;
+  DynamicTypeTest1 t(&d);
 };
+
+struct DynTBase2 {
+  int x; // expected-note{{uninitialized field 'this->bptr->DynTBase2::x'}}
+};
+struct DynTDerived2 : DynTBase2 {
+  int y; // expected-note{{uninitialized field 'this->bptr->y'}}
+};
+
+struct DynamicTypeTest2 {
+  DynTBase2 *bptr;
+  int i = 0;
+
+  DynamicTypeTest2(DynTBase2 *bptr) : bptr(bptr) {} // expected-warning{{2 
uninitialized fields}}
+};
+
+void fDynamicTypeTest2() {
+  DynTDerived2 d;
+  DynamicTypeTest2 t(&d);
+}
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
@@ -240,5 +240,9 @@
   break;
   }
 
+  while (R->getAs()) {
+R = R->getSuperRegion()->getAs();
+  }
+
   return std::make_pair(R, NeedsCastBack);
 }


Index: test/Analysis/cxx-uninitialized-object-inheritance.cpp
===
--- test/Analysis/cxx-uninitialized-object-inheritance.cpp
+++ test/Analysis/cxx-uninitialized-object-inheritance.cpp
@@ -781,21 +781,38 @@
 // Dynamic type test.
 //===--===//
 
-struct DynTBase {};
-struct DynTDerived : DynTBase {
-  // TODO: we'd expect the note: {{uninitialized field 'this->x'}}
-  int x; // no-note
+struct DynTBase1 {};
+struct DynTDerived1 : DynTBase1 {
+  int y; // expected-note{{uninitialized field 'this->bptr->y'}}
 };
 
-struct DynamicTypeTest {
-  DynTBase *bptr;
+struct DynamicTypeTest1 {
+  DynTBase1 *bptr;
   int i = 0;
 
-  // TODO: we'd expect the warning: {{1 uninitialized field}}
-  DynamicTypeTest(DynTBase *bptr) : bptr(bptr) {} // no-warning
+  DynamicTypeTest1(DynTBase1 *bptr) : bptr(bptr) {} // expected-warning{{1 uninitialized field}}
 };
 
-void f() {
-  DynTDerived d;
-  DynamicTypeTest t(&d);
+void fDynamicTypeTest1() {
+  DynTDerived1 d;
+  DynamicTypeTest1 t(&d);
 };
+
+struct DynTBase2 {
+  int x; // expected-note{{uninitialized field 'this->bptr->DynTBase2::x'}}
+};
+struct DynTDerived2 : DynTBase2 {
+  int y; // expected-note{{uninitialized field 'this->bptr->y'}}
+};
+
+struct DynamicTypeTest2 {
+  DynTBase2 *bptr;
+  int i = 0;
+
+  DynamicTypeTest2(DynTBase2 *bptr) : bptr(bptr) {} // expected-warning{{2 uninitialized fields}}
+};
+
+void fDynamicTypeTest2() {
+  DynTDerived2 d;
+  DynamicTypeTest2 t(&d);
+}
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
@@ -240,5 +240,9 @@
   break;
   }
 
+  while (R->getAs()) {
+R = R->getSuperRegion()->getAs();
+  }
+
   return std::make_pair(R, NeedsCastBack);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

The discussion kind of moved away from the original patch, probably because the 
problem is larger than the defition of some host macros. However I still think 
that this patch improves the situation.


Repository:
  rC Clang

https://reviews.llvm.org/D50845



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51172: [libcxx] Comment out #define __cpp_lib_node_extract, we only support half of that functionality

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: ldionne, mclow.lists, hans.
Herald added a reviewer: EricWF.
Herald added subscribers: dexonsmith, christof.

The other half of this is in https://reviews.llvm.org/D48896, so we shouldn't 
claim that we support this feature. This should be cherry-picked into the 7.0 
release, since I believe the first half landed before the branch point.


Repository:
  rCXX libc++

https://reviews.llvm.org/D51172

Files:
  libcxx/include/__node_handle


Index: libcxx/include/__node_handle
===
--- libcxx/include/__node_handle
+++ libcxx/include/__node_handle
@@ -26,7 +26,8 @@
 
 #if _LIBCPP_STD_VER > 14
 
-#define __cpp_lib_node_extract 201606L
+// FIXME: Uncomment this when we support the 'merge' functionality.
+// #define __cpp_lib_node_extract 201606L
 
 // Specialized in __tree & __hash_table for their _NodeType.
 template 


Index: libcxx/include/__node_handle
===
--- libcxx/include/__node_handle
+++ libcxx/include/__node_handle
@@ -26,7 +26,8 @@
 
 #if _LIBCPP_STD_VER > 14
 
-#define __cpp_lib_node_extract 201606L
+// FIXME: Uncomment this when we support the 'merge' functionality.
+// #define __cpp_lib_node_extract 201606L
 
 // Specialized in __tree & __hash_table for their _NodeType.
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51172: [libcxx] Comment out #define __cpp_lib_node_extract, we only support half of that functionality

[Louis Dionne via Phabricator via cfe-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

LGTM. Don't forget to update https://reviews.llvm.org/D48896 so it uncomments 
this. Also, this should be merged into LLVM 7.


Repository:
  rCXX libc++

https://reviews.llvm.org/D51172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r340544 - Comment out #define __cpp_lib_node_extract, we only support half of that functionality

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Thu Aug 23 10:08:02 2018
New Revision: 340544

URL: http://llvm.org/viewvc/llvm-project?rev=340544&view=rev
Log:
Comment out #define __cpp_lib_node_extract, we only support half of that 
functionality

Differential revision: https://reviews.llvm.org/D51172

Modified:
libcxx/trunk/include/__node_handle

Modified: libcxx/trunk/include/__node_handle
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__node_handle?rev=340544&r1=340543&r2=340544&view=diff
==
--- libcxx/trunk/include/__node_handle (original)
+++ libcxx/trunk/include/__node_handle Thu Aug 23 10:08:02 2018
@@ -26,7 +26,8 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 #if _LIBCPP_STD_VER > 14
 
-#define __cpp_lib_node_extract 201606L
+// FIXME: Uncomment this when we support the 'merge' functionality.
+// #define __cpp_lib_node_extract 201606L
 
 // Specialized in __tree & __hash_table for their _NodeType.
 template 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51172: [libcxx] Comment out #define __cpp_lib_node_extract, we only support half of that functionality

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCXX340544: Comment out #define __cpp_lib_node_extract, we 
only support half of that… (authored by epilk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51172?vs=162208&id=162215#toc

Repository:
  rCXX libc++

https://reviews.llvm.org/D51172

Files:
  include/__node_handle


Index: include/__node_handle
===
--- include/__node_handle
+++ include/__node_handle
@@ -26,7 +26,8 @@
 
 #if _LIBCPP_STD_VER > 14
 
-#define __cpp_lib_node_extract 201606L
+// FIXME: Uncomment this when we support the 'merge' functionality.
+// #define __cpp_lib_node_extract 201606L
 
 // Specialized in __tree & __hash_table for their _NodeType.
 template 


Index: include/__node_handle
===
--- include/__node_handle
+++ include/__node_handle
@@ -26,7 +26,8 @@
 
 #if _LIBCPP_STD_VER > 14
 
-#define __cpp_lib_node_extract 201606L
+// FIXME: Uncomment this when we support the 'merge' functionality.
+// #define __cpp_lib_node_extract 201606L
 
 // Specialized in __tree & __hash_table for their _NodeType.
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51084: Implement -Watomic-implicit-seq-cst

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:4924
+<< Callee->getSourceRange();
+  }
+

Why is the diagnostic at the end location?  And why are you separately checking 
whether it's ignored at the begin location?



Comment at: lib/Sema/SemaChecking.cpp:10003
+static void DiagnoseImplicitAtomicSeqCst(Sema &S, Expr *E) {
+  if (S.Diags.isIgnored(diag::warn_atomic_implicit_seq_cst, E->getBeginLoc()))
+return;

`isIgnored` is not actually a cheap operation; you should *definitely* be 
checking for atomic types first.  And usually we just fire off the diagnostic 
without checking `isIgnored` because the setup cost of a diagnostic is not 
assumed to be that high.


Repository:
  rC Clang

https://reviews.llvm.org/D51084



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51172: [libcxx] Comment out #define __cpp_lib_node_extract, we only support half of that functionality

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

Landed as r340544. @hans: Can you cherry-pick?


Repository:
  rCXX libc++

https://reviews.llvm.org/D51172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst:16
+
+   string s = StrCat("A", StrCat("B", StrCat("C", "D")));
+   ==> string s = StrCat("A", "B", "C", "D");

Please add namespaces and use empty line instead of ==>


https://reviews.llvm.org/D51132



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r340547 - [ASTMatchers] Let hasObjectExpression also support UnresolvedMemberExpr, CXXDependentScopeMemberExpr

[Shuai Wang via cfe-commits]

Author: shuaiwang
Date: Thu Aug 23 10:16:06 2018
New Revision: 340547

URL: http://llvm.org/viewvc/llvm-project?rev=340547&view=rev
Log:
[ASTMatchers] Let hasObjectExpression also support UnresolvedMemberExpr, 
CXXDependentScopeMemberExpr

Reviewers: aaron.ballman

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D50617

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=340547&r1=340546&r2=340547&view=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Thu Aug 23 10:16:06 2018
@@ -4668,6 +4668,20 @@ with withInitializer matching (1)
 
 
 
+MatcherCXXDependentScopeMemberExpr>hasObjectExpressionMatcherExpr> 
InnerMatcher
+Matches a 
member expression where the object expression is
+matched by a given matcher.
+
+Given
+  struct X { int m; };
+  void f(X x) { x.m; m; }
+memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")))
+  matches "x.m" and "m"
+with hasObjectExpression(...)
+  matching "x" and the implicit object expression of "m" which has type X*.
+
+
+
 MatcherCXXForRangeStmt>hasBodyMatcherStmt> 
InnerMatcher
 Matches a 'for', 'while', 
'do while' statement or a function
 definition that has a given body.
@@ -6692,6 +6706,20 @@ Example matches true (matcher = hasUnary
 
 
 
+MatcherUnresolvedMemberExpr>hasObjectExpressionMatcherExpr> 
InnerMatcher
+Matches a 
member expression where the object expression is
+matched by a given matcher.
+
+Given
+  struct X { int m; };
+  void f(X x) { x.m; m; }
+memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")))
+  matches "x.m" and "m"
+with hasObjectExpression(...)
+  matching "x" and the implicit object expression of "m" which has type X*.
+
+
+
 MatcherUnresolvedUsingType>hasDeclarationconst MatcherDecl>  
InnerMatcher
 Matches a node if 
the declaration associated with that node
 matches the given matcher.

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=340547&r1=340546&r2=340547&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Thu Aug 23 10:16:06 2018
@@ -4825,8 +4825,17 @@ AST_MATCHER_P(MemberExpr, member,
 ///   matches "x.m" and "m"
 /// with hasObjectExpression(...)
 ///   matching "x" and the implicit object expression of "m" which has type X*.
-AST_MATCHER_P(MemberExpr, hasObjectExpression,
-  internal::Matcher, InnerMatcher) {
+AST_POLYMORPHIC_MATCHER_P(
+hasObjectExpression,
+AST_POLYMORPHIC_SUPPORTED_TYPES(MemberExpr, UnresolvedMemberExpr,
+CXXDependentScopeMemberExpr),
+internal::Matcher, InnerMatcher) {
+  if (const auto *E = dyn_cast(&Node))
+if (E->isImplicitAccess())
+  return false;
+  if (const auto *E = dyn_cast(&Node))
+if (E->isImplicitAccess())
+  return false;
   return InnerMatcher.matches(*Node.getBase(), Finder, Builder);
 }
 

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp?rev=340547&r1=340546&r2=340547&view=diff
==
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp Thu Aug 23 
10:16:06 2018
@@ -1517,6 +1517,26 @@ TEST(HasObjectExpression, MatchesBaseOfV
 "struct X { int m; }; void f(X* x) { x->m; }",
 memberExpr(hasObjectExpression(
   hasType(pointsTo(recordDecl(hasName("X";
+  EXPECT_TRUE(matches("template  struct X { void f() { T t; t.m; } 
};",
+  cxxDependentScopeMemberExpr(hasObjectExpression(
+  declRefExpr(to(namedDecl(hasName("t";
+  EXPECT_TRUE(
+  matches("template  struct X { void f() { T t; t->m; } };",
+  cxxDependentScopeMemberExpr(has

[PATCH] D50617: [ASTMatchers] Let hasObjectExpression also support UnresolvedMemberExpr, CXXDependentScopeMemberExpr

[Shuai Wang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340547: [ASTMatchers] Let hasObjectExpression also support 
UnresolvedMemberExpr… (authored by shuaiwang, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50617?vs=161830&id=162218#toc

Repository:
  rC Clang

https://reviews.llvm.org/D50617

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1517,6 +1517,26 @@
 "struct X { int m; }; void f(X* x) { x->m; }",
 memberExpr(hasObjectExpression(
   hasType(pointsTo(recordDecl(hasName("X";
+  EXPECT_TRUE(matches("template  struct X { void f() { T t; t.m; } };",
+  cxxDependentScopeMemberExpr(hasObjectExpression(
+  declRefExpr(to(namedDecl(hasName("t";
+  EXPECT_TRUE(
+  matches("template  struct X { void f() { T t; t->m; } };",
+  cxxDependentScopeMemberExpr(hasObjectExpression(
+  declRefExpr(to(namedDecl(hasName("t";
+}
+
+TEST(HasObjectExpression, MatchesBaseOfMemberFunc) {
+  EXPECT_TRUE(matches(
+  "struct X { void f(); }; void g(X x) { x.f(); }",
+  memberExpr(hasObjectExpression(hasType(recordDecl(hasName("X")));
+  EXPECT_TRUE(matches("struct X { template  void f(); };"
+  "template  void g(X x) { x.f(); }",
+  unresolvedMemberExpr(hasObjectExpression(
+  hasType(recordDecl(hasName("X")));
+  EXPECT_TRUE(matches("template  void f(T t) { t.g(); }",
+  cxxDependentScopeMemberExpr(hasObjectExpression(
+  declRefExpr(to(namedDecl(hasName("t";
 }
 
 TEST(HasObjectExpression,
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -4668,6 +4668,20 @@
 
 
 
+MatcherCXXDependentScopeMemberExpr>hasObjectExpressionMatcherExpr> InnerMatcher
+Matches a member expression where the object expression is
+matched by a given matcher.
+
+Given
+  struct X { int m; };
+  void f(X x) { x.m; m; }
+memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")))
+  matches "x.m" and "m"
+with hasObjectExpression(...)
+  matching "x" and the implicit object expression of "m" which has type X*.
+
+
+
 MatcherCXXForRangeStmt>hasBodyMatcherStmt> InnerMatcher
 Matches a 'for', 'while', 'do while' statement or a function
 definition that has a given body.
@@ -6692,6 +6706,20 @@
 
 
 
+MatcherUnresolvedMemberExpr>hasObjectExpressionMatcherExpr> InnerMatcher
+Matches a member expression where the object expression is
+matched by a given matcher.
+
+Given
+  struct X { int m; };
+  void f(X x) { x.m; m; }
+memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")))
+  matches "x.m" and "m"
+with hasObjectExpression(...)
+  matching "x" and the implicit object expression of "m" which has type X*.
+
+
+
 MatcherUnresolvedUsingType>hasDeclarationconst MatcherDecl>  InnerMatcher
 Matches a node if the declaration associated with that node
 matches the given matcher.
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -4825,8 +4825,17 @@
 ///   matches "x.m" and "m"
 /// with hasObjectExpression(...)
 ///   matching "x" and the implicit object expression of "m" which has type X*.
-AST_MATCHER_P(MemberExpr, hasObjectExpression,
-  internal::Matcher, InnerMatcher) {
+AST_POLYMORPHIC_MATCHER_P(
+hasObjectExpression,
+AST_POLYMORPHIC_SUPPORTED_TYPES(MemberExpr, UnresolvedMemberExpr,
+CXXDependentScopeMemberExpr),
+internal::Matcher, InnerMatcher) {
+  if (const auto *E = dyn_cast(&Node))
+if (E->isImplicitAccess())
+  return false;
+  if (const auto *E = dyn_cast(&Node))
+if (E->isImplicitAccess())
+  return false;
   return InnerMatcher.matches(*Node.getBase(), Finder, Builder);
 }
 
___
cfe

[PATCH] D50892: [analyzer][UninitializedObjectChecker] Correct dynamic type is acquired for record pointees

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:187-191
   // If FR is a pointer pointing to a non-primitive type.
   if (Optional RecordV =
   DerefdV.getAs()) {
 
 const TypedValueRegion *R = RecordV->getRegion();

NoQ wrote:
> Szelethus wrote:
> > NoQ wrote:
> > > This looks like one more situation where we dereference a location to get 
> > > a value and then struggle to get back to the location that we've 
> > > dereferenced by looking at the value. Can we just use `V`?
> > I've struggled with derefencing for months now -- I'm afraid I just don't 
> > really get what you'd like to see here.
> > 
> > Here's what I attempted to implement:
> > I'd like to obtain the pointee's region of a `Loc` region, even if it has 
> > to be casted to another type, like through void pointers and 
> > `nonloc::LocAsInteger`, and continue analysis on said region as usual.
> > 
> > The trickiest part I can't seem to get right is the acquisition of the 
> > pointee region. For the problem this patch attempts to solve, even though 
> > `DynT` correctly says that the dynamic type is `DynTDerived2 *`, `DerefdV` 
> > contains a region for `DynTBase`.
> > 
> > I uploaded a new patch, D51057, which hopefully settles derefence related 
> > issues. Please note that it **does not **replace this diff, as the acquired 
> > region is still of type `DynTBase`.
> > 
> > I find understanding these intricate details of the analyzer very 
> > difficult, as I found very little documentation about how this works, which 
> > often left me guessing what the proper way to do this is. Can you recommend 
> > some literature for me on this field?
> > Can you recommend some literature for me on this field?
> 
> This is pretty specific to our analyzer. `SVal`/`SymExpr`/`MemRegion` 
> hierarchy is tightly coupled to implementation details of the `RegionStore`, 
> which is our memory model. There's a paper on it [1]. We have some in-tree 
> documentation of the `RegionStore` [2] (other docs there are also interesting 
> to read). And there's my old workbook [3]. And i guess that's it.
> 
> [1] Xu, Zhongxing & Kremenek, Ted & Zhang, Jian. (2010). A Memory Model for 
> Static Analysis of C Programs. 535-548. 10.1007/978-3-642-16558-0_44.
> [2] 
> https://github.com/llvm-mirror/clang/blob/master/docs/analyzer/RegionStore.txt
> [3] 
> https://github.com/haoNoQ/clang-analyzer-guide/releases/download/v0.1/clang-analyzer-guide-v0.1.pdf
Thank you! I'm aware of these works, and have read them already.

The actual implementation of the analyzer is most described in your book, and 
I've often got the best ideas from that. However, some very well described 
things in there have absolutely no documentation in the actual code -- for 
example, it isn't obvious at all to me what `CompundValue` is, and I was 
surprised to learn what it does from your guide. Other examples are the 
difference between `TypedValueRegion`s `getLocationType` and `getValueType`, 
`SymExpr` in general, and so on.

I think it would also be very valuable to have a link in `docs/analyzer` to 
your book. On another note, would you mind if I were to put some of the 
information from there into the actual code?



Comment at: test/Analysis/cxx-uninitialized-object-inheritance.cpp:787
   // TODO: we'd expect the note: {{uninitialized field 'this->x'}}
   int x; // no-note
 };

NoQ wrote:
> Szelethus wrote:
> > The checker should be able to catch this one -- for some reason it is 
> > regarded as an unknown region. Odd, as the test case right after this one 
> > works perfectly.
> There's a variety of problems we have with empty base classes, might be one 
> of those, and they are usually easy to fix because, well, yes it's a special 
> case, but it's also an extremely simple case.
> 
> I encourage you to open up the Exploded Graph and study it carefully to see 
> what and where goes wrong (not for this revision).
I'd love to learn about other parts of the analyzer (besides chasing pointers), 
but for now, this seems to have fixed itself ;)


https://reviews.llvm.org/D50892



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162219.

https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char &operator[](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view a, string_view b);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string &str);
+  AlphaNum(const string_view &pc);
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum &operator=(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum &a);
+string StrCat(const AlphaNum &a, const AlphaNum &b);
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c);
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c,
+  const AlphaNum &d);
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c,
+  const AlphaNum &d, const AlphaNum &e, const AV &... args);
+
+void StrAppend(string *dest, const AlphaNum &a);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c, const AlphaNum &d);
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c, const AlphaNum &d, const AlphaNum &e,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, StrCat("A", StrCat(1.1)));
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: redundant StrCat calls
+
+  S 

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst:16
+
+   string s = absl::StrCat("A", absl::StrCat("B", absll::StrCat("C", 
"D")));
+   string s = absl::StrCat("A", "B", "C", "D");

std::string. Please insert empty line.



Comment at: docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst:17
+   string s = absl::StrCat("A", absl::StrCat("B", absll::StrCat("C", 
"D")));
+   string s = absl::StrCat("A", "B", "C", "D");
+   absl::StrAppend(&s, absl::StrCat("E", "F", "G"));

std::string. Same indentation as in previous line. Same for second example.


https://reviews.llvm.org/D51132



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50962: [clangd] Speculative code completion index request before Sema is run.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 162221.
ioeric marked 5 inline comments as done.
ioeric added a comment.
Herald added subscribers: jfb, javed.absar.

- Moved most logic into CodeComplete.cc


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50962

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/TUScheduler.h
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -923,7 +923,11 @@
llvm::function_ref
Callback) const override {}
 
-  const std::vector allRequests() const { return Requests; }
+  const std::vector consumeRequests() const {
+auto Reqs = std::move(Requests);
+Requests.clear();
+return Reqs;
+  }
 
 private:
   mutable std::vector Requests;
@@ -934,7 +938,7 @@
   IndexRequestCollector Requests;
   Opts.Index = &Requests;
   completions(Code, {}, Opts);
-  return Requests.allRequests();
+  return Requests.consumeRequests();
 }
 
 TEST(CompletionTest, UnqualifiedIdQuery) {
@@ -1705,6 +1709,75 @@
 Not(Contains(Labeled("void vfunc(bool param) override");
 }
 
+TEST(SpeculateCompletionFilter, Filters) {
+  Annotations F(R"cpp($bof^
+  $bol^
+  ab$ab^
+  x.ab$dot^
+  x.$dotempty^
+  x::ab$scoped^
+  x::$scopedempty^
+
+  )cpp");
+  auto speculate = [&](StringRef PointName) {
+auto Filter = speculateCompletionFilter(F.code(), F.point(PointName));
+assert(Filter);
+return *Filter;
+  };
+  EXPECT_EQ(speculate("bof"), "");
+  EXPECT_EQ(speculate("bol"), "");
+  EXPECT_EQ(speculate("ab"), "ab");
+  EXPECT_EQ(speculate("dot"), "ab");
+  EXPECT_EQ(speculate("dotempty"), "");
+  EXPECT_EQ(speculate("scoped"), "ab");
+  EXPECT_EQ(speculate("scopedempty"), "");
+}
+
+TEST(CompletionTest, EnableSpeculativeIndexRequest) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto File = testPath("foo.cpp");
+  Annotations Test(R"cpp(
+  namespace ns1 { int abc; }
+  namespace ns2 { int abc; }
+  void f() { ns1::ab$1^; ns1::ab$2^; }
+  void f() { ns2::ab$3^; }
+  )cpp");
+  runAddDocument(Server, File, Test.code());
+  clangd::CodeCompleteOptions Opts = {};
+
+  IndexRequestCollector Requests;
+  Opts.Index = &Requests;
+  Opts.SpeculativeIndexRequest = true;
+
+  auto CompleteAtPoint = [&](StringRef P) {
+cantFail(runCodeComplete(Server, File, Test.point(P), Opts));
+// Sleep for a while to make sure asynchronous call (if applicable) is also
+// triggered before callback is invoked.
+std::this_thread::sleep_for(std::chrono::milliseconds(100));
+  };
+
+  CompleteAtPoint("1");
+  auto Reqs1 = Requests.consumeRequests();
+  ASSERT_EQ(Reqs1.size(), 1u);
+  EXPECT_THAT(Reqs1[0].Scopes, UnorderedElementsAre("ns1::"));
+
+  CompleteAtPoint("2");
+  auto Reqs2 = Requests.consumeRequests();
+  // Speculation succeeded. Used speculative index result.
+  ASSERT_EQ(Reqs2.size(), 1u);
+  EXPECT_EQ(Reqs2[0], Reqs1[0]);
+
+  CompleteAtPoint("3");
+  // Speculation failed. Sent speculative index request and the new index
+  // request after sema.
+  auto Reqs3 = Requests.consumeRequests();
+  ASSERT_EQ(Reqs3.size(), 2u);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -308,6 +308,7 @@
 CCOpts.IncludeIndicator.Insert.clear();
 CCOpts.IncludeIndicator.NoInsert.clear();
   }
+  CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
 
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -19,7 +19,9 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/StringSaver.h"
 #include 
+#include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -343,6 +345,14 @@
   /// Contextually relevant files (e.g. the file we're code-completing in).
   /// Paths should be absolute.
   std::vector ProximityPaths;
+
+  bool operator==(const FuzzyFindRequest &Req) const {
+return std::tie(Query, Scopes, MaxCandidateCount, RestrictForCodeCompletion,
+ProximityPaths) ==
+   std::tie(Req.Query, Req.Scopes, Req.MaxCandidateCount,
+Req.RestrictForCodeCompletion, Req.ProximityPaths);
+  }
+  bool operator!=(const FuzzyFindRequest &Req) const { return !(*this == Req); }
 };
 
 struct LookupRequest {
Index: clangd/index/In

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162223.

https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char &operator[](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view a, string_view b);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string &str);
+  AlphaNum(const string_view &pc);
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum &operator=(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum &a);
+string StrCat(const AlphaNum &a, const AlphaNum &b);
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c);
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c,
+  const AlphaNum &d);
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c,
+  const AlphaNum &d, const AlphaNum &e, const AV &... args);
+
+void StrAppend(string *dest, const AlphaNum &a);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c, const AlphaNum &d);
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c, const AlphaNum &d, const AlphaNum &e,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, StrCat("A", StrCat(1.1)));
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: redundant StrCat calls
+
+  S 

[PATCH] D50962: [clangd] Speculative code completion index request before Sema is run.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 16.
ioeric added a comment.

- minor cleanup


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50962

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/TUScheduler.h
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -923,7 +923,11 @@
llvm::function_ref
Callback) const override {}
 
-  const std::vector allRequests() const { return Requests; }
+  const std::vector consumeRequests() const {
+auto Reqs = std::move(Requests);
+Requests.clear();
+return Reqs;
+  }
 
 private:
   mutable std::vector Requests;
@@ -934,7 +938,7 @@
   IndexRequestCollector Requests;
   Opts.Index = &Requests;
   completions(Code, {}, Opts);
-  return Requests.allRequests();
+  return Requests.consumeRequests();
 }
 
 TEST(CompletionTest, UnqualifiedIdQuery) {
@@ -1705,6 +1709,75 @@
 Not(Contains(Labeled("void vfunc(bool param) override");
 }
 
+TEST(SpeculateCompletionFilter, Filters) {
+  Annotations F(R"cpp($bof^
+  $bol^
+  ab$ab^
+  x.ab$dot^
+  x.$dotempty^
+  x::ab$scoped^
+  x::$scopedempty^
+
+  )cpp");
+  auto speculate = [&](StringRef PointName) {
+auto Filter = speculateCompletionFilter(F.code(), F.point(PointName));
+assert(Filter);
+return *Filter;
+  };
+  EXPECT_EQ(speculate("bof"), "");
+  EXPECT_EQ(speculate("bol"), "");
+  EXPECT_EQ(speculate("ab"), "ab");
+  EXPECT_EQ(speculate("dot"), "ab");
+  EXPECT_EQ(speculate("dotempty"), "");
+  EXPECT_EQ(speculate("scoped"), "ab");
+  EXPECT_EQ(speculate("scopedempty"), "");
+}
+
+TEST(CompletionTest, EnableSpeculativeIndexRequest) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto File = testPath("foo.cpp");
+  Annotations Test(R"cpp(
+  namespace ns1 { int abc; }
+  namespace ns2 { int abc; }
+  void f() { ns1::ab$1^; ns1::ab$2^; }
+  void f() { ns2::ab$3^; }
+  )cpp");
+  runAddDocument(Server, File, Test.code());
+  clangd::CodeCompleteOptions Opts = {};
+
+  IndexRequestCollector Requests;
+  Opts.Index = &Requests;
+  Opts.SpeculativeIndexRequest = true;
+
+  auto CompleteAtPoint = [&](StringRef P) {
+cantFail(runCodeComplete(Server, File, Test.point(P), Opts));
+// Sleep for a while to make sure asynchronous call (if applicable) is also
+// triggered before callback is invoked.
+std::this_thread::sleep_for(std::chrono::milliseconds(100));
+  };
+
+  CompleteAtPoint("1");
+  auto Reqs1 = Requests.consumeRequests();
+  ASSERT_EQ(Reqs1.size(), 1u);
+  EXPECT_THAT(Reqs1[0].Scopes, UnorderedElementsAre("ns1::"));
+
+  CompleteAtPoint("2");
+  auto Reqs2 = Requests.consumeRequests();
+  // Speculation succeeded. Used speculative index result.
+  ASSERT_EQ(Reqs2.size(), 1u);
+  EXPECT_EQ(Reqs2[0], Reqs1[0]);
+
+  CompleteAtPoint("3");
+  // Speculation failed. Sent speculative index request and the new index
+  // request after sema.
+  auto Reqs3 = Requests.consumeRequests();
+  ASSERT_EQ(Reqs3.size(), 2u);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -308,6 +308,7 @@
 CCOpts.IncludeIndicator.Insert.clear();
 CCOpts.IncludeIndicator.NoInsert.clear();
   }
+  CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
 
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -20,6 +20,7 @@
 #include "llvm/Support/StringSaver.h"
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -343,6 +344,14 @@
   /// Contextually relevant files (e.g. the file we're code-completing in).
   /// Paths should be absolute.
   std::vector ProximityPaths;
+
+  bool operator==(const FuzzyFindRequest &Req) const {
+return std::tie(Query, Scopes, MaxCandidateCount, RestrictForCodeCompletion,
+ProximityPaths) ==
+   std::tie(Req.Query, Req.Scopes, Req.MaxCandidateCount,
+Req.RestrictForCodeCompletion, Req.ProximityPaths);
+  }
+  bool operator!=(const FuzzyFindRequest &Req) const { return !(*this == Req); }
 };
 
 struct LookupRequest {
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -8,6 +8,9 @@
 //===

[PATCH] D50962: [clangd] Speculative code completion index request before Sema is run.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 162224.
ioeric added a comment.

- another minior cleanup


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50962

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/TUScheduler.h
  clangd/index/Index.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -923,7 +923,11 @@
llvm::function_ref
Callback) const override {}
 
-  const std::vector allRequests() const { return Requests; }
+  const std::vector consumeRequests() const {
+auto Reqs = std::move(Requests);
+Requests.clear();
+return Reqs;
+  }
 
 private:
   mutable std::vector Requests;
@@ -934,7 +938,7 @@
   IndexRequestCollector Requests;
   Opts.Index = &Requests;
   completions(Code, {}, Opts);
-  return Requests.allRequests();
+  return Requests.consumeRequests();
 }
 
 TEST(CompletionTest, UnqualifiedIdQuery) {
@@ -1705,6 +1709,75 @@
 Not(Contains(Labeled("void vfunc(bool param) override");
 }
 
+TEST(SpeculateCompletionFilter, Filters) {
+  Annotations F(R"cpp($bof^
+  $bol^
+  ab$ab^
+  x.ab$dot^
+  x.$dotempty^
+  x::ab$scoped^
+  x::$scopedempty^
+
+  )cpp");
+  auto speculate = [&](StringRef PointName) {
+auto Filter = speculateCompletionFilter(F.code(), F.point(PointName));
+assert(Filter);
+return *Filter;
+  };
+  EXPECT_EQ(speculate("bof"), "");
+  EXPECT_EQ(speculate("bol"), "");
+  EXPECT_EQ(speculate("ab"), "ab");
+  EXPECT_EQ(speculate("dot"), "ab");
+  EXPECT_EQ(speculate("dotempty"), "");
+  EXPECT_EQ(speculate("scoped"), "ab");
+  EXPECT_EQ(speculate("scopedempty"), "");
+}
+
+TEST(CompletionTest, EnableSpeculativeIndexRequest) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto File = testPath("foo.cpp");
+  Annotations Test(R"cpp(
+  namespace ns1 { int abc; }
+  namespace ns2 { int abc; }
+  void f() { ns1::ab$1^; ns1::ab$2^; }
+  void f() { ns2::ab$3^; }
+  )cpp");
+  runAddDocument(Server, File, Test.code());
+  clangd::CodeCompleteOptions Opts = {};
+
+  IndexRequestCollector Requests;
+  Opts.Index = &Requests;
+  Opts.SpeculativeIndexRequest = true;
+
+  auto CompleteAtPoint = [&](StringRef P) {
+cantFail(runCodeComplete(Server, File, Test.point(P), Opts));
+// Sleep for a while to make sure asynchronous call (if applicable) is also
+// triggered before callback is invoked.
+std::this_thread::sleep_for(std::chrono::milliseconds(100));
+  };
+
+  CompleteAtPoint("1");
+  auto Reqs1 = Requests.consumeRequests();
+  ASSERT_EQ(Reqs1.size(), 1u);
+  EXPECT_THAT(Reqs1[0].Scopes, UnorderedElementsAre("ns1::"));
+
+  CompleteAtPoint("2");
+  auto Reqs2 = Requests.consumeRequests();
+  // Speculation succeeded. Used speculative index result.
+  ASSERT_EQ(Reqs2.size(), 1u);
+  EXPECT_EQ(Reqs2[0], Reqs1[0]);
+
+  CompleteAtPoint("3");
+  // Speculation failed. Sent speculative index request and the new index
+  // request after sema.
+  auto Reqs3 = Requests.consumeRequests();
+  ASSERT_EQ(Reqs3.size(), 2u);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -308,6 +308,7 @@
 CCOpts.IncludeIndicator.Insert.clear();
 CCOpts.IncludeIndicator.NoInsert.clear();
   }
+  CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
 
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -20,6 +20,7 @@
 #include "llvm/Support/StringSaver.h"
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -343,6 +344,14 @@
   /// Contextually relevant files (e.g. the file we're code-completing in).
   /// Paths should be absolute.
   std::vector ProximityPaths;
+
+  bool operator==(const FuzzyFindRequest &Req) const {
+return std::tie(Query, Scopes, MaxCandidateCount, RestrictForCodeCompletion,
+ProximityPaths) ==
+   std::tie(Req.Query, Req.Scopes, Req.MaxCandidateCount,
+Req.RestrictForCodeCompletion, Req.ProximityPaths);
+  }
+  bool operator!=(const FuzzyFindRequest &Req) const { return !(*this == Req); }
 };
 
 struct LookupRequest {
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -14,6 +14,7 @@
 #include "Function.h"
 #inclu

[PATCH] D50962: [clangd] Speculative code completion index request before Sema is run.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/ClangdServer.cpp:159
+  }
+  if (SpecFuzzyReq) {
+if (auto Filter = speculateCompletionFilter(Content, Pos)) {

ilya-biryukov wrote:
> NIT: maybe invert condition to reduce nesting?
It would become something like:
```
if (!SpecFuzzyReq)
  return SpecFuzzyReq;
 // some work
return SpecFuzzyReq;
```

Having to return the same thing twice seems a bit worse IMO.



Comment at: clangd/CodeComplete.cpp:1388
+  if (Len > 0)
+St++; // Shift to the first valid character.
+  return Content.substr(St, Len);

ilya-biryukov wrote:
> This looks incorrect in case of the identifiers starting at offset 0. A 
> corner case, but still.
`Offset == 0` is handled above.



Comment at: clangd/tool/ClangdMain.cpp:179
+should be effective for a number of code completions.)"),
+llvm::cl::init(true));
+

ilya-biryukov wrote:
> Maybe remove this option?
> Doing a speculative request seems like the right thing to do in all cases. It 
> never hurts completion latency. Any reasons to avoid it?
Sure, was trying to be conservative :)

Removed the command-line option but still keeping the code completion option. 
As async call has some overhead, I think we should only enable it when static 
index is provided.



Comment at: unittests/clangd/CodeCompleteTests.cpp:926
 
-  const std::vector allRequests() const { return Requests; }
+  const std::vector consumeRequests() const {
+auto Reqs = std::move(Requests);

ilya-biryukov wrote:
> Why do we want to change this? To avoid tests getting requests from the 
> previous tests?
Yes. 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50962



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51159: [FileManager] Do not call 'real_path' in getFile().

[Simon Marchi via Phabricator via cfe-commits]

simark added inline comments.



Comment at: lib/Basic/FileManager.cpp:319
 
-  SmallString<128> RealPathName;
-  if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
+  if (UFE.File) {
+if (auto Path = UFE.File->getName()) {

What's the rationale for only computing the field if `UFE.File` is non-null?

Previously, if you looked up the file with `openFile == false` and then later 
`openFile == true`, the `RealPathName` field would not be set because of this.  
That doesn't seem right.



Comment at: lib/Basic/FileManager.cpp:326
+  llvm::sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true);
+  UFE.RealPathName = AbsPath.str();
+}

If the path contains symlinks, doesn't this put a non-real path in the 
RealPathName field?  Won't users (e.g. clangd) use this value thinking it is a 
real path, when it is actually not?


Repository:
  rC Clang

https://reviews.llvm.org/D51159



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

Let me know if there's anything else I can fix to move the process along.


https://reviews.llvm.org/D51061



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/abseil-str-cat-append.rst:11
+
+  a = StrCat(a, b); // Use StrAppend(&a, b) instead.
+

Please add namespace.


https://reviews.llvm.org/D51061



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits