[PATCH] D50502: [clangd] Initial cancellation mechanism for LSP requests.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 161428.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Check cancellation earlier && Fix normalization difference between string and 
integer request ids.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50502

Files:
  clangd/CMakeLists.txt
  clangd/Cancellation.cpp
  clangd/Cancellation.h
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/JSONRPCDispatcher.cpp
  clangd/JSONRPCDispatcher.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  unittests/clangd/CMakeLists.txt
  unittests/clangd/CancellationTests.cpp

Index: unittests/clangd/CancellationTests.cpp
===
--- /dev/null
+++ unittests/clangd/CancellationTests.cpp
@@ -0,0 +1,79 @@
+#include "Cancellation.h"
+#include "Context.h"
+#include "Threading.h"
+#include "llvm/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(CancellationTest, CancellationTest) {
+  TaskHandle TH = TaskHandle::create();
+  WithContext ContextWithCancellation(
+  setCurrentCancellationToken(TH.createCancellationToken()));
+  EXPECT_FALSE(isCancelled());
+  TH.cancel();
+  EXPECT_TRUE(isCancelled());
+}
+
+TEST(CancellationTest, TaskHandleTestHandleDiesContextLives) {
+  llvm::Optional ContextWithCancellation;
+  {
+auto CancellableTaskHandle = TaskHandle::create();
+ContextWithCancellation.emplace(setCurrentCancellationToken(
+CancellableTaskHandle.createCancellationToken()));
+EXPECT_FALSE(isCancelled());
+CancellableTaskHandle.cancel();
+EXPECT_TRUE(isCancelled());
+  }
+  EXPECT_TRUE(isCancelled());
+}
+
+TEST(CancellationTest, TaskHandleContextDiesHandleLives) {
+  auto CancellableTaskHandle = TaskHandle::create();
+  {
+WithContext ContextWithCancellation(setCurrentCancellationToken(
+CancellableTaskHandle.createCancellationToken()));
+EXPECT_FALSE(isCancelled());
+CancellableTaskHandle.cancel();
+EXPECT_TRUE(isCancelled());
+  }
+  // Still should be able to cancel without any problems.
+  CancellableTaskHandle.cancel();
+}
+
+TEST(CancellationTest, CancellationToken) {
+  auto CancellableTaskHandle = TaskHandle::create();
+  WithContext ContextWithCancellation(setCurrentCancellationToken(
+  CancellableTaskHandle.createCancellationToken()));
+  const auto &CT = getCurrentCancellationToken();
+  EXPECT_FALSE(CT);
+  CancellableTaskHandle.cancel();
+  EXPECT_TRUE(CT);
+}
+
+TEST(CancellationTest, AsynCancellationTest) {
+  std::atomic HasCancelled(false);
+  Notification Cancelled;
+  auto TaskToBeCancelled = [&](CancellationToken CT) {
+WithContext ContextGuard(setCurrentCancellationToken(std::move(CT)));
+Cancelled.wait();
+HasCancelled = isCancelled();
+  };
+  TaskHandle TH = TaskHandle::create();
+  std::thread AsyncTask(TaskToBeCancelled, TH.createCancellationToken());
+  TH.cancel();
+  Cancelled.notify();
+  AsyncTask.join();
+
+  EXPECT_TRUE(HasCancelled);
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: unittests/clangd/CMakeLists.txt
===
--- unittests/clangd/CMakeLists.txt
+++ unittests/clangd/CMakeLists.txt
@@ -10,6 +10,7 @@
 
 add_extra_unittest(ClangdTests
   Annotations.cpp
+  CancellationTests.cpp
   ClangdTests.cpp
   ClangdUnitTests.cpp
   CodeCompleteTests.cpp
Index: clangd/ProtocolHandlers.h
===
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -55,6 +55,7 @@
   virtual void onDocumentHighlight(TextDocumentPositionParams &Params) = 0;
   virtual void onHover(TextDocumentPositionParams &Params) = 0;
   virtual void onChangeConfiguration(DidChangeConfigurationParams &Params) = 0;
+  virtual void onCancelRequest(CancelParams &Params) = 0;
 };
 
 void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher,
Index: clangd/ProtocolHandlers.cpp
===
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -75,4 +75,5 @@
   Register("workspace/didChangeConfiguration",
&ProtocolCallbacks::onChangeConfiguration);
   Register("workspace/symbol", &ProtocolCallbacks::onWorkspaceSymbol);
+  Register("$/cancelRequest", &ProtocolCallbacks::onCancelRequest);
 }
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -867,6 +867,16 @@
 llvm::json::Value toJSON(const DocumentHighlight &DH);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const DocumentHighlight &);
 
+struct CancelParams {
+  /// The request id to cancel.
+  /// This can be either a number or string, if it is a number simply print it
+  /

[PATCH] D50955: [clangd] Implement TRUE Iterator

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added a reviewer: ioeric.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.

This patch introduces TRUE Iterator which efficiently handles posting lists 
containing all items within `[0, Size)` range.


https://reviews.llvm.org/D50955

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
@@ -262,6 +262,19 @@
   EXPECT_THAT(consume(*DocIterator, 0), ElementsAre());
 }
 
+TEST(DexIndexIterators, True) {
+  auto TrueIterator = createTrue(0U);
+  EXPECT_THAT(TrueIterator->reachedEnd(), true);
+  EXPECT_THAT(consume(*TrueIterator), ElementsAre());
+
+  PostingList L0 = {1, 2, 5, 7};
+  TrueIterator = createTrue(7U);
+  EXPECT_THAT(TrueIterator->peek(), 0);
+  auto AndIterator = createAnd(create(L0), move(TrueIterator));
+  EXPECT_THAT(AndIterator->reachedEnd(), false);
+  EXPECT_THAT(consume(*AndIterator), ElementsAre(1, 2, 5));
+}
+
 testing::Matcher>
 trigramsAre(std::initializer_list Trigrams) {
   std::vector Tokens;
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
@@ -129,6 +129,10 @@
 std::unique_ptr
 createOr(std::vector> Children);
 
+/// Returns TRUE Iterator which iterates over "virtual" PostingList containing
+/// all items in range [0, Size) in an efficient manner.
+std::unique_ptr createTrue(DocID Size);
+
 /// This allows createAnd(create(...), create(...)) syntax.
 template  std::unique_ptr createAnd(Args... args) {
   std::vector> Children;
Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -225,6 +225,45 @@
   std::vector> Children;
 };
 
+/// TrueIterator handles PostingLists which contain all items of the index. In
+/// order to prevent additional memory consumption, it only stores the size of
+/// this virtual posting list because posting lists of such density are likely
+/// to consume a lot of memory. All operations are performed in O(1) as a
+/// result which also significantly improves performance of the iterator,
+/// because iterating such posting list via DocumentIterator would result in
+/// increased cost of advanceTo().
+class TrueIterator : public Iterator {
+public:
+  TrueIterator(DocID Size) : Size(Size) {}
+
+  bool reachedEnd() const override { return Index >= Size; }
+
+  void advance() override {
+assert(!reachedEnd() && "Can't advance iterator after it reached the 
end.");
+++Index;
+  }
+
+  void advanceTo(DocID ID) override {
+assert(!reachedEnd() && "Can't advance iterator after it reached the 
end.");
+Index = ID;
+  }
+
+  DocID peek() const override {
+assert(!reachedEnd() && "TrueIterator can't call peek() at the end.");
+return Index;
+  }
+
+private:
+  llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
+OS << "(TrueIterator {" << Index << "} out of " << Size << ")";
+return OS;
+  }
+
+  DocID Index = 0;
+  /// Size of the underlying virtual PostingList.
+  DocID Size;
+};
+
 } // end namespace
 
 std::vector consume(Iterator &It, size_t Limit) {
@@ -249,6 +288,10 @@
   return llvm::make_unique(move(Children));
 }
 
+std::unique_ptr createTrue(DocID Size) {
+  return llvm::make_unique(Size);
+}
+
 } // namespace dex
 } // namespace clangd
 } // namespace clang


Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -262,6 +262,19 @@
   EXPECT_THAT(consume(*DocIterator, 0), ElementsAre());
 }
 
+TEST(DexIndexIterators, True) {
+  auto TrueIterator = createTrue(0U);
+  EXPECT_THAT(TrueIterator->reachedEnd(), true);
+  EXPECT_THAT(consume(*TrueIterator), ElementsAre());
+
+  PostingList L0 = {1, 2, 5, 7};
+  TrueIterator = createTrue(7U);
+  EXPECT_THAT(TrueIterator->peek(), 0);
+  auto AndIterator = createAnd(create(L0), move(TrueIterator));
+  EXPECT_THAT(AndIterator->reachedEnd(), false);
+  EXPECT_THAT(consume(*AndIterator), ElementsAre(1, 2, 5));
+}
+
 testing::Matcher>
 trigramsAre(std::initializer_list Trigrams) {
   std::vector Tokens;
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
@@ -129,6 +129,10 @@
 std::uniqu

[PATCH] D50956: [clangd] NFC: Cleanup Dex Iterator comments and simplify tests

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added a reviewer: ioeric.
Herald added subscribers: kadircet, arphaman, omtcyfz, jkorous, MaskRay, 
ilya-biryukov.

Proposed changes:

- Cleanup comments in `clangd/index/dex/Iterator.h`: Vim's `gq` formatting 
added redundant spaces instead of newlines in few places
- Few comments in `OrIterator` are wrong
- Use `EXPECT_TRUE(Condition)` instead of `EXPECT_THAT(Condition, true)` (same 
with `EXPECT_FALSE`)

This patch does not affect functionality.


https://reviews.llvm.org/D50956

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
@@ -28,33 +28,33 @@
   auto DocIterator = create(L);
 
   EXPECT_EQ(DocIterator->peek(), 4U);
-  EXPECT_EQ(DocIterator->reachedEnd(), false);
+  EXPECT_FALSE(DocIterator->reachedEnd());
 
   DocIterator->advance();
   EXPECT_EQ(DocIterator->peek(), 7U);
-  EXPECT_EQ(DocIterator->reachedEnd(), false);
+  EXPECT_FALSE(DocIterator->reachedEnd());
 
   DocIterator->advanceTo(20);
   EXPECT_EQ(DocIterator->peek(), 20U);
-  EXPECT_EQ(DocIterator->reachedEnd(), false);
+  EXPECT_FALSE(DocIterator->reachedEnd());
 
   DocIterator->advanceTo(65);
   EXPECT_EQ(DocIterator->peek(), 100U);
-  EXPECT_EQ(DocIterator->reachedEnd(), false);
+  EXPECT_FALSE(DocIterator->reachedEnd());
 
   DocIterator->advanceTo(420);
-  EXPECT_EQ(DocIterator->reachedEnd(), true);
+  EXPECT_TRUE(DocIterator->reachedEnd());
 }
 
 TEST(DexIndexIterators, AndWithEmpty) {
   const PostingList L0;
   const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000};
 
   auto AndEmpty = createAnd(create(L0));
-  EXPECT_EQ(AndEmpty->reachedEnd(), true);
+  EXPECT_TRUE(AndEmpty->reachedEnd());
 
   auto AndWithEmpty = createAnd(create(L0), create(L1));
-  EXPECT_EQ(AndWithEmpty->reachedEnd(), true);
+  EXPECT_TRUE(AndWithEmpty->reachedEnd());
 
   EXPECT_THAT(consume(*AndWithEmpty), ElementsAre());
 }
@@ -65,7 +65,7 @@
 
   auto And = createAnd(create(L1), create(L0));
 
-  EXPECT_EQ(And->reachedEnd(), false);
+  EXPECT_FALSE(And->reachedEnd());
   EXPECT_THAT(consume(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U));
 
   And = createAnd(create(L0), create(L1));
@@ -94,18 +94,18 @@
   EXPECT_EQ(And->peek(), 320U);
   And->advanceTo(10);
 
-  EXPECT_EQ(And->reachedEnd(), true);
+  EXPECT_TRUE(And->reachedEnd());
 }
 
 TEST(DexIndexIterators, OrWithEmpty) {
   const PostingList L0;
   const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000};
 
   auto OrEmpty = createOr(create(L0));
-  EXPECT_EQ(OrEmpty->reachedEnd(), true);
+  EXPECT_TRUE(OrEmpty->reachedEnd());
 
   auto OrWithEmpty = createOr(create(L0), create(L1));
-  EXPECT_EQ(OrWithEmpty->reachedEnd(), false);
+  EXPECT_FALSE(OrWithEmpty->reachedEnd());
 
   EXPECT_THAT(consume(*OrWithEmpty),
   ElementsAre(0U, 5U, 7U, 10U, 42U, 320U, 9000U));
@@ -117,7 +117,7 @@
 
   auto Or = createOr(create(L0), create(L1));
 
-  EXPECT_EQ(Or->reachedEnd(), false);
+  EXPECT_FALSE(Or->reachedEnd());
   EXPECT_EQ(Or->peek(), 0U);
   Or->advance();
   EXPECT_EQ(Or->peek(), 4U);
@@ -136,7 +136,7 @@
   Or->advanceTo(9000);
   EXPECT_EQ(Or->peek(), 9000U);
   Or->advanceTo(9001);
-  EXPECT_EQ(Or->reachedEnd(), true);
+  EXPECT_TRUE(Or->reachedEnd());
 
   Or = createOr(create(L0), create(L1));
 
@@ -151,7 +151,7 @@
 
   auto Or = createOr(create(L0), create(L1), create(L2));
 
-  EXPECT_EQ(Or->reachedEnd(), false);
+  EXPECT_FALSE(Or->reachedEnd());
   EXPECT_EQ(Or->peek(), 0U);
 
   Or->advance();
@@ -166,7 +166,7 @@
   EXPECT_EQ(Or->peek(), 60U);
 
   Or->advanceTo(9001);
-  EXPECT_EQ(Or->reachedEnd(), true);
+  EXPECT_TRUE(Or->reachedEnd());
 }
 
 // FIXME(kbobyrev): The testcase below is similar to what is expected in real
@@ -208,7 +208,7 @@
   // Lower Or Iterator: [0, 1, 5]
   createOr(create(L2), create(L3), create(L4)));
 
-  EXPECT_EQ(Root->reachedEnd(), false);
+  EXPECT_FALSE(Root->reachedEnd());
   EXPECT_EQ(Root->peek(), 1U);
   Root->advanceTo(0);
   // Advance multiple times. Shouldn't do anything.
@@ -220,7 +220,7 @@
   Root->advanceTo(5);
   EXPECT_EQ(Root->peek(), 5U);
   Root->advanceTo(9000);
-  EXPECT_EQ(Root->reachedEnd(), true);
+  EXPECT_TRUE(Root->reachedEnd());
 }
 
 TEST(DexIndexIterators, StringRepresentation) {
@@ -264,14 +264,14 @@
 
 TEST(DexIndexIterators, True) {
   auto TrueIterator = createTrue(0U);
-  EXPECT_THAT(TrueIterator->reachedEnd(), true);
+  EXPECT_TRUE(TrueIterator->reachedEnd());
   EXPECT_THAT(consume(*TrueIterator), ElementsAre());
 
   PostingList L0 = {1, 2, 5, 7};
   TrueIterator = createTrue(7U);
   EXPECT_THAT(TrueIterator->peek(), 0);
   auto AndIterator = createAnd(create(L0), move(TrueIterator));
-  EXPECT_THAT(AndIterator->reachedEnd(),

[PATCH] D50955: [clangd] Implement TRUE Iterator

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:230
+/// order to prevent additional memory consumption, it only stores the size of
+/// this virtual posting list because posting lists of such density are likely
+/// to consume a lot of memory. All operations are performed in O(1) as a

nit: The documentation here is a bit verbose as. How about `... It stores size 
of the virtual posting list, and all operations are performed in O(1).`? Other 
statements can be easily derived from this IMO.



Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:248
+assert(!reachedEnd() && "Can't advance iterator after it reached the 
end.");
+Index = ID;
+  }

Should we check `ID < Size` here?



Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:258
+  llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
+OS << "(TrueIterator {" << Index << "} out of " << Size << ")";
+return OS;

nit: maybe `s/TrueIterator/TRUE/`?


https://reviews.llvm.org/D50955



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

[PATCH] D50956: [clangd] NFC: Cleanup Dex Iterator comments and simplify tests

[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


https://reviews.llvm.org/D50956



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

[PATCH] D50337: [clangd] DexIndex implementation prototype

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 161432.
kbobyrev marked 2 inline comments as done.
kbobyrev added a comment.
Herald added a subscriber: kadircet.

Use TRUE iterator to ensure validity of the query processing.


https://reviews.llvm.org/D50337

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/DexIndex.cpp
  clang-tools-extra/clangd/index/dex/DexIndex.h
  clang-tools-extra/clangd/index/dex/Token.h
  clang-tools-extra/unittests/clangd/CMakeLists.txt
  clang-tools-extra/unittests/clangd/DexIndexTests.cpp
  clang-tools-extra/unittests/clangd/IndexTests.cpp
  clang-tools-extra/unittests/clangd/TestIndex.cpp
  clang-tools-extra/unittests/clangd/TestIndex.h

Index: clang-tools-extra/unittests/clangd/TestIndex.h
===
--- /dev/null
+++ clang-tools-extra/unittests/clangd/TestIndex.h
@@ -0,0 +1,57 @@
+//===-- IndexHelpers.h --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_INDEXTESTCOMMON_H
+#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_INDEXTESTCOMMON_H
+
+#include "index/Index.h"
+#include "index/Merge.h"
+#include "index/dex/DexIndex.h"
+#include "index/dex/Iterator.h"
+#include "index/dex/Token.h"
+#include "index/dex/Trigram.h"
+
+namespace clang {
+namespace clangd {
+
+Symbol symbol(llvm::StringRef QName);
+
+struct SlabAndPointers {
+  SymbolSlab Slab;
+  std::vector Pointers;
+};
+
+// Create a slab of symbols with the given qualified names as both IDs and
+// names. The life time of the slab is managed by the returned shared pointer.
+// If \p WeakSymbols is provided, it will be pointed to the managed object in
+// the returned shared pointer.
+std::shared_ptr>
+generateSymbols(std::vector QualifiedNames,
+std::weak_ptr *WeakSymbols = nullptr);
+
+// Create a slab of symbols with IDs and names [Begin, End], otherwise identical
+// to the `generateSymbols` above.
+std::shared_ptr>
+generateNumSymbols(int Begin, int End,
+   std::weak_ptr *WeakSymbols = nullptr);
+
+std::string getQualifiedName(const Symbol &Sym);
+
+std::vector match(const SymbolIndex &I,
+   const FuzzyFindRequest &Req,
+   bool *Incomplete = nullptr);
+
+// Returns qualified names of symbols with any of IDs in the index.
+std::vector lookup(const SymbolIndex &I,
+llvm::ArrayRef IDs);
+
+} // namespace clangd
+} // namespace clang
+
+#endif
Index: clang-tools-extra/unittests/clangd/TestIndex.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clangd/TestIndex.cpp
@@ -0,0 +1,89 @@
+//===-- IndexHelpers.cpp *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "TestIndex.h"
+
+namespace clang {
+namespace clangd {
+
+Symbol symbol(llvm::StringRef QName) {
+  Symbol Sym;
+  Sym.ID = SymbolID(QName.str());
+  size_t Pos = QName.rfind("::");
+  if (Pos == llvm::StringRef::npos) {
+Sym.Name = QName;
+Sym.Scope = "";
+  } else {
+Sym.Name = QName.substr(Pos + 2);
+Sym.Scope = QName.substr(0, Pos + 2);
+  }
+  return Sym;
+}
+
+// Create a slab of symbols with the given qualified names as both IDs and
+// names. The life time of the slab is managed by the returned shared pointer.
+// If \p WeakSymbols is provided, it will be pointed to the managed object in
+// the returned shared pointer.
+std::shared_ptr>
+generateSymbols(std::vector QualifiedNames,
+std::weak_ptr *WeakSymbols) {
+  SymbolSlab::Builder Slab;
+  for (llvm::StringRef QName : QualifiedNames)
+Slab.insert(symbol(QName));
+
+  auto Storage = std::make_shared();
+  Storage->Slab = std::move(Slab).build();
+  for (const auto &Sym : Storage->Slab)
+Storage->Pointers.push_back(&Sym);
+  if (WeakSymbols)
+*WeakSymbols = Storage;
+  auto *Pointers = &Storage->Pointers;
+  return {std::move(Storage), Pointers};
+}
+
+// Create a slab of symbols with IDs and names [Begin, End], otherwise identical
+// to the `generateSymbols` above.
+std::shared_ptr>
+generateNumSymbols(int Begin, int End,
+   std::weak_ptr *WeakSymbols) {
+  std::vector Names;
+  for (int i = Begin; i <= End; i++)
+Names.push_back(std::to_string(i));
+  return generateSymbols(Names, WeakSymbols);
+}
+
+std::string getQualifiedName(const Symbol &Sym) {
+  return (Sym.Scope + Sym.

[PATCH] D50955: [clangd] Implement TRUE Iterator

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Address a couple of comments.


https://reviews.llvm.org/D50955

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
@@ -262,6 +262,19 @@
   EXPECT_THAT(consume(*DocIterator, 0), ElementsAre());
 }
 
+TEST(DexIndexIterators, True) {
+  auto TrueIterator = createTrue(0U);
+  EXPECT_THAT(TrueIterator->reachedEnd(), true);
+  EXPECT_THAT(consume(*TrueIterator), ElementsAre());
+
+  PostingList L0 = {1, 2, 5, 7};
+  TrueIterator = createTrue(7U);
+  EXPECT_THAT(TrueIterator->peek(), 0);
+  auto AndIterator = createAnd(create(L0), move(TrueIterator));
+  EXPECT_THAT(AndIterator->reachedEnd(), false);
+  EXPECT_THAT(consume(*AndIterator), ElementsAre(1, 2, 5));
+}
+
 testing::Matcher>
 trigramsAre(std::initializer_list Trigrams) {
   std::vector Tokens;
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
@@ -129,6 +129,10 @@
 std::unique_ptr
 createOr(std::vector> Children);
 
+/// Returns TRUE Iterator which iterates over "virtual" PostingList containing
+/// all items in range [0, Size) in an efficient manner.
+std::unique_ptr createTrue(DocID Size);
+
 /// This allows createAnd(create(...), create(...)) syntax.
 template  std::unique_ptr createAnd(Args... args) {
   std::vector> Children;
Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -225,6 +225,41 @@
   std::vector> Children;
 };
 
+/// TrueIterator handles PostingLists which contain all items of the index. It
+/// stores size of the virtual posting list, and all operations are performed
+/// in O(1).
+class TrueIterator : public Iterator {
+public:
+  TrueIterator(DocID Size) : Size(Size) {}
+
+  bool reachedEnd() const override { return Index >= Size; }
+
+  void advance() override {
+assert(!reachedEnd() && "Can't advance iterator after it reached the 
end.");
+++Index;
+  }
+
+  void advanceTo(DocID ID) override {
+assert(!reachedEnd() && "Can't advance iterator after it reached the 
end.");
+Index = std::min(ID, Size);
+  }
+
+  DocID peek() const override {
+assert(!reachedEnd() && "TrueIterator can't call peek() at the end.");
+return Index;
+  }
+
+private:
+  llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
+OS << "(TRUE {" << Index << "} out of " << Size << ")";
+return OS;
+  }
+
+  DocID Index = 0;
+  /// Size of the underlying virtual PostingList.
+  DocID Size;
+};
+
 } // end namespace
 
 std::vector consume(Iterator &It, size_t Limit) {
@@ -249,6 +284,10 @@
   return llvm::make_unique(move(Children));
 }
 
+std::unique_ptr createTrue(DocID Size) {
+  return llvm::make_unique(Size);
+}
+
 } // namespace dex
 } // namespace clangd
 } // namespace clang


Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -262,6 +262,19 @@
   EXPECT_THAT(consume(*DocIterator, 0), ElementsAre());
 }
 
+TEST(DexIndexIterators, True) {
+  auto TrueIterator = createTrue(0U);
+  EXPECT_THAT(TrueIterator->reachedEnd(), true);
+  EXPECT_THAT(consume(*TrueIterator), ElementsAre());
+
+  PostingList L0 = {1, 2, 5, 7};
+  TrueIterator = createTrue(7U);
+  EXPECT_THAT(TrueIterator->peek(), 0);
+  auto AndIterator = createAnd(create(L0), move(TrueIterator));
+  EXPECT_THAT(AndIterator->reachedEnd(), false);
+  EXPECT_THAT(consume(*AndIterator), ElementsAre(1, 2, 5));
+}
+
 testing::Matcher>
 trigramsAre(std::initializer_list Trigrams) {
   std::vector Tokens;
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
@@ -129,6 +129,10 @@
 std::unique_ptr
 createOr(std::vector> Children);
 
+/// Returns TRUE Iterator which iterates over "virtual" PostingList containing
+/// all items in range [0, Size) in an efficient manner.
+std::unique_ptr createTrue(DocID Size);
+
 /// This allows createAnd(create(...), create(...)) syntax.
 template  std::unique_ptr createAnd(Args... args) {
   std::vector> Children;
Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
==

[PATCH] D50955: [clangd] Implement TRUE Iterator

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:248
+assert(!reachedEnd() && "Can't advance iterator after it reached the 
end.");
+Index = ID;
+  }

ioeric wrote:
> Should we check `ID < Size` here?
Not really, here's an example: `(& ({0}, 42) (TRUE {0} out of 10)`.

When called `advance()`, underlying AND iterator would point to `42` and 
`advanceTo()` would be called on TRUE iterator, it will move it to the END but 
it would be completely valid (same behavior for every iterator, actually, since 
none of them check for `ID < LastDocID` equivalent).

I should, however, do `Index = std::min(ID, Size)` since calling `dump()` and 
getting something like `(TRUE {9000} out of 42)` would be implicit.


https://reviews.llvm.org/D50955



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

[PATCH] D50957: Rename -mlink-cuda-bitcode to -mlink-builtin-bitcode

[Matt Arsenault via Phabricator via cfe-commits]

arsenm created this revision.
arsenm added reviewers: tra, jlebar.
Herald added subscribers: Anastasia, wdng.

The same semantics work for OpenCL, and probably any offload
language. Keep the old name around as an alias.


https://reviews.llvm.org/D50957

Files:
  include/clang/Driver/CC1Options.td
  lib/Driver/ToolChains/Cuda.cpp
  test/CodeGenCUDA/link-device-bitcode.cu
  test/CodeGenCUDA/propagate-metadata.cu
  test/Driver/cuda-detect.cu
  test/Driver/openmp-offload-gpu.c

Index: test/Driver/openmp-offload-gpu.c
===
--- test/Driver/openmp-offload-gpu.c
+++ test/Driver/openmp-offload-gpu.c
@@ -152,7 +152,7 @@
 // RUN:   -fopenmp-relocatable-target -save-temps -no-canonical-prefixes %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-BCLIB %s
 
-// CHK-BCLIB: clang{{.*}}-triple{{.*}}nvptx64-nvidia-cuda{{.*}}-mlink-cuda-bitcode{{.*}}libomptarget-nvptx-sm_20.bc
+// CHK-BCLIB: clang{{.*}}-triple{{.*}}nvptx64-nvidia-cuda{{.*}}-mlink-builtin-bitcode{{.*}}libomptarget-nvptx-sm_20.bc
 // CHK-BCLIB-NOT: {{error:|warning:}}
 
 /// ###
Index: test/Driver/cuda-detect.cu
===
--- test/Driver/cuda-detect.cu
+++ test/Driver/cuda-detect.cu
@@ -145,8 +145,8 @@
 
 // COMMON: "-triple" "nvptx-nvidia-cuda"
 // COMMON-SAME: "-fcuda-is-device"
-// LIBDEVICE-SAME: "-mlink-cuda-bitcode"
-// NOLIBDEVICE-NOT: "-mlink-cuda-bitcode"
+// LIBDEVICE-SAME: "-mlink-builtin-bitcode"
+// NOLIBDEVICE-NOT: "-mlink-builtin-bitcode"
 // LIBDEVICE20-SAME: libdevice.compute_20.10.bc
 // LIBDEVICE30-SAME: libdevice.compute_30.10.bc
 // LIBDEVICE35-SAME: libdevice.compute_35.10.bc
Index: test/CodeGenCUDA/propagate-metadata.cu
===
--- test/CodeGenCUDA/propagate-metadata.cu
+++ test/CodeGenCUDA/propagate-metadata.cu
@@ -1,5 +1,5 @@
 // Check that when we link a bitcode module into a file using
-// -mlink-cuda-bitcode, we apply the same attributes to the functions in that
+// -mlink-builtin-bitcode, we apply the same attributes to the functions in that
 // bitcode module as we apply to functions we generate.
 //
 // In particular, we check that ftz and unsafe-math are propagated into the
@@ -14,17 +14,17 @@
 // RUN: %clang_cc1 -x c++ -emit-llvm-bc -ftrapping-math -DLIB \
 // RUN:   %s -o %t.bc -triple nvptx-unknown-unknown
 
-// RUN: %clang_cc1 -x cuda %s -emit-llvm -mlink-cuda-bitcode %t.bc -o - \
+// RUN: %clang_cc1 -x cuda %s -emit-llvm -mlink-builtin-bitcode %t.bc -o - \
 // RUN:   -fno-trapping-math -fcuda-is-device -triple nvptx-unknown-unknown \
 // RUN: | FileCheck %s --check-prefix=CHECK --check-prefix=NOFTZ --check-prefix=NOFAST
 
-// RUN: %clang_cc1 -x cuda %s -emit-llvm -mlink-cuda-bitcode %t.bc \
+// RUN: %clang_cc1 -x cuda %s -emit-llvm -mlink-builtin-bitcode %t.bc \
 // RUN:   -fno-trapping-math -fcuda-flush-denormals-to-zero -o - \
 // RUN:   -fcuda-is-device -triple nvptx-unknown-unknown \
 // RUN: | FileCheck %s --check-prefix=CHECK --check-prefix=FTZ \
 // RUN:   --check-prefix=NOFAST
 
-// RUN: %clang_cc1 -x cuda %s -emit-llvm -mlink-cuda-bitcode %t.bc \
+// RUN: %clang_cc1 -x cuda %s -emit-llvm -mlink-builtin-bitcode %t.bc \
 // RUN:   -fno-trapping-math -fcuda-flush-denormals-to-zero -o - \
 // RUN:   -fcuda-is-device -menable-unsafe-fp-math -triple nvptx-unknown-unknown \
 // RUN: | FileCheck %s --check-prefix=CHECK --check-prefix=FAST
Index: test/CodeGenCUDA/link-device-bitcode.cu
===
--- test/CodeGenCUDA/link-device-bitcode.cu
+++ test/CodeGenCUDA/link-device-bitcode.cu
@@ -11,13 +11,19 @@
 //
 // Make sure function in device-code gets linked in and internalized.
 // RUN: %clang_cc1 -triple nvptx-unknown-cuda -fcuda-is-device \
+// RUN:-mlink-builtin-bitcode %t.bc  -emit-llvm \
+// RUN:-disable-llvm-passes -o - %s \
+// RUN:| FileCheck %s -check-prefix CHECK-IR
+
+// Make sure legacy flag name works
+// RUN: %clang_cc1 -triple nvptx-unknown-cuda -fcuda-is-device \
 // RUN:-mlink-cuda-bitcode %t.bc  -emit-llvm \
 // RUN:-disable-llvm-passes -o - %s \
 // RUN:| FileCheck %s -check-prefix CHECK-IR
 //
 // Make sure we can link two bitcode files.
 // RUN: %clang_cc1 -triple nvptx-unknown-cuda -fcuda-is-device \
-// RUN:-mlink-cuda-bitcode %t.bc -mlink-cuda-bitcode %t-2.bc \
+// RUN:-mlink-builtin-bitcode %t.bc -mlink-builtin-bitcode %t-2.bc \
 // RUN:-emit-llvm -disable-llvm-passes -o - %s \
 // RUN:| FileCheck %s -check-prefix CHECK-IR -check-prefix CHECK-IR-2
 //
@@ -30,7 +36,7 @@
 //
 // Make sure NVVMReflect pass is enabled in NVPTX back-end.
 // RUN: %clang_cc1 -triple nvptx-unknown-cuda -fcuda-is-device \
-// RUN:-mlink-cuda-bitcode %t.bc -S -o /dev/null %s \
+// RUN:-mlink-builtin-bitcode %t.bc -S -o /dev/null %s \
 // RUN:-mllvm -debug-pass=Structure 

[PATCH] D50955: [clangd] Implement TRUE Iterator

[Eric Liu via Phabricator via cfe-commits]

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

lg


https://reviews.llvm.org/D50955



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

[PATCH] D50958: [clangd] WIP: xrefs for dynamic index.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added reviewers: ilya-biryukov, ioeric.
Herald added subscribers: kadircet, arphaman, mgrang, jkorous, MaskRay, 
javed.absar, mgorny.

Depends on the AST callback interfaces.

- https://reviews.llvm.org/D50847
- https://reviews.llvm.org/D50889


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50958

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  clangd/XRefs.cpp
  clangd/XRefs.h
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/MemIndex.cpp
  clangd/index/Merge.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test
  unittests/clangd/CMakeLists.txt
  unittests/clangd/TUSchedulerTests.cpp

Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -17,10 +17,11 @@
 
 namespace clang {
 namespace clangd {
+namespace {
 
 using ::testing::_;
-using ::testing::Each;
 using ::testing::AnyOf;
+using ::testing::Each;
 using ::testing::Pair;
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
@@ -30,6 +31,18 @@
   handleAllErrors(std::move(Err), [](const llvm::ErrorInfoBase &) {});
 }
 
+class NoopParsingCallbacks : public ParsingCallbacks {
+public:
+  static NoopParsingCallbacks& instance() {
+static NoopParsingCallbacks* Instance = new NoopParsingCallbacks;
+return *Instance;
+  }
+
+  void onPreambleAST(PathRef Path, ASTContext &Ctx,
+ std::shared_ptr PP) override {}
+  void onMainAST(PathRef Path, ParsedAST &AST) override {}
+};
+
 class TUSchedulerTests : public ::testing::Test {
 protected:
   ParseInputs getInputs(PathRef File, std::string Contents) {
@@ -45,7 +58,7 @@
 TEST_F(TUSchedulerTests, MissingFiles) {
   TUScheduler S(getDefaultAsyncThreadsCount(),
 /*StorePreamblesInMemory=*/true,
-/*PreambleParsedCallback=*/nullptr,
+NoopParsingCallbacks::instance(),
 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
 ASTRetentionPolicy());
 
@@ -102,7 +115,7 @@
 TUScheduler S(
 getDefaultAsyncThreadsCount(),
 /*StorePreamblesInMemory=*/true,
-/*PreambleParsedCallback=*/nullptr,
+NoopParsingCallbacks::instance(),
 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
 ASTRetentionPolicy());
 auto Path = testPath("foo.cpp");
@@ -129,11 +142,10 @@
 TEST_F(TUSchedulerTests, Debounce) {
   std::atomic CallbackCount(0);
   {
-TUScheduler S(getDefaultAsyncThreadsCount(),
-  /*StorePreamblesInMemory=*/true,
-  /*PreambleParsedCallback=*/nullptr,
-  /*UpdateDebounce=*/std::chrono::seconds(1),
-  ASTRetentionPolicy());
+TUScheduler S(
+getDefaultAsyncThreadsCount(),
+/*StorePreamblesInMemory=*/true, NoopParsingCallbacks::instance(),
+/*UpdateDebounce=*/std::chrono::seconds(1), ASTRetentionPolicy());
 // FIXME: we could probably use timeouts lower than 1 second here.
 auto Path = testPath("foo.cpp");
 S.update(Path, getInputs(Path, "auto (debounced)"), WantDiagnostics::Auto,
@@ -163,7 +175,7 @@
   {
 TUScheduler S(getDefaultAsyncThreadsCount(),
   /*StorePreamblesInMemory=*/true,
-  /*PreambleParsedCallback=*/nullptr,
+  NoopParsingCallbacks::instance(),
   /*UpdateDebounce=*/std::chrono::milliseconds(50),
   ASTRetentionPolicy());
 
@@ -261,7 +273,7 @@
   Policy.MaxRetainedASTs = 2;
   TUScheduler S(
   /*AsyncThreadsCount=*/1, /*StorePreambleInMemory=*/true,
-  PreambleParsedCallback(),
+  NoopParsingCallbacks::instance(),
   /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), Policy);
 
   llvm::StringLiteral SourceContents = R"cpp(
@@ -358,7 +370,7 @@
   // the same time. All reads should get the same non-null preamble.
   TUScheduler S(
   /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
-  PreambleParsedCallback(),
+  NoopParsingCallbacks::instance(),
   /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
   ASTRetentionPolicy());
   auto Foo = testPath("foo.cpp");
@@ -391,7 +403,7 @@
 TEST_F(TUSchedulerTests, NoopOnEmptyChanges) {
   TUScheduler S(
   /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
-  /*StorePreambleInMemory=*/true, PreambleParsedCallback(),
+  /*StorePreambleInMemory=*/true, NoopParsingCallbacks::instance(),
   /*UpdateDebounce=*/st

[PATCH] D50957: Rename -mlink-cuda-bitcode to -mlink-builtin-bitcode

[Matt Arsenault via Phabricator via cfe-commits]

arsenm updated this revision to Diff 161439.
arsenm added a comment.

Forgot to commit part


https://reviews.llvm.org/D50957

Files:
  include/clang/Driver/CC1Options.td
  lib/Driver/ToolChains/Cuda.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGenCUDA/link-device-bitcode.cu
  test/CodeGenCUDA/propagate-metadata.cu
  test/Driver/cuda-detect.cu
  test/Driver/openmp-offload-gpu.c

Index: test/Driver/openmp-offload-gpu.c
===
--- test/Driver/openmp-offload-gpu.c
+++ test/Driver/openmp-offload-gpu.c
@@ -152,7 +152,7 @@
 // RUN:   -fopenmp-relocatable-target -save-temps -no-canonical-prefixes %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-BCLIB %s
 
-// CHK-BCLIB: clang{{.*}}-triple{{.*}}nvptx64-nvidia-cuda{{.*}}-mlink-cuda-bitcode{{.*}}libomptarget-nvptx-sm_20.bc
+// CHK-BCLIB: clang{{.*}}-triple{{.*}}nvptx64-nvidia-cuda{{.*}}-mlink-builtin-bitcode{{.*}}libomptarget-nvptx-sm_20.bc
 // CHK-BCLIB-NOT: {{error:|warning:}}
 
 /// ###
Index: test/Driver/cuda-detect.cu
===
--- test/Driver/cuda-detect.cu
+++ test/Driver/cuda-detect.cu
@@ -145,8 +145,8 @@
 
 // COMMON: "-triple" "nvptx-nvidia-cuda"
 // COMMON-SAME: "-fcuda-is-device"
-// LIBDEVICE-SAME: "-mlink-cuda-bitcode"
-// NOLIBDEVICE-NOT: "-mlink-cuda-bitcode"
+// LIBDEVICE-SAME: "-mlink-builtin-bitcode"
+// NOLIBDEVICE-NOT: "-mlink-builtin-bitcode"
 // LIBDEVICE20-SAME: libdevice.compute_20.10.bc
 // LIBDEVICE30-SAME: libdevice.compute_30.10.bc
 // LIBDEVICE35-SAME: libdevice.compute_35.10.bc
Index: test/CodeGenCUDA/propagate-metadata.cu
===
--- test/CodeGenCUDA/propagate-metadata.cu
+++ test/CodeGenCUDA/propagate-metadata.cu
@@ -1,5 +1,5 @@
 // Check that when we link a bitcode module into a file using
-// -mlink-cuda-bitcode, we apply the same attributes to the functions in that
+// -mlink-builtin-bitcode, we apply the same attributes to the functions in that
 // bitcode module as we apply to functions we generate.
 //
 // In particular, we check that ftz and unsafe-math are propagated into the
@@ -14,17 +14,17 @@
 // RUN: %clang_cc1 -x c++ -emit-llvm-bc -ftrapping-math -DLIB \
 // RUN:   %s -o %t.bc -triple nvptx-unknown-unknown
 
-// RUN: %clang_cc1 -x cuda %s -emit-llvm -mlink-cuda-bitcode %t.bc -o - \
+// RUN: %clang_cc1 -x cuda %s -emit-llvm -mlink-builtin-bitcode %t.bc -o - \
 // RUN:   -fno-trapping-math -fcuda-is-device -triple nvptx-unknown-unknown \
 // RUN: | FileCheck %s --check-prefix=CHECK --check-prefix=NOFTZ --check-prefix=NOFAST
 
-// RUN: %clang_cc1 -x cuda %s -emit-llvm -mlink-cuda-bitcode %t.bc \
+// RUN: %clang_cc1 -x cuda %s -emit-llvm -mlink-builtin-bitcode %t.bc \
 // RUN:   -fno-trapping-math -fcuda-flush-denormals-to-zero -o - \
 // RUN:   -fcuda-is-device -triple nvptx-unknown-unknown \
 // RUN: | FileCheck %s --check-prefix=CHECK --check-prefix=FTZ \
 // RUN:   --check-prefix=NOFAST
 
-// RUN: %clang_cc1 -x cuda %s -emit-llvm -mlink-cuda-bitcode %t.bc \
+// RUN: %clang_cc1 -x cuda %s -emit-llvm -mlink-builtin-bitcode %t.bc \
 // RUN:   -fno-trapping-math -fcuda-flush-denormals-to-zero -o - \
 // RUN:   -fcuda-is-device -menable-unsafe-fp-math -triple nvptx-unknown-unknown \
 // RUN: | FileCheck %s --check-prefix=CHECK --check-prefix=FAST
Index: test/CodeGenCUDA/link-device-bitcode.cu
===
--- test/CodeGenCUDA/link-device-bitcode.cu
+++ test/CodeGenCUDA/link-device-bitcode.cu
@@ -11,13 +11,19 @@
 //
 // Make sure function in device-code gets linked in and internalized.
 // RUN: %clang_cc1 -triple nvptx-unknown-cuda -fcuda-is-device \
+// RUN:-mlink-builtin-bitcode %t.bc  -emit-llvm \
+// RUN:-disable-llvm-passes -o - %s \
+// RUN:| FileCheck %s -check-prefix CHECK-IR
+
+// Make sure legacy flag name works
+// RUN: %clang_cc1 -triple nvptx-unknown-cuda -fcuda-is-device \
 // RUN:-mlink-cuda-bitcode %t.bc  -emit-llvm \
 // RUN:-disable-llvm-passes -o - %s \
 // RUN:| FileCheck %s -check-prefix CHECK-IR
 //
 // Make sure we can link two bitcode files.
 // RUN: %clang_cc1 -triple nvptx-unknown-cuda -fcuda-is-device \
-// RUN:-mlink-cuda-bitcode %t.bc -mlink-cuda-bitcode %t-2.bc \
+// RUN:-mlink-builtin-bitcode %t.bc -mlink-builtin-bitcode %t-2.bc \
 // RUN:-emit-llvm -disable-llvm-passes -o - %s \
 // RUN:| FileCheck %s -check-prefix CHECK-IR -check-prefix CHECK-IR-2
 //
@@ -30,7 +36,7 @@
 //
 // Make sure NVVMReflect pass is enabled in NVPTX back-end.
 // RUN: %clang_cc1 -triple nvptx-unknown-cuda -fcuda-is-device \
-// RUN:-mlink-cuda-bitcode %t.bc -S -o /dev/null %s \
+// RUN:-mlink-builtin-bitcode %t.bc -S -o /dev/null %s \
 // RUN:-mllvm -debug-pass=Structure 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-REFLECT
 
Index: lib/Frontend/Compil

[PATCH] D50847: [clangd] Add callbacks on parsed AST in addition to parsed preambles

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.
Herald added a subscriber: kadircet.



Comment at: clangd/TUScheduler.h:66
+  /// instead.
+  virtual void onMainAST(PathRef Path, ParsedAST &AST) = 0;
+};

ilya-biryukov wrote:
> hokein wrote:
> > Does the callback get called every time we built an AST? clangd only has 3 
> > idle ASTs, if the AST is not there, we rebuild it when needed (even the 
> > source code is not changed), and we will update the dynamic index, which 
> > seems unnecessary.
> > 
> > It may rarely happen, 3 idle ASTs  might cover most cases, I think? 
> Currently we only call this when AST is built for diagnostics, so we will 
> have only a single callback, even if the AST will be rebuilt multiple times 
> because it was flushed out of the cash (as long as the file contents didn't 
> change, of course).
> 
> So we do behave optimally in that case and I suggest we keep it this way
is there any overlap between preamble AST and main AST? If so, could you 
elaborate in the documentation? 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50847



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

[clang-tools-extra] r340155 - [clangd] Implement TRUE Iterator

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Mon Aug 20 01:47:30 2018
New Revision: 340155

URL: http://llvm.org/viewvc/llvm-project?rev=340155&view=rev
Log:
[clangd] Implement TRUE Iterator

This patch introduces TRUE Iterator which efficiently handles posting
lists containing all items within `[0, Size)` range.

Reviewed by: ioeric

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

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

Modified: clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp?rev=340155&r1=340154&r2=340155&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp Mon Aug 20 01:47:30 
2018
@@ -225,6 +225,41 @@ private:
   std::vector> Children;
 };
 
+/// TrueIterator handles PostingLists which contain all items of the index. It
+/// stores size of the virtual posting list, and all operations are performed
+/// in O(1).
+class TrueIterator : public Iterator {
+public:
+  TrueIterator(DocID Size) : Size(Size) {}
+
+  bool reachedEnd() const override { return Index >= Size; }
+
+  void advance() override {
+assert(!reachedEnd() && "Can't advance iterator after it reached the 
end.");
+++Index;
+  }
+
+  void advanceTo(DocID ID) override {
+assert(!reachedEnd() && "Can't advance iterator after it reached the 
end.");
+Index = std::min(ID, Size);
+  }
+
+  DocID peek() const override {
+assert(!reachedEnd() && "TrueIterator can't call peek() at the end.");
+return Index;
+  }
+
+private:
+  llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
+OS << "(TRUE {" << Index << "} out of " << Size << ")";
+return OS;
+  }
+
+  DocID Index = 0;
+  /// Size of the underlying virtual PostingList.
+  DocID Size;
+};
+
 } // end namespace
 
 std::vector consume(Iterator &It, size_t Limit) {
@@ -249,6 +284,10 @@ createOr(std::vector(move(Children));
 }
 
+std::unique_ptr createTrue(DocID Size) {
+  return llvm::make_unique(Size);
+}
+
 } // namespace dex
 } // namespace clangd
 } // namespace clang

Modified: clang-tools-extra/trunk/clangd/index/dex/Iterator.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Iterator.h?rev=340155&r1=340154&r2=340155&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.h (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.h Mon Aug 20 01:47:30 2018
@@ -129,6 +129,10 @@ createAnd(std::vector
 createOr(std::vector> Children);
 
+/// Returns TRUE Iterator which iterates over "virtual" PostingList containing
+/// all items in range [0, Size) in an efficient manner.
+std::unique_ptr createTrue(DocID Size);
+
 /// This allows createAnd(create(...), create(...)) syntax.
 template  std::unique_ptr createAnd(Args... args) {
   std::vector> Children;

Modified: clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp?rev=340155&r1=340154&r2=340155&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp Mon Aug 20 
01:47:30 2018
@@ -262,6 +262,19 @@ TEST(DexIndexIterators, Limit) {
   EXPECT_THAT(consume(*DocIterator, 0), ElementsAre());
 }
 
+TEST(DexIndexIterators, True) {
+  auto TrueIterator = createTrue(0U);
+  EXPECT_THAT(TrueIterator->reachedEnd(), true);
+  EXPECT_THAT(consume(*TrueIterator), ElementsAre());
+
+  PostingList L0 = {1, 2, 5, 7};
+  TrueIterator = createTrue(7U);
+  EXPECT_THAT(TrueIterator->peek(), 0);
+  auto AndIterator = createAnd(create(L0), move(TrueIterator));
+  EXPECT_THAT(AndIterator->reachedEnd(), false);
+  EXPECT_THAT(consume(*AndIterator), ElementsAre(1, 2, 5));
+}
+
 testing::Matcher>
 trigramsAre(std::initializer_list Trigrams) {
   std::vector Tokens;


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

[PATCH] D50955: [clangd] Implement TRUE Iterator

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340155: [clangd] Implement TRUE Iterator (authored by 
omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50955?vs=161435&id=161440#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50955

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


Index: clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
@@ -262,6 +262,19 @@
   EXPECT_THAT(consume(*DocIterator, 0), ElementsAre());
 }
 
+TEST(DexIndexIterators, True) {
+  auto TrueIterator = createTrue(0U);
+  EXPECT_THAT(TrueIterator->reachedEnd(), true);
+  EXPECT_THAT(consume(*TrueIterator), ElementsAre());
+
+  PostingList L0 = {1, 2, 5, 7};
+  TrueIterator = createTrue(7U);
+  EXPECT_THAT(TrueIterator->peek(), 0);
+  auto AndIterator = createAnd(create(L0), move(TrueIterator));
+  EXPECT_THAT(AndIterator->reachedEnd(), false);
+  EXPECT_THAT(consume(*AndIterator), ElementsAre(1, 2, 5));
+}
+
 testing::Matcher>
 trigramsAre(std::initializer_list Trigrams) {
   std::vector Tokens;
Index: clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
@@ -225,6 +225,41 @@
   std::vector> Children;
 };
 
+/// TrueIterator handles PostingLists which contain all items of the index. It
+/// stores size of the virtual posting list, and all operations are performed
+/// in O(1).
+class TrueIterator : public Iterator {
+public:
+  TrueIterator(DocID Size) : Size(Size) {}
+
+  bool reachedEnd() const override { return Index >= Size; }
+
+  void advance() override {
+assert(!reachedEnd() && "Can't advance iterator after it reached the 
end.");
+++Index;
+  }
+
+  void advanceTo(DocID ID) override {
+assert(!reachedEnd() && "Can't advance iterator after it reached the 
end.");
+Index = std::min(ID, Size);
+  }
+
+  DocID peek() const override {
+assert(!reachedEnd() && "TrueIterator can't call peek() at the end.");
+return Index;
+  }
+
+private:
+  llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
+OS << "(TRUE {" << Index << "} out of " << Size << ")";
+return OS;
+  }
+
+  DocID Index = 0;
+  /// Size of the underlying virtual PostingList.
+  DocID Size;
+};
+
 } // end namespace
 
 std::vector consume(Iterator &It, size_t Limit) {
@@ -249,6 +284,10 @@
   return llvm::make_unique(move(Children));
 }
 
+std::unique_ptr createTrue(DocID Size) {
+  return llvm::make_unique(Size);
+}
+
 } // namespace dex
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.h
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.h
@@ -129,6 +129,10 @@
 std::unique_ptr
 createOr(std::vector> Children);
 
+/// Returns TRUE Iterator which iterates over "virtual" PostingList containing
+/// all items in range [0, Size) in an efficient manner.
+std::unique_ptr createTrue(DocID Size);
+
 /// This allows createAnd(create(...), create(...)) syntax.
 template  std::unique_ptr createAnd(Args... args) {
   std::vector> Children;


Index: clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
@@ -262,6 +262,19 @@
   EXPECT_THAT(consume(*DocIterator, 0), ElementsAre());
 }
 
+TEST(DexIndexIterators, True) {
+  auto TrueIterator = createTrue(0U);
+  EXPECT_THAT(TrueIterator->reachedEnd(), true);
+  EXPECT_THAT(consume(*TrueIterator), ElementsAre());
+
+  PostingList L0 = {1, 2, 5, 7};
+  TrueIterator = createTrue(7U);
+  EXPECT_THAT(TrueIterator->peek(), 0);
+  auto AndIterator = createAnd(create(L0), move(TrueIterator));
+  EXPECT_THAT(AndIterator->reachedEnd(), false);
+  EXPECT_THAT(consume(*AndIterator), ElementsAre(1, 2, 5));
+}
+
 testing::Matcher>
 trigramsAre(std::initializer_list Trigrams) {
   std::vector Tokens;
Index: clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
@@ -225,6 +225,41 @@
   std::vector> Children;
 };
 
+/// TrueIterator handles PostingLists which contain all items of the index. It
+/// stores size of the virtual posting list

[PATCH] D50958: [clangd] WIP: xrefs for dynamic index.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 161441.
hokein added a comment.

More cleanups.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50958

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  clangd/XRefs.cpp
  clangd/XRefs.h
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/MemIndex.cpp
  clangd/index/Merge.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test
  unittests/clangd/TUSchedulerTests.cpp

Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -17,10 +17,11 @@
 
 namespace clang {
 namespace clangd {
+namespace {
 
 using ::testing::_;
-using ::testing::Each;
 using ::testing::AnyOf;
+using ::testing::Each;
 using ::testing::Pair;
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
@@ -30,6 +31,18 @@
   handleAllErrors(std::move(Err), [](const llvm::ErrorInfoBase &) {});
 }
 
+class NoopParsingCallbacks : public ParsingCallbacks {
+public:
+  static NoopParsingCallbacks& instance() {
+static NoopParsingCallbacks* Instance = new NoopParsingCallbacks;
+return *Instance;
+  }
+
+  void onPreambleAST(PathRef Path, ASTContext &Ctx,
+ std::shared_ptr PP) override {}
+  void onMainAST(PathRef Path, ParsedAST &AST) override {}
+};
+
 class TUSchedulerTests : public ::testing::Test {
 protected:
   ParseInputs getInputs(PathRef File, std::string Contents) {
@@ -45,7 +58,7 @@
 TEST_F(TUSchedulerTests, MissingFiles) {
   TUScheduler S(getDefaultAsyncThreadsCount(),
 /*StorePreamblesInMemory=*/true,
-/*PreambleParsedCallback=*/nullptr,
+NoopParsingCallbacks::instance(),
 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
 ASTRetentionPolicy());
 
@@ -102,7 +115,7 @@
 TUScheduler S(
 getDefaultAsyncThreadsCount(),
 /*StorePreamblesInMemory=*/true,
-/*PreambleParsedCallback=*/nullptr,
+NoopParsingCallbacks::instance(),
 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
 ASTRetentionPolicy());
 auto Path = testPath("foo.cpp");
@@ -129,11 +142,10 @@
 TEST_F(TUSchedulerTests, Debounce) {
   std::atomic CallbackCount(0);
   {
-TUScheduler S(getDefaultAsyncThreadsCount(),
-  /*StorePreamblesInMemory=*/true,
-  /*PreambleParsedCallback=*/nullptr,
-  /*UpdateDebounce=*/std::chrono::seconds(1),
-  ASTRetentionPolicy());
+TUScheduler S(
+getDefaultAsyncThreadsCount(),
+/*StorePreamblesInMemory=*/true, NoopParsingCallbacks::instance(),
+/*UpdateDebounce=*/std::chrono::seconds(1), ASTRetentionPolicy());
 // FIXME: we could probably use timeouts lower than 1 second here.
 auto Path = testPath("foo.cpp");
 S.update(Path, getInputs(Path, "auto (debounced)"), WantDiagnostics::Auto,
@@ -163,7 +175,7 @@
   {
 TUScheduler S(getDefaultAsyncThreadsCount(),
   /*StorePreamblesInMemory=*/true,
-  /*PreambleParsedCallback=*/nullptr,
+  NoopParsingCallbacks::instance(),
   /*UpdateDebounce=*/std::chrono::milliseconds(50),
   ASTRetentionPolicy());
 
@@ -261,7 +273,7 @@
   Policy.MaxRetainedASTs = 2;
   TUScheduler S(
   /*AsyncThreadsCount=*/1, /*StorePreambleInMemory=*/true,
-  PreambleParsedCallback(),
+  NoopParsingCallbacks::instance(),
   /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), Policy);
 
   llvm::StringLiteral SourceContents = R"cpp(
@@ -358,7 +370,7 @@
   // the same time. All reads should get the same non-null preamble.
   TUScheduler S(
   /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
-  PreambleParsedCallback(),
+  NoopParsingCallbacks::instance(),
   /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
   ASTRetentionPolicy());
   auto Foo = testPath("foo.cpp");
@@ -391,7 +403,7 @@
 TEST_F(TUSchedulerTests, NoopOnEmptyChanges) {
   TUScheduler S(
   /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
-  /*StorePreambleInMemory=*/true, PreambleParsedCallback(),
+  /*StorePreambleInMemory=*/true, NoopParsingCallbacks::instance(),
   /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
   ASTRetentionPolicy());
 
@@ -444,7 +456,7 @@
 TEST_F(TUSchedulerTests, NoChangeDiags) {
   TUScheduler S(
   /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
-  /*StorePreambleInMemory=*/true,

[PATCH] D50958: [clangd] WIP: xrefs for dynamic index.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

This patch is incomplete, and it is a **prototype** based on our discussion 
last week. You can patch it locally, and play around with VSCode.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50958



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

[PATCH] D50960: [clangd] Add missing lock in the lookup.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: ioeric.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ilya-biryukov.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50960

Files:
  clangd/index/MemIndex.cpp


Index: clangd/index/MemIndex.cpp
===
--- clangd/index/MemIndex.cpp
+++ clangd/index/MemIndex.cpp
@@ -64,6 +64,7 @@
 
 void MemIndex::lookup(const LookupRequest &Req,
   llvm::function_ref Callback) const 
{
+  std::lock_guard Lock(Mutex);
   for (const auto &ID : Req.IDs) {
 auto I = Index.find(ID);
 if (I != Index.end())


Index: clangd/index/MemIndex.cpp
===
--- clangd/index/MemIndex.cpp
+++ clangd/index/MemIndex.cpp
@@ -64,6 +64,7 @@
 
 void MemIndex::lookup(const LookupRequest &Req,
   llvm::function_ref Callback) const {
+  std::lock_guard Lock(Mutex);
   for (const auto &ID : Req.IDs) {
 auto I = Index.find(ID);
 if (I != Index.end())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50960: [clangd] Add missing lock in the lookup.

[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:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50960



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

[PATCH] D50956: [clangd] NFC: Cleanup Dex Iterator comments and simplify tests

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 161443.
kbobyrev edited the summary of this revision.
kbobyrev added a comment.

Something I initially forgot: fix misplaced `private:` so that `dump()` stays 
private as it was in the interface.


https://reviews.llvm.org/D50956

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
@@ -28,33 +28,33 @@
   auto DocIterator = create(L);
 
   EXPECT_EQ(DocIterator->peek(), 4U);
-  EXPECT_EQ(DocIterator->reachedEnd(), false);
+  EXPECT_FALSE(DocIterator->reachedEnd());
 
   DocIterator->advance();
   EXPECT_EQ(DocIterator->peek(), 7U);
-  EXPECT_EQ(DocIterator->reachedEnd(), false);
+  EXPECT_FALSE(DocIterator->reachedEnd());
 
   DocIterator->advanceTo(20);
   EXPECT_EQ(DocIterator->peek(), 20U);
-  EXPECT_EQ(DocIterator->reachedEnd(), false);
+  EXPECT_FALSE(DocIterator->reachedEnd());
 
   DocIterator->advanceTo(65);
   EXPECT_EQ(DocIterator->peek(), 100U);
-  EXPECT_EQ(DocIterator->reachedEnd(), false);
+  EXPECT_FALSE(DocIterator->reachedEnd());
 
   DocIterator->advanceTo(420);
-  EXPECT_EQ(DocIterator->reachedEnd(), true);
+  EXPECT_TRUE(DocIterator->reachedEnd());
 }
 
 TEST(DexIndexIterators, AndWithEmpty) {
   const PostingList L0;
   const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000};
 
   auto AndEmpty = createAnd(create(L0));
-  EXPECT_EQ(AndEmpty->reachedEnd(), true);
+  EXPECT_TRUE(AndEmpty->reachedEnd());
 
   auto AndWithEmpty = createAnd(create(L0), create(L1));
-  EXPECT_EQ(AndWithEmpty->reachedEnd(), true);
+  EXPECT_TRUE(AndWithEmpty->reachedEnd());
 
   EXPECT_THAT(consume(*AndWithEmpty), ElementsAre());
 }
@@ -65,7 +65,7 @@
 
   auto And = createAnd(create(L1), create(L0));
 
-  EXPECT_EQ(And->reachedEnd(), false);
+  EXPECT_FALSE(And->reachedEnd());
   EXPECT_THAT(consume(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U));
 
   And = createAnd(create(L0), create(L1));
@@ -94,18 +94,18 @@
   EXPECT_EQ(And->peek(), 320U);
   And->advanceTo(10);
 
-  EXPECT_EQ(And->reachedEnd(), true);
+  EXPECT_TRUE(And->reachedEnd());
 }
 
 TEST(DexIndexIterators, OrWithEmpty) {
   const PostingList L0;
   const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000};
 
   auto OrEmpty = createOr(create(L0));
-  EXPECT_EQ(OrEmpty->reachedEnd(), true);
+  EXPECT_TRUE(OrEmpty->reachedEnd());
 
   auto OrWithEmpty = createOr(create(L0), create(L1));
-  EXPECT_EQ(OrWithEmpty->reachedEnd(), false);
+  EXPECT_FALSE(OrWithEmpty->reachedEnd());
 
   EXPECT_THAT(consume(*OrWithEmpty),
   ElementsAre(0U, 5U, 7U, 10U, 42U, 320U, 9000U));
@@ -117,7 +117,7 @@
 
   auto Or = createOr(create(L0), create(L1));
 
-  EXPECT_EQ(Or->reachedEnd(), false);
+  EXPECT_FALSE(Or->reachedEnd());
   EXPECT_EQ(Or->peek(), 0U);
   Or->advance();
   EXPECT_EQ(Or->peek(), 4U);
@@ -136,7 +136,7 @@
   Or->advanceTo(9000);
   EXPECT_EQ(Or->peek(), 9000U);
   Or->advanceTo(9001);
-  EXPECT_EQ(Or->reachedEnd(), true);
+  EXPECT_TRUE(Or->reachedEnd());
 
   Or = createOr(create(L0), create(L1));
 
@@ -151,7 +151,7 @@
 
   auto Or = createOr(create(L0), create(L1), create(L2));
 
-  EXPECT_EQ(Or->reachedEnd(), false);
+  EXPECT_FALSE(Or->reachedEnd());
   EXPECT_EQ(Or->peek(), 0U);
 
   Or->advance();
@@ -166,7 +166,7 @@
   EXPECT_EQ(Or->peek(), 60U);
 
   Or->advanceTo(9001);
-  EXPECT_EQ(Or->reachedEnd(), true);
+  EXPECT_TRUE(Or->reachedEnd());
 }
 
 // FIXME(kbobyrev): The testcase below is similar to what is expected in real
@@ -208,7 +208,7 @@
   // Lower Or Iterator: [0, 1, 5]
   createOr(create(L2), create(L3), create(L4)));
 
-  EXPECT_EQ(Root->reachedEnd(), false);
+  EXPECT_FALSE(Root->reachedEnd());
   EXPECT_EQ(Root->peek(), 1U);
   Root->advanceTo(0);
   // Advance multiple times. Shouldn't do anything.
@@ -220,7 +220,7 @@
   Root->advanceTo(5);
   EXPECT_EQ(Root->peek(), 5U);
   Root->advanceTo(9000);
-  EXPECT_EQ(Root->reachedEnd(), true);
+  EXPECT_TRUE(Root->reachedEnd());
 }
 
 TEST(DexIndexIterators, StringRepresentation) {
@@ -264,14 +264,14 @@
 
 TEST(DexIndexIterators, True) {
   auto TrueIterator = createTrue(0U);
-  EXPECT_THAT(TrueIterator->reachedEnd(), true);
+  EXPECT_TRUE(TrueIterator->reachedEnd());
   EXPECT_THAT(consume(*TrueIterator), ElementsAre());
 
   PostingList L0 = {1, 2, 5, 7};
   TrueIterator = createTrue(7U);
   EXPECT_THAT(TrueIterator->peek(), 0);
   auto AndIterator = createAnd(create(L0), move(TrueIterator));
-  EXPECT_THAT(AndIterator->reachedEnd(), false);
+  EXPECT_FALSE(AndIterator->reachedEnd());
   EXPECT_THAT(consume(*AndIterator), ElementsAre(1, 2, 5));
 }
 
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-ext

[PATCH] D50961: [clang-tidy] Add missing check documentation.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: ioeric.
Herald added subscribers: kadircet, arphaman, jkorous, ilya-biryukov, xazax.hun.

[clangd] Simplify the code using UniqueStringSaver, NFC.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50961

Files:
  clangd/index/Index.cpp
  clangd/index/Index.h
  docs/clang-tidy/checks/abseil-duration-division.rst

Index: docs/clang-tidy/checks/abseil-duration-division.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-duration-division.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-duration-division
+
+abseil-duration-division
+
+
+``absl::Duration`` arithmetic works like it does with integers. That means that
+division of two ``absl::Duration`` objects returns an ``int64`` with any fractional
+component truncated toward 0. See `this link `_ for more information on arithmetic with ``absl::Duration``.
+
+For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ int64 sec1 = d / absl::Seconds(1); // Truncates toward 0.
+ int64 sec2 = absl::ToInt64Seconds(d);  // Equivalent to division.
+ assert(sec1 == 3 && sec2 == 3);
+
+ double dsec = d / absl::Seconds(1);  // WRONG: Still truncates toward 0.
+ assert(dsec == 3.0);
+
+If you want floating-point division, you should use either the
+``absl::FDivDuration()`` function, or one of the unit conversion functions such
+as ``absl::ToDoubleSeconds()``. For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ double dsec1 = absl::FDivDuration(d, absl::Seconds(1));  // GOOD: No truncation.
+ double dsec2 = absl::ToDoubleSeconds(d); // GOOD: No truncation.
+ assert(dsec1 == 3.5 && dsec2 == 3.5);
+
+
+This check looks for uses of ``absl::Duration`` division that is done in a
+floating-point context, and recommends the use of a function that returns a
+floating-point value.
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -17,6 +17,7 @@
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/StringSaver.h"
 #include 
 #include 
 
@@ -257,6 +258,8 @@
   // The frozen SymbolSlab will use less memory.
   class Builder {
   public:
+Builder() : UniqueStrings(Arena) {}
+
 // Adds a symbol, overwriting any existing one with the same ID.
 // This is a deep copy: underlying strings will be owned by the slab.
 void insert(const Symbol &S);
@@ -273,7 +276,7 @@
   private:
 llvm::BumpPtrAllocator Arena;
 // Intern table for strings. Contents are on the arena.
-llvm::DenseSet Strings;
+llvm::UniqueStringSaver UniqueStrings;
 std::vector Symbols;
 // Values are indices into Symbols vector.
 llvm::DenseMap SymbolIndex;
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -77,16 +77,10 @@
 }
 
 // Copy the underlying data of the symbol into the owned arena.
-static void own(Symbol &S, DenseSet &Strings,
+static void own(Symbol &S, llvm::UniqueStringSaver &Strings,
 BumpPtrAllocator &Arena) {
   // Intern replaces V with a reference to the same string owned by the arena.
-  auto Intern = [&](StringRef &V) {
-auto R = Strings.insert(V);
-if (R.second) { // New entry added to the table, copy the string.
-  *R.first = V.copy(Arena);
-}
-V = *R.first;
-  };
+  auto Intern = [&](StringRef &V) { V = Strings.save(V); };
 
   // We need to copy every StringRef field onto the arena.
   Intern(S.Name);
@@ -114,10 +108,10 @@
   auto R = SymbolIndex.try_emplace(S.ID, Symbols.size());
   if (R.second) {
 Symbols.push_back(S);
-own(Symbols.back(), Strings, Arena);
+own(Symbols.back(), UniqueStrings, Arena);
   } else {
 auto &Copy = Symbols[R.first->second] = S;
-own(Copy, Strings, Arena);
+own(Copy, UniqueStrings, Arena);
   }
 }
 
@@ -128,7 +122,7 @@
 [](const Symbol &L, const Symbol &R) { return L.ID < R.ID; });
   // We may have unused strings from overwritten symbols. Build a new arena.
   BumpPtrAllocator NewArena;
-  DenseSet Strings;
+  llvm::UniqueStringSaver Strings(NewArena);
   for (auto &S : Symbols)
 own(S, Strings, NewArena);
   return SymbolSlab(std::move(NewArena), std::move(Symbols));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50961: [clangd] Simplify the code using UniqueStringSaver, NFC.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 161445.
hokein added a comment.

Correct the patch.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50961

Files:
  clangd/index/Index.cpp
  clangd/index/Index.h
  docs/clang-tidy/checks/abseil-duration-division.rst

Index: docs/clang-tidy/checks/abseil-duration-division.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-duration-division.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-duration-division
+
+abseil-duration-division
+
+
+``absl::Duration`` arithmetic works like it does with integers. That means that
+division of two ``absl::Duration`` objects returns an ``int64`` with any fractional
+component truncated toward 0. See `this link `_ for more information on arithmetic with ``absl::Duration``.
+
+For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ int64 sec1 = d / absl::Seconds(1); // Truncates toward 0.
+ int64 sec2 = absl::ToInt64Seconds(d);  // Equivalent to division.
+ assert(sec1 == 3 && sec2 == 3);
+
+ double dsec = d / absl::Seconds(1);  // WRONG: Still truncates toward 0.
+ assert(dsec == 3.0);
+
+If you want floating-point division, you should use either the
+``absl::FDivDuration()`` function, or one of the unit conversion functions such
+as ``absl::ToDoubleSeconds()``. For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ double dsec1 = absl::FDivDuration(d, absl::Seconds(1));  // GOOD: No truncation.
+ double dsec2 = absl::ToDoubleSeconds(d); // GOOD: No truncation.
+ assert(dsec1 == 3.5 && dsec2 == 3.5);
+
+
+This check looks for uses of ``absl::Duration`` division that is done in a
+floating-point context, and recommends the use of a function that returns a
+floating-point value.
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -17,6 +17,7 @@
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/StringSaver.h"
 #include 
 #include 
 
@@ -257,6 +258,8 @@
   // The frozen SymbolSlab will use less memory.
   class Builder {
   public:
+Builder() : UniqueStrings(Arena) {}
+
 // Adds a symbol, overwriting any existing one with the same ID.
 // This is a deep copy: underlying strings will be owned by the slab.
 void insert(const Symbol &S);
@@ -273,7 +276,7 @@
   private:
 llvm::BumpPtrAllocator Arena;
 // Intern table for strings. Contents are on the arena.
-llvm::DenseSet Strings;
+llvm::UniqueStringSaver UniqueStrings;
 std::vector Symbols;
 // Values are indices into Symbols vector.
 llvm::DenseMap SymbolIndex;
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -77,16 +77,10 @@
 }
 
 // Copy the underlying data of the symbol into the owned arena.
-static void own(Symbol &S, DenseSet &Strings,
+static void own(Symbol &S, llvm::UniqueStringSaver &Strings,
 BumpPtrAllocator &Arena) {
   // Intern replaces V with a reference to the same string owned by the arena.
-  auto Intern = [&](StringRef &V) {
-auto R = Strings.insert(V);
-if (R.second) { // New entry added to the table, copy the string.
-  *R.first = V.copy(Arena);
-}
-V = *R.first;
-  };
+  auto Intern = [&](StringRef &V) { V = Strings.save(V); };
 
   // We need to copy every StringRef field onto the arena.
   Intern(S.Name);
@@ -114,10 +108,10 @@
   auto R = SymbolIndex.try_emplace(S.ID, Symbols.size());
   if (R.second) {
 Symbols.push_back(S);
-own(Symbols.back(), Strings, Arena);
+own(Symbols.back(), UniqueStrings, Arena);
   } else {
 auto &Copy = Symbols[R.first->second] = S;
-own(Copy, Strings, Arena);
+own(Copy, UniqueStrings, Arena);
   }
 }
 
@@ -128,7 +122,7 @@
 [](const Symbol &L, const Symbol &R) { return L.ID < R.ID; });
   // We may have unused strings from overwritten symbols. Build a new arena.
   BumpPtrAllocator NewArena;
-  DenseSet Strings;
+  llvm::UniqueStringSaver Strings(NewArena);
   for (auto &S : Symbols)
 own(S, Strings, NewArena);
   return SymbolSlab(std::move(NewArena), std::move(Symbols));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50961: [clangd] Simplify the code using UniqueStringSaver, NFC.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 161446.
hokein added a comment.

Update


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50961

Files:
  clangd/index/Index.cpp
  clangd/index/Index.h


Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -17,6 +17,7 @@
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/StringSaver.h"
 #include 
 #include 
 
@@ -257,6 +258,8 @@
   // The frozen SymbolSlab will use less memory.
   class Builder {
   public:
+Builder() : UniqueStrings(Arena) {}
+
 // Adds a symbol, overwriting any existing one with the same ID.
 // This is a deep copy: underlying strings will be owned by the slab.
 void insert(const Symbol &S);
@@ -273,7 +276,7 @@
   private:
 llvm::BumpPtrAllocator Arena;
 // Intern table for strings. Contents are on the arena.
-llvm::DenseSet Strings;
+llvm::UniqueStringSaver UniqueStrings;
 std::vector Symbols;
 // Values are indices into Symbols vector.
 llvm::DenseMap SymbolIndex;
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -77,16 +77,10 @@
 }
 
 // Copy the underlying data of the symbol into the owned arena.
-static void own(Symbol &S, DenseSet &Strings,
+static void own(Symbol &S, llvm::UniqueStringSaver &Strings,
 BumpPtrAllocator &Arena) {
   // Intern replaces V with a reference to the same string owned by the arena.
-  auto Intern = [&](StringRef &V) {
-auto R = Strings.insert(V);
-if (R.second) { // New entry added to the table, copy the string.
-  *R.first = V.copy(Arena);
-}
-V = *R.first;
-  };
+  auto Intern = [&](StringRef &V) { V = Strings.save(V); };
 
   // We need to copy every StringRef field onto the arena.
   Intern(S.Name);
@@ -114,10 +108,10 @@
   auto R = SymbolIndex.try_emplace(S.ID, Symbols.size());
   if (R.second) {
 Symbols.push_back(S);
-own(Symbols.back(), Strings, Arena);
+own(Symbols.back(), UniqueStrings, Arena);
   } else {
 auto &Copy = Symbols[R.first->second] = S;
-own(Copy, Strings, Arena);
+own(Copy, UniqueStrings, Arena);
   }
 }
 
@@ -128,7 +122,7 @@
 [](const Symbol &L, const Symbol &R) { return L.ID < R.ID; });
   // We may have unused strings from overwritten symbols. Build a new arena.
   BumpPtrAllocator NewArena;
-  DenseSet Strings;
+  llvm::UniqueStringSaver Strings(NewArena);
   for (auto &S : Symbols)
 own(S, Strings, NewArena);
   return SymbolSlab(std::move(NewArena), std::move(Symbols));


Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -17,6 +17,7 @@
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/StringSaver.h"
 #include 
 #include 
 
@@ -257,6 +258,8 @@
   // The frozen SymbolSlab will use less memory.
   class Builder {
   public:
+Builder() : UniqueStrings(Arena) {}
+
 // Adds a symbol, overwriting any existing one with the same ID.
 // This is a deep copy: underlying strings will be owned by the slab.
 void insert(const Symbol &S);
@@ -273,7 +276,7 @@
   private:
 llvm::BumpPtrAllocator Arena;
 // Intern table for strings. Contents are on the arena.
-llvm::DenseSet Strings;
+llvm::UniqueStringSaver UniqueStrings;
 std::vector Symbols;
 // Values are indices into Symbols vector.
 llvm::DenseMap SymbolIndex;
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -77,16 +77,10 @@
 }
 
 // Copy the underlying data of the symbol into the owned arena.
-static void own(Symbol &S, DenseSet &Strings,
+static void own(Symbol &S, llvm::UniqueStringSaver &Strings,
 BumpPtrAllocator &Arena) {
   // Intern replaces V with a reference to the same string owned by the arena.
-  auto Intern = [&](StringRef &V) {
-auto R = Strings.insert(V);
-if (R.second) { // New entry added to the table, copy the string.
-  *R.first = V.copy(Arena);
-}
-V = *R.first;
-  };
+  auto Intern = [&](StringRef &V) { V = Strings.save(V); };
 
   // We need to copy every StringRef field onto the arena.
   Intern(S.Name);
@@ -114,10 +108,10 @@
   auto R = SymbolIndex.try_emplace(S.ID, Symbols.size());
   if (R.second) {
 Symbols.push_back(S);
-own(Symbols.back(), Strings, Arena);
+own(Symbols.back(), UniqueStrings, Arena);
   } else {
 auto &Copy = Symbols[R.first->second] = S;
-own(Copy, Strings, Arena);
+own(Copy, UniqueStrings, Arena);
   }
 }
 
@@ -128,7 +122,7 @@
 [](const Symbol &L, const Symbol &R) { return L.ID < R.ID; });
   // We may have unused st

[clang-tools-extra] r340156 - [clangd] Add missing lock in the lookup.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Mon Aug 20 02:07:59 2018
New Revision: 340156

URL: http://llvm.org/viewvc/llvm-project?rev=340156&view=rev
Log:
[clangd] Add missing lock in the lookup.

Reviewers: ioeric

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/index/MemIndex.cpp

Modified: clang-tools-extra/trunk/clangd/index/MemIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/MemIndex.cpp?rev=340156&r1=340155&r2=340156&view=diff
==
--- clang-tools-extra/trunk/clangd/index/MemIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/MemIndex.cpp Mon Aug 20 02:07:59 2018
@@ -64,6 +64,7 @@ bool MemIndex::fuzzyFind(
 
 void MemIndex::lookup(const LookupRequest &Req,
   llvm::function_ref Callback) const 
{
+  std::lock_guard Lock(Mutex);
   for (const auto &ID : Req.IDs) {
 auto I = Index.find(ID);
 if (I != Index.end())


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

[PATCH] D50960: [clangd] Add missing lock in the lookup.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE340156: [clangd] Add missing lock in the lookup. (authored 
by hokein, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50960?vs=161442&id=161447#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50960

Files:
  clangd/index/MemIndex.cpp


Index: clangd/index/MemIndex.cpp
===
--- clangd/index/MemIndex.cpp
+++ clangd/index/MemIndex.cpp
@@ -64,6 +64,7 @@
 
 void MemIndex::lookup(const LookupRequest &Req,
   llvm::function_ref Callback) const 
{
+  std::lock_guard Lock(Mutex);
   for (const auto &ID : Req.IDs) {
 auto I = Index.find(ID);
 if (I != Index.end())


Index: clangd/index/MemIndex.cpp
===
--- clangd/index/MemIndex.cpp
+++ clangd/index/MemIndex.cpp
@@ -64,6 +64,7 @@
 
 void MemIndex::lookup(const LookupRequest &Req,
   llvm::function_ref Callback) const {
+  std::lock_guard Lock(Mutex);
   for (const auto &ID : Req.IDs) {
 auto I = Index.find(ID);
 if (I != Index.end())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50961: [clangd] Simplify the code using UniqueStringSaver, NFC.

[Eric Liu via Phabricator via cfe-commits]

ioeric accepted this revision.
ioeric added inline comments.
This revision is now accepted and ready to land.



Comment at: docs/clang-tidy/checks/abseil-duration-division.rst:1
+.. title:: clang-tidy - abseil-duration-division
+

Please revert.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50961



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

[PATCH] D50571: [clangd] add an extension field to LSP to transfer the diagnostic's category

[João Távora via Phabricator via cfe-commits]

joaotavora added a comment.
Herald added a subscriber: kadircet.

> LGTM. Let's watch out for possible breakages in any of the clients, though. 
> I've checked VSCode and it seems to be fine with the added fields.

This isn't in the spec and broke the LSP client eglot 
(https://github.com/joaotavora/eglot/pull/81). Why don't you put this in the 
"source" field, or concat it to the "message" field.  Who can even use this 
information if it's not in the spec? Are clients supposed to code against every 
LSP server's whim?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50571



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

[clang-tools-extra] r340157 - [clangd] NFC: Cleanup Dex Iterator comments and simplify tests

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Mon Aug 20 02:16:14 2018
New Revision: 340157

URL: http://llvm.org/viewvc/llvm-project?rev=340157&view=rev
Log:
[clangd] NFC: Cleanup Dex Iterator comments and simplify tests

Proposed changes:

* Cleanup comments in `clangd/index/dex/Iterator.h`: Vim's `gq`
  formatting added redundant spaces instead of newlines in few
  places
* Few comments in `OrIterator` are wrong
* Use `EXPECT_TRUE(Condition)` instead of
  `EXPECT_THAT(Condition, true)` (same with `EXPECT_FALSE`)
* Don't expose `dump()` method to the public by misplacing
  `private:`

This patch does not affect functionality.

Reviewed by: ioeric

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

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

Modified: clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp?rev=340157&r1=340156&r2=340157&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp Mon Aug 20 02:16:14 
2018
@@ -46,6 +46,7 @@ public:
 return *Index;
   }
 
+private:
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
 OS << '[';
 auto Separator = "";
@@ -66,7 +67,6 @@ public:
 return OS;
   }
 
-private:
   PostingListRef Documents;
   PostingListRef::const_iterator Index;
 };
@@ -103,6 +103,7 @@ public:
 
   DocID peek() const override { return Children.front()->peek(); }
 
+private:
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
 OS << "(& ";
 auto Separator = "";
@@ -114,7 +115,6 @@ public:
 return OS;
   }
 
-private:
   /// Restores class invariants: each child will point to the same element 
after
   /// sync.
   void sync() {
@@ -180,7 +180,7 @@ public:
   /// Moves each child pointing to the smallest DocID to the next item.
   void advance() override {
 assert(!reachedEnd() &&
-   "OrIterator must have at least one child to advance().");
+   "OrIterator can't call advance() after it reached the end.");
 const auto SmallestID = peek();
 for (const auto &Child : Children)
   if (!Child->reachedEnd() && Child->peek() == SmallestID)
@@ -199,7 +199,7 @@ public:
   /// value.
   DocID peek() const override {
 assert(!reachedEnd() &&
-   "OrIterator must have at least one child to peek().");
+   "OrIterator can't peek() after it reached the end.");
 DocID Result = std::numeric_limits::max();
 
 for (const auto &Child : Children)
@@ -209,6 +209,7 @@ public:
 return Result;
   }
 
+private:
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
 OS << "(| ";
 auto Separator = "";
@@ -220,7 +221,6 @@ public:
 return OS;
   }
 
-private:
   // FIXME(kbobyrev): Would storing Children in min-heap be faster?
   std::vector> Children;
 };

Modified: clang-tools-extra/trunk/clangd/index/dex/Iterator.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Iterator.h?rev=340157&r1=340156&r2=340157&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.h (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.h Mon Aug 20 02:16:14 2018
@@ -11,14 +11,14 @@
 // symbol, such as high fuzzy matching score, scope, type etc. The lists of all
 // symbols matching some criteria (e.g. belonging to "clang::clangd::" scope)
 // are expressed in a form of Search Tokens which are stored in the inverted
-// index.  Inverted index maps these tokens to the posting lists - sorted ( by
-// symbol quality) sequences of symbol IDs matching the token, e.g.  scope 
token
+// index. Inverted index maps these tokens to the posting lists - sorted (by
+// symbol quality) sequences of symbol IDs matching the token, e.g. scope token
 // "clangd::clangd::" is mapped to the list of IDs of all symbols which are
 // declared in this namespace. Search queries are build from a set of
 // requirements which can be combined with each other forming the query trees.
 // The leafs of such trees are posting lists, and the nodes are operations on
-// these posting lists, e.g. intersection or union.  Efficient processing of
-// these multi-level queries is handled by Iterators.  Iterators advance 
through
+// these posting lists, e.g. intersection or union. Efficient processing of
+// these multi-level queries is handled by Iterators. Iterators advance through
 // all leaf posting lists producing the result of search query, which preserves
 // the sorted order of IDs. Having the resulting IDs sorted is important,
 // because it allows receiving a certain number of the most valuable items 
(e.g.

Modified: 

[PATCH] D50956: [clangd] NFC: Cleanup Dex Iterator comments and simplify tests

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340157: [clangd] NFC: Cleanup Dex Iterator comments and 
simplify tests (authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50956?vs=161443&id=161448#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50956

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

Index: clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
@@ -28,33 +28,33 @@
   auto DocIterator = create(L);
 
   EXPECT_EQ(DocIterator->peek(), 4U);
-  EXPECT_EQ(DocIterator->reachedEnd(), false);
+  EXPECT_FALSE(DocIterator->reachedEnd());
 
   DocIterator->advance();
   EXPECT_EQ(DocIterator->peek(), 7U);
-  EXPECT_EQ(DocIterator->reachedEnd(), false);
+  EXPECT_FALSE(DocIterator->reachedEnd());
 
   DocIterator->advanceTo(20);
   EXPECT_EQ(DocIterator->peek(), 20U);
-  EXPECT_EQ(DocIterator->reachedEnd(), false);
+  EXPECT_FALSE(DocIterator->reachedEnd());
 
   DocIterator->advanceTo(65);
   EXPECT_EQ(DocIterator->peek(), 100U);
-  EXPECT_EQ(DocIterator->reachedEnd(), false);
+  EXPECT_FALSE(DocIterator->reachedEnd());
 
   DocIterator->advanceTo(420);
-  EXPECT_EQ(DocIterator->reachedEnd(), true);
+  EXPECT_TRUE(DocIterator->reachedEnd());
 }
 
 TEST(DexIndexIterators, AndWithEmpty) {
   const PostingList L0;
   const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000};
 
   auto AndEmpty = createAnd(create(L0));
-  EXPECT_EQ(AndEmpty->reachedEnd(), true);
+  EXPECT_TRUE(AndEmpty->reachedEnd());
 
   auto AndWithEmpty = createAnd(create(L0), create(L1));
-  EXPECT_EQ(AndWithEmpty->reachedEnd(), true);
+  EXPECT_TRUE(AndWithEmpty->reachedEnd());
 
   EXPECT_THAT(consume(*AndWithEmpty), ElementsAre());
 }
@@ -65,7 +65,7 @@
 
   auto And = createAnd(create(L1), create(L0));
 
-  EXPECT_EQ(And->reachedEnd(), false);
+  EXPECT_FALSE(And->reachedEnd());
   EXPECT_THAT(consume(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U));
 
   And = createAnd(create(L0), create(L1));
@@ -94,18 +94,18 @@
   EXPECT_EQ(And->peek(), 320U);
   And->advanceTo(10);
 
-  EXPECT_EQ(And->reachedEnd(), true);
+  EXPECT_TRUE(And->reachedEnd());
 }
 
 TEST(DexIndexIterators, OrWithEmpty) {
   const PostingList L0;
   const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000};
 
   auto OrEmpty = createOr(create(L0));
-  EXPECT_EQ(OrEmpty->reachedEnd(), true);
+  EXPECT_TRUE(OrEmpty->reachedEnd());
 
   auto OrWithEmpty = createOr(create(L0), create(L1));
-  EXPECT_EQ(OrWithEmpty->reachedEnd(), false);
+  EXPECT_FALSE(OrWithEmpty->reachedEnd());
 
   EXPECT_THAT(consume(*OrWithEmpty),
   ElementsAre(0U, 5U, 7U, 10U, 42U, 320U, 9000U));
@@ -117,7 +117,7 @@
 
   auto Or = createOr(create(L0), create(L1));
 
-  EXPECT_EQ(Or->reachedEnd(), false);
+  EXPECT_FALSE(Or->reachedEnd());
   EXPECT_EQ(Or->peek(), 0U);
   Or->advance();
   EXPECT_EQ(Or->peek(), 4U);
@@ -136,7 +136,7 @@
   Or->advanceTo(9000);
   EXPECT_EQ(Or->peek(), 9000U);
   Or->advanceTo(9001);
-  EXPECT_EQ(Or->reachedEnd(), true);
+  EXPECT_TRUE(Or->reachedEnd());
 
   Or = createOr(create(L0), create(L1));
 
@@ -151,7 +151,7 @@
 
   auto Or = createOr(create(L0), create(L1), create(L2));
 
-  EXPECT_EQ(Or->reachedEnd(), false);
+  EXPECT_FALSE(Or->reachedEnd());
   EXPECT_EQ(Or->peek(), 0U);
 
   Or->advance();
@@ -166,7 +166,7 @@
   EXPECT_EQ(Or->peek(), 60U);
 
   Or->advanceTo(9001);
-  EXPECT_EQ(Or->reachedEnd(), true);
+  EXPECT_TRUE(Or->reachedEnd());
 }
 
 // FIXME(kbobyrev): The testcase below is similar to what is expected in real
@@ -208,7 +208,7 @@
   // Lower Or Iterator: [0, 1, 5]
   createOr(create(L2), create(L3), create(L4)));
 
-  EXPECT_EQ(Root->reachedEnd(), false);
+  EXPECT_FALSE(Root->reachedEnd());
   EXPECT_EQ(Root->peek(), 1U);
   Root->advanceTo(0);
   // Advance multiple times. Shouldn't do anything.
@@ -220,7 +220,7 @@
   Root->advanceTo(5);
   EXPECT_EQ(Root->peek(), 5U);
   Root->advanceTo(9000);
-  EXPECT_EQ(Root->reachedEnd(), true);
+  EXPECT_TRUE(Root->reachedEnd());
 }
 
 TEST(DexIndexIterators, StringRepresentation) {
@@ -264,14 +264,14 @@
 
 TEST(DexIndexIterators, True) {
   auto TrueIterator = createTrue(0U);
-  EXPECT_THAT(TrueIterator->reachedEnd(), true);
+  EXPECT_TRUE(TrueIterator->reachedEnd());
   EXPECT_THAT(consume(*TrueIterator), ElementsAre());
 
   PostingList L0 = {1, 2, 5, 7};
   TrueIterator = createTrue(7U);
   EXPECT_THAT(TrueIterator->peek(), 0);
   auto AndIterator = createAnd(create(L0), move(TrueIterator));
-  EXPECT_THAT(AndIterator->reachedEnd(), false);
+  EXPECT_FALSE(AndIterator->reachedEnd());
   EXPECT_THAT(consume(*AndIterator), ElementsAre(1, 2,

[PATCH] D50847: [clangd] Add callbacks on parsed AST in addition to parsed preambles

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

The interfaces look mostly good to me.




Comment at: clangd/ClangdServer.cpp:73
+
+  void onPreambleAST(PathRef Path, ASTContext &Ctx,
+ std::shared_ptr PP) override {

I think `Ctx` should be a `pointer` which gives us a way (passing a nullptr) to 
clean up the `FileIndex`, the same as `ParsedAST` below.

And I discover that we don't cleanup dynamic index when a file is being closed, 
is it expected or a bug?



Comment at: clangd/TUScheduler.cpp:406
 // We only need to build the AST if diagnostics were requested.
 if (WantDiags == WantDiagnostics::No)
   return;

ilya-biryukov wrote:
> hokein wrote:
> > The AST might not get built if `WantDiags::No`, and will be built in 
> > `runWithAST`.
> I suggest we keep it a known limitation and not rebuild the index in that 
> case.
> The original intent of `WantDiags::No` was to update the contents of the 
> file, so that upcoming request can see the new contents (e.g. this is a hack 
> to avoid passing overriden contents of the file in the request itself).
> `WantDiags::No` is always followed by an actual request that wants the 
> diagnostics (and will, therefore, build the AST and emit the callback).
> 
> Alternatively, we could unify the code paths building the AST. Would be a bit 
> more intrusive change, but I guess it's worth it.
I see, SG, thanks!



Comment at: clangd/TUScheduler.h:66
+  /// instead.
+  virtual void onMainAST(PathRef Path, ParsedAST &AST) = 0;
+};

ioeric wrote:
> ilya-biryukov wrote:
> > hokein wrote:
> > > Does the callback get called every time we built an AST? clangd only has 
> > > 3 idle ASTs, if the AST is not there, we rebuild it when needed (even the 
> > > source code is not changed), and we will update the dynamic index, which 
> > > seems unnecessary.
> > > 
> > > It may rarely happen, 3 idle ASTs  might cover most cases, I think? 
> > Currently we only call this when AST is built for diagnostics, so we will 
> > have only a single callback, even if the AST will be rebuilt multiple times 
> > because it was flushed out of the cash (as long as the file contents didn't 
> > change, of course).
> > 
> > So we do behave optimally in that case and I suggest we keep it this way
> is there any overlap between preamble AST and main AST? If so, could you 
> elaborate in the documentation? 
> Currently we only call this when AST is built for diagnostics, so we will 
> have only a single callback, even if the AST will be rebuilt multiple times 
> because it was flushed out of the cash (as long as the file contents didn't 
> change, of course).

It sounds reasonable, thanks for the explanation. Could you clarify it in the 
comment?

> is there any overlap between preamble AST and main AST? If so, could you 
> elaborate in the documentation?

AFAIK, both of them contain different `TopLevelDecls`, but it is still possible 
to get the `Decl*` (declared in header) when visiting the main AST (e.g. when 
visiting the `void foo() {}` definition in `MainAST`, we can still get the 
`void foo();` declaration in `PreambleAST`). 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50847



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

[clang-tools-extra] r340161 - [clangd] Simplify the code using UniqueStringSaver, NFC.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Mon Aug 20 02:47:12 2018
New Revision: 340161

URL: http://llvm.org/viewvc/llvm-project?rev=340161&view=rev
Log:
[clangd] Simplify the code using UniqueStringSaver, NFC.

Modified:
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h

Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=340161&r1=340160&r2=340161&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Mon Aug 20 02:47:12 2018
@@ -77,16 +77,10 @@ SymbolSlab::const_iterator SymbolSlab::f
 }
 
 // Copy the underlying data of the symbol into the owned arena.
-static void own(Symbol &S, DenseSet &Strings,
+static void own(Symbol &S, llvm::UniqueStringSaver &Strings,
 BumpPtrAllocator &Arena) {
   // Intern replaces V with a reference to the same string owned by the arena.
-  auto Intern = [&](StringRef &V) {
-auto R = Strings.insert(V);
-if (R.second) { // New entry added to the table, copy the string.
-  *R.first = V.copy(Arena);
-}
-V = *R.first;
-  };
+  auto Intern = [&](StringRef &V) { V = Strings.save(V); };
 
   // We need to copy every StringRef field onto the arena.
   Intern(S.Name);
@@ -114,10 +108,10 @@ void SymbolSlab::Builder::insert(const S
   auto R = SymbolIndex.try_emplace(S.ID, Symbols.size());
   if (R.second) {
 Symbols.push_back(S);
-own(Symbols.back(), Strings, Arena);
+own(Symbols.back(), UniqueStrings, Arena);
   } else {
 auto &Copy = Symbols[R.first->second] = S;
-own(Copy, Strings, Arena);
+own(Copy, UniqueStrings, Arena);
   }
 }
 
@@ -128,7 +122,7 @@ SymbolSlab SymbolSlab::Builder::build()
 [](const Symbol &L, const Symbol &R) { return L.ID < R.ID; });
   // We may have unused strings from overwritten symbols. Build a new arena.
   BumpPtrAllocator NewArena;
-  DenseSet Strings;
+  llvm::UniqueStringSaver Strings(NewArena);
   for (auto &S : Symbols)
 own(S, Strings, NewArena);
   return SymbolSlab(std::move(NewArena), std::move(Symbols));

Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=340161&r1=340160&r2=340161&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Mon Aug 20 02:47:12 2018
@@ -17,6 +17,7 @@
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/StringSaver.h"
 #include 
 #include 
 
@@ -257,6 +258,8 @@ public:
   // The frozen SymbolSlab will use less memory.
   class Builder {
   public:
+Builder() : UniqueStrings(Arena) {}
+
 // Adds a symbol, overwriting any existing one with the same ID.
 // This is a deep copy: underlying strings will be owned by the slab.
 void insert(const Symbol &S);
@@ -273,7 +276,7 @@ public:
   private:
 llvm::BumpPtrAllocator Arena;
 // Intern table for strings. Contents are on the arena.
-llvm::DenseSet Strings;
+llvm::UniqueStringSaver UniqueStrings;
 std::vector Symbols;
 // Values are indices into Symbols vector.
 llvm::DenseMap SymbolIndex;


___
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 created this revision.
ioeric added reviewers: hokein, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.

For index-based code completion, send an asynchronous speculative index
request, based on the index request for the last code completion on the same
file and the filter text typed before the cursor, before sema code completion
is invoked. This can reduce the code completion latency (by roughly latency of
sema code completion) if the speculative request is the same as the one
generated for the ongoing code completion from sema. As a sequence of code
completions often have the same scopes and proximity paths etc, this should be
effective for a number of code completions.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50962

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/CodeComplete.cpp
  clangd/CodeComplete.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
@@ -1594,6 +1594,66 @@
   ElementsAre(Sig("foo(T, U) -> void", {"T", "U"})));
 }
 
+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("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());
+
+  FS.Files[testPath("bar.h")] =
+  R"cpp(namespace ns { struct preamble { int member; }; })cpp";
+  auto File = testPath("foo.cpp");
+  Annotations Test(R"cpp(
+  #include "bar.h"
+  namespace ns1 { int local1; }
+  namespace ns2 { int local2; }
+  void f() { ns1::^; }
+  void f() { ns2::$2^; }
+  )cpp");
+  runAddDocument(Server, File, Test.code());
+  clangd::CodeCompleteOptions Opts = {};
+
+  auto I = memIndex({var("ns1::index1"), var("ns2::index2")});
+  Opts.Index = I.get();
+  Opts.SpeculativeIndexRequest = true;
+
+  auto First = cantFail(runCodeComplete(Server, File, Test.point(), Opts));
+  EXPECT_THAT(First.Completions,
+  UnorderedElementsAre(Named("local1"), Named("index1")));
+
+  auto Second = cantFail(runCodeComplete(Server, File, Test.point(), Opts));
+  EXPECT_THAT(Second.Completions,
+  UnorderedElementsAre(Named("local1"), Named("index1")));
+
+  auto CacheMiss =
+  cantFail(runCodeComplete(Server, File, Test.point("2"), Opts));
+  EXPECT_THAT(CacheMiss.Completions,
+  UnorderedElementsAre(Named("local2"), Named("index2")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -164,6 +164,20 @@
"an include line will be inserted or not."),
 llvm::cl::init(true));
 
+static llvm::cl::opt SpeculateCompletionIndexRequest(
+"speculative-completion-index-request",
+llvm::cl::desc(
+R"(If set to true and static index is enabled, this will send an
+asynchronous speculative index request, based on the index request for
+the last code completion on the same file and the filter text typed
+before the cursor, before sema code completion is invoked. This can
+reduce the code completion latency (by roughly latency of sema code
+completion) if the speculative request is the same as the one generated
+for the ongoing code completion from sema. As a sequence of code
+completions often have the same scopes and proximity paths etc, this
+should be effective for a number of code completions.)"),
+llvm::cl::init(true));
+
 static llvm::cl::opt YamlSymbolFile(
 "yaml-symbol-file",
 llvm::cl::desc(
@@ -299,6 +313,8 @@
 CCOpts.IncludeIndicator.Insert.clear();
 CCOpts.IncludeIndicator.NoInsert.clear();
   }
+  CCOpts.SpeculativeIndexRequest =
+  SpeculateCompletionIndexRequest && Opts.StaticIndex;
 
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -18,7 +18,10 @@
 

[PATCH] D50889: [clangd] Make FileIndex aware of the main file

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.
Herald added a subscriber: kadircet.



Comment at: clangd/index/FileIndex.h:70
+  void
+  update(PathRef Path, ASTContext *AST, std::shared_ptr PP,
+ llvm::Optional> TopLevelDecls = llvm::None);

Any strong reason to unify the interface (and `indexAST`)? 

It seems to me that we do different things on `PreambleAST` and `MainAST`:

- on `PreambleAST`, we only index `Symbols` for code completion.
- on `MainAST`, we index `Symbols` (to collect other information missing from 
`PreambleAST`, e.g. definition location), and collect symbol references.

Therefore, we'd need to set different options for `SymbolCollector` 
accordingly. Having a single interface would make the implementation tricky 
(checking `TopLevelDecls` to see whether this is a `PreambleASt`).  


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50889



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

[PATCH] D50961: [clangd] Simplify the code using UniqueStringSaver, NFC.

[Haojian Wu via Phabricator via cfe-commits]

hokein closed this revision.
hokein added a comment.

committed in https://reviews.llvm.org/rL340161.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50961



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

[PATCH] D50889: [clangd] Make FileIndex aware of the main file

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/ClangdServer.cpp:70
+// FIXME(ibiryukov): this should be a generic helper instead.
+class NoopCallbacks : public ParsingCallbacks {
 public:

Maybe provide noop implementations for `ParsingCallbacks::onPreambleAST()` and 
`ParsingCallbacks::onMainAST` by default?



Comment at: clangd/ClangdServer.h:213
+  /// from the file itself. Those have different lifetimes and we merge 
results from both 
+  class DynamicIndex : public ParsingCallbacks {
+  public:

Can we move this into ClangdServer.cpp as an implementation helper? 



Comment at: clangd/ClangdServer.h:246
+  /// If present, an up-to-date of symbols in open files. Read via Index.
+  std::unique_ptr FileIdx;
   // If present, a merged view of FileIdx and an external index. Read via 
Index.

nit: `s/FileIdx/DymIdx`? (also need to update the comment above)



Comment at: clangd/index/FileIndex.cpp:46
+   AST.getTranslationUnitDecl()->decls().end());
+TopLevelDecls = Storage;
+  }

nit: I'd try avoiding modify the input argument if possible. Maybe set 
`Storage` properly according to `TopLevelDecls`?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50889



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

[PATCH] D50926: [SourceManager] Extract 'findFileIDsForFile' from 'translateFile' to allow mapping from one file entry to multiple FileIDs

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: include/clang/Basic/SourceManager.h:1533
 
+  /// Looks through all the local and imported source locations to find the set
+  /// of FileIDs that correspond to the given entry.

nit: put this closer to the closely related `translateFile`?



Comment at: include/clang/Basic/SourceManager.h:1539
+  /// \returns true if the callback returned true, false otherwise.
+  bool findFileIDsForFile(const FileEntry *SourceFile,
+  llvm::function_ref Callback) const;

Callback pattern seems uncommon in LLVM/Clang. I'd suggest making this return a 
set of `FileID`s and put the callback-based function as a helper (shared by 
this and `translateFile`)in the implementation.


Repository:
  rC Clang

https://reviews.llvm.org/D50926



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

[PATCH] D50963: [NFC] Don't define static function in header (UninitializedObject.h)

[Andrei Elovikov via Phabricator via cfe-commits]

a.elovikov created this revision.
a.elovikov added reviewers: Szelethus, erichkeane.

See also http://lists.llvm.org/pipermail/cfe-users/2016-January/000854.html for
the reasons why it's bad.


https://reviews.llvm.org/D50963

Files:
  clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
  clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp


Index: 
clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
===
--- 
clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
+++ 
clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
@@ -215,12 +215,10 @@
 llvm_unreachable("All cases are handled!");
   }
 
-  // Temporary variable to avoid warning from -Wunused-function.
-  bool IsPrimitive = isPrimitiveType(DynT->getPointeeType());
-  assert((IsPrimitive || DynT->isAnyPointerType() || DynT->isReferenceType()) 
&&
+  assert((isPrimitiveType(DynT->getPointeeType()) || DynT->isAnyPointerType() 
||
+  DynT->isReferenceType()) &&
  "At this point FR must either have a primitive dynamic type, or it "
  "must be a null, undefined, unknown or concrete pointer!");
-  (void)IsPrimitive;
 
   if (isPrimitiveUninit(DerefdV)) {
 if (NeedsCastBack)
Index: 
clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
===
--- clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
+++ clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
@@ -233,7 +233,7 @@
 /// value is undefined or not, such as ints and doubles, can be analyzed with
 /// ease. This also helps ensuring that every special field type is handled
 /// correctly.
-static bool isPrimitiveType(const QualType &T) {
+inline bool isPrimitiveType(const QualType &T) {
   return T->isBuiltinType() || T->isEnumeralType() || T->isMemberPointerType();
 }
 


Index: clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
+++ clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
@@ -215,12 +215,10 @@
 llvm_unreachable("All cases are handled!");
   }
 
-  // Temporary variable to avoid warning from -Wunused-function.
-  bool IsPrimitive = isPrimitiveType(DynT->getPointeeType());
-  assert((IsPrimitive || DynT->isAnyPointerType() || DynT->isReferenceType()) &&
+  assert((isPrimitiveType(DynT->getPointeeType()) || DynT->isAnyPointerType() ||
+  DynT->isReferenceType()) &&
  "At this point FR must either have a primitive dynamic type, or it "
  "must be a null, undefined, unknown or concrete pointer!");
-  (void)IsPrimitive;
 
   if (isPrimitiveUninit(DerefdV)) {
 if (NeedsCastBack)
Index: clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
===
--- clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
+++ clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
@@ -233,7 +233,7 @@
 /// value is undefined or not, such as ints and doubles, can be analyzed with
 /// ease. This also helps ensuring that every special field type is handled
 /// correctly.
-static bool isPrimitiveType(const QualType &T) {
+inline bool isPrimitiveType(const QualType &T) {
   return T->isBuiltinType() || T->isEnumeralType() || T->isMemberPointerType();
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

In https://reviews.llvm.org/D50616#1204588, @leonardchan wrote:

> Would it be simpler instead just to have the logic contained in the virtual 
> function for `TargetCodeGenInfo` as opposed to returning `nullptr` since any 
> custom target will end up overriding it anyway and ideally not return 
> `nullptr`?


I guess that's also possible. I figured it would be clearer to have the generic 
logic in the more obvious location (CGExprScalar) and fall back to the custom 
handling if needed. Many of the handlers in TargetCodeGenInfo are empty.

> I haven't brought this up to llvm-dev, but the main reason for why we decided 
> to only implement this in clang was because we thought it would be a lot 
> harder pushing a new type into upstream llvm, especially since a lot of the 
> features can be supported on the frontend using clang's existing 
> infrastructure. We are open to adding fixed point types to LLVM IR in the 
> future though once all the frontend stuff is laid out and if the community is 
> willing to accept it.

I know there are multiple threads on llvm-dev asking about fixed-point support 
(some of them from us) and the verdict usually seems to be "no new types, use 
integers and intrinsics". This has worked fairly well for us, so it should be 
possible to adapt the design for upstream if there is a desire to have it.

I do not think a new type is required. As there are no in-tree architectures 
with native fixed-point support (as far as I know? Does AVR or ARM support it, 
perhaps? Probably not enough to cover the entire E-C spec, though.), there 
isn't really much to warrant adding a new type (or even new IR operators) for 
it. Furthermore, many of the operations on fixed-point are simple integer 
operations (except ones like saturation/saturating operations, multiplication, 
division etc) so those would be better off implemented in terms of those 
operators rather than inventing new ones that do pretty much the same thing.

In https://reviews.llvm.org/D50616#1204754, @rjmccall wrote:

> Very few things in LLVM actually try to exhaustively handle all operations.  
> There are a
>  couple of generic predicates on `Instruction` like `mayHaveSideEffects`, 
> there's
>  serialization/`.ll`-writing, and there's code-gen.  The first two are not 
> hard at all
>  to implement, and it'd be quite simple to write a legalization pass in 
> code-gen that
>  turns all these operations into integer operations and which could easily be 
> customized
>  to support targets that want to do something more precise.


I certainly support the idea of representing some (not all) fixed-point 
operations as intrinsics (or instructions, although I suspect that's more work 
than one might think) and converting them into integer instructions for targets 
which do not care to support the specific operation natively. A type is 
overkill; fixed-point values **are** 'bags of bits' like LLVM integers, just 
with a different interpretation of the bits. It's no different than the 
distinction between signed and unsigned, and we model that on the LLVM level 
through the operations rather than through the types.

Creating instruction/intrinsic definitions of fixed-point operations does come 
with problems. If an IR producer wants to emit fixed-point operations with 
slightly different semantics than what our instructions/intrinsics have, 
they're out of luck. It might also be a problem for targets that have 
fixed-point support, but cannot perform the operations in exactly the same 
manner as the IR operation specifies. Making the semantics of the operation too 
tight could cause this to happen, but making them too loose makes it hard to 
really do anything with it. This is another argument for frontend 
customization. Every operation also introduces a bit of optimization/codegen 
work, unlike pure integer operations which the passes already know.

Still, I think intrinsics for the complicated operations would provide a good 
balance.

> The advantages of having real IR support include:
> 
> - It's significant simpler for frontends.  All of this logic in Clang will 
> need to be reimplemented in any other frontend that wants to support 
> fixed-point types.

As I mentioned, this only simplifies things if the semantics of the operations 
work for the other frontend/language/target.

> 
> 
> - It's much easier to test.  The frontend needs to handle a lot of different 
> code patterns that ultimately turn into the same small set of basic 
> operations, like compound arithmetic and atomic operations and a million 
> different things that are supposed to trigger implicit conversions.  It's ver 
> frustrating to write tests for these things when constants aren't readable 
> and the generated IR for every operation is a ten-instruction sequence.

I think this is primarily a problem for saturating operations, as those require 
a whole bunch of conditional assignment. Nonsaturating multiplication is only 
about 5 inst

[PATCH] D50502: [clangd] Initial cancellation mechanism for LSP requests.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/Cancellation.h:96
+/// checks using it to avoid extra lookups in the Context.
+class CancellationToken {
+public:

As chatted offline, I have questions about the motivation of the 
`CancellationToken` class. Intuitively, it seems to me that this can be merged 
into `TaskHandle`, and we can simply stash the `TaskHandle` instead of a token 
into the context. There would be fewer states to maintain, and the design would 
be a bit more straightforward. I might be missing context/concerns here, so I'd 
like to hear what you and Ilya think. @ilya-biryukov 



Comment at: clangd/ClangdLSPServer.cpp:75
+std::string NormalizeRequestID(const json::Value &ID) {
+  CancelParams CP;
+  fromJSON(json::Object{{"id", ID}}, CP);

Use `ID.getAsString()`?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50502



___
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 added a comment.

I think it is reasonable to make Sema support suggesting override methods, 
instead of implementing it in clangd side?




Comment at: clangd/CodeComplete.cpp:206
+  llvm::StringMap> Overrides;
+  for (auto *Method : dyn_cast(DC)->methods()) {
+if (!Method->isVirtual())

nit: CR->methods().



Comment at: clangd/CodeComplete.cpp:210
+const std::string Name = Method->getNameAsString();
+const auto it = Overrides.find(Name);
+if (it == Overrides.end())

nit: we can simplify the code like `Overrides[Name].push_back(Method)`.



Comment at: clangd/CodeComplete.cpp:219
+  for (const auto &Base : CR->bases()) {
+const auto *BR = Base.getType().getTypePtr()->getAsCXXRecordDecl();
+for (auto *Method : BR->methods()) {

I think we should check whether `BR == nullptr` here.



Comment at: clangd/CodeComplete.cpp:1233
+// struct/class/union definition.
+const auto Overrides = getVirtualNonOverridenMethods(
+Recorder->CCSema->CurContext, Recorder->CCSema);

It seems that we treat it as a special case, the code path here runs out of the 
`ranking` path.


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] D50966: Fix an undefined behavior when storing an empty StringRef.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: ioeric.
Herald added a subscriber: llvm-commits.

Passing a nullptr to memcpy is UB.


Repository:
  rL LLVM

https://reviews.llvm.org/D50966

Files:
  lib/Support/StringSaver.cpp


Index: lib/Support/StringSaver.cpp
===
--- lib/Support/StringSaver.cpp
+++ lib/Support/StringSaver.cpp
@@ -12,6 +12,8 @@
 using namespace llvm;
 
 StringRef StringSaver::save(StringRef S) {
+  if (S.empty())
+return StringRef();
   char *P = Alloc.Allocate(S.size() + 1);
   memcpy(P, S.data(), S.size());
   P[S.size()] = '\0';


Index: lib/Support/StringSaver.cpp
===
--- lib/Support/StringSaver.cpp
+++ lib/Support/StringSaver.cpp
@@ -12,6 +12,8 @@
 using namespace llvm;
 
 StringRef StringSaver::save(StringRef S) {
+  if (S.empty())
+return StringRef();
   char *P = Alloc.Allocate(S.size() + 1);
   memcpy(P, S.data(), S.size());
   P[S.size()] = '\0';
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50337: [clangd] DexIndex implementation prototype

[Eric Liu via Phabricator via cfe-commits]

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

LG. Last few nits and then good to go.




Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:97
+}
+TopLevelChildren.push_back(createAnd(move(TrigramIterators)));
+

Check `!TrigramIterators.empty()`?



Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:128
+  const llvm::Optional Score = Filter.match(Sym->Name);
+  if (!Score.hasValue())
+continue;

nit: `if (!Score)`



Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:149
+  llvm::function_ref Callback) const 
{
+  for (const auto &ID : Req.IDs) {
+auto I = LookupTable.find(ID);

This also needs lock.



Comment at: clang-tools-extra/unittests/clangd/TestIndex.h:23
+
+Symbol symbol(llvm::StringRef QName);
+

Add comment about what `SymbolID` is?



Comment at: clang-tools-extra/unittests/clangd/TestIndex.h:25
+
+struct SlabAndPointers {
+  SymbolSlab Slab;

Please add documentation.



Comment at: clang-tools-extra/unittests/clangd/TestIndex.h:46
+
+std::vector match(const SymbolIndex &I,
+   const FuzzyFindRequest &Req,

Please add documentation.


https://reviews.llvm.org/D50337



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

[PATCH] D50967: [Preamble] Fix an undefined behavior when checking an empty preamble can be reused.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: ilya-biryukov.

Passing nullptr to memcmp is UB.


Repository:
  rC Clang

https://reviews.llvm.org/D50967

Files:
  lib/Frontend/PrecompiledPreamble.cpp


Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -416,6 +416,9 @@
   Bounds.Size <= MainFileBuffer->getBufferSize() &&
   "Buffer is too large. Bounds were calculated from a different buffer?");
 
+  if (Bounds.Size == 0)
+return PreambleBytes.size() == Bounds.Size;
+
   auto PreambleInvocation = std::make_shared(Invocation);
   PreprocessorOptions &PreprocessorOpts =
   PreambleInvocation->getPreprocessorOpts();


Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -416,6 +416,9 @@
   Bounds.Size <= MainFileBuffer->getBufferSize() &&
   "Buffer is too large. Bounds were calculated from a different buffer?");
 
+  if (Bounds.Size == 0)
+return PreambleBytes.size() == Bounds.Size;
+
   auto PreambleInvocation = std::make_shared(Invocation);
   PreprocessorOptions &PreprocessorOpts =
   PreambleInvocation->getPreprocessorOpts();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50966: Fix an undefined behavior when storing an empty StringRef.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: lib/Support/StringSaver.cpp:14
 
 StringRef StringSaver::save(StringRef S) {
+  if (S.empty())

Is it possible to reuse `StringRef::copy(Allocator &)` here?


Repository:
  rL LLVM

https://reviews.llvm.org/D50966



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

[PATCH] D50502: [clangd] Initial cancellation mechanism for LSP requests.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:75
+std::string NormalizeRequestID(const json::Value &ID) {
+  CancelParams CP;
+  fromJSON(json::Object{{"id", ID}}, CP);

ioeric wrote:
> Use `ID.getAsString()`?
Unfortunately id can be both a string or a number according to LSP specs 
therefore we normalize both cases into a string.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50502



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

[PATCH] D50963: [NFC] Don't define static function in header (UninitializedObject.h)

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

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

Thanks! Learned something new today then :) This solution looks a lot cleaner 
as well.


https://reviews.llvm.org/D50963



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

[PATCH] D50966: Fix an undefined behavior when storing an empty StringRef.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 161467.
hokein added a comment.

Correct the fix.


Repository:
  rL LLVM

https://reviews.llvm.org/D50966

Files:
  lib/Support/StringSaver.cpp


Index: lib/Support/StringSaver.cpp
===
--- lib/Support/StringSaver.cpp
+++ lib/Support/StringSaver.cpp
@@ -13,7 +13,8 @@
 
 StringRef StringSaver::save(StringRef S) {
   char *P = Alloc.Allocate(S.size() + 1);
-  memcpy(P, S.data(), S.size());
+  if (!S.empty())
+memcpy(P, S.data(), S.size());
   P[S.size()] = '\0';
   return StringRef(P, S.size());
 }


Index: lib/Support/StringSaver.cpp
===
--- lib/Support/StringSaver.cpp
+++ lib/Support/StringSaver.cpp
@@ -13,7 +13,8 @@
 
 StringRef StringSaver::save(StringRef S) {
   char *P = Alloc.Allocate(S.size() + 1);
-  memcpy(P, S.data(), S.size());
+  if (!S.empty())
+memcpy(P, S.data(), S.size());
   P[S.size()] = '\0';
   return StringRef(P, S.size());
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50966: Fix an undefined behavior when storing an empty StringRef.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: lib/Support/StringSaver.cpp:14
 
 StringRef StringSaver::save(StringRef S) {
+  if (S.empty())

ioeric wrote:
> Is it possible to reuse `StringRef::copy(Allocator &)` here?
Unfortunately, not, `StringRef::copy` will change the behavior. The `StringRef` 
returned here is null-terminated.


Repository:
  rL LLVM

https://reviews.llvm.org/D50966



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

[PATCH] D50897: [clangd] Allow using experimental Dex index

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.
Herald added a subscriber: kadircet.



Comment at: clang-tools-extra/clangd/index/Index.h:360
+ template 
+ static std::unique_ptr build(SymbolSlab Slab) {
+   struct Snapshot {

I'd try to avoid this pattern as it implicitly requires 
`T::build(std::shared_ptr>)` to be implemented. 
Alternatively, you could pull out a helper function that turns a `SymbolSlab` 
into `std::shared_ptr>`, which can be shared by 
different indexes. It can probably live in `MemIndex.h`.


https://reviews.llvm.org/D50897



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

[PATCH] D50970: [clangd] Introduce BOOST iterators

[Kirill Bobyrev via Phabricator via cfe-commits]

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

https://reviews.llvm.org/D50970

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
@@ -175,26 +175,29 @@
 // beneficial to implement automatic generation of query trees for more
 // comprehensive testing.
 TEST(DexIndexIterators, QueryTree) {
-  // An example of more complicated query
   //
   //  +-+
   //  |And Iterator:1, 5|
   //  +++
   //   |
   //   |
-  // ++
+  // +-+--+
   // ||
   // ||
   //  +--v--+  +--v-+
   //  |And Iterator: 1, 5, 9|  |Or Iterator: 0, 1, 5|
   //  +--+--+  +--+-+
   // ||
-  //  +--+-++-+---+
+  //  +--+-++-+
   //  ||| |   |
-  //  +---v-+ +v-+   +--v--++-V--++---v---+
-  //  |1, 3, 5, 8, 9| |1, 5, 7, 9|   |Empty||0, 5||0, 1, 5|
-  //  +-+ +--+   +-++++---+
-
+  //  +---v-+ ++---+ +--v--+  +---v+ +v---+
+  //  |1, 3, 5, 8, 9| |Boost: 2| |Empty|  |Boost: 3| |Boost: 4|
+  //  +-+ ++---+ +-+  +---++ ++---+
+  //   |  |   |
+  //  +v-+  +-v--++---v---+
+  //  |1, 5, 7, 9|  |0, 5||0, 1, 5|
+  //  +--+  +++---+
+  //
   const PostingList L0 = {1, 3, 5, 8, 9};
   const PostingList L1 = {1, 5, 7, 9};
   const PostingList L2 = {0, 5};
@@ -204,21 +207,26 @@
   // Root of the query tree: [1, 5]
   auto Root = createAnd(
   // Lower And Iterator: [1, 5, 9]
-  createAnd(create(L0), create(L1)),
+  createAnd(create(L0), createBoost(create(L1), 2U)),
   // Lower Or Iterator: [0, 1, 5]
-  createOr(create(L2), create(L3), create(L4)));
+  createOr(create(L2), createBoost(create(L3), 3U),
+   createBoost(create(L4), 4U)));
 
   EXPECT_FALSE(Root->reachedEnd());
   EXPECT_EQ(Root->peek(), 1U);
   Root->advanceTo(0);
   // Advance multiple times. Shouldn't do anything.
   Root->advanceTo(1);
   Root->advanceTo(0);
   EXPECT_EQ(Root->peek(), 1U);
+  auto ElementBoost = Root->boost(Root->peek());
+  EXPECT_THAT(ElementBoost, 6);
   Root->advance();
   EXPECT_EQ(Root->peek(), 5U);
   Root->advanceTo(5);
   EXPECT_EQ(Root->peek(), 5U);
+  ElementBoost = Root->boost(Root->peek());
+  EXPECT_THAT(ElementBoost, 8);
   Root->advanceTo(9000);
   EXPECT_TRUE(Root->reachedEnd());
 }
@@ -275,6 +283,34 @@
   EXPECT_THAT(consume(*AndIterator), ElementsAre(1, 2, 5));
 }
 
+TEST(DexIndexIterators, Boost) {
+  auto BoostIterator = createBoost(createTrue(5U), 42U);
+  EXPECT_FALSE(BoostIterator->reachedEnd());
+  auto ElementBoost = BoostIterator->boost(BoostIterator->peek());
+  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->boost(Root->peek());
+  EXPECT_THAT(ElementBoost, Iterator::DEFAULT_BOOST_SCORE);
+  Root->advance();
+  EXPECT_THAT(Root->peek(), 1U);
+  ElementBoost = Root->boost(Root->peek());
+  EXPECT_THAT(ElementBoost, 3);
+
+  Root->advance();
+  EXPECT_THAT(Root->peek(), 2U);
+  ElementBoost = Root->boost(Root->peek());
+  EXPECT_THAT(ElementBoost, 2);
+
+  Root->advanceTo(4);
+  ElementBoost = Root->boost(Root->peek());
+  EXPECT_THAT(ElementBoost, 3);
+}
+
 testing::Matcher>
 trigramsAre(std::initializer_list Trigrams) {
   std::vector Tokens;
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
@@ -89,6 +89,8 @@
   ///

[PATCH] D50970: [clangd] Introduce BOOST iterators

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev planned changes to this revision.
kbobyrev added a comment.

This patch is in preview mode, documentation overhaul and minor cleanup 
incoming.


https://reviews.llvm.org/D50970



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

[PATCH] D50966: Fix an undefined behavior when storing an empty StringRef.

[Eric Liu via Phabricator via cfe-commits]

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

lg




Comment at: lib/Support/StringSaver.cpp:14
 
 StringRef StringSaver::save(StringRef S) {
+  if (S.empty())

hokein wrote:
> ioeric wrote:
> > Is it possible to reuse `StringRef::copy(Allocator &)` here?
> Unfortunately, not, `StringRef::copy` will change the behavior. The 
> `StringRef` returned here is null-terminated.
Hmm, that seems strange but out of the scope.


Repository:
  rL LLVM

https://reviews.llvm.org/D50966



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

[PATCH] D50966: Fix an undefined behavior when storing an empty StringRef.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340170: Fix an undefined behavior when storing an empty 
StringRef. (authored by hokein, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D50966

Files:
  llvm/trunk/lib/Support/StringSaver.cpp


Index: llvm/trunk/lib/Support/StringSaver.cpp
===
--- llvm/trunk/lib/Support/StringSaver.cpp
+++ llvm/trunk/lib/Support/StringSaver.cpp
@@ -13,7 +13,8 @@
 
 StringRef StringSaver::save(StringRef S) {
   char *P = Alloc.Allocate(S.size() + 1);
-  memcpy(P, S.data(), S.size());
+  if (!S.empty())
+memcpy(P, S.data(), S.size());
   P[S.size()] = '\0';
   return StringRef(P, S.size());
 }


Index: llvm/trunk/lib/Support/StringSaver.cpp
===
--- llvm/trunk/lib/Support/StringSaver.cpp
+++ llvm/trunk/lib/Support/StringSaver.cpp
@@ -13,7 +13,8 @@
 
 StringRef StringSaver::save(StringRef S) {
   char *P = Alloc.Allocate(S.size() + 1);
-  memcpy(P, S.data(), S.size());
+  if (!S.empty())
+memcpy(P, S.data(), S.size());
   P[S.size()] = '\0';
   return StringRef(P, S.size());
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50966: Fix an undefined behavior when storing an empty StringRef.

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Why do we need to allocate memory in this case at all? I.e. why can't this just 
be:

  if (S.empty())
return StringRef("", 0);
  ...


Repository:
  rL LLVM

https://reviews.llvm.org/D50966



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

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

[Deanna Garcia via Phabricator via cfe-commits]

deannagarcia updated this revision to Diff 161472.
deannagarcia marked 11 inline comments as done.

https://reviews.llvm.org/D50862

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
  clang-tidy/abseil/FasterStrsplitDelimiterCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-faster-strsplit-delimiter.cpp

Index: test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
===
--- test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
+++ test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
@@ -0,0 +1,95 @@
+// RUN: %check_clang_tidy %s abseil-faster-strsplit-delimiter %t
+
+namespace absl {
+
+class string_view {
+  public:
+string_view();
+string_view(const char *);
+};
+
+namespace strings_internal {
+struct Splitter {};
+struct MaxSplitsImpl {
+  MaxSplitsImpl();
+  ~MaxSplitsImpl();
+};
+} //namespace strings_internal
+
+template 
+strings_internal::Splitter StrSplit(absl::string_view, Delim) {
+  return {};
+}
+template 
+strings_internal::Splitter StrSplit(absl::string_view, Delim, Pred) {
+  return {};
+}
+
+class ByAnyChar {
+  public:
+explicit ByAnyChar(absl::string_view);
+~ByAnyChar();
+};
+
+template 
+strings_internal::MaxSplitsImpl MaxSplits(Delim, int) {
+  return {};
+}
+
+} //namespace absl
+
+void SplitDelimiters() {
+  absl::StrSplit("ABC", "A");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit() called with a string literal consisting of a single character; consider using the character overload [abseil-faster-strsplit-delimiter]
+  // CHECK-FIXES: absl::StrSplit("ABC", 'A');
+
+  absl::StrSplit("ABC", absl::ByAnyChar("\n"));
+  // CHECK-MESSAGES: [[@LINE-1]]:41: warning: absl::StrSplit()
+  // CHECK-FIXES: absl::StrSplit("ABC", '\n');
+
+  // Works with predicate
+  absl::StrSplit("ABC", "A", [](absl::string_view) { return true; });
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+  // CHECK-FIXES: absl::StrSplit("ABC", 'A', [](absl::string_view) { return true; });
+
+  // Doesn't do anything with other strings lenghts.
+  absl::StrSplit("ABC", "AB");
+  absl::StrSplit("ABC", absl::ByAnyChar(""));
+  absl::StrSplit("ABC", absl::ByAnyChar(" \t"));
+
+  // Escapes a single quote in the resulting character literal.
+  absl::StrSplit("ABC", "'");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+  // CHECK-FIXES: absl::StrSplit("ABC", '\'');
+
+  absl::StrSplit("ABC", absl::MaxSplits("\t", 1));
+  // CHECK-MESSAGES: [[@LINE-1]]:41: warning: absl::MaxSplits()
+  // CHECK-FIXES: absl::StrSplit("ABC", absl::MaxSplits('\t', 1));
+
+  auto delim = absl::MaxSplits(absl::ByAnyChar(" "), 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:48: warning: absl::MaxSplits()
+  // CHECK-FIXES: auto delim = absl::MaxSplits(' ', 1);
+}
+
+#define MACRO(str) absl::StrSplit("ABC", str)
+
+void Macro() {
+  MACRO("A");
+}
+
+template 
+void FunctionTemplate() {
+  // This one should not warn because ByAnyChar is a dependent type.
+  absl::StrSplit("TTT", T("A"));
+
+  // This one will warn, but we are checking that we get a correct warning only
+  // once.
+  absl::StrSplit("TTT", "A");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+  // CHECK-FIXES: absl::StrSplit("TTT", 'A');
+}
+
+void FunctionTemplateCaller() {
+  FunctionTemplate();
+  FunctionTemplate();
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
 =
 
 .. toctree::
+   abseil-faster-strsplit-delimiter
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
===
--- docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
+++ docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
@@ -0,0 +1,41 @@
+.. title:: clang-tidy - abseil-faster-strsplit-delimiter
+
+abseil-faster-strsplit-delimiter
+
+
+Finds instances of ``absl::StrSplit()`` or ``absl::MaxSplits()`` where the
+delimiter is a single character string literal and replaces with a character.
+The check will offer a suggestion to change the string literal into a character.
+It will also catch code using ``absl::ByAnyChar()`` for just a single character
+and will transform that into a single character as well.
+
+These changes will give the same result, but using characters rather than
+single character string literals is more efficient and readable.
+
+Examples:
+
+.. code-block:: c++
+
+  // Original - the argument is a string literal.
+  for (auto piece : absl::StrSplit(str, "B")) {
+
+  // Suggested - the argument is a character, 

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

[Deanna Garcia via Phabricator via cfe-commits]

deannagarcia added inline comments.



Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:54
+  // Now replace the " with '.
+  auto Pos = Result.find_first_of('"');
+  if (Pos == Result.npos)

JonasToth wrote:
> deannagarcia wrote:
> > JonasToth wrote:
> > > This expects at max 2 `"` characters. couldnt there be more?
> > The only way this will be run is if the string is a single character, so 
> > the only possibility if there are more than 2 " characters is that the 
> > character that is the delimiter is actually " which is taken care of in the 
> > check. Does that make sense/do you think I need to add a comment about this?
> Ok I see, you could add an assertion before this section. Having the 
> precondition checked is always a good thing and usually adds clarity as well 
> :)
I can't think of a simple thing to assert because most literals will look like: 
"a" but there is also the possibility that it looks like "\"" and I can't think 
of an easy way to combine those two. Do you have an idea/maybe I could just put 
a comment at the beginning saying this is only meant for single character 
string literals?


https://reviews.llvm.org/D50862



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

r340174 - [NFC] Don't define static function in header (UninitializedObject.h)

[Andrei Elovikov via cfe-commits]

Author: a.elovikov
Date: Mon Aug 20 06:45:38 2018
New Revision: 340174

URL: http://llvm.org/viewvc/llvm-project?rev=340174&view=rev
Log:
[NFC] Don't define static function in header (UninitializedObject.h)

Summary:
See also http://lists.llvm.org/pipermail/cfe-users/2016-January/000854.html for
the reasons why it's bad.

Reviewers: Szelethus, erichkeane

Reviewed By: Szelethus

Subscribers: cfe-commits

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

Modified:

cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h

cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h?rev=340174&r1=340173&r2=340174&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h 
(original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h 
Mon Aug 20 06:45:38 2018
@@ -233,7 +233,7 @@ private:
 /// value is undefined or not, such as ints and doubles, can be analyzed with
 /// ease. This also helps ensuring that every special field type is handled
 /// correctly.
-static bool isPrimitiveType(const QualType &T) {
+inline bool isPrimitiveType(const QualType &T) {
   return T->isBuiltinType() || T->isEnumeralType() || T->isMemberPointerType();
 }
 

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp?rev=340174&r1=340173&r2=340174&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
 (original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
 Mon Aug 20 06:45:38 2018
@@ -215,12 +215,10 @@ bool FindUninitializedFields::isPointerO
 llvm_unreachable("All cases are handled!");
   }
 
-  // Temporary variable to avoid warning from -Wunused-function.
-  bool IsPrimitive = isPrimitiveType(DynT->getPointeeType());
-  assert((IsPrimitive || DynT->isAnyPointerType() || DynT->isReferenceType()) 
&&
+  assert((isPrimitiveType(DynT->getPointeeType()) || DynT->isAnyPointerType() 
||
+  DynT->isReferenceType()) &&
  "At this point FR must either have a primitive dynamic type, or it "
  "must be a null, undefined, unknown or concrete pointer!");
-  (void)IsPrimitive;
 
   if (isPrimitiveUninit(DerefdV)) {
 if (NeedsCastBack)


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

[PATCH] D50963: [NFC] Don't define static function in header (UninitializedObject.h)

[Andrei Elovikov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340174: [NFC] Don't define static function in header 
(UninitializedObject.h) (authored by a.elovikov, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50963?vs=161458&id=161473#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50963

Files:
  
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
  
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp


Index: 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
===
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
@@ -215,12 +215,10 @@
 llvm_unreachable("All cases are handled!");
   }
 
-  // Temporary variable to avoid warning from -Wunused-function.
-  bool IsPrimitive = isPrimitiveType(DynT->getPointeeType());
-  assert((IsPrimitive || DynT->isAnyPointerType() || DynT->isReferenceType()) 
&&
+  assert((isPrimitiveType(DynT->getPointeeType()) || DynT->isAnyPointerType() 
||
+  DynT->isReferenceType()) &&
  "At this point FR must either have a primitive dynamic type, or it "
  "must be a null, undefined, unknown or concrete pointer!");
-  (void)IsPrimitive;
 
   if (isPrimitiveUninit(DerefdV)) {
 if (NeedsCastBack)
Index: 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
===
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
@@ -233,7 +233,7 @@
 /// value is undefined or not, such as ints and doubles, can be analyzed with
 /// ease. This also helps ensuring that every special field type is handled
 /// correctly.
-static bool isPrimitiveType(const QualType &T) {
+inline bool isPrimitiveType(const QualType &T) {
   return T->isBuiltinType() || T->isEnumeralType() || T->isMemberPointerType();
 }
 


Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
@@ -215,12 +215,10 @@
 llvm_unreachable("All cases are handled!");
   }
 
-  // Temporary variable to avoid warning from -Wunused-function.
-  bool IsPrimitive = isPrimitiveType(DynT->getPointeeType());
-  assert((IsPrimitive || DynT->isAnyPointerType() || DynT->isReferenceType()) &&
+  assert((isPrimitiveType(DynT->getPointeeType()) || DynT->isAnyPointerType() ||
+  DynT->isReferenceType()) &&
  "At this point FR must either have a primitive dynamic type, or it "
  "must be a null, undefined, unknown or concrete pointer!");
-  (void)IsPrimitive;
 
   if (isPrimitiveUninit(DerefdV)) {
 if (NeedsCastBack)
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
@@ -233,7 +233,7 @@
 /// value is undefined or not, such as ints and doubles, can be analyzed with
 /// ease. This also helps ensuring that every special field type is handled
 /// correctly.
-static bool isPrimitiveType(const QualType &T) {
+inline bool isPrimitiveType(const QualType &T) {
   return T->isBuiltinType() || T->isEnumeralType() || T->isMemberPointerType();
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:54
+  // Now replace the " with '.
+  auto Pos = Result.find_first_of('"');
+  if (Pos == Result.npos)

deannagarcia wrote:
> JonasToth wrote:
> > deannagarcia wrote:
> > > JonasToth wrote:
> > > > This expects at max 2 `"` characters. couldnt there be more?
> > > The only way this will be run is if the string is a single character, so 
> > > the only possibility if there are more than 2 " characters is that the 
> > > character that is the delimiter is actually " which is taken care of in 
> > > the check. Does that make sense/do you think I need to add a comment 
> > > about this?
> > Ok I see, you could add an assertion before this section. Having the 
> > precondition checked is always a good thing and usually adds clarity as 
> > well :)
> I can't think of a simple thing to assert because most literals will look 
> like: "a" but there is also the possibility that it looks like "\"" and I 
> can't think of an easy way to combine those two. Do you have an idea/maybe I 
> could just put a comment at the beginning saying this is only meant for 
> single character string literals?
Wouldnt that result in an assert approximatly this:

`assert(Result.size() == 1 || (Result.size() == 2 && Result.startswith('\\'))`

Stating the size constraint alone is already a valuable assert in my opinion.



Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:85
+
+  // Find uses of absl::StrSplit(..., "x") and absl::StrSplit(..., 
absl::ByAnyChar("x"))
+  // to transform them into absl::StrSplit(..., 'x').

These comments seem to be longer then 80, if true please wrap them.


https://reviews.llvm.org/D50862



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

[PATCH] D50557: [clang][mips] Set __mips_fpr correctly for -mfpxx

[Simon Atanasyan via Phabricator via cfe-commits]

atanasyan added inline comments.



Comment at: lib/Basic/Targets/Mips.cpp:141
+  Twine(32 / (FPMode == FP64 || IsSingleFloat ? 1 : 2)));
 
   if (IsMips16)

What do you think about this variant of the code? It's longer, but imho looks 
more clear.
```
switch (FPMode) {
case FPXX:
  Builder.defineMacro("__mips_fpr", Twine(0));
  break;
case FP32:
  Builder.defineMacro("__mips_fpr", Twine(32));
  break;
case FP64:
  Builder.defineMacro("__mips_fpr", Twine(64));
  break;
}

if (FPMode == FP64 || IsSingleFloat)
  Builder.defineMacro("_MIPS_FPSET", Twine(32));
else
  Builder.defineMacro("_MIPS_FPSET", Twine(16));
```


https://reviews.llvm.org/D50557



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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

Anymore changes I should make or issues I should be aware of?


https://reviews.llvm.org/D50542



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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D50542#1205880, @hugoeg wrote:

> Anymore changes I should make or issues I should be aware of?


From my side not!
You still have unresolved comments that cause the revision to not be `Ready To 
Review` for the main reviewers, maybe that causes them to overlook it.




Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:2
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+

JonasToth wrote:
> hugoeg wrote:
> > JonasToth wrote:
> > > hugoeg wrote:
> > > > hokein wrote:
> > > > > nit: please make sure the code follow LLVM code style, even for test 
> > > > > code :)
> > > > what is this in reference too?
> > > > Will the test still work if I wrap the CHECK MESSAGE lines?
> > > CHECK-MESSAGE can be on one line, even if its longer (that is common in 
> > > the clang-tidy tests).
> > > 
> > > But dont use many empty lines and respect naming conventions and run 
> > > clang-format over the code (except there is a valid reason that the 
> > > formatting would infer with the tested logic).
> > Do my function names have to be verbs, they're not doing anything.
> > 
> > I could rename InternalFunction to something like InternallyProcessString 
> > annd StringFunction to process String
> > Would this be preferred?
> It helps if you somehow show the "topic of the function". But I wrote some 
> tests, that did not strictly follow and they passed review too ;)
> 
> Foo is just too generic, sth like `DirectAccess`, `FriendUsage`, 
> `OpeningNamespace` or so is already telling and I guess good enough :)
I think the names are ok.


https://reviews.llvm.org/D50542



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

[PATCH] D50970: [clangd] Implement BOOST iterator

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 161477.
kbobyrev retitled this revision from "[clangd] Introduce BOOST iterators" to 
"[clangd] Implement BOOST iterator".
kbobyrev edited the summary of this revision.
kbobyrev added a comment.

Add documentation, cleanup tests.


https://reviews.llvm.org/D50970

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
@@ -172,53 +172,61 @@
 // FIXME(kbobyrev): The testcase below is similar to what is expected in real
 // queries. It should be updated once new iterators (such as boosting, limiting,
 // etc iterators) appear. However, it is not exhaustive and it would be
-// beneficial to implement automatic generation of query trees for more
-// comprehensive testing.
+// beneficial to implement automatic generation (e.g. fuzzing) of query trees
+// for more comprehensive testing.
 TEST(DexIndexIterators, QueryTree) {
-  // An example of more complicated query
   //
   //  +-+
   //  |And Iterator:1, 5|
   //  +++
   //   |
   //   |
-  // ++
+  // +-+--+
   // ||
   // ||
-  //  +--v--+  +--v-+
-  //  |And Iterator: 1, 5, 9|  |Or Iterator: 0, 1, 5|
-  //  +--+--+  +--+-+
+  //  +--v--+  +--v+
+  //  |And Iterator: 1, 5, 9|  |Or Iterator: 0, 1, 3, 5|
+  //  +--+--+  +--++
   // ||
-  //  +--+-++-+---+
+  //  +--+-++-+
   //  ||| |   |
-  //  +---v-+ +v-+   +--v--++-V--++---v---+
-  //  |1, 3, 5, 8, 9| |1, 5, 7, 9|   |Empty||0, 5||0, 1, 5|
-  //  +-+ +--+   +-++++---+
-
+  //  +---v-+ ++---+ +--v--+  +---v+ +v---+
+  //  |1, 3, 5, 8, 9| |Boost: 2| |Empty|  |Boost: 3| |Boost: 4|
+  //  +-+ ++---+ +-+  +---++ ++---+
+  //   |  |   |
+  //  +v-+  +-v--++---v---+
+  //  |1, 5, 7, 9|  |1, 5||0, 3, 5|
+  //  +--+  +++---+
+  //
   const PostingList L0 = {1, 3, 5, 8, 9};
   const PostingList L1 = {1, 5, 7, 9};
-  const PostingList L2 = {0, 5};
-  const PostingList L3 = {0, 1, 5};
-  const PostingList L4;
+  const PostingList L3;
+  const PostingList L4 = {1, 5};
+  const PostingList L5 = {0, 3, 5};
 
   // Root of the query tree: [1, 5]
   auto Root = createAnd(
   // Lower And Iterator: [1, 5, 9]
-  createAnd(create(L0), create(L1)),
+  createAnd(create(L0), createBoost(create(L1), 2U)),
   // Lower Or Iterator: [0, 1, 5]
-  createOr(create(L2), create(L3), create(L4)));
+  createOr(create(L3), createBoost(create(L4), 3U),
+   createBoost(create(L5), 4U)));
 
   EXPECT_FALSE(Root->reachedEnd());
   EXPECT_EQ(Root->peek(), 1U);
   Root->advanceTo(0);
   // Advance multiple times. Shouldn't do anything.
   Root->advanceTo(1);
   Root->advanceTo(0);
   EXPECT_EQ(Root->peek(), 1U);
+  auto ElementBoost = Root->boost(Root->peek());
+  EXPECT_THAT(ElementBoost, 6);
   Root->advance();
   EXPECT_EQ(Root->peek(), 5U);
   Root->advanceTo(5);
   EXPECT_EQ(Root->peek(), 5U);
+  ElementBoost = Root->boost(Root->peek());
+  EXPECT_THAT(ElementBoost, 8);
   Root->advanceTo(9000);
   EXPECT_TRUE(Root->reachedEnd());
 }
@@ -275,6 +283,34 @@
   EXPECT_THAT(consume(*AndIterator), ElementsAre(1, 2, 5));
 }
 
+TEST(DexIndexIterators, Boost) {
+  auto BoostIterator = createBoost(createTrue(5U), 42U);
+  EXPECT_FALSE(BoostIterator->reachedEnd());
+  auto ElementBoost = BoostIterator->boost(BoostIterator->peek());
+  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->b

[PATCH] D50337: [clangd] DexIndex implementation prototype

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Address post-LGTM comments.


https://reviews.llvm.org/D50337

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/DexIndex.cpp
  clang-tools-extra/clangd/index/dex/DexIndex.h
  clang-tools-extra/clangd/index/dex/Token.h
  clang-tools-extra/unittests/clangd/CMakeLists.txt
  clang-tools-extra/unittests/clangd/DexIndexTests.cpp
  clang-tools-extra/unittests/clangd/IndexTests.cpp
  clang-tools-extra/unittests/clangd/TestIndex.cpp
  clang-tools-extra/unittests/clangd/TestIndex.h

Index: clang-tools-extra/unittests/clangd/TestIndex.h
===
--- /dev/null
+++ clang-tools-extra/unittests/clangd/TestIndex.h
@@ -0,0 +1,64 @@
+//===-- IndexHelpers.h --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_INDEXTESTCOMMON_H
+#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_INDEXTESTCOMMON_H
+
+#include "index/Index.h"
+#include "index/Merge.h"
+#include "index/dex/DexIndex.h"
+#include "index/dex/Iterator.h"
+#include "index/dex/Token.h"
+#include "index/dex/Trigram.h"
+
+namespace clang {
+namespace clangd {
+
+// Creates Symbol instance and sets SymbolID to given QualifiedName.
+Symbol symbol(llvm::StringRef QName);
+
+// Bundles symbol pointers with the actual symbol slab the pointers refer to in
+// order to ensure that the slab isn't destroyed while it's used by and index.
+struct SlabAndPointers {
+  SymbolSlab Slab;
+  std::vector Pointers;
+};
+
+// Create a slab of symbols with the given qualified names as both IDs and
+// names. The life time of the slab is managed by the returned shared pointer.
+// If \p WeakSymbols is provided, it will be pointed to the managed object in
+// the returned shared pointer.
+std::shared_ptr>
+generateSymbols(std::vector QualifiedNames,
+std::weak_ptr *WeakSymbols = nullptr);
+
+// Create a slab of symbols with IDs and names [Begin, End], otherwise identical
+// to the `generateSymbols` above.
+std::shared_ptr>
+generateNumSymbols(int Begin, int End,
+   std::weak_ptr *WeakSymbols = nullptr);
+
+// Returns fully-qualified name out of given symbol.
+std::string getQualifiedName(const Symbol &Sym);
+
+// Performs fuzzy matching-based symbol lookup given a query and an index.
+// Incomplete is set true if more items than requested can be retrieved, false
+// otherwise.
+std::vector match(const SymbolIndex &I,
+   const FuzzyFindRequest &Req,
+   bool *Incomplete = nullptr);
+
+// Returns qualified names of symbols with any of IDs in the index.
+std::vector lookup(const SymbolIndex &I,
+llvm::ArrayRef IDs);
+
+} // namespace clangd
+} // namespace clang
+
+#endif
Index: clang-tools-extra/unittests/clangd/TestIndex.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clangd/TestIndex.cpp
@@ -0,0 +1,83 @@
+//===-- IndexHelpers.cpp *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "TestIndex.h"
+
+namespace clang {
+namespace clangd {
+
+Symbol symbol(llvm::StringRef QName) {
+  Symbol Sym;
+  Sym.ID = SymbolID(QName.str());
+  size_t Pos = QName.rfind("::");
+  if (Pos == llvm::StringRef::npos) {
+Sym.Name = QName;
+Sym.Scope = "";
+  } else {
+Sym.Name = QName.substr(Pos + 2);
+Sym.Scope = QName.substr(0, Pos + 2);
+  }
+  return Sym;
+}
+
+std::shared_ptr>
+generateSymbols(std::vector QualifiedNames,
+std::weak_ptr *WeakSymbols) {
+  SymbolSlab::Builder Slab;
+  for (llvm::StringRef QName : QualifiedNames)
+Slab.insert(symbol(QName));
+
+  auto Storage = std::make_shared();
+  Storage->Slab = std::move(Slab).build();
+  for (const auto &Sym : Storage->Slab)
+Storage->Pointers.push_back(&Sym);
+  if (WeakSymbols)
+*WeakSymbols = Storage;
+  auto *Pointers = &Storage->Pointers;
+  return {std::move(Storage), Pointers};
+}
+
+std::shared_ptr>
+generateNumSymbols(int Begin, int End,
+   std::weak_ptr *WeakSymbols) {
+  std::vector Names;
+  for (int i = Begin; i <= End; i++)
+Names.push_back(std::to_string(i));
+  return generateSymbols(Names, WeakSymbols);
+}
+
+std::string getQualifiedName(const Symbol &Sym) {
+  return (Sym.Scope + Sym.N

[clang-tools-extra] r340175 - [clangd] DexIndex implementation prototype

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Mon Aug 20 07:39:32 2018
New Revision: 340175

URL: http://llvm.org/viewvc/llvm-project?rev=340175&view=rev
Log:
[clangd] DexIndex implementation prototype

This patch is a proof-of-concept Dex index implementation. It has
several flaws, which don't allow replacing static MemIndex yet, such as:

* Not being able to handle queries of small size (less than 3 symbols);
  a way to solve this is generating trigrams of smaller size and having
  such incomplete trigrams in the index structure.
* Speed measurements: while manually editing files in Vim and requesting
  autocompletion gives an impression that the performance is at least
  comparable with the current static index, having actual numbers is
  important because we don't want to hurt the users and roll out slow
  code. Eric (@ioeric) suggested that we should only replace MemIndex as
  soon as we have the evidence that this is not a regression in terms of
  performance. An approach which is likely to be successful here is to
  wait until we have benchmark library in the LLVM core repository, which
  is something I have suggested in the LLVM mailing lists, received
  positive feedback on and started working on. I will add a dependency as
  soon as the suggested patch is out for a review (currently there's at
  least one complication which is being addressed by
  https://github.com/google/benchmark/pull/649). Key performance
  improvements for iterators are sorting by cost and the limit iterator.
* Quality measurements: currently, boosting iterator and two-phase
  lookup stage are not implemented, without these the quality is likely to
  be worse than the current implementation can yield. Measuring quality is
  tricky, but another suggestion in the offline discussion was that the
  drop-in replacement should only happen after Boosting iterators
  implementation (and subsequent query enhancement).

The proposed changes do not affect Clangd functionality or performance,
`DexIndex` is only used in unit tests and not in production code.

Reviewed by: ioeric

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

Added:
clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp
clang-tools-extra/trunk/clangd/index/dex/DexIndex.h
clang-tools-extra/trunk/unittests/clangd/TestIndex.cpp
clang-tools-extra/trunk/unittests/clangd/TestIndex.h
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/index/dex/Token.h
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=340175&r1=340174&r2=340175&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Mon Aug 20 07:39:32 2018
@@ -43,6 +43,7 @@ add_clang_library(clangDaemon
   index/SymbolCollector.cpp
   index/SymbolYAML.cpp
 
+  index/dex/DexIndex.cpp
   index/dex/Iterator.cpp
   index/dex/Trigram.cpp
 

Added: clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp?rev=340175&view=auto
==
--- clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp (added)
+++ clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp Mon Aug 20 07:39:32 
2018
@@ -0,0 +1,167 @@
+//===--- DexIndex.cpp - Dex Symbol Index Implementation -*- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "DexIndex.h"
+#include "../../FuzzyMatch.h"
+#include "../../Logger.h"
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+namespace dex {
+
+namespace {
+
+// Returns the tokens which are given symbol's characteristics. Currently, the
+// generated tokens only contain fuzzy matching trigrams and symbol's scope,
+// but in the future this will also return path proximity tokens and other
+// types of tokens such as symbol type (if applicable).
+// Returns the tokens which are given symbols's characteristics. For example,
+// trigrams and scopes.
+// FIXME(kbobyrev): Support more token types:
+// * Path proximity
+// * Types
+std::vector generateSearchTokens(const Symbol &Sym) {
+  std::vector Result = generateIdentifierTrigrams(Sym.Name);
+  Result.push_back(Token(Token::Kind::Scope, Sym.Scope));
+  return Result;
+}
+
+} // namespace
+
+void DexIndex::build(std::shared_ptr> Syms) {
+  llvm::DenseMap TempLookupTable;
+  llvm::DenseMap TempSymbolQuality;

[PATCH] D50337: [clangd] DexIndex implementation prototype

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340175: [clangd] DexIndex implementation prototype (authored 
by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50337?vs=161480&id=161481#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50337

Files:
  clang-tools-extra/trunk/clangd/CMakeLists.txt
  clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp
  clang-tools-extra/trunk/clangd/index/dex/DexIndex.h
  clang-tools-extra/trunk/clangd/index/dex/Token.h
  clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
  clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
  clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
  clang-tools-extra/trunk/unittests/clangd/TestIndex.cpp
  clang-tools-extra/trunk/unittests/clangd/TestIndex.h

Index: clang-tools-extra/trunk/unittests/clangd/TestIndex.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/TestIndex.cpp
+++ clang-tools-extra/trunk/unittests/clangd/TestIndex.cpp
@@ -0,0 +1,83 @@
+//===-- IndexHelpers.cpp *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "TestIndex.h"
+
+namespace clang {
+namespace clangd {
+
+Symbol symbol(llvm::StringRef QName) {
+  Symbol Sym;
+  Sym.ID = SymbolID(QName.str());
+  size_t Pos = QName.rfind("::");
+  if (Pos == llvm::StringRef::npos) {
+Sym.Name = QName;
+Sym.Scope = "";
+  } else {
+Sym.Name = QName.substr(Pos + 2);
+Sym.Scope = QName.substr(0, Pos + 2);
+  }
+  return Sym;
+}
+
+std::shared_ptr>
+generateSymbols(std::vector QualifiedNames,
+std::weak_ptr *WeakSymbols) {
+  SymbolSlab::Builder Slab;
+  for (llvm::StringRef QName : QualifiedNames)
+Slab.insert(symbol(QName));
+
+  auto Storage = std::make_shared();
+  Storage->Slab = std::move(Slab).build();
+  for (const auto &Sym : Storage->Slab)
+Storage->Pointers.push_back(&Sym);
+  if (WeakSymbols)
+*WeakSymbols = Storage;
+  auto *Pointers = &Storage->Pointers;
+  return {std::move(Storage), Pointers};
+}
+
+std::shared_ptr>
+generateNumSymbols(int Begin, int End,
+   std::weak_ptr *WeakSymbols) {
+  std::vector Names;
+  for (int i = Begin; i <= End; i++)
+Names.push_back(std::to_string(i));
+  return generateSymbols(Names, WeakSymbols);
+}
+
+std::string getQualifiedName(const Symbol &Sym) {
+  return (Sym.Scope + Sym.Name).str();
+}
+
+std::vector match(const SymbolIndex &I,
+   const FuzzyFindRequest &Req, bool *Incomplete) {
+  std::vector Matches;
+  bool IsIncomplete = I.fuzzyFind(Req, [&](const Symbol &Sym) {
+Matches.push_back(clang::clangd::getQualifiedName(Sym));
+  });
+  if (Incomplete)
+*Incomplete = IsIncomplete;
+  return Matches;
+}
+
+// Returns qualified names of symbols with any of IDs in the index.
+std::vector lookup(const SymbolIndex &I,
+llvm::ArrayRef IDs) {
+  LookupRequest Req;
+  Req.IDs.insert(IDs.begin(), IDs.end());
+  std::vector Results;
+  I.lookup(Req, [&](const Symbol &Sym) {
+Results.push_back(getQualifiedName(Sym));
+  });
+  return Results;
+}
+
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
@@ -7,6 +7,10 @@
 //
 //===--===//
 
+#include "TestIndex.h"
+#include "index/Index.h"
+#include "index/Merge.h"
+#include "index/dex/DexIndex.h"
 #include "index/dex/Iterator.h"
 #include "index/dex/Token.h"
 #include "index/dex/Trigram.h"
@@ -17,11 +21,13 @@
 #include 
 #include 
 
+using ::testing::ElementsAre;
+using ::testing::UnorderedElementsAre;
+
 namespace clang {
 namespace clangd {
 namespace dex {
-
-using ::testing::ElementsAre;
+namespace {
 
 TEST(DexIndexIterators, DocumentIterator) {
   const PostingList L = {4, 7, 8, 20, 42, 100};
@@ -359,6 +365,175 @@
"hij", "ijk", "jkl", "klm"}));
 }
 
+TEST(DexIndex, Lookup) {
+  DexIndex I;
+  I.build(generateSymbols({"ns::abc", "ns::xyz"}));
+  EXPECT_THAT(lookup(I, SymbolID("ns::abc")), UnorderedElementsAre("ns::abc"));
+  EXPECT_THAT(lookup(I, {SymbolID("ns::abc"), SymbolID("ns::xyz")}),
+  UnorderedElementsAre("ns::abc", "ns::xyz"));
+  EXPECT_THAT(lookup(I, {SymbolID("ns::nonono"), SymbolID("ns::xyz")}),
+  UnorderedElementsAre("ns::xyz"));
+  EXPECT_THAT(lookup(I, SymbolID("ns::nonono")), UnorderedElements

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

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

fixed some minor things in the test


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/strings/str_cat.h
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t,  -- -- -I %S/Inputs/absl/strings
+
+#include "str_cat.h"
+
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl {
+void OpeningNamespace() {
+  strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+SomeContainer b;
+std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/strings/str_cat.h
===
--- test/clang-tidy/Inputs/absl/strings/str_cat.h
+++ test/clang-tidy/Inputs/absl/strings/str_cat.h
@@ -0,0 +1,33 @@
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namespace std
+
+namespace absl {
+std::string StringsFunction(std::string s1) { return s1; }
+class SomeContainer {};
+namespace strings_internal {
+void InternalFunction() {}
+template  P InternalTemplateFunction(P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+struct InternalStruct {};
+} // namespace container_internal
+} // namespace absl
+
+// should not trigger warnings because inside Abseil files
+void DirectAcessInternal() {
+  absl::strings_internal::InternalFunction();
+  absl::strings_internal::InternalTemplateFunction("a");
+}
+
+class FriendUsageInternal {
+  friend struct absl::container_internal::InternalStruct;
+};
+
+namespace absl {
+void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
 =
 
 .. toctree::
+   abseil-no-internal-deps
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,21 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something
+is in a namespace that includes the word “internal”, code is not allowed to 
+depend upon it beaucse it’s an implementation detail. They cannot friend it, 
+include it, you mention it or refer to it in any way. Doing so violates 
+Abseil's compatibility guidelines and may result in breakage. See 
+https://abseil.io/about/compatibility for more information.
+
+The following cases will result in warnings:
+
+.. code-block:: c++
+
+  absl::strings_internal::foo();
+  class foo{
+friend struct absl::container_internal::faa;
+  };
+  absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,7 +57,10 @@
 Improvements to clang-tidy
 --
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Gives a warning if code using Abseil depends on internal details.
 
 Improvements to include-fixer
 -
Index: clang-tidy/abseil/NoInternalDepsCheck.h

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

[Eric Liu via Phabricator via cfe-commits]

ioeric planned changes to this revision.
ioeric added a comment.

I just realized that `CodeCompleteFlow` would be blocked by the speculative 
index request even when index request is not needed (e.g. member access), 
because it owns the future object. This can make some completion requests 
slower. We may need to move the async index query into the 
`ClangdServer::codeComplete` so that completion callback can be invoked as soon 
as results are ready, without having to wait for speculative index results.

F7000668: future-redundant-wait.png 


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] D50897: [clangd] Allow using experimental Dex index

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 161487.
kbobyrev marked an inline comment as done.

https://reviews.llvm.org/D50897

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
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -12,6 +12,7 @@
 #include "Path.h"
 #include "Trace.h"
 #include "index/SymbolYAML.h"
+#include "index/dex/DexIndex.h"
 #include "clang/Basic/Version.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
@@ -28,27 +29,6 @@
 using namespace clang;
 using namespace clang::clangd;
 
-namespace {
-enum class PCHStorageFlag { Disk, Memory };
-
-// Build an in-memory static index for global symbols from a YAML-format file.
-// The size of global symbols should be relatively small, so that all symbols
-// can be managed in memory.
-std::unique_ptr buildStaticIndex(llvm::StringRef YamlSymbolFile) {
-  auto Buffer = llvm::MemoryBuffer::getFile(YamlSymbolFile);
-  if (!Buffer) {
-llvm::errs() << "Can't open " << YamlSymbolFile << "\n";
-return nullptr;
-  }
-  auto Slab = symbolsFromYAML(Buffer.get()->getBuffer());
-  SymbolSlab::Builder SymsBuilder;
-  for (auto Sym : Slab)
-SymsBuilder.insert(Sym);
-
-  return MemIndex::build(std::move(SymsBuilder).build());
-}
-} // namespace
-
 static llvm::cl::opt CompileCommandsDir(
 "compile-commands-dir",
 llvm::cl::desc("Specify a path to look for compile_commands.json. If path "
@@ -113,6 +93,30 @@
 "Intended to simplify lit tests."),
 llvm::cl::init(false), llvm::cl::Hidden);
 
+namespace {
+
+enum class PCHStorageFlag { Disk, Memory };
+
+// Build an in-memory static index for global symbols from a YAML-format file.
+// The size of global symbols should be relatively small, so that all symbols
+// can be managed in memory.
+std::unique_ptr buildStaticIndex(llvm::StringRef YamlSymbolFile) {
+  auto Buffer = llvm::MemoryBuffer::getFile(YamlSymbolFile);
+  if (!Buffer) {
+llvm::errs() << "Can't open " << YamlSymbolFile << "\n";
+return nullptr;
+  }
+  auto Slab = symbolsFromYAML(Buffer.get()->getBuffer());
+  SymbolSlab::Builder SymsBuilder;
+  for (auto Sym : Slab)
+SymsBuilder.insert(Sym);
+
+  return UseDex ? DexIndex::build(std::move(SymsBuilder).build())
+: MemIndex::build(std::move(SymsBuilder).build());
+}
+
+} // namespace
+
 static llvm::cl::opt PCHStorage(
 "pch-storage",
 llvm::cl::desc("Storing PCHs in memory increases memory usages, but may "
@@ -185,6 +189,11 @@
 "'compile_commands.json' files")),
 llvm::cl::init(FilesystemCompileArgs), llvm::cl::Hidden);
 
+static llvm::cl::opt
+UseDex("use-dex-index",
+   llvm::cl::desc("Use experimental Dex static index."),
+   llvm::cl::init(false), llvm::cl::Hidden);
+
 int main(int argc, char *argv[]) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) {
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
@@ -43,6 +43,9 @@
   /// accessible as long as `Symbols` is kept alive.
   void build(std::shared_ptr> Symbols);
 
+  /// \brief Build index from a symbol slab.
+  static std::unique_ptr build(SymbolSlab Slab);
+
   bool
   fuzzyFind(const FuzzyFindRequest &Req,
 llvm::function_ref Callback) const override;
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
@@ -69,6 +69,12 @@
   }
 }
 
+std::unique_ptr DexIndex::build(SymbolSlab Slab) {
+  auto Idx = llvm::make_unique();
+  Idx->build(getSymbolsFromSlab(std::move(Slab)));
+  return std::move(Idx);
+}
+
 /// Constructs iterators over tokens extracted from the query and exhausts it
 /// while applying Callback to each symbol in the order of decreasing quality
 /// of the matched symbols.
Index: clang-tools-extra/clangd/index/MemIndex.h
===
--- clang-tools-extra/clangd/index/MemIndex.h
+++ clang-tools-extra/clangd/index/MemIndex.h
@@ -47,6 +47,10 @@
   mutable std::mutex Mutex;
 };
 
+// FIXME(kbobyrev): Document this one.
+std::shared_ptr>
+getSymbolsFromSlab(SymbolSlab Slab);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/index/MemIndex.cpp
===
--- clang-tools-extra/clangd/index/

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:20
+
+bool IsInAbseilFile(const SourceManager& manager, SourceLocation loc){
+  if (loc.isInvalid()) {

hugoeg wrote:
> hokein wrote:
> > I think we can make it as an ASTMatcher instead of a function like:
> > 
> > ```
> > AST_POLYMORPHIC_MATCHER_P(isInAbseilFile,
> >   AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, 
> > TypeLoc)) {
> >// your code here.
> > }
> > ```
> Unfortunately, I do not think we can.  That was the way I originally tried to 
> implement it. It worked on no-namespace-check, but not in this one. This is 
> because as an AST_POLYMORPHIC_MATCHER_P, we are trying to match an usage of a 
> Decl, not the Decl itself and since we are matching a TypeLoc in 
> no-internal-deps-check, not really the usage, it doesn't work.
> 
> As a result, I modified my original implementation, which you will see in 
> no-namespace-check and turned it into IsInAbseilFile(SourceManager&,  
> SourceLocation), which is just takes a source location and returns whether 
> the location we matched is in an Abseil file
Could you explain a bit more why it won't work? I don't understand why it 
doesn't work. 

Basically you define the matcher like:

```
AST_POLYMORPHIC_MATCHER(
isInAbseilFile, AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc,
NestedNameSpecifierLoc)) {
  auto &SourceManager = Finder->getASTContext().getSourceManager();
  SourceLocation loc = Node.getBeginLoc();
  if (loc.isInvalid()) return false;
  const FileEntry *FileEntry =
  SourceManager.getFileEntryForID(SourceManager.getFileID(loc));
  if (!FileEntry) return false;
  StringRef Filename = FileEntry->getName();
  llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
 "synchronization|types|utiliy)");
  return RE.match(Filename);
}
```

And use it for example in your check

```
Finder->addMatcher(nestedNameSpecifierLoc(
 loc(specifiesNamespace(namespaceDecl(
 matchesName("internal"),
 hasParent(namespaceDecl(hasName("absl")),
 unless(isInAbseilFile()))
 .bind("InternalDep"),
 this);
```



Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:20
+
+bool IsInAbseilFile(const SourceManager &manager, SourceLocation loc) {
+  if (loc.isInvalid()) return false;

nit: LLVM code guideline uses `Camel` for variable names.



Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:26
+  llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
+ "synchronization|types|utiliy)");
+  return RE.match(Filename);

typo: utiliy => utility.



Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:51
+  InternalDependency->getBeginLoc())) {
+
+diag(InternalDependency->getBeginLoc(),

nit: remove the empty line.



Comment at: docs/clang-tidy/checks/abseil-no-internal-deps.rst:8
+is in a namespace that includes the word “internal”, code is not allowed to 
+depend upon it beaucse it’s an implementation detail. They cannot friend it, 
+include it, you mention it or refer to it in any way. Doing so violates 

This sentence doesn't parse.



Comment at: docs/clang-tidy/checks/abseil-no-internal-deps.rst:17
+
+  absl::strings_internal::foo();
+  class foo{

nit: Please add a trailing comment on the line where it'd trigger the warning.



Comment at: docs/clang-tidy/checks/abseil-no-internal-deps.rst:18
+  absl::strings_internal::foo();
+  class foo{
+friend struct absl::container_internal::faa;

nit:  space between `foo` and `{`.


https://reviews.llvm.org/D50542



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

r340180 - [ASTImporter] Test for importing condition variable from a ForStmt

[Raphael Isemann via cfe-commits]

Author: teemperor
Date: Mon Aug 20 08:51:41 2018
New Revision: 340180

URL: http://llvm.org/viewvc/llvm-project?rev=340180&view=rev
Log:
[ASTImporter] Test for importing condition variable from a ForStmt

Reviewers: a.sidorin, a_sidorin

Reviewed By: a_sidorin

Subscribers: cfe-commits, martong

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

Modified:
cfe/trunk/test/Import/for-stmt/Inputs/F.cpp
cfe/trunk/test/Import/for-stmt/test.cpp

Modified: cfe/trunk/test/Import/for-stmt/Inputs/F.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/for-stmt/Inputs/F.cpp?rev=340180&r1=340179&r2=340180&view=diff
==
--- cfe/trunk/test/Import/for-stmt/Inputs/F.cpp (original)
+++ cfe/trunk/test/Import/for-stmt/Inputs/F.cpp Mon Aug 20 08:51:41 2018
@@ -3,6 +3,8 @@ void f() {
 ;
   for (int i = 0;;)
 continue;
+  for (; bool j = false;)
+continue;
   for (int i = 0; i != 0; ++i) {
 i++;
   }

Modified: cfe/trunk/test/Import/for-stmt/test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/for-stmt/test.cpp?rev=340180&r1=340179&r2=340180&view=diff
==
--- cfe/trunk/test/Import/for-stmt/test.cpp (original)
+++ cfe/trunk/test/Import/for-stmt/test.cpp Mon Aug 20 08:51:41 2018
@@ -17,6 +17,18 @@
 // CHECK-NEXT: ContinueStmt
 
 // CHECK: ForStmt
+// CHECK-NEXT: <>
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl
+// CHECK-NEXT: CXXBoolLiteralExpr
+// CHECK-NEXT: ImplicitCastExpr
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: 'j'
+// CHECK-SAME: 'bool'
+// CHECK-NEXT: <>
+// CHECK-NEXT: ContinueStmt
+
+// CHECK: ForStmt
 // CHECK-NEXT: DeclStmt
 // CHECK-NEXT: VarDecl
 // CHECK-NEXT: IntegerLiteral


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

[PATCH] D50928: [ASTImporter] Test for importing condition variable from a ForStmt

[Raphael Isemann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340180: [ASTImporter] Test for importing condition variable 
from a ForStmt (authored by teemperor, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50928?vs=161342&id=161493#toc

Repository:
  rC Clang

https://reviews.llvm.org/D50928

Files:
  test/Import/for-stmt/Inputs/F.cpp
  test/Import/for-stmt/test.cpp


Index: test/Import/for-stmt/Inputs/F.cpp
===
--- test/Import/for-stmt/Inputs/F.cpp
+++ test/Import/for-stmt/Inputs/F.cpp
@@ -3,6 +3,8 @@
 ;
   for (int i = 0;;)
 continue;
+  for (; bool j = false;)
+continue;
   for (int i = 0; i != 0; ++i) {
 i++;
   }
Index: test/Import/for-stmt/test.cpp
===
--- test/Import/for-stmt/test.cpp
+++ test/Import/for-stmt/test.cpp
@@ -17,6 +17,18 @@
 // CHECK-NEXT: ContinueStmt
 
 // CHECK: ForStmt
+// CHECK-NEXT: <>
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl
+// CHECK-NEXT: CXXBoolLiteralExpr
+// CHECK-NEXT: ImplicitCastExpr
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: 'j'
+// CHECK-SAME: 'bool'
+// CHECK-NEXT: <>
+// CHECK-NEXT: ContinueStmt
+
+// CHECK: ForStmt
 // CHECK-NEXT: DeclStmt
 // CHECK-NEXT: VarDecl
 // CHECK-NEXT: IntegerLiteral


Index: test/Import/for-stmt/Inputs/F.cpp
===
--- test/Import/for-stmt/Inputs/F.cpp
+++ test/Import/for-stmt/Inputs/F.cpp
@@ -3,6 +3,8 @@
 ;
   for (int i = 0;;)
 continue;
+  for (; bool j = false;)
+continue;
   for (int i = 0; i != 0; ++i) {
 i++;
   }
Index: test/Import/for-stmt/test.cpp
===
--- test/Import/for-stmt/test.cpp
+++ test/Import/for-stmt/test.cpp
@@ -17,6 +17,18 @@
 // CHECK-NEXT: ContinueStmt
 
 // CHECK: ForStmt
+// CHECK-NEXT: <>
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl
+// CHECK-NEXT: CXXBoolLiteralExpr
+// CHECK-NEXT: ImplicitCastExpr
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: 'j'
+// CHECK-SAME: 'bool'
+// CHECK-NEXT: <>
+// CHECK-NEXT: ContinueStmt
+
+// CHECK: ForStmt
 // CHECK-NEXT: DeclStmt
 // CHECK-NEXT: VarDecl
 // CHECK-NEXT: IntegerLiteral
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r340181 - [OPENMP][BLOCKS]Fix PR38923: reference to a global variable is captured

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Aug 20 09:00:22 2018
New Revision: 340181

URL: http://llvm.org/viewvc/llvm-project?rev=340181&view=rev
Log:
[OPENMP][BLOCKS]Fix PR38923: reference to a global variable is captured
by a block.

Added checks for capturing of the variable in the block when trying to
emit correct address for the variable with the reference type. This
extra check allows correctly identify the variables that are not
captured in the block context.

Added:
cfe/trunk/test/CodeGenCXX/block-byref.cpp
Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=340181&r1=340180&r2=340181&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Mon Aug 20 09:00:22 2018
@@ -2437,6 +2437,7 @@ LValue CodeGenFunction::EmitDeclRefLValu
 // A DeclRefExpr for a reference initialized by a constant expression can
 // appear without being odr-used. Directly emit the constant initializer.
 const Expr *Init = VD->getAnyInitializer(VD);
+const auto *BD = dyn_cast_or_null(CurCodeDecl);
 if (Init && !isa(VD) && VD->getType()->isReferenceType() &&
 VD->isUsableInConstantExpressions(getContext()) &&
 VD->checkInitIsICE() &&
@@ -2446,7 +2447,7 @@ LValue CodeGenFunction::EmitDeclRefLValu
 (LocalDeclMap.count(VD->getCanonicalDecl()) ||
  CapturedStmtInfo->lookup(VD->getCanonicalDecl( ||
LambdaCaptureFields.lookup(VD->getCanonicalDecl()) ||
-   isa(CurCodeDecl {
+   (BD && BD->capturesVariable(VD) {
   llvm::Constant *Val =
 ConstantEmitter(*this).emitAbstract(E->getLocation(),
 *VD->evaluateValue(),

Added: cfe/trunk/test/CodeGenCXX/block-byref.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/block-byref.cpp?rev=340181&view=auto
==
--- cfe/trunk/test/CodeGenCXX/block-byref.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/block-byref.cpp Mon Aug 20 09:00:22 2018
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 %s -fblocks -triple x86_64-apple-darwin -std=c++11 
-emit-llvm -o - | FileCheck %s
+// REQUIRES: x86-registered-target
+
+// CHECK: @b = global i32 0,
+
+// CHECK: define {{.*}}void @{{.*}}test{{.*}}_block_invoke(
+// CHECK: store i32 2, i32* @b,
+// CHECK: ret void
+
+int b;
+
+void test() {
+  int &a = b;
+  ^{ a = 2; };
+}


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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 161494.
hugoeg marked 5 inline comments as done.
hugoeg added a comment.

corrections applied


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/strings/str_cat.h
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,34 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t,  -- -- -I %S/Inputs
+
+#include "absl/strings/str_cat.h"
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl {
+void OpeningNamespace() {
+  strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+SomeContainer b;
+std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/strings/str_cat.h
===
--- test/clang-tidy/Inputs/absl/strings/str_cat.h
+++ test/clang-tidy/Inputs/absl/strings/str_cat.h
@@ -0,0 +1,33 @@
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namespace std
+
+namespace absl {
+std::string StringsFunction(std::string s1) { return s1; }
+class SomeContainer {};
+namespace strings_internal {
+void InternalFunction() {}
+template  P InternalTemplateFunction(P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+struct InternalStruct {};
+} // namespace container_internal
+} // namespace absl
+
+// should not trigger warnings because inside Abseil files
+void DirectAcessInternal() {
+  absl::strings_internal::InternalFunction();
+  absl::strings_internal::InternalTemplateFunction("a");
+}
+
+class FriendUsageInternal {
+  friend struct absl::container_internal::InternalStruct;
+};
+
+namespace absl {
+void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
 =
 
 .. toctree::
+   abseil-no-internal-deps
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,23 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something
+is in a namespace that includes the word “internal”, code is not allowed to 
+depend upon it beaucse it’s an implementation detail. They cannot friend it, 
+include it, you mention it or refer to it in any way. Doing so violates 
+Abseil's compatibility guidelines and may result in breakage. See 
+https://abseil.io/about/compatibility for more information.
+
+The following cases will result in warnings:
+
+.. code-block:: c++
+
+  absl::strings_internal::foo();
+  class foo{
+friend struct absl::container_internal::faa;
+// warning triggered on this line
+  };
+  absl::memory_internal::MakeUniqueResult();
+  // warning triggered on the line 
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,7 +57,10 @@
 Improvements to clang-tidy
 --
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Gives a warning if code using Abseil depends on internal details.
 
 Improvements to include-fixer
 --

[PATCH] D50932: [ASTImporter] Add test for C++ casts and fix broken const_cast importing.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor updated this revision to Diff 161496.
teemperor added a comment.

- Added comment why we enable RTTI.


https://reviews.llvm.org/D50932

Files:
  lib/AST/ASTImporter.cpp
  test/Import/cxx-casts/Inputs/F.cpp
  test/Import/cxx-casts/test.cpp
  tools/clang-import-test/clang-import-test.cpp


Index: tools/clang-import-test/clang-import-test.cpp
===
--- tools/clang-import-test/clang-import-test.cpp
+++ tools/clang-import-test/clang-import-test.cpp
@@ -194,6 +194,8 @@
   Inv->getLangOpts()->ThreadsafeStatics = false;
   Inv->getLangOpts()->AccessControl = false;
   Inv->getLangOpts()->DollarIdents = true;
+  // Needed for testing dynamic_cast.
+  Inv->getLangOpts()->RTTI = true;
   Inv->getCodeGenOpts().setDebugInfo(codegenoptions::FullDebugInfo);
   Inv->getTargetOpts().Triple = llvm::sys::getDefaultTargetTriple();
 
Index: test/Import/cxx-casts/test.cpp
===
--- /dev/null
+++ test/Import/cxx-casts/test.cpp
@@ -0,0 +1,21 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | 
FileCheck %s
+
+// CHECK: CXXDynamicCastExpr
+// CHECK-SAME: dynamic_cast
+// CHECK-SAME: 
+
+// CHECK: CXXStaticCastExpr
+// CHECK-SAME: static_cast
+// CHECK-SAME: 
+
+// CHECK: CXXReinterpretCastExpr
+// CHECK-SAME: reinterpret_cast
+// CHECK-SAME: 
+
+// CHECK: CXXConstCastExpr
+// CHECK-SAME: const_cast
+// CHECK-SAME: 
+
+void expr() {
+  f();
+}
Index: test/Import/cxx-casts/Inputs/F.cpp
===
--- /dev/null
+++ test/Import/cxx-casts/Inputs/F.cpp
@@ -0,0 +1,12 @@
+struct A {
+  virtual ~A() {}
+};
+struct B : public A {};
+
+void f() {
+  const A *b = new B();
+  const B *c1 = dynamic_cast(b);
+  const B *c2 = static_cast(b);
+  const B *c3 = reinterpret_cast(b);
+  A *c4 = const_cast(b);
+}
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -6897,6 +6897,10 @@
 return CXXReinterpretCastExpr::Create(
 Importer.getToContext(), ToType, VK, CK, ToOp, &BasePath,
 ToWritten, ToOperatorLoc, ToRParenLoc, ToAngleBrackets);
+  } else if (isa(E)) {
+return CXXConstCastExpr::Create(Importer.getToContext(), ToType, VK, ToOp,
+ToWritten, ToOperatorLoc, ToRParenLoc,
+ToAngleBrackets);
   } else {
 return nullptr;
   }


Index: tools/clang-import-test/clang-import-test.cpp
===
--- tools/clang-import-test/clang-import-test.cpp
+++ tools/clang-import-test/clang-import-test.cpp
@@ -194,6 +194,8 @@
   Inv->getLangOpts()->ThreadsafeStatics = false;
   Inv->getLangOpts()->AccessControl = false;
   Inv->getLangOpts()->DollarIdents = true;
+  // Needed for testing dynamic_cast.
+  Inv->getLangOpts()->RTTI = true;
   Inv->getCodeGenOpts().setDebugInfo(codegenoptions::FullDebugInfo);
   Inv->getTargetOpts().Triple = llvm::sys::getDefaultTargetTriple();
 
Index: test/Import/cxx-casts/test.cpp
===
--- /dev/null
+++ test/Import/cxx-casts/test.cpp
@@ -0,0 +1,21 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | FileCheck %s
+
+// CHECK: CXXDynamicCastExpr
+// CHECK-SAME: dynamic_cast
+// CHECK-SAME: 
+
+// CHECK: CXXStaticCastExpr
+// CHECK-SAME: static_cast
+// CHECK-SAME: 
+
+// CHECK: CXXReinterpretCastExpr
+// CHECK-SAME: reinterpret_cast
+// CHECK-SAME: 
+
+// CHECK: CXXConstCastExpr
+// CHECK-SAME: const_cast
+// CHECK-SAME: 
+
+void expr() {
+  f();
+}
Index: test/Import/cxx-casts/Inputs/F.cpp
===
--- /dev/null
+++ test/Import/cxx-casts/Inputs/F.cpp
@@ -0,0 +1,12 @@
+struct A {
+  virtual ~A() {}
+};
+struct B : public A {};
+
+void f() {
+  const A *b = new B();
+  const B *c1 = dynamic_cast(b);
+  const B *c2 = static_cast(b);
+  const B *c3 = reinterpret_cast(b);
+  A *c4 = const_cast(b);
+}
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -6897,6 +6897,10 @@
 return CXXReinterpretCastExpr::Create(
 Importer.getToContext(), ToType, VK, CK, ToOp, &BasePath,
 ToWritten, ToOperatorLoc, ToRParenLoc, ToAngleBrackets);
+  } else if (isa(E)) {
+return CXXConstCastExpr::Create(Importer.getToContext(), ToType, VK, ToOp,
+ToWritten, ToOperatorLoc, ToRParenLoc,
+ToAngleBrackets);
   } else {
 return nullptr;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r340182 - [ASTImporter] Add test for C++ casts and fix broken const_cast importing.

[Raphael Isemann via cfe-commits]

Author: teemperor
Date: Mon Aug 20 09:20:01 2018
New Revision: 340182

URL: http://llvm.org/viewvc/llvm-project?rev=340182&view=rev
Log:
[ASTImporter] Add test for C++ casts and fix broken const_cast importing.

Summary:
The ASTImporter does currently not handle const_casts. This patch adds the
missing const_cast importer code and the test case that discovered this.

Reviewers: a.sidorin, a_sidorin

Reviewed By: a_sidorin

Subscribers: a_sidorin, martong, cfe-commits

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

Added:
cfe/trunk/test/Import/cxx-casts/
cfe/trunk/test/Import/cxx-casts/Inputs/
cfe/trunk/test/Import/cxx-casts/Inputs/F.cpp
cfe/trunk/test/Import/cxx-casts/test.cpp
Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/tools/clang-import-test/clang-import-test.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=340182&r1=340181&r2=340182&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Mon Aug 20 09:20:01 2018
@@ -6897,6 +6897,10 @@ Expr *ASTNodeImporter::VisitCXXNamedCast
 return CXXReinterpretCastExpr::Create(
 Importer.getToContext(), ToType, VK, CK, ToOp, &BasePath,
 ToWritten, ToOperatorLoc, ToRParenLoc, ToAngleBrackets);
+  } else if (isa(E)) {
+return CXXConstCastExpr::Create(Importer.getToContext(), ToType, VK, ToOp,
+ToWritten, ToOperatorLoc, ToRParenLoc,
+ToAngleBrackets);
   } else {
 return nullptr;
   }

Added: cfe/trunk/test/Import/cxx-casts/Inputs/F.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/cxx-casts/Inputs/F.cpp?rev=340182&view=auto
==
--- cfe/trunk/test/Import/cxx-casts/Inputs/F.cpp (added)
+++ cfe/trunk/test/Import/cxx-casts/Inputs/F.cpp Mon Aug 20 09:20:01 2018
@@ -0,0 +1,12 @@
+struct A {
+  virtual ~A() {}
+};
+struct B : public A {};
+
+void f() {
+  const A *b = new B();
+  const B *c1 = dynamic_cast(b);
+  const B *c2 = static_cast(b);
+  const B *c3 = reinterpret_cast(b);
+  A *c4 = const_cast(b);
+}

Added: cfe/trunk/test/Import/cxx-casts/test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/cxx-casts/test.cpp?rev=340182&view=auto
==
--- cfe/trunk/test/Import/cxx-casts/test.cpp (added)
+++ cfe/trunk/test/Import/cxx-casts/test.cpp Mon Aug 20 09:20:01 2018
@@ -0,0 +1,21 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | 
FileCheck %s
+
+// CHECK: CXXDynamicCastExpr
+// CHECK-SAME: dynamic_cast
+// CHECK-SAME: 
+
+// CHECK: CXXStaticCastExpr
+// CHECK-SAME: static_cast
+// CHECK-SAME: 
+
+// CHECK: CXXReinterpretCastExpr
+// CHECK-SAME: reinterpret_cast
+// CHECK-SAME: 
+
+// CHECK: CXXConstCastExpr
+// CHECK-SAME: const_cast
+// CHECK-SAME: 
+
+void expr() {
+  f();
+}

Modified: cfe/trunk/tools/clang-import-test/clang-import-test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-import-test/clang-import-test.cpp?rev=340182&r1=340181&r2=340182&view=diff
==
--- cfe/trunk/tools/clang-import-test/clang-import-test.cpp (original)
+++ cfe/trunk/tools/clang-import-test/clang-import-test.cpp Mon Aug 20 09:20:01 
2018
@@ -194,6 +194,8 @@ std::unique_ptr BuildC
   Inv->getLangOpts()->ThreadsafeStatics = false;
   Inv->getLangOpts()->AccessControl = false;
   Inv->getLangOpts()->DollarIdents = true;
+  // Needed for testing dynamic_cast.
+  Inv->getLangOpts()->RTTI = true;
   Inv->getCodeGenOpts().setDebugInfo(codegenoptions::FullDebugInfo);
   Inv->getTargetOpts().Triple = llvm::sys::getDefaultTargetTriple();
 


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

[PATCH] D50932: [ASTImporter] Add test for C++ casts and fix broken const_cast importing.

[Raphael Isemann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340182: [ASTImporter] Add test for C++ casts and fix broken 
const_cast importing. (authored by teemperor, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50932?vs=161496&id=161498#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50932

Files:
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/test/Import/cxx-casts/Inputs/F.cpp
  cfe/trunk/test/Import/cxx-casts/test.cpp
  cfe/trunk/tools/clang-import-test/clang-import-test.cpp


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -6897,6 +6897,10 @@
 return CXXReinterpretCastExpr::Create(
 Importer.getToContext(), ToType, VK, CK, ToOp, &BasePath,
 ToWritten, ToOperatorLoc, ToRParenLoc, ToAngleBrackets);
+  } else if (isa(E)) {
+return CXXConstCastExpr::Create(Importer.getToContext(), ToType, VK, ToOp,
+ToWritten, ToOperatorLoc, ToRParenLoc,
+ToAngleBrackets);
   } else {
 return nullptr;
   }
Index: cfe/trunk/tools/clang-import-test/clang-import-test.cpp
===
--- cfe/trunk/tools/clang-import-test/clang-import-test.cpp
+++ cfe/trunk/tools/clang-import-test/clang-import-test.cpp
@@ -194,6 +194,8 @@
   Inv->getLangOpts()->ThreadsafeStatics = false;
   Inv->getLangOpts()->AccessControl = false;
   Inv->getLangOpts()->DollarIdents = true;
+  // Needed for testing dynamic_cast.
+  Inv->getLangOpts()->RTTI = true;
   Inv->getCodeGenOpts().setDebugInfo(codegenoptions::FullDebugInfo);
   Inv->getTargetOpts().Triple = llvm::sys::getDefaultTargetTriple();
 
Index: cfe/trunk/test/Import/cxx-casts/Inputs/F.cpp
===
--- cfe/trunk/test/Import/cxx-casts/Inputs/F.cpp
+++ cfe/trunk/test/Import/cxx-casts/Inputs/F.cpp
@@ -0,0 +1,12 @@
+struct A {
+  virtual ~A() {}
+};
+struct B : public A {};
+
+void f() {
+  const A *b = new B();
+  const B *c1 = dynamic_cast(b);
+  const B *c2 = static_cast(b);
+  const B *c3 = reinterpret_cast(b);
+  A *c4 = const_cast(b);
+}
Index: cfe/trunk/test/Import/cxx-casts/test.cpp
===
--- cfe/trunk/test/Import/cxx-casts/test.cpp
+++ cfe/trunk/test/Import/cxx-casts/test.cpp
@@ -0,0 +1,21 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | 
FileCheck %s
+
+// CHECK: CXXDynamicCastExpr
+// CHECK-SAME: dynamic_cast
+// CHECK-SAME: 
+
+// CHECK: CXXStaticCastExpr
+// CHECK-SAME: static_cast
+// CHECK-SAME: 
+
+// CHECK: CXXReinterpretCastExpr
+// CHECK-SAME: reinterpret_cast
+// CHECK-SAME: 
+
+// CHECK: CXXConstCastExpr
+// CHECK-SAME: const_cast
+// CHECK-SAME: 
+
+void expr() {
+  f();
+}


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -6897,6 +6897,10 @@
 return CXXReinterpretCastExpr::Create(
 Importer.getToContext(), ToType, VK, CK, ToOp, &BasePath,
 ToWritten, ToOperatorLoc, ToRParenLoc, ToAngleBrackets);
+  } else if (isa(E)) {
+return CXXConstCastExpr::Create(Importer.getToContext(), ToType, VK, ToOp,
+ToWritten, ToOperatorLoc, ToRParenLoc,
+ToAngleBrackets);
   } else {
 return nullptr;
   }
Index: cfe/trunk/tools/clang-import-test/clang-import-test.cpp
===
--- cfe/trunk/tools/clang-import-test/clang-import-test.cpp
+++ cfe/trunk/tools/clang-import-test/clang-import-test.cpp
@@ -194,6 +194,8 @@
   Inv->getLangOpts()->ThreadsafeStatics = false;
   Inv->getLangOpts()->AccessControl = false;
   Inv->getLangOpts()->DollarIdents = true;
+  // Needed for testing dynamic_cast.
+  Inv->getLangOpts()->RTTI = true;
   Inv->getCodeGenOpts().setDebugInfo(codegenoptions::FullDebugInfo);
   Inv->getTargetOpts().Triple = llvm::sys::getDefaultTargetTriple();
 
Index: cfe/trunk/test/Import/cxx-casts/Inputs/F.cpp
===
--- cfe/trunk/test/Import/cxx-casts/Inputs/F.cpp
+++ cfe/trunk/test/Import/cxx-casts/Inputs/F.cpp
@@ -0,0 +1,12 @@
+struct A {
+  virtual ~A() {}
+};
+struct B : public A {};
+
+void f() {
+  const A *b = new B();
+  const B *c1 = dynamic_cast(b);
+  const B *c2 = static_cast(b);
+  const B *c3 = reinterpret_cast(b);
+  A *c4 = const_cast(b);
+}
Index: cfe/trunk/test/Import/cxx-casts/test.cpp
===
--- cfe/trunk/test/Import/cxx-casts/test.cpp
+++ cfe/trunk/test/Import/cxx-casts/test.cpp
@@ -0,0 +1,21 @@
+// RUN: clang-imp

[PATCH] D50571: [clangd] add an extension field to LSP to transfer the diagnostic's category

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

In https://reviews.llvm.org/D50571#1205650, @joaotavora wrote:

> > LGTM. Let's watch out for possible breakages in any of the clients, though. 
> > I've checked VSCode and it seems to be fine with the added fields.
>
> This isn't in the spec and broke the LSP client eglot 
> (https://github.com/joaotavora/eglot/pull/81). Why don't you put this in the 
> "source" field, or concat it to the "message" field.  Who can even use this 
> information if it's not in the spec? Are clients supposed to code against 
> every LSP server's whim?


Thanks for the feedback. I'll make a patch that turns this off by default so 
that clients can opt-in into it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50571



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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg marked an inline comment as done.
hugoeg added inline comments.



Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:20
+
+bool IsInAbseilFile(const SourceManager& manager, SourceLocation loc){
+  if (loc.isInvalid()) {

hokein wrote:
> hugoeg wrote:
> > hokein wrote:
> > > I think we can make it as an ASTMatcher instead of a function like:
> > > 
> > > ```
> > > AST_POLYMORPHIC_MATCHER_P(isInAbseilFile,
> > >   AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, 
> > > TypeLoc)) {
> > >// your code here.
> > > }
> > > ```
> > Unfortunately, I do not think we can.  That was the way I originally tried 
> > to implement it. It worked on no-namespace-check, but not in this one. This 
> > is because as an AST_POLYMORPHIC_MATCHER_P, we are trying to match an usage 
> > of a Decl, not the Decl itself and since we are matching a TypeLoc in 
> > no-internal-deps-check, not really the usage, it doesn't work.
> > 
> > As a result, I modified my original implementation, which you will see in 
> > no-namespace-check and turned it into IsInAbseilFile(SourceManager&,  
> > SourceLocation), which is just takes a source location and returns whether 
> > the location we matched is in an Abseil file
> Could you explain a bit more why it won't work? I don't understand why it 
> doesn't work. 
> 
> Basically you define the matcher like:
> 
> ```
> AST_POLYMORPHIC_MATCHER(
> isInAbseilFile, AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc,
> NestedNameSpecifierLoc)) {
>   auto &SourceManager = Finder->getASTContext().getSourceManager();
>   SourceLocation loc = Node.getBeginLoc();
>   if (loc.isInvalid()) return false;
>   const FileEntry *FileEntry =
>   SourceManager.getFileEntryForID(SourceManager.getFileID(loc));
>   if (!FileEntry) return false;
>   StringRef Filename = FileEntry->getName();
>   llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
>  "synchronization|types|utiliy)");
>   return RE.match(Filename);
> }
> ```
> 
> And use it for example in your check
> 
> ```
> Finder->addMatcher(nestedNameSpecifierLoc(
>  loc(specifiesNamespace(namespaceDecl(
>  matchesName("internal"),
>  hasParent(namespaceDecl(hasName("absl")),
>  unless(isInAbseilFile()))
>  .bind("InternalDep"),
>  this);
> ```
You're right, this implementation seems to work, I seemed to have a simple 
logic error in my original implementation, the new diff will include this 
version 


https://reviews.llvm.org/D50542



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

[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: lib/Parse/ParseExpr.cpp:1126
+
+Actions.StartCheckingNoDeref();
+

This parser-driven start/stop mechanism will not work in C++ templates. 
Instead, can you remove the "start" part and check the noderef exprs as part of 
popping the ExprEvaluationContextRecord?



Comment at: lib/Sema/SemaExpr.cpp:14230-14242
+class DeclRefFinder : public ConstEvaluatedExprVisitor {
+public:
+  typedef ConstEvaluatedExprVisitor Inherited;
+
+  DeclRefFinder(ASTContext &Ctx) : Inherited(Ctx) {}
+
+  void VisitDeclRefExpr(const DeclRefExpr *E) { DeclRef = E; }

rsmith wrote:
> I don't see any reason to assume that the `DeclRefExpr` found by this visitor 
> will be the one you're interested in (the one the `noderef` attribute came 
> from).
> 
> How about instead you walk over only expressions that you *know* preserve 
> `noderef`, and if you can't find a declaration as the source of the `noderef` 
> attribute, produce a differently-worded diagnostic that doesn't give a 
> variable name?
This comment is marked done but has not been addressed.



Comment at: lib/Sema/SemaExpr.cpp:4267
+  } else if (const auto *Member = dyn_cast(StrippedExpr)) {
+// Getting the address of a member expr in the form &(*s).b
+const Expr *Base = Member->getBase()->IgnoreParenImpCasts();

You need to loop/recurse here; there could be a sequence of `.` member accesses 
to strip.



Comment at: lib/Sema/SemaExpr.cpp:4291
+  const QualType &BaseTy = Base->getType();
+  QualType ElemTy;
+  if (const auto *ArrayTy = dyn_cast(BaseTy))

Did you mean to use ElemTy somewhere below? (I'd guess you intended to check 
whether it's marked noderef?)



Comment at: lib/Sema/SemaType.cpp:4209
+  bool IsNoDeref = false;
+  if (const auto *AT = dyn_cast(T))
+IsNoDeref = AT->getAttrKind() == AttributedType::attr_noderef;

This is not correct (there could be multiple attributes on the type). See below.



Comment at: lib/Sema/SemaType.cpp:4816
+
+if (const auto *AT = dyn_cast(T))
+  IsNoDeref = AT->getAttrKind() == AttributedType::attr_noderef;

Move this after the check below (directly setting ExpectNoDerefChunk instead), 
and remove the unnecessary IsNoDeref variable.



Comment at: lib/Sema/SemaType.cpp:4816
+
+if (const auto *AT = dyn_cast(T))
+  IsNoDeref = AT->getAttrKind() == AttributedType::attr_noderef;

rsmith wrote:
> Move this after the check below (directly setting ExpectNoDerefChunk 
> instead), and remove the unnecessary IsNoDeref variable.
This is not correct: there could be multiple attributes at this level. You 
could fix this locally either by looping through the attributes on the type or 
iterating through the ParsedAttrs on the chunk. But I think my preference would 
be to store state indicating that we have an unvalidated noderef attribute on 
the TypeProcessingState at the point where you create the AttributedType, and 
check and clear that flag here when creating a type chunk.


Repository:
  rC Clang

https://reviews.llvm.org/D49511



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

r340188 - Close FileEntries of cached files in ModuleManager::addModule().

[Adrian Prantl via cfe-commits]

Author: adrian
Date: Mon Aug 20 10:10:27 2018
New Revision: 340188

URL: http://llvm.org/viewvc/llvm-project?rev=340188&view=rev
Log:
Close FileEntries of cached files in ModuleManager::addModule().

While investigating why LLDB (which can build hundreds of clang
modules during one debug session) was getting "too many open files"
errors, I found that most of them are .pcm files that are kept open by
ModuleManager. Pretty much all of the open file dscriptors are
FileEntries that are refering to `.pcm` files for which a buffer
already exists in a CompilerInstance's PCMCache.

Before PCMCache was added it was necessary to hold on to open file
descriptors to ensure that all ModuleManagers using the same
FileManager read the a consistent version of a given `.pcm` file on
disk, even when a concurrent clang process overwrites the file halfway
through. The PCMCache makes this practice unnecessary, since it caches
the entire contents of a `.pcm` file, while the FileManager caches all
the stat() information.

This patch adds a call to FileEntry::closeFile() to the path where a
Buffer has already been created. This is necessary because even for a
freshly written `.pcm` file the file is stat()ed once immediately
after writing to generate a FileEntry in the FileManager. Because a
freshly-generated file's contents is stored in the PCMCache, it is
fine to close the file immediately thereafter.  The second change this
patch makes is to set the `ShouldClose` flag to true when reading a
`.pcm` file into the PCMCache for the first time.

[For reference, in 1 Clang instance there is
 - 1 FileManager and
 - n ModuleManagers with
 - n PCMCaches.]

rdar://problem/40906753

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

Modified:
cfe/trunk/lib/Serialization/ModuleManager.cpp

Modified: cfe/trunk/lib/Serialization/ModuleManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ModuleManager.cpp?rev=340188&r1=340187&r2=340188&view=diff
==
--- cfe/trunk/lib/Serialization/ModuleManager.cpp (original)
+++ cfe/trunk/lib/Serialization/ModuleManager.cpp Mon Aug 20 10:10:27 2018
@@ -161,21 +161,24 @@ ModuleManager::addModule(StringRef FileN
   if (std::unique_ptr Buffer = lookupBuffer(FileName)) {
 // The buffer was already provided for us.
 NewModule->Buffer = &PCMCache->addBuffer(FileName, std::move(Buffer));
+// Since the cached buffer is reused, it is safe to close the file
+// descriptor that was opened while stat()ing the PCM in
+// lookupModuleFile() above, it won't be needed any longer.
+Entry->closeFile();
   } else if (llvm::MemoryBuffer *Buffer = PCMCache->lookupBuffer(FileName)) {
 NewModule->Buffer = Buffer;
+// As above, the file descriptor is no longer needed.
+Entry->closeFile();
   } else {
 // Open the AST file.
 llvm::ErrorOr> 
Buf((std::error_code()));
 if (FileName == "-") {
   Buf = llvm::MemoryBuffer::getSTDIN();
 } else {
-  // Leave the FileEntry open so if it gets read again by another
-  // ModuleManager it must be the same underlying file.
-  // FIXME: Because FileManager::getFile() doesn't guarantee that it will
-  // give us an open file, this may not be 100% reliable.
+  // Get a buffer of the file and close the file descriptor when done.
   Buf = FileMgr.getBufferForFile(NewModule->File,
  /*IsVolatile=*/false,
- /*ShouldClose=*/false);
+ /*ShouldClose=*/true);
 }
 
 if (!Buf) {


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

[PATCH] D50870: Close FileEntries of cached files in ModuleManager::addModule().

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340188: Close FileEntries of cached files in 
ModuleManager::addModule(). (authored by adrian, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50870?vs=161119&id=161501#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50870

Files:
  cfe/trunk/lib/Serialization/ModuleManager.cpp


Index: cfe/trunk/lib/Serialization/ModuleManager.cpp
===
--- cfe/trunk/lib/Serialization/ModuleManager.cpp
+++ cfe/trunk/lib/Serialization/ModuleManager.cpp
@@ -161,21 +161,24 @@
   if (std::unique_ptr Buffer = lookupBuffer(FileName)) {
 // The buffer was already provided for us.
 NewModule->Buffer = &PCMCache->addBuffer(FileName, std::move(Buffer));
+// Since the cached buffer is reused, it is safe to close the file
+// descriptor that was opened while stat()ing the PCM in
+// lookupModuleFile() above, it won't be needed any longer.
+Entry->closeFile();
   } else if (llvm::MemoryBuffer *Buffer = PCMCache->lookupBuffer(FileName)) {
 NewModule->Buffer = Buffer;
+// As above, the file descriptor is no longer needed.
+Entry->closeFile();
   } else {
 // Open the AST file.
 llvm::ErrorOr> 
Buf((std::error_code()));
 if (FileName == "-") {
   Buf = llvm::MemoryBuffer::getSTDIN();
 } else {
-  // Leave the FileEntry open so if it gets read again by another
-  // ModuleManager it must be the same underlying file.
-  // FIXME: Because FileManager::getFile() doesn't guarantee that it will
-  // give us an open file, this may not be 100% reliable.
+  // Get a buffer of the file and close the file descriptor when done.
   Buf = FileMgr.getBufferForFile(NewModule->File,
  /*IsVolatile=*/false,
- /*ShouldClose=*/false);
+ /*ShouldClose=*/true);
 }
 
 if (!Buf) {


Index: cfe/trunk/lib/Serialization/ModuleManager.cpp
===
--- cfe/trunk/lib/Serialization/ModuleManager.cpp
+++ cfe/trunk/lib/Serialization/ModuleManager.cpp
@@ -161,21 +161,24 @@
   if (std::unique_ptr Buffer = lookupBuffer(FileName)) {
 // The buffer was already provided for us.
 NewModule->Buffer = &PCMCache->addBuffer(FileName, std::move(Buffer));
+// Since the cached buffer is reused, it is safe to close the file
+// descriptor that was opened while stat()ing the PCM in
+// lookupModuleFile() above, it won't be needed any longer.
+Entry->closeFile();
   } else if (llvm::MemoryBuffer *Buffer = PCMCache->lookupBuffer(FileName)) {
 NewModule->Buffer = Buffer;
+// As above, the file descriptor is no longer needed.
+Entry->closeFile();
   } else {
 // Open the AST file.
 llvm::ErrorOr> Buf((std::error_code()));
 if (FileName == "-") {
   Buf = llvm::MemoryBuffer::getSTDIN();
 } else {
-  // Leave the FileEntry open so if it gets read again by another
-  // ModuleManager it must be the same underlying file.
-  // FIXME: Because FileManager::getFile() doesn't guarantee that it will
-  // give us an open file, this may not be 100% reliable.
+  // Get a buffer of the file and close the file descriptor when done.
   Buf = FileMgr.getBufferForFile(NewModule->File,
  /*IsVolatile=*/false,
- /*ShouldClose=*/false);
+ /*ShouldClose=*/true);
 }
 
 if (!Buf) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50978: [ASTImporter] Add test for C++'s try/catch statements.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor created this revision.
Herald added subscribers: cfe-commits, martong.
Herald added a reviewer: a.sidorin.

Also enable exceptions in clang-import-test so that we can parse the test files.


Repository:
  rC Clang

https://reviews.llvm.org/D50978

Files:
  test/Import/cxx-try-catch/Inputs/F.cpp
  test/Import/cxx-try-catch/test.cpp
  tools/clang-import-test/clang-import-test.cpp


Index: tools/clang-import-test/clang-import-test.cpp
===
--- tools/clang-import-test/clang-import-test.cpp
+++ tools/clang-import-test/clang-import-test.cpp
@@ -196,6 +196,8 @@
   Inv->getLangOpts()->DollarIdents = true;
   // Needed for testing dynamic_cast.
   Inv->getLangOpts()->RTTI = true;
+  Inv->getLangOpts()->Exceptions = true;
+  Inv->getLangOpts()->CXXExceptions = true;
   Inv->getCodeGenOpts().setDebugInfo(codegenoptions::FullDebugInfo);
   Inv->getTargetOpts().Triple = llvm::sys::getDefaultTargetTriple();
 
Index: test/Import/cxx-try-catch/test.cpp
===
--- /dev/null
+++ test/Import/cxx-try-catch/test.cpp
@@ -0,0 +1,39 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | 
FileCheck %s
+
+// CHECK: CXXTryStmt
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: CXXCatchStmt
+// CHECK-NEXT: <>
+// CHECK-NEXT: CompoundStmt
+
+// CHECK: CXXTryStmt
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: CXXCatchStmt
+// CHECK-NEXT: VarDecl
+// CHECK-SAME: 'int'
+// CHECK-NEXT: CompoundStmt
+
+// CHECK: CXXTryStmt
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: CXXCatchStmt
+// CHECK-NEXT: VarDecl
+// CHECK-SAME: varname
+// CHECK-SAME: 'int'
+// CHECK-NEXT: CompoundStmt
+
+// CHECK: CXXTryStmt
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: CXXCatchStmt
+// CHECK-NEXT: VarDecl
+// CHECK-SAME: varname1
+// CHECK-SAME: 'int'
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: CXXCatchStmt
+// CHECK-NEXT: VarDecl
+// CHECK-SAME: varname2
+// CHECK-SAME: 'long'
+// CHECK-NEXT: CompoundStmt
+
+void expr() {
+  f();
+}
Index: test/Import/cxx-try-catch/Inputs/F.cpp
===
--- /dev/null
+++ test/Import/cxx-try-catch/Inputs/F.cpp
@@ -0,0 +1,18 @@
+void f() {
+  try {
+  } catch (...) {
+  }
+
+  try {
+  } catch (int) {
+  }
+
+  try {
+  } catch (int varname) {
+  }
+
+  try {
+  } catch (int varname1) {
+  } catch (long varname2) {
+  }
+}


Index: tools/clang-import-test/clang-import-test.cpp
===
--- tools/clang-import-test/clang-import-test.cpp
+++ tools/clang-import-test/clang-import-test.cpp
@@ -196,6 +196,8 @@
   Inv->getLangOpts()->DollarIdents = true;
   // Needed for testing dynamic_cast.
   Inv->getLangOpts()->RTTI = true;
+  Inv->getLangOpts()->Exceptions = true;
+  Inv->getLangOpts()->CXXExceptions = true;
   Inv->getCodeGenOpts().setDebugInfo(codegenoptions::FullDebugInfo);
   Inv->getTargetOpts().Triple = llvm::sys::getDefaultTargetTriple();
 
Index: test/Import/cxx-try-catch/test.cpp
===
--- /dev/null
+++ test/Import/cxx-try-catch/test.cpp
@@ -0,0 +1,39 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | FileCheck %s
+
+// CHECK: CXXTryStmt
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: CXXCatchStmt
+// CHECK-NEXT: <>
+// CHECK-NEXT: CompoundStmt
+
+// CHECK: CXXTryStmt
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: CXXCatchStmt
+// CHECK-NEXT: VarDecl
+// CHECK-SAME: 'int'
+// CHECK-NEXT: CompoundStmt
+
+// CHECK: CXXTryStmt
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: CXXCatchStmt
+// CHECK-NEXT: VarDecl
+// CHECK-SAME: varname
+// CHECK-SAME: 'int'
+// CHECK-NEXT: CompoundStmt
+
+// CHECK: CXXTryStmt
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: CXXCatchStmt
+// CHECK-NEXT: VarDecl
+// CHECK-SAME: varname1
+// CHECK-SAME: 'int'
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: CXXCatchStmt
+// CHECK-NEXT: VarDecl
+// CHECK-SAME: varname2
+// CHECK-SAME: 'long'
+// CHECK-NEXT: CompoundStmt
+
+void expr() {
+  f();
+}
Index: test/Import/cxx-try-catch/Inputs/F.cpp
===
--- /dev/null
+++ test/Import/cxx-try-catch/Inputs/F.cpp
@@ -0,0 +1,18 @@
+void f() {
+  try {
+  } catch (...) {
+  }
+
+  try {
+  } catch (int) {
+  }
+
+  try {
+  } catch (int varname) {
+  }
+
+  try {
+  } catch (int varname1) {
+  } catch (long varname2) {
+  }
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42043: c-index: CXString: fix MSAN read-past-end bug

[Vedant Kumar via Phabricator via cfe-commits]

vsk added subscribers: jkorous, vsapsai.
vsk added a comment.

+ Jan and Volodymyr. This seemed to be in good shape the last time I looked at 
it. Not having touched libclang for a while I don't think I can give an 
official lgtm.


Repository:
  rC Clang

https://reviews.llvm.org/D42043



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

[PATCH] D50926: [SourceManager] Extract 'findFileIDsForFile' from 'translateFile' to allow mapping from one file entry to multiple FileIDs

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: include/clang/Basic/SourceManager.h:1539
+  /// \returns true if the callback returned true, false otherwise.
+  bool findFileIDsForFile(const FileEntry *SourceFile,
+  llvm::function_ref Callback) const;

ioeric wrote:
> Callback pattern seems uncommon in LLVM/Clang. I'd suggest making this return 
> a set of `FileID`s and put the callback-based function as a helper (shared by 
> this and `translateFile`)in the implementation.
I created a helper, but unfortunately it needs to be a member of SourceManager 
as FileID::get is private.


Repository:
  rC Clang

https://reviews.llvm.org/D50926



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

[PATCH] D50926: [SourceManager] Extract 'findFileIDsForFile' from 'translateFile' to allow mapping from one file entry to multiple FileIDs

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 161504.
arphaman marked 2 inline comments as done.
arphaman added a comment.

Address review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D50926

Files:
  include/clang/Basic/SourceManager.h
  lib/Basic/SourceManager.cpp
  unittests/Basic/SourceManagerTest.cpp

Index: unittests/Basic/SourceManagerTest.cpp
===
--- unittests/Basic/SourceManagerTest.cpp
+++ unittests/Basic/SourceManagerTest.cpp
@@ -377,6 +377,51 @@
   EXPECT_TRUE(SourceMgr.isBeforeInTranslationUnit(Macros[10].Loc, Macros[11].Loc));
 }
 
+TEST_F(SourceManagerTest, findFileIDsForFile) {
+  const char *header = "int x;";
+
+  const char *main = "#include \n"
+ "#include \n";
+
+  std::unique_ptr HeaderBuf =
+  llvm::MemoryBuffer::getMemBuffer(header);
+  std::unique_ptr MainBuf =
+  llvm::MemoryBuffer::getMemBuffer(main);
+  SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(MainBuf)));
+
+  const FileEntry *headerFile =
+  FileMgr.getVirtualFile("/test-header.h", HeaderBuf->getBufferSize(), 0);
+  SourceMgr.overrideFileContents(headerFile, std::move(HeaderBuf));
+
+  TrivialModuleLoader ModLoader;
+  MemoryBufferCache PCMCache;
+  HeaderSearch HeaderInfo(std::make_shared(), SourceMgr,
+  Diags, LangOpts, &*Target);
+  Preprocessor PP(std::make_shared(), Diags, LangOpts,
+  SourceMgr, PCMCache, HeaderInfo, ModLoader,
+  /*IILookup =*/nullptr,
+  /*OwnsHeaderSearch =*/false);
+  PP.Initialize(*Target);
+
+  PP.EnterMainSourceFile();
+
+  while (1) {
+Token tok;
+PP.Lex(tok);
+if (tok.is(tok::eof))
+  break;
+  }
+
+  llvm::SmallVector Files = SourceMgr.findFileIDsForFile(headerFile);
+
+  ASSERT_EQ(2U, Files.size());
+  ASSERT_NE(Files[0], Files[1]);
+  SourceLocation Loc1 = SourceMgr.translateLineCol(Files[0], 1, 1);
+  SourceLocation Loc2 = SourceMgr.translateLineCol(Files[1], 1, 1);
+  ASSERT_NE(Loc1, Loc2);
+  ASSERT_TRUE(SourceMgr.isBeforeInTranslationUnit(Loc1, Loc2));
+}
+
 #endif
 
 } // anonymous namespace
Index: lib/Basic/SourceManager.cpp
===
--- lib/Basic/SourceManager.cpp
+++ lib/Basic/SourceManager.cpp
@@ -1602,6 +1602,47 @@
   return translateLineCol(FirstFID, Line, Col);
 }
 
+bool SourceManager::findFileIDsForFile(
+const FileEntry *SourceFile,
+llvm::function_ref Callback) const {
+  assert(SourceFile && "Null source file!");
+
+  // Look through all of the local source locations.
+  for (unsigned I = 0, N = local_sloc_entry_size(); I != N; ++I) {
+bool Invalid = false;
+const SLocEntry &SLoc = getLocalSLocEntry(I, &Invalid);
+if (Invalid)
+  return false;
+
+if (SLoc.isFile() && SLoc.getFile().getContentCache() &&
+SLoc.getFile().getContentCache()->OrigEntry == SourceFile) {
+  if (Callback(FileID::get(I)))
+return true;
+}
+  }
+
+  // If that still didn't help, try the modules.
+  for (unsigned I = 0, N = loaded_sloc_entry_size(); I != N; ++I) {
+const SLocEntry &SLoc = getLoadedSLocEntry(I);
+if (SLoc.isFile() && SLoc.getFile().getContentCache() &&
+SLoc.getFile().getContentCache()->OrigEntry == SourceFile) {
+  if (Callback(FileID::get(-int(I) - 2)))
+return true;
+}
+  }
+  return false;
+}
+
+llvm::SmallVector
+SourceManager::findFileIDsForFile(const FileEntry *SourceFile) const {
+  llvm::SmallVector Result;
+  findFileIDsForFile(SourceFile, [&](FileID F) {
+Result.push_back(F);
+return false;
+  });
+  return Result;
+}
+
 /// Get the FileID for the given file.
 ///
 /// If the source file is included multiple times, the FileID will be the
@@ -1650,35 +1691,16 @@
 }
   }
 
-  if (FirstFID.isInvalid()) {
-// The location we're looking for isn't in the main file; look
-// through all of the local source locations.
-for (unsigned I = 0, N = local_sloc_entry_size(); I != N; ++I) {
-  bool Invalid = false;
-  const SLocEntry &SLoc = getLocalSLocEntry(I, &Invalid);
-  if (Invalid)
-return FileID();
+  if (FirstFID.isValid())
+return FirstFID;
 
-  if (SLoc.isFile() &&
-  SLoc.getFile().getContentCache() &&
-  SLoc.getFile().getContentCache()->OrigEntry == SourceFile) {
-FirstFID = FileID::get(I);
-break;
-  }
-}
-// If that still didn't help, try the modules.
-if (FirstFID.isInvalid()) {
-  for (unsigned I = 0, N = loaded_sloc_entry_size(); I != N; ++I) {
-const SLocEntry &SLoc = getLoadedSLocEntry(I);
-if (SLoc.isFile() &&
-SLoc.getFile().getContentCache() &&
-SLoc.getFile().getContentCache()->OrigEntry == SourceFile) {
-  FirstFID = FileID::get(-int(I) - 2);
-  break;
-}
-  }
-}
-  }
+  // The location we're looking for isn't in the main file; look
+  // through all

[PATCH] D50535: Fix selective formatting of ObjC scope

[Jacek Olesiak via Phabricator via cfe-commits]

jolesiak added a comment.

Nice! That looks very promising.

It still fails when we pass a longer range (maybe mimicking `PreviousRBrace` 
will help), e.g.:
Base code:

  @protocol A
   @optional
  // comment
  - (void)f;
  @end
  MACRO

formatted with `clang-format -lines=3:6 file.m` gives:

  @protocol A
   @optional
   // comment
   - (void)f;
   @end
   MACRO

This is not happening for Cpp, e.g.:
Base code:

  class A {
   void f ();
  // comment
  void g ();
  };
  MACRO

running `clang-format -lines=3:6 file.cpp` gives:

  class A {
   void f ();
   // comment
   void g ();
  };
  MACRO

I think it would be good to add all these tests to unit tests, but I see that 
an interface to pass line ranges is not very pleasant.


Repository:
  rC Clang

https://reviews.llvm.org/D50535



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

[PATCH] D50979: Eliminate instances of `EmitScalarExpr(E->getArg(n))` in EmitX86BuiltinExpr().

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: rnk.
Herald added a reviewer: javed.absar.
Herald added a subscriber: kristof.beyls.

EmitX86BuiltinExpr() emits all args into Ops at the beginning, so don't do that 
work again. No intended behavior change.

(TNorthover: EmitAArch64BuiltinExpr() also emits args into Ops before the big 
switch (with some more subtlety around the last arg that I don't understand), 
but then almost every switch case does `EmitScalarExpr(E->getArg(n))`. It's 
been like this since the commit that added the arm64 code. Was the Ops pushing 
code added later and all the EmitScalarExpr()s are now unneeded, or is some 
reason for this?)


https://reviews.llvm.org/D50979

Files:
  clang/lib/CodeGen/CGBuiltin.cpp


Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -10480,14 +10480,11 @@
 llvm::Type *Int128PtrTy = Int128Ty->getPointerTo();
 
 Value *Destination =
-Builder.CreateBitCast(EmitScalarExpr(E->getArg(0)), Int128PtrTy);
-Value *ExchangeHigh128 =
-Builder.CreateZExt(EmitScalarExpr(E->getArg(1)), Int128Ty);
-Value *ExchangeLow128 =
-Builder.CreateZExt(EmitScalarExpr(E->getArg(2)), Int128Ty);
-Address ComparandResult(
-Builder.CreateBitCast(EmitScalarExpr(E->getArg(3)), Int128PtrTy),
-getContext().toCharUnitsFromBits(128));
+Builder.CreateBitCast(Ops[0], Int128PtrTy);
+Value *ExchangeHigh128 = Builder.CreateZExt(Ops[1], Int128Ty);
+Value *ExchangeLow128 = Builder.CreateZExt(Ops[2], Int128Ty);
+Address ComparandResult(Builder.CreateBitCast(Ops[3], Int128PtrTy),
+getContext().toCharUnitsFromBits(128));
 
 Value *Exchange = Builder.CreateOr(
 Builder.CreateShl(ExchangeHigh128, 64, "", false, false),
@@ -10538,8 +10535,8 @@
   case X86::BI__readfsdword:
   case X86::BI__readfsqword: {
 llvm::Type *IntTy = ConvertType(E->getType());
-Value *Ptr = Builder.CreateIntToPtr(EmitScalarExpr(E->getArg(0)),
-llvm::PointerType::get(IntTy, 257));
+Value *Ptr =
+Builder.CreateIntToPtr(Ops[0], llvm::PointerType::get(IntTy, 257));
 LoadInst *Load = Builder.CreateAlignedLoad(
 IntTy, Ptr, getContext().getTypeAlignInChars(E->getType()));
 Load->setVolatile(true);
@@ -10550,8 +10547,8 @@
   case X86::BI__readgsdword:
   case X86::BI__readgsqword: {
 llvm::Type *IntTy = ConvertType(E->getType());
-Value *Ptr = Builder.CreateIntToPtr(EmitScalarExpr(E->getArg(0)),
-llvm::PointerType::get(IntTy, 256));
+Value *Ptr =
+Builder.CreateIntToPtr(Ops[0], llvm::PointerType::get(IntTy, 256));
 LoadInst *Load = Builder.CreateAlignedLoad(
 IntTy, Ptr, getContext().getTypeAlignInChars(E->getType()));
 Load->setVolatile(true);


Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -10480,14 +10480,11 @@
 llvm::Type *Int128PtrTy = Int128Ty->getPointerTo();
 
 Value *Destination =
-Builder.CreateBitCast(EmitScalarExpr(E->getArg(0)), Int128PtrTy);
-Value *ExchangeHigh128 =
-Builder.CreateZExt(EmitScalarExpr(E->getArg(1)), Int128Ty);
-Value *ExchangeLow128 =
-Builder.CreateZExt(EmitScalarExpr(E->getArg(2)), Int128Ty);
-Address ComparandResult(
-Builder.CreateBitCast(EmitScalarExpr(E->getArg(3)), Int128PtrTy),
-getContext().toCharUnitsFromBits(128));
+Builder.CreateBitCast(Ops[0], Int128PtrTy);
+Value *ExchangeHigh128 = Builder.CreateZExt(Ops[1], Int128Ty);
+Value *ExchangeLow128 = Builder.CreateZExt(Ops[2], Int128Ty);
+Address ComparandResult(Builder.CreateBitCast(Ops[3], Int128PtrTy),
+getContext().toCharUnitsFromBits(128));
 
 Value *Exchange = Builder.CreateOr(
 Builder.CreateShl(ExchangeHigh128, 64, "", false, false),
@@ -10538,8 +10535,8 @@
   case X86::BI__readfsdword:
   case X86::BI__readfsqword: {
 llvm::Type *IntTy = ConvertType(E->getType());
-Value *Ptr = Builder.CreateIntToPtr(EmitScalarExpr(E->getArg(0)),
-llvm::PointerType::get(IntTy, 257));
+Value *Ptr =
+Builder.CreateIntToPtr(Ops[0], llvm::PointerType::get(IntTy, 257));
 LoadInst *Load = Builder.CreateAlignedLoad(
 IntTy, Ptr, getContext().getTypeAlignInChars(E->getType()));
 Load->setVolatile(true);
@@ -10550,8 +10547,8 @@
   case X86::BI__readgsdword:
   case X86::BI__readgsqword: {
 llvm::Type *IntTy = ConvertType(E->getType());
-Value *Ptr = Builder.CreateIntToPtr(EmitScalarExpr(E->getArg(0)),
-llvm::PointerType::get(IntTy, 256));
+Value *Ptr =
+Builder.CreateIntToPtr(Ops

[PATCH] D50945: [Lex] Make HeaderMaps a unique_ptr vector

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In https://reviews.llvm.org/D50945#1205337, @kristina wrote:

> Given the context (class an file name itself) and documentation around the 
> function, I don't think in this particular case it improves readability or 
> maintainability, the lifetime of the `HeaderMap` is (IMHO) fairly obvious 
> from the const qualifier and from the documentation of the function itself. I 
> would say leave it as is.


I'd vote in favor of the change - since it makes the ownership obvious 
at-a-glance (documentation or not) & helps catch any mistakes that can be 
easily introduced when handling object ownership.


Repository:
  rC Clang

https://reviews.llvm.org/D50945



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

[PATCH] D50945: [Lex] Make HeaderMaps a unique_ptr vector

[David Blaikie via Phabricator via cfe-commits]

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

Looks good - thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D50945



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

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

[Deanna Garcia via Phabricator via cfe-commits]

deannagarcia updated this revision to Diff 161513.

https://reviews.llvm.org/D50862

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
  clang-tidy/abseil/FasterStrsplitDelimiterCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-faster-strsplit-delimiter.cpp

Index: test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
===
--- test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
+++ test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
@@ -0,0 +1,99 @@
+// RUN: %check_clang_tidy %s abseil-faster-strsplit-delimiter %t
+
+namespace absl {
+
+class string_view {
+  public:
+string_view();
+string_view(const char *);
+};
+
+namespace strings_internal {
+struct Splitter {};
+struct MaxSplitsImpl {
+  MaxSplitsImpl();
+  ~MaxSplitsImpl();
+};
+} //namespace strings_internal
+
+template 
+strings_internal::Splitter StrSplit(absl::string_view, Delim) {
+  return {};
+}
+template 
+strings_internal::Splitter StrSplit(absl::string_view, Delim, Pred) {
+  return {};
+}
+
+class ByAnyChar {
+  public:
+explicit ByAnyChar(absl::string_view);
+~ByAnyChar();
+};
+
+template 
+strings_internal::MaxSplitsImpl MaxSplits(Delim, int) {
+  return {};
+}
+
+} //namespace absl
+
+void SplitDelimiters() {
+  absl::StrSplit("ABC", "A");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit() called with a string literal consisting of a single character; consider using the character overload [abseil-faster-strsplit-delimiter]
+  // CHECK-FIXES: absl::StrSplit("ABC", 'A');
+
+  absl::StrSplit("ABC", absl::ByAnyChar("\n"));
+  // CHECK-MESSAGES: [[@LINE-1]]:41: warning: absl::StrSplit()
+  // CHECK-FIXES: absl::StrSplit("ABC", '\n');
+
+  // Works with predicate
+  absl::StrSplit("ABC", "A", [](absl::string_view) { return true; });
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+  // CHECK-FIXES: absl::StrSplit("ABC", 'A', [](absl::string_view) { return true; });
+
+  // Doesn't do anything with other strings lenghts.
+  absl::StrSplit("ABC", "AB");
+  absl::StrSplit("ABC", absl::ByAnyChar(""));
+  absl::StrSplit("ABC", absl::ByAnyChar(" \t"));
+
+  // Escapes a single quote in the resulting character literal.
+  absl::StrSplit("ABC", "'");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+  // CHECK-FIXES: absl::StrSplit("ABC", '\'');
+
+  absl::StrSplit("ABC", "\"");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+  // CHECK-FIXES: absl::StrSplit("ABC", '\"');
+
+  absl::StrSplit("ABC", absl::MaxSplits("\t", 1));
+  // CHECK-MESSAGES: [[@LINE-1]]:41: warning: absl::MaxSplits()
+  // CHECK-FIXES: absl::StrSplit("ABC", absl::MaxSplits('\t', 1));
+
+  auto delim = absl::MaxSplits(absl::ByAnyChar(" "), 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:48: warning: absl::MaxSplits()
+  // CHECK-FIXES: auto delim = absl::MaxSplits(' ', 1);
+}
+
+#define MACRO(str) absl::StrSplit("ABC", str)
+
+void Macro() {
+  MACRO("A");
+}
+
+template 
+void FunctionTemplate() {
+  // This one should not warn because ByAnyChar is a dependent type.
+  absl::StrSplit("TTT", T("A"));
+
+  // This one will warn, but we are checking that we get a correct warning only
+  // once.
+  absl::StrSplit("TTT", "A");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+  // CHECK-FIXES: absl::StrSplit("TTT", 'A');
+}
+
+void FunctionTemplateCaller() {
+  FunctionTemplate();
+  FunctionTemplate();
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
 =
 
 .. toctree::
+   abseil-faster-strsplit-delimiter
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
===
--- docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
+++ docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
@@ -0,0 +1,41 @@
+.. title:: clang-tidy - abseil-faster-strsplit-delimiter
+
+abseil-faster-strsplit-delimiter
+
+
+Finds instances of ``absl::StrSplit()`` or ``absl::MaxSplits()`` where the
+delimiter is a single character string literal and replaces with a character.
+The check will offer a suggestion to change the string literal into a character.
+It will also catch code using ``absl::ByAnyChar()`` for just a single character
+and will transform that into a single character as well.
+
+These changes will give the same result, but using characters rather than
+single character string literals is more efficient and readable.
+
+Examples:
+
+.. code-block:: c++
+
+  // Original - the argument is a string literal.

r340191 - [OPENMP] Fix crash on the emission of the weak function declaration.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Aug 20 11:03:40 2018
New Revision: 340191

URL: http://llvm.org/viewvc/llvm-project?rev=340191&view=rev
Log:
[OPENMP] Fix crash on the emission of the weak function declaration.

If the function is actually a weak reference, it should not be marked as
deferred definition as this is only a declaration. Patch adds checks for
the definitions if they must be emitted. Otherwise, only declaration is
emitted.

Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/test/OpenMP/declare_target_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=340191&r1=340190&r2=340191&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Mon Aug 20 11:03:40 2018
@@ -2558,15 +2558,17 @@ llvm::Constant *CodeGenModule::GetOrCrea
 if (getLangOpts().OpenMPIsDevice && OpenMPRuntime &&
 !OpenMPRuntime->markAsGlobalTarget(GD) && FD->isDefined() &&
 !DontDefer && !IsForDefinition) {
-  const FunctionDecl *FDDef = FD->getDefinition();
-  GlobalDecl GDDef;
-  if (const auto *CD = dyn_cast(FDDef))
-GDDef = GlobalDecl(CD, GD.getCtorType());
-  else if (const auto *DD = dyn_cast(FDDef))
-GDDef = GlobalDecl(DD, GD.getDtorType());
-  else
-GDDef = GlobalDecl(FDDef);
-  addDeferredDeclToEmit(GDDef);
+  if (const FunctionDecl *FDDef = FD->getDefinition())
+if (getContext().DeclMustBeEmitted(FDDef)) {
+  GlobalDecl GDDef;
+  if (const auto *CD = dyn_cast(FDDef))
+GDDef = GlobalDecl(CD, GD.getCtorType());
+  else if (const auto *DD = dyn_cast(FDDef))
+GDDef = GlobalDecl(DD, GD.getDtorType());
+  else
+GDDef = GlobalDecl(FDDef);
+  addDeferredDeclToEmit(GDDef);
+}
 }
 
 if (FD->isMultiVersion()) {

Modified: cfe/trunk/test/OpenMP/declare_target_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_target_codegen.cpp?rev=340191&r1=340190&r2=340191&view=diff
==
--- cfe/trunk/test/OpenMP/declare_target_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/declare_target_codegen.cpp Mon Aug 20 11:03:40 2018
@@ -128,5 +128,19 @@ int baz2() {
   return 2 + baz3();
 }
 
+extern int create() throw();
+
+static __typeof(create) __t_create __attribute__((__weakref__("__create")));
+
+int baz5() {
+  bool a;
+// CHECK-DAG: define weak void 
@__omp_offloading_{{.*}}baz5{{.*}}_l[[@LINE+1]](i64 {{.*}})
+#pragma omp target
+  a = __extension__(void *) & __t_create != 0;
+  return a;
+}
+
+// CHECK-DAG: declare extern_weak signext i32 @__create()
+
 // CHECK-NOT: define {{.*}}{{baz1|baz4|maini1}}
 #endif // HEADER


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

[PATCH] D50683: [Android] Set NewAlign for 64-bit Android to 8 bytes

[Pirama Arumuga Nainar via Phabricator via cfe-commits]

pirama abandoned this revision.
pirama added a comment.

Thanks for the clarification Richard and Eli.  I agree that leaving the status 
quo will match the intent of the macro.  I'll abandon this.


Repository:
  rC Clang

https://reviews.llvm.org/D50683



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

[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

I made a post on llvm-dev 
(http://lists.llvm.org/pipermail/llvm-dev/2018-August/125433.html) to see if 
other people have opinions on this. In the meantime, do you think I should make 
a separate patch that moves this into an LLVM intrinsic function?


Repository:
  rC Clang

https://reviews.llvm.org/D50616



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

r340193 - Rename -mlink-cuda-bitcode to -mlink-builtin-bitcode

[Matt Arsenault via cfe-commits]

Author: arsenm
Date: Mon Aug 20 11:16:48 2018
New Revision: 340193

URL: http://llvm.org/viewvc/llvm-project?rev=340193&view=rev
Log:
Rename -mlink-cuda-bitcode to -mlink-builtin-bitcode

The same semantics work for OpenCL, and probably any offload
language. Keep the old name around as an alias.

Modified:
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/lib/Driver/ToolChains/Cuda.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/CodeGenCUDA/link-device-bitcode.cu
cfe/trunk/test/CodeGenCUDA/propagate-metadata.cu
cfe/trunk/test/Driver/cuda-detect.cu
cfe/trunk/test/Driver/openmp-offload-gpu.c

Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=340193&r1=340192&r2=340193&view=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Mon Aug 20 11:16:48 2018
@@ -287,9 +287,11 @@ def mconstructor_aliases : Flag<["-"], "
   HelpText<"Emit complete constructors and destructors as aliases when 
possible">;
 def mlink_bitcode_file : Separate<["-"], "mlink-bitcode-file">,
   HelpText<"Link the given bitcode file before performing optimizations.">;
-def mlink_cuda_bitcode : Separate<["-"], "mlink-cuda-bitcode">,
+def mlink_builtin_bitcode : Separate<["-"], "mlink-builtin-bitcode">,
   HelpText<"Link and internalize needed symbols from the given bitcode file "
"before performing optimizations.">;
+def mlink_cuda_bitcode : Separate<["-"], "mlink-cuda-bitcode">,
+  Alias;
 def vectorize_loops : Flag<["-"], "vectorize-loops">,
   HelpText<"Run the Loop vectorization passes">;
 def vectorize_slp : Flag<["-"], "vectorize-slp">,

Modified: cfe/trunk/lib/Driver/ToolChains/Cuda.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Cuda.cpp?rev=340193&r1=340192&r2=340193&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Cuda.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Cuda.cpp Mon Aug 20 11:16:48 2018
@@ -621,7 +621,7 @@ void CudaToolChain::addClangTargetOption
 return;
   }
 
-  CC1Args.push_back("-mlink-cuda-bitcode");
+  CC1Args.push_back("-mlink-builtin-bitcode");
   CC1Args.push_back(DriverArgs.MakeArgString(LibDeviceFile));
 
   // Libdevice in CUDA-7.0 requires PTX version that's more recent than LLVM
@@ -667,7 +667,7 @@ void CudaToolChain::addClangTargetOption
   SmallString<128> LibOmpTargetFile(LibraryPath);
   llvm::sys::path::append(LibOmpTargetFile, LibOmpTargetName);
   if (llvm::sys::fs::exists(LibOmpTargetFile)) {
-CC1Args.push_back("-mlink-cuda-bitcode");
+CC1Args.push_back("-mlink-builtin-bitcode");
 CC1Args.push_back(DriverArgs.MakeArgString(LibOmpTargetFile));
 FoundBCLibrary = true;
 break;

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=340193&r1=340192&r2=340193&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Mon Aug 20 11:16:48 2018
@@ -912,10 +912,10 @@ static bool ParseCodeGenArgs(CodeGenOpti
   Opts.RelaxELFRelocations = Args.hasArg(OPT_mrelax_relocations);
   Opts.DebugCompilationDir = Args.getLastArgValue(OPT_fdebug_compilation_dir);
   for (auto *A :
-   Args.filtered(OPT_mlink_bitcode_file, OPT_mlink_cuda_bitcode)) {
+   Args.filtered(OPT_mlink_bitcode_file, OPT_mlink_builtin_bitcode)) {
 CodeGenOptions::BitcodeFileToLink F;
 F.Filename = A->getValue();
-if (A->getOption().matches(OPT_mlink_cuda_bitcode)) {
+if (A->getOption().matches(OPT_mlink_builtin_bitcode)) {
   F.LinkFlags = llvm::Linker::Flags::LinkOnlyNeeded;
   // When linking CUDA bitcode, propagate function attributes so that
   // e.g. libdevice gets fast-math attrs if we're building with fast-math.

Modified: cfe/trunk/test/CodeGenCUDA/link-device-bitcode.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCUDA/link-device-bitcode.cu?rev=340193&r1=340192&r2=340193&view=diff
==
--- cfe/trunk/test/CodeGenCUDA/link-device-bitcode.cu (original)
+++ cfe/trunk/test/CodeGenCUDA/link-device-bitcode.cu Mon Aug 20 11:16:48 2018
@@ -11,13 +11,19 @@
 //
 // Make sure function in device-code gets linked in and internalized.
 // RUN: %clang_cc1 -triple nvptx-unknown-cuda -fcuda-is-device \
+// RUN:-mlink-builtin-bitcode %t.bc  -emit-llvm \
+// RUN:-disable-llvm-passes -o - %s \
+// RUN:| FileCheck %s -check-prefix CHECK-IR
+
+// Make sure legacy flag name works
+// RUN: %clang_cc1 -triple nvptx-unknown-cuda -fcuda-is-d

[PATCH] D50957: Rename -mlink-cuda-bitcode to -mlink-builtin-bitcode

[Matt Arsenault via Phabricator via cfe-commits]

arsenm closed this revision.
arsenm added a comment.

r340193


https://reviews.llvm.org/D50957



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

[PATCH] D50462: Try building complete AST after a fatal error was emitted if further diagnostics are expected

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.
Herald added a subscriber: kadircet.

Thanks, Dmitry, for your explanation. It does help to understand your use case 
better.

What about having a mode that treats missing header as non-fatal error? Because 
I believe there are cases where there is no sense to continue after a fatal 
error. For example, consider hitting the error limit. Showing too many errors 
isn't useful and also after 50 errors it is a little bit too optimistic to 
assume that AST is in a good shape. I don't know if there are other 
fatal-but-we-can-continue errors and if we need a more general solution. So far 
looks like missing include is the biggest blocker for IDE-like functionality.


Repository:
  rC Clang

https://reviews.llvm.org/D50462



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

[PATCH] D50979: Eliminate instances of `EmitScalarExpr(E->getArg(n))` in EmitX86BuiltinExpr().

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

I don't think this is NFC. Testcase:

  long long int a, b, c, d;
  unsigned char f() { return _InterlockedCompareExchange128(&(++a), ++b, ++c, 
&(++d)); }

Today, Clang increments `a`, `b`, `c`, and `d` twice each in `f()`.


https://reviews.llvm.org/D50979



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

[PATCH] D50984: AMDGPU: Move target code into TargetParser

[Matt Arsenault via Phabricator via cfe-commits]

arsenm created this revision.
arsenm added reviewers: rampitec, kzhuravl, yaxunl.
Herald added subscribers: t-tye, tpr, dstuttard, nhaehnle, wdng, jvesely.
arsenm added a dependency: D50983: AMDGPU: Partially move target handling code 
from clang to TargetParser.

https://reviews.llvm.org/D50984

Files:
  lib/Basic/Targets/AMDGPU.cpp
  lib/Basic/Targets/AMDGPU.h

Index: lib/Basic/Targets/AMDGPU.h
===
--- lib/Basic/Targets/AMDGPU.h
+++ lib/Basic/Targets/AMDGPU.h
@@ -19,6 +19,7 @@
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/Compiler.h"
+#include "llvm/Support/TargetParser.h"
 
 namespace clang {
 namespace targets {
@@ -38,160 +39,61 @@
   static const LangASMap AMDGPUDefIsGenMap;
   static const LangASMap AMDGPUDefIsPrivMap;
 
-  /// GPU kinds supported by the AMDGPU target.
-  enum GPUKind : uint32_t {
-// Not specified processor.
-GK_NONE = 0,
-
-// R600-based processors.
-GK_R600,
-GK_R630,
-GK_RS880,
-GK_RV670,
-GK_RV710,
-GK_RV730,
-GK_RV770,
-GK_CEDAR,
-GK_CYPRESS,
-GK_JUNIPER,
-GK_REDWOOD,
-GK_SUMO,
-GK_BARTS,
-GK_CAICOS,
-GK_CAYMAN,
-GK_TURKS,
-
-GK_R600_FIRST = GK_R600,
-GK_R600_LAST = GK_TURKS,
-
-// AMDGCN-based processors.
-GK_GFX600,
-GK_GFX601,
-GK_GFX700,
-GK_GFX701,
-GK_GFX702,
-GK_GFX703,
-GK_GFX704,
-GK_GFX801,
-GK_GFX802,
-GK_GFX803,
-GK_GFX810,
-GK_GFX900,
-GK_GFX902,
-GK_GFX904,
-GK_GFX906,
-
-GK_AMDGCN_FIRST = GK_GFX600,
-GK_AMDGCN_LAST = GK_GFX906,
-  };
+  llvm::AMDGPU::GPUKind GPUKind;
+  unsigned GPUFeatures;
 
-  struct GPUInfo {
-llvm::StringLiteral Name;
-llvm::StringLiteral CanonicalName;
-AMDGPUTargetInfo::GPUKind Kind;
-bool HasFMAF;
-bool HasFastFMAF;
-bool HasLDEXPF;
-bool HasFP64;
-bool HasFastFMA;
-bool HasFullRateF32Denorms;
-  };
 
-  static constexpr GPUInfo InvalidGPU =
-{{""}, {""}, GK_NONE, false, false, false, false, false, false};
-  static constexpr GPUInfo R600GPUs[26] = {
-  // Name CanonicalKindHasHasHasHasHasHas
-  //  Name FMAF   Fast   LDEXPF FP64   Fast   Fast
-  //  FMAF FMADenorm
-{{"r600"},{"r600"},GK_R600,false, false, false, false, false, false},
-{{"rv630"},   {"r600"},GK_R600,false, false, false, false, false, false},
-{{"rv635"},   {"r600"},GK_R600,false, false, false, false, false, false},
-{{"r630"},{"r630"},GK_R630,false, false, false, false, false, false},
-{{"rs780"},   {"rs880"},   GK_RS880,   false, false, false, false, false, false},
-{{"rs880"},   {"rs880"},   GK_RS880,   false, false, false, false, false, false},
-{{"rv610"},   {"rs880"},   GK_RS880,   false, false, false, false, false, false},
-{{"rv620"},   {"rs880"},   GK_RS880,   false, false, false, false, false, false},
-{{"rv670"},   {"rv670"},   GK_RV670,   false, false, false, false, false, false},
-{{"rv710"},   {"rv710"},   GK_RV710,   false, false, false, false, false, false},
-{{"rv730"},   {"rv730"},   GK_RV730,   false, false, false, false, false, false},
-{{"rv740"},   {"rv770"},   GK_RV770,   false, false, false, false, false, false},
-{{"rv770"},   {"rv770"},   GK_RV770,   false, false, false, false, false, false},
-{{"cedar"},   {"cedar"},   GK_CEDAR,   false, false, false, false, false, false},
-{{"palm"},{"cedar"},   GK_CEDAR,   false, false, false, false, false, false},
-{{"cypress"}, {"cypress"}, GK_CYPRESS, true,  false, false, false, false, false},
-{{"hemlock"}, {"cypress"}, GK_CYPRESS, true,  false, false, false, false, false},
-{{"juniper"}, {"juniper"}, GK_JUNIPER, false, false, false, false, false, false},
-{{"redwood"}, {"redwood"}, GK_REDWOOD, false, false, false, false, false, false},
-{{"sumo"},{"sumo"},GK_SUMO,false, false, false, false, false, false},
-{{"sumo2"},   {"sumo"},GK_SUMO,false, false, false, false, false, false},
-{{"barts"},   {"barts"},   GK_BARTS,   false, false, false, false, false, false},
-{{"caicos"},  {"caicos"},  GK_CAICOS,  false, false, false, false, false, false},
-{{"aruba"},   {"cayman"},  GK_CAYMAN,  true,  false, false, false, false, false},
-{{"cayman"},  {"cayman"},  GK_CAYMAN,  true,  false, false, false, false, false},
-{{"turks"},   {"turks"},   GK_TURKS,   false, false, false, false, false, false},
-  };
-  static constexpr GPUInfo AMDGCNGPUs[32] = {
-  // Name   CanonicalKindHas   HasHasHas   Has   Has
-  //Name FMAF  Fast   LDEXPF FP64  Fast  Fast
-  //   FMAFFMA   Denorm
-{{"gfx600"},{"gfx600

[PATCH] D50413: [libunwind][include] Add some missing definitions to .

[Charles Davis via Phabricator via cfe-commits]

cdavis5x added a comment.

Ping.

Are y'all waiting for me to do something?


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50413



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