r340251 - Removed unused variable [NFC]

2018-08-21 Thread Mikael Holmen via cfe-commits
Author: uabelho
Date: Tue Aug 21 00:22:45 2018
New Revision: 340251

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

The compiler warned:
../tools/clang/lib/Sema/SemaType.cpp:6788:31: error: unused variable 'AT' 
[-Werror,-Wunused-variable]
if (const AttributedType *AT = S.getCallingConvAttributedType(type)) {
  ^
1 error generated.

Modified:
cfe/trunk/lib/Sema/SemaType.cpp

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=340251&r1=340250&r2=340251&view=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Tue Aug 21 00:22:45 2018
@@ -6785,7 +6785,7 @@ static bool handleFunctionTypeAttr(TypeP
   if (CCOld != CC) {
 // Error out on when there's already an attribute on the type
 // and the CCs don't match.
-if (const AttributedType *AT = S.getCallingConvAttributedType(type)) {
+if (S.getCallingConvAttributedType(type)) {
   S.Diag(attr.getLoc(), diag::err_attributes_are_not_compatible)
 << FunctionType::getNameForCallConv(CC)
 << FunctionType::getNameForCallConv(CCOld);


___
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.

2018-08-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 161651.
ioeric marked an inline comment as done.
ioeric added a comment.

- Improve tests.


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
  unittests/clangd/IndexTests.cpp

Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -321,6 +321,21 @@
   EXPECT_EQ(M.Name, "right");
 }
 
+TEST(AsyncFuzzyFind, Simple) {
+  MemIndex Idx;
+  Idx.build(generateSymbols({"ns::abc", "ns::xyz"}));
+
+  FuzzyFindRequest Req;
+  Req.Query = "";
+  Req.Scopes = {"ns::"};
+
+  AsyncFuzzyFind Async(Idx, Req);
+
+  EXPECT_EQ(Async.getRequest(), Req);
+  EXPECT_THAT(Async.getResult(),
+  UnorderedElementsAre(Named("abc"), Named("xyz")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -923,7 +923,11 @@
llvm::function_ref
Callback) const override {}
 
-  const std::vector allRequests() const { return Requests; }
+  const std::vector consumeRequests() const {
+auto Reqs = std::move(Requests);
+Requests.clear();
+return Reqs;
+  }
 
 private:
   mutable std::vector Requests;
@@ -934,7 +938,7 @@
   IndexRequestCollector Requests;
   Opts.Index = &Requests;
   completions(Code, {}, Opts);
-  return Requests.allRequests();
+  return Requests.consumeRequests();
 }
 
 TEST(CompletionTest, UnqualifiedIdQuery) {
@@ -1700,6 +1704,75 @@
   }
 }
 
+TEST(SpeculateCompletionFilter, Filters) {
+  Annotations F(R"cpp($bof^
+  $bol^
+  ab$ab^
+  x.ab$dot^
+  x.$dotempty^
+  x::ab$scoped^
+  x::$scopedempty^
+
+  )cpp");
+  auto speculate = [&](StringRef PointName) {
+auto Filter = speculateCompletionFilter(F.code(), F.point(PointName));
+assert(Filter);
+return *Filter;
+  };
+  EXPECT_EQ(speculate("bof"), "");
+  EXPECT_EQ(speculate("bol"), "");
+  EXPECT_EQ(speculate("ab"), "ab");
+  EXPECT_EQ(speculate("dot"), "ab");
+  EXPECT_EQ(speculate("dotempty"), "");
+  EXPECT_EQ(speculate("scoped"), "ab");
+  EXPECT_EQ(speculate("scopedempty"), "");
+}
+
+TEST(CompletionTest, EnableSpeculativeIndexRequest) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto File = testPath("foo.cpp");
+  Annotations Test(R"cpp(
+  namespace ns1 { int abc; }
+  namespace ns2 { int abc; }
+  void f() { ns1::ab$1^; ns1::ab$2^; }
+  void f() { ns2::ab$3^; }
+  )cpp");
+  runAddDocument(Server, File, Test.code());
+  clangd::CodeCompleteOptions Opts = {};
+
+  IndexRequestCollector Requests;
+  Opts.Index = &Requests;
+  Opts.SpeculativeIndexRequest = true;
+
+  auto CompleteAtPoint = [&](StringRef P) {
+cantFail(runCodeComplete(Server, File, Test.point(P), Opts));
+// Sleep for a while to make sure asynchronous call (if applicable) is also
+// triggered before callback is invoked.
+std::this_thread::sleep_for(std::chrono::milliseconds(100));
+  };
+
+  CompleteAtPoint("1");
+  auto Reqs1 = Requests.consumeRequests();
+  ASSERT_EQ(Reqs1.size(), 1u);
+  EXPECT_THAT(Reqs1[0].Scopes, UnorderedElementsAre("ns1::"));
+
+  CompleteAtPoint("2");
+  auto Reqs2 = Requests.consumeRequests();
+  // Speculation succeeded. Used speculative index result.
+  ASSERT_EQ(Reqs2.size(), 1u);
+  EXPECT_EQ(Reqs2[0], Reqs1[0]);
+
+  CompleteAtPoint("3");
+  // Speculation failed. Sent speculative index request and the new index
+  // request after sema.
+  auto Reqs3 = Requests.consumeRequests();
+  ASSERT_EQ(Reqs3.size(), 2u);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -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 reque

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

2018-08-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments.



Comment at: unittests/clangd/CodeCompleteTests.cpp:1710
+  $bol^
+  ab$ab^
+  x.ab$dot^

kadircet wrote:
> Maybe an EXPECT for this one as well?
Nice catch!


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] D51026: [CodeGen] Implicitly set stackrealign on the main function, if custom stack alignment is used

2018-08-21 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo created this revision.
mstorsjo added reviewers: craig.topper, erichkeane, rnk.

If using a custom stack alignment, one is expected to make sure that all 
callers provide such alignment, or realign the stack in all entry points (and 
callbacks).

Despite this, the compiler can assume that the main function will need 
realignment in these cases, since the startup routines calling the main 
function most probably won't provide the custom alignment.

This matches what GCC does in similar cases; if compiling with 
-mincoming-stack-boundary=X -mpreferred-stack-boundary=X, GCC normally assumes 
such alignment on entry to a function, but specifically for the main function 
still does realignment.


Repository:
  rC Clang

https://reviews.llvm.org/D51026

Files:
  lib/CodeGen/CodeGenFunction.cpp
  test/CodeGen/stackrealign-main.c


Index: test/CodeGen/stackrealign-main.c
===
--- /dev/null
+++ test/CodeGen/stackrealign-main.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm -o - 
-mstack-alignment=64 %s | FileCheck %s
+
+// CHECK-LABEL: define void @other()
+// CHECK: [[OTHER:#[0-9]+]]
+// CHECK: {
+void other(void) {}
+
+// CHECK-LABEL: define i32 @main(
+// CHECK: [[MAIN:#[0-9]+]]
+// CHECK: {
+int main(int argc, char **argv) {
+  other();
+  return 0;
+}
+
+// CHECK: attributes [[OTHER]] = { noinline nounwind optnone
+// CHECK-NOT: "stackrealign"
+// CHECK: }
+// CHECK: attributes [[MAIN]] = { noinline nounwind optnone 
{{.*}}"stackrealign"{{.*}} }
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -981,6 +981,13 @@
   if (FD->isMain())
 Fn->addFnAttr(llvm::Attribute::NoRecurse);
 
+  // If a custom alignment is used, force realigning to this alignment on
+  // any main function which certainly will need it.
+  if (const FunctionDecl *FD = dyn_cast_or_null(D))
+if ((FD->isMain() || FD->isMSVCRTEntryPoint()) &&
+CGM.getCodeGenOpts().StackAlignment)
+  Fn->addFnAttr("stackrealign");
+
   llvm::BasicBlock *EntryBB = createBasicBlock("entry", CurFn);
 
   // Create a marker to make it easy to insert allocas into the entryblock


Index: test/CodeGen/stackrealign-main.c
===
--- /dev/null
+++ test/CodeGen/stackrealign-main.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm -o - -mstack-alignment=64 %s | FileCheck %s
+
+// CHECK-LABEL: define void @other()
+// CHECK: [[OTHER:#[0-9]+]]
+// CHECK: {
+void other(void) {}
+
+// CHECK-LABEL: define i32 @main(
+// CHECK: [[MAIN:#[0-9]+]]
+// CHECK: {
+int main(int argc, char **argv) {
+  other();
+  return 0;
+}
+
+// CHECK: attributes [[OTHER]] = { noinline nounwind optnone
+// CHECK-NOT: "stackrealign"
+// CHECK: }
+// CHECK: attributes [[MAIN]] = { noinline nounwind optnone {{.*}}"stackrealign"{{.*}} }
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -981,6 +981,13 @@
   if (FD->isMain())
 Fn->addFnAttr(llvm::Attribute::NoRecurse);
 
+  // If a custom alignment is used, force realigning to this alignment on
+  // any main function which certainly will need it.
+  if (const FunctionDecl *FD = dyn_cast_or_null(D))
+if ((FD->isMain() || FD->isMSVCRTEntryPoint()) &&
+CGM.getCodeGenOpts().StackAlignment)
+  Fn->addFnAttr("stackrealign");
+
   llvm::BasicBlock *EntryBB = createBasicBlock("entry", CurFn);
 
   // Create a marker to make it easy to insert allocas into the entryblock
___
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

2018-08-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments.



Comment at: lib/Basic/SourceManager.cpp:1705
 
   // If we haven't found what we want yet, try again, but this time stat()
   // each of the files in case the files have changed since we originally

Do we also need this in `findFileIDsForFile`?



Comment at: unittests/Basic/SourceManagerTest.cpp:380
 
+TEST_F(SourceManagerTest, findFileIDsForFile) {
+  const char *header = "int x;";

Could you add a test case for getting file ID for main file, just to make sure 
we also covered cases handled by `if (MainFileID.isValid()) {...}`  code path 
in `translateFile()`?


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] D50580: [clang-tidy] Abseil: no namespace check

2018-08-21 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tidy/abseil/AbseilMatcher.h:16
+
+/// Matches AST nodes that were expanded within abseil-header-files.
+AST_POLYMORPHIC_MATCHER(isExpansionNotInAbseilHeader,

nit: We need proper documentation for this matcher, since it is exposed to 
users.



Comment at: clang-tidy/abseil/AbseilMatcher.h:17
+/// Matches AST nodes that were expanded within abseil-header-files.
+AST_POLYMORPHIC_MATCHER(isExpansionNotInAbseilHeader,
+AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc,

- using negative for AST matcher seems uncommon in ASTMatchers, for negative we 
have `unless`, so I'd suggest implementing `isInAbseilHeader`.
- it seems that this matcher is very similar to the matcher in 
https://reviews.llvm.org/D50542.  I think `isInAbseilFile` should also cover 
your case here, and also ignore the warning if you run the check on abseil core 
files.



Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+  auto Filename = FileEntry->getName();
+  llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
+ "synchronization|types|utiliy)");

The regex seems incomplete, we are missing `algorithm`, `time` subdirectory. 



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

typo: `utility`



Comment at: clang-tidy/abseil/AbseilMatcher.h:34
+ "synchronization|types|utiliy)");
+  return (!RE.match(Filename));
+}

nit: remove the outer `()`. 


https://reviews.llvm.org/D50580



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


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

2018-08-21 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



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

ioeric wrote:
> 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 
I am for splitting cancellation checks from cancellation triggers.
The processing code only needs to check if it was cancelled and exposing the 
`cancel()` does not add any useful functionality, merely ways to misuse it.


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] D50502: [clangd] Initial cancellation mechanism for LSP requests.

2018-08-21 Thread 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:

ilya-biryukov wrote:
> ioeric wrote:
> > 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 
> I am for splitting cancellation checks from cancellation triggers.
> The processing code only needs to check if it was cancelled and exposing the 
> `cancel()` does not add any useful functionality, merely ways to misuse it.
Couldn't we prevent this by passing only const handle references to users who 
are not expect to call `cancel()`?


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] D50580: [clang-tidy] Abseil: no namespace check

2018-08-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+  auto Filename = FileEntry->getName();
+  llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
+ "synchronization|types|utiliy)");

hokein wrote:
> The regex seems incomplete, we are missing `algorithm`, `time` subdirectory. 
So what happens if open the namespace in a file that is located in my personal 
`absl/base` directory?
It will be false-negatively not reported?


https://reviews.llvm.org/D50580



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


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

2018-08-21 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

The patch looks good. I'll commit for you once @JonasToth has no further 
comments.


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] D50580: [clang-tidy] Abseil: no namespace check

2018-08-21 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+  auto Filename = FileEntry->getName();
+  llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
+ "synchronization|types|utiliy)");

lebedev.ri wrote:
> hokein wrote:
> > The regex seems incomplete, we are missing `algorithm`, `time` 
> > subdirectory. 
> So what happens if open the namespace in a file that is located in my 
> personal `absl/base` directory?
> It will be false-negatively not reported?
 Yes, I'd say this is a known limitation.


https://reviews.llvm.org/D50580



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


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

2018-08-21 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a subscriber: deannagarcia.
hokein added a comment.

Your patch seems have some comments unaddressed but you marked done.

About the abseil matcher stuff, you and @deannagarcia have an overlap, maybe 
just wait for https://reviews.llvm.org/D50580 to submit, and reuse the matcher 
in this patch?




Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:24
+  auto &SourceManager = Finder->getASTContext().getSourceManager();
+  SourceLocation loc = Node.getBeginLoc();
+  if (loc.isInvalid())

nit: loc => Loc


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] D50580: [clang-tidy] Abseil: no namespace check

2018-08-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+  auto Filename = FileEntry->getName();
+  llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
+ "synchronization|types|utiliy)");

hokein wrote:
> lebedev.ri wrote:
> > hokein wrote:
> > > The regex seems incomplete, we are missing `algorithm`, `time` 
> > > subdirectory. 
> > So what happens if open the namespace in a file that is located in my 
> > personal `absl/base` directory?
> > It will be false-negatively not reported?
>  Yes, I'd say this is a known limitation.
Similarly, the check will break (start producing false-positives) as soon as a 
new directory is added to abseil proper.
I don't have any reliable idea on how to do this better, but the current 
solution seems rather suboptimal..


https://reviews.llvm.org/D50580



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


[PATCH] D50814: [clangd] transfer the fixits with the notes instead of adding them to the main diagnostic if request by the client

2018-08-21 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.
Herald added a subscriber: kadircet.

Just to make sure I fully understand the use-case: could you elaborate a little 
more? Do you need to get exactly the same set of notes that clang provides?
Our current model seems to follow the clang's model, but it removes some notes 
and shows them as fix-its for the main diagnostic instead.
(We think it should provide a better user experience, because those notes do 
look like a way to present a fix-it to the user when you have only text-based 
output and they seem redundant when one has an editor-based UI with fix-its).

Not opposed to changing the model to meet other needs, but want to make sure 
what we're currently doing in clangd makes sense and understand your use-case 
better.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50814



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


[PATCH] D50703: [clangd] NFC: Mark Workspace Symbol feature complete in the documentation

2018-08-21 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

In https://reviews.llvm.org/D50703#1205167, @malaperle wrote:

> Nah, I guess other things are partial too, like Go to Definition. Let's 
> assume they will be completed "soon" ;)


https://reviews.llvm.org/D50889 should finally add the symbols from the main 
files. Sorry for the delays.


Repository:
  rL LLVM

https://reviews.llvm.org/D50703



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


[PATCH] D51029: [clangd] Implement LIMIT iterator

2018-08-21 Thread 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/D51029

Files:
  clang-tools-extra/clangd/index/dex/DexIndex.cpp
  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
@@ -62,7 +62,7 @@
   auto AndWithEmpty = createAnd(create(L0), create(L1));
   EXPECT_TRUE(AndWithEmpty->reachedEnd());
 
-  EXPECT_THAT(consume(*AndWithEmpty), ElementsAre());
+  EXPECT_THAT(consume(move(AndWithEmpty)), ElementsAre());
 }
 
 TEST(DexIndexIterators, AndTwoLists) {
@@ -72,7 +72,7 @@
   auto And = createAnd(create(L1), create(L0));
 
   EXPECT_FALSE(And->reachedEnd());
-  EXPECT_THAT(consume(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U));
+  EXPECT_THAT(consume(move(And)), ElementsAre(0U, 7U, 10U, 320U, 9000U));
 
   And = createAnd(create(L0), create(L1));
 
@@ -113,7 +113,7 @@
   auto OrWithEmpty = createOr(create(L0), create(L1));
   EXPECT_FALSE(OrWithEmpty->reachedEnd());
 
-  EXPECT_THAT(consume(*OrWithEmpty),
+  EXPECT_THAT(consume(move(OrWithEmpty)),
   ElementsAre(0U, 5U, 7U, 10U, 42U, 320U, 9000U));
 }
 
@@ -146,7 +146,7 @@
 
   Or = createOr(create(L0), create(L1));
 
-  EXPECT_THAT(consume(*Or),
+  EXPECT_THAT(consume(move(Or)),
   ElementsAre(0U, 4U, 5U, 7U, 10U, 30U, 42U, 60U, 320U, 9000U));
 }
 
@@ -256,29 +256,29 @@
   const PostingList L5;
 
   auto DocIterator = create(L0);
-  EXPECT_THAT(consume(*DocIterator, 42), ElementsAre(4, 7, 8, 20, 42, 100));
+  EXPECT_THAT(consume(move(DocIterator), 42), ElementsAre(4, 7, 8, 20, 42, 100));
 
   DocIterator = create(L0);
-  EXPECT_THAT(consume(*DocIterator), ElementsAre(4, 7, 8, 20, 42, 100));
+  EXPECT_THAT(consume(move(DocIterator)), ElementsAre(4, 7, 8, 20, 42, 100));
 
   DocIterator = create(L0);
-  EXPECT_THAT(consume(*DocIterator, 3), ElementsAre(4, 7, 8));
+  EXPECT_THAT(consume(move(DocIterator), 3), ElementsAre(4, 7, 8));
 
   DocIterator = create(L0);
-  EXPECT_THAT(consume(*DocIterator, 0), ElementsAre());
+  EXPECT_THAT(consume(move(DocIterator), 0), ElementsAre());
 }
 
 TEST(DexIndexIterators, True) {
   auto TrueIterator = createTrue(0U);
   EXPECT_TRUE(TrueIterator->reachedEnd());
-  EXPECT_THAT(consume(*TrueIterator), ElementsAre());
+  EXPECT_THAT(consume(move(TrueIterator)), ElementsAre());
 
   PostingList L0 = {1, 2, 5, 7};
   TrueIterator = createTrue(7U);
   EXPECT_THAT(TrueIterator->peek(), 0);
   auto AndIterator = createAnd(create(L0), move(TrueIterator));
   EXPECT_FALSE(AndIterator->reachedEnd());
-  EXPECT_THAT(consume(*AndIterator), ElementsAre(1, 2, 5));
+  EXPECT_THAT(consume(move(AndIterator)), ElementsAre(1, 2, 5));
 }
 
 testing::Matcher>
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,13 @@
   ///
   /// Note: reachedEnd() must be false.
   virtual DocID peek() const = 0;
+  /// Marks an element as "consumed" and triggers limit decrement for every
+  /// LIMIT iterator which yields given item. Root iterator should call
+  /// consume(peek) as the argument is used to propagate the tree, otherwise
+  /// the behavior will be incorrect.
+  virtual void consume(DocID ID) = 0;
+  /// Returns true if the iterator can retrieve more items upon request.
+  virtual bool canRetrieveMore() const { return !reachedEnd(); }
 
   virtual ~Iterator() {}
 
@@ -113,7 +120,7 @@
 
 /// Advances the iterator until it is either exhausted or the number of
 /// requested items is reached. The result contains sorted DocumentIDs.
-std::vector consume(Iterator &It,
+std::vector consume(std::unique_ptr It,
size_t Limit = std::numeric_limits::max());
 
 /// Returns a document iterator over given PostingList.
@@ -133,6 +140,11 @@
 /// all items in range [0, Size) in an efficient manner.
 std::unique_ptr createTrue(DocID Size);
 
+/// Returns LIMIT iterator, which ...
+// FIXME(kbobyrev): Add docs.
+std::unique_ptr
+createLimit(std::unique_ptr Child, 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
@@ -46,6 +46,8 @@
 return *Index;
   }
 
+  void consume (DocID ID) override {}
+
 private:
   llvm::raw

[PATCH] D51029: [clangd] Implement LIMIT iterator

2018-08-21 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev planned changes to this revision.
kbobyrev added a comment.

Patch is in the preview mode, I will add documentation and more tests before 
opening a review.


https://reviews.llvm.org/D51029



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


[PATCH] D51029: [clangd] Implement LIMIT iterator

2018-08-21 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 161677.
kbobyrev added a comment.

Add comprehensive tests, improve documentation.


https://reviews.llvm.org/D51029

Files:
  clang-tools-extra/clangd/index/dex/DexIndex.cpp
  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
@@ -62,7 +62,7 @@
   auto AndWithEmpty = createAnd(create(L0), create(L1));
   EXPECT_TRUE(AndWithEmpty->reachedEnd());
 
-  EXPECT_THAT(consume(*AndWithEmpty), ElementsAre());
+  EXPECT_THAT(consume(move(AndWithEmpty)), ElementsAre());
 }
 
 TEST(DexIndexIterators, AndTwoLists) {
@@ -72,7 +72,7 @@
   auto And = createAnd(create(L1), create(L0));
 
   EXPECT_FALSE(And->reachedEnd());
-  EXPECT_THAT(consume(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U));
+  EXPECT_THAT(consume(move(And)), ElementsAre(0U, 7U, 10U, 320U, 9000U));
 
   And = createAnd(create(L0), create(L1));
 
@@ -113,7 +113,7 @@
   auto OrWithEmpty = createOr(create(L0), create(L1));
   EXPECT_FALSE(OrWithEmpty->reachedEnd());
 
-  EXPECT_THAT(consume(*OrWithEmpty),
+  EXPECT_THAT(consume(move(OrWithEmpty)),
   ElementsAre(0U, 5U, 7U, 10U, 42U, 320U, 9000U));
 }
 
@@ -146,7 +146,7 @@
 
   Or = createOr(create(L0), create(L1));
 
-  EXPECT_THAT(consume(*Or),
+  EXPECT_THAT(consume(move(Or)),
   ElementsAre(0U, 4U, 5U, 7U, 10U, 30U, 42U, 60U, 320U, 9000U));
 }
 
@@ -248,37 +248,40 @@
 }
 
 TEST(DexIndexIterators, Limit) {
-  const PostingList L0 = {4, 7, 8, 20, 42, 100};
-  const PostingList L1 = {1, 3, 5, 8, 9};
-  const PostingList L2 = {1, 5, 7, 9};
-  const PostingList L3 = {0, 5};
-  const PostingList L4 = {0, 1, 5};
-  const PostingList L5;
+  const PostingList L0 = {3, 6, 7, 20, 42, 100};
+  const PostingList L1 = {1, 3, 5, 6, 7, 30, 100};
+  const PostingList L2 = {0, 3, 5, 7, 8, 100};
 
   auto DocIterator = create(L0);
-  EXPECT_THAT(consume(*DocIterator, 42), ElementsAre(4, 7, 8, 20, 42, 100));
+  EXPECT_THAT(consume(move(DocIterator), 42),
+  ElementsAre(3, 6, 7, 20, 42, 100));
 
   DocIterator = create(L0);
-  EXPECT_THAT(consume(*DocIterator), ElementsAre(4, 7, 8, 20, 42, 100));
+  EXPECT_THAT(consume(move(DocIterator)), ElementsAre(3, 6, 7, 20, 42, 100));
 
   DocIterator = create(L0);
-  EXPECT_THAT(consume(*DocIterator, 3), ElementsAre(4, 7, 8));
+  EXPECT_THAT(consume(move(DocIterator), 3), ElementsAre(3, 6, 7));
 
   DocIterator = create(L0);
-  EXPECT_THAT(consume(*DocIterator, 0), ElementsAre());
+  EXPECT_THAT(consume(move(DocIterator), 0), ElementsAre());
+
+  auto AndIterator =
+  createAnd(createLimit(createTrue(9000), 343), createLimit(create(L0), 2),
+createLimit(create(L1), 3), createLimit(create(L2), 42));
+  EXPECT_THAT(consume(move(AndIterator)), ElementsAre(3, 7));
 }
 
 TEST(DexIndexIterators, True) {
   auto TrueIterator = createTrue(0U);
   EXPECT_TRUE(TrueIterator->reachedEnd());
-  EXPECT_THAT(consume(*TrueIterator), ElementsAre());
+  EXPECT_THAT(consume(move(TrueIterator)), ElementsAre());
 
   PostingList L0 = {1, 2, 5, 7};
   TrueIterator = createTrue(7U);
   EXPECT_THAT(TrueIterator->peek(), 0);
   auto AndIterator = createAnd(create(L0), move(TrueIterator));
   EXPECT_FALSE(AndIterator->reachedEnd());
-  EXPECT_THAT(consume(*AndIterator), ElementsAre(1, 2, 5));
+  EXPECT_THAT(consume(move(AndIterator)), ElementsAre(1, 2, 5));
 }
 
 testing::Matcher>
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,13 @@
   ///
   /// Note: reachedEnd() must be false.
   virtual DocID peek() const = 0;
+  /// Marks an element as "consumed" and triggers limit decrement for every
+  /// LIMIT iterator which yields given item. Root iterator should call
+  /// consume(peek) as the argument is used to propagate the tree, otherwise
+  /// the behavior will be incorrect.
+  virtual void consume(DocID ID) = 0;
+  /// Returns true if the iterator can retrieve more items upon request.
+  virtual bool canRetrieveMore() const { return !reachedEnd(); }
 
   virtual ~Iterator() {}
 
@@ -113,7 +120,7 @@
 
 /// Advances the iterator until it is either exhausted or the number of
 /// requested items is reached. The result contains sorted DocumentIDs.
-std::vector consume(Iterator &It,
+std::vector consume(std::unique_ptr It,
size_t Limit = std::numeric_limits::max());
 
 /// Returns a document iterator over given PostingList.
@@ -133,6 +140,11 @@
 /// all items in range [0, Size) in an efficient manner.
 std::unique_ptr createTrue(DocID Size);
 
+/// Returns LIMIT iterator, 

[PATCH] D51029: [clangd] Implement LIMIT iterator

2018-08-21 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev planned changes to this revision.
kbobyrev added a comment.

Since it's bundled with the BOOST iterator and doesn't make too much sense 
without it, I should probable rebase on top of https://reviews.llvm.org/D50970 
and add it as the parent revision.


https://reviews.llvm.org/D51029



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


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

2018-08-21 Thread Stanislav Mekhanoshin via Phabricator via cfe-commits
rampitec accepted this revision.
rampitec added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D50984



___
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

2018-08-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments.



Comment at: clang-tools-extra/clangd/index/MemIndex.h:50
 
+// FIXME(kbobyrev): Document this one.
+std::shared_ptr>

Please add documentation.



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:96
 
+namespace {
+

Why the move? A chunk of code in the middle of flags seems strange.


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

2018-08-21 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 161679.
kbobyrev marked 2 inline comments as done.
kbobyrev added a comment.

Aww, the previous diff was the wrong one and didn't contain docs.

The move of the code to the middle of Clangd driver was justified by the 
assumption that it might be better to move the code that uses `UseDex` flag 
closer to the flag itself, but I agree: it looks strange. I moved it back.


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"
@@ -29,6 +30,7 @@
 using namespace clang::clangd;
 
 namespace {
+
 enum class PCHStorageFlag { Disk, Memory };
 
 // Build an in-memory static index for global symbols from a YAML-format file.
@@ -45,8 +47,10 @@
   for (auto Sym : Slab)
 SymsBuilder.insert(Sym);
 
-  return MemIndex::build(std::move(SymsBuilder).build());
+  return UseDex ? DexIndex::build(std::move(SymsBuilder).build())
+: MemIndex::build(std::move(SymsBuilder).build());
 }
+
 } // namespace
 
 static llvm::cl::opt CompileCommandsDir(
@@ -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,11 @@
   mutable std::mutex Mutex;
 };
 
+// Returns pointers to the symbols in given slab and bundles slab lifetime with
+// returned symbol pointers so that the pointers are never invalid.
+std::shared_ptr>
+getSymbolsFromSlab(SymbolSlab Slab);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/index/MemIndex.cpp
===
--- clang-tools-extra/clangd/index/MemIndex.cpp
+++ clang-tools-extra/clangd/index/MemIndex.cpp
@@ -28,6 +28,12 @@
   }
 }
 
+std::unique_ptr MemIndex::build(SymbolSlab Slab) {
+  auto Idx = llvm::make_unique();
+  Idx->build(getSymbolsFromSlab(std::move(Slab)));
+  return std::move(Idx);
+}
+
 bool MemIndex::fuzzyFind(
 const FuzzyFindRequest &Req,
 llvm::function_ref Callback) const {
@@ -72,26 +78,24 @@
   }
 }
 
-std::unique_ptr MemIndex::build(SymbolSlab Slab) {
+void MemIndex::findOccurrences(
+const OccurrencesRequest &Req,
+llvm::function_ref Callback) const {
+  log("findOccurrences is not implemented.");
+}
+
+std::shared_ptr>
+getSymbolsFromSlab(SymbolSlab Slab) {
   struct Snapshot {
 SymbolSlab Slab;
 std::vector Pointers;
   };
   auto Snap = std::make_shared();
   Snap->Slab = std::move(Slab);
   for (auto &Sym : Snap->Slab)
 Snap->Pointers.push_back(&Sym);
-  auto S = std::shared_ptr>(std::move(Snap),
-&Snap->Pointers);
-  auto MemIdx = llvm::make_uni

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

2018-08-21 Thread 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/D50897



___
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.

2018-08-21 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



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

ioeric wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > 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 
> > I am for splitting cancellation checks from cancellation triggers.
> > The processing code only needs to check if it was cancelled and exposing 
> > the `cancel()` does not add any useful functionality, merely ways to misuse 
> > it.
> Couldn't we prevent this by passing only const handle references to users who 
> are not expect to call `cancel()`?
As long as `TaskHandle` does not have a copy constructor, that LG.
I.e. the scheme that seems to work is:
1. return `shared_ptr` from async computations like code complete.
2. stash `shared_ptr` into the context.
3. return `const TaskHandle*` from the context.

This should give a simpler implementation.
The cons from my point of view are:
1. We return copyable task handles (i.e. the shared_ptr) from the API by 
default. This provokes usages that don't take ownership issues into account, 
i.e. it's easy to just copy the task everywhere without having clear ownership 
of it. As long as it's just cancellation there, we're probably fine, but if we 
add more stuff into it that might become a problem.
2. We expose implementation details (i.e. shared_ptr) by removing the layer of 
abstraction. This makes potential refactorings of the client code harder.

In general, I think having our own wrappers there opens up more room for 
experiments with the API later (the API is very constrained, so it's easy to 
refactor the implementation).  OTOH, if we believe the design is right or 
changing the clients is not too much work (which is probably true), either way 
is fine with me.

But either solution has its pros and cons, I'm happy with trying out any of 
those on code completion, see how it goes and decide whether we want to change 
anything before finalizing our design and adding cancellation to all operations 
in clangd.


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

2018-08-21 Thread Kirill Bobyrev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL340262: [clangd] Allow using experimental Dex index 
(authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50897?vs=161679&id=161682#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50897

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

Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/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"
@@ -29,6 +30,7 @@
 using namespace clang::clangd;
 
 namespace {
+
 enum class PCHStorageFlag { Disk, Memory };
 
 // Build an in-memory static index for global symbols from a YAML-format file.
@@ -45,8 +47,10 @@
   for (auto Sym : Slab)
 SymsBuilder.insert(Sym);
 
-  return MemIndex::build(std::move(SymsBuilder).build());
+  return UseDex ? DexIndex::build(std::move(SymsBuilder).build())
+: MemIndex::build(std::move(SymsBuilder).build());
 }
+
 } // namespace
 
 static llvm::cl::opt CompileCommandsDir(
@@ -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/trunk/clangd/index/MemIndex.h
===
--- clang-tools-extra/trunk/clangd/index/MemIndex.h
+++ clang-tools-extra/trunk/clangd/index/MemIndex.h
@@ -47,6 +47,11 @@
   mutable std::mutex Mutex;
 };
 
+// Returns pointers to the symbols in given slab and bundles slab lifetime with
+// returned symbol pointers so that the pointers are never invalid.
+std::shared_ptr>
+getSymbolsFromSlab(SymbolSlab Slab);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/trunk/clangd/index/MemIndex.cpp
===
--- clang-tools-extra/trunk/clangd/index/MemIndex.cpp
+++ clang-tools-extra/trunk/clangd/index/MemIndex.cpp
@@ -28,6 +28,12 @@
   }
 }
 
+std::unique_ptr MemIndex::build(SymbolSlab Slab) {
+  auto Idx = llvm::make_unique();
+  Idx->build(getSymbolsFromSlab(std::move(Slab)));
+  return std::move(Idx);
+}
+
 bool MemIndex::fuzzyFind(
 const FuzzyFindRequest &Req,
 llvm::function_ref Callback) const {
@@ -72,26 +78,24 @@
   }
 }
 
-std::unique_ptr MemIndex::build(SymbolSlab Slab) {
+void MemIndex::findOccurrences(
+const OccurrencesRequest &Req,
+llvm::function_ref Callback) const {
+  log("findOccurrences is not implemented.");
+}
+
+std::shared_ptr>
+getSymbolsFromSlab(SymbolSlab Slab) {
   struct Snapshot {
 SymbolSlab Slab;
 std::vector Pointers;
   };
   auto Snap = std::make_shared();
   Snap->Slab = std::move(Slab);
   for (auto &Sym : Snap->Slab)
 Snap->Pointers.push_back(&Sym);
-  auto S = std::shared_ptr>(std::move(Snap),
-&Snap->Pointers);
-  auto MemIdx = llvm::make_unique();
-  MemIdx->build(std::move(S));
-  return std::move(MemIdx);
-}
-
-void MemIndex::findOccurrences(
-const OccurrencesRequest &Req,
-llvm::function_ref Callback) const {
-  log("findOccurrences is not implemented.");
+  return std::shared_ptr>(std::move(Snap),
+  &Snap->Pointers);
 }
 
 } // namespace clangd
Index: clang-tools-extra/trunk/clangd/index/dex/DexIndex.h
===
--- clang-tools-extra/trunk/clangd/index/dex/DexIndex.h
+++ clang-tools-extra/trunk/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/trunk/clangd/index/dex/DexIndex.cpp
===
--- clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp
+++ clang-tools-extra/trunk

[clang-tools-extra] r340262 - [clangd] Allow using experimental Dex index

2018-08-21 Thread Kirill Bobyrev via cfe-commits
Author: omtcyfz
Date: Tue Aug 21 03:32:27 2018
New Revision: 340262

URL: http://llvm.org/viewvc/llvm-project?rev=340262&view=rev
Log:
[clangd] Allow using experimental Dex index

This patch adds hidden Clangd flag ("use-dex-index") which replaces
(currently) default `MemIndex` with `DexIndex` for the static index.

Reviewed by: ioeric

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

Modified:
clang-tools-extra/trunk/clangd/index/MemIndex.cpp
clang-tools-extra/trunk/clangd/index/MemIndex.h
clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp
clang-tools-extra/trunk/clangd/index/dex/DexIndex.h
clang-tools-extra/trunk/clangd/tool/ClangdMain.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=340262&r1=340261&r2=340262&view=diff
==
--- clang-tools-extra/trunk/clangd/index/MemIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/MemIndex.cpp Tue Aug 21 03:32:27 2018
@@ -28,6 +28,12 @@ void MemIndex::build(std::shared_ptr MemIndex::build(SymbolSlab Slab) {
+  auto Idx = llvm::make_unique();
+  Idx->build(getSymbolsFromSlab(std::move(Slab)));
+  return std::move(Idx);
+}
+
 bool MemIndex::fuzzyFind(
 const FuzzyFindRequest &Req,
 llvm::function_ref Callback) const {
@@ -72,7 +78,14 @@ void MemIndex::lookup(const LookupReques
   }
 }
 
-std::unique_ptr MemIndex::build(SymbolSlab Slab) {
+void MemIndex::findOccurrences(
+const OccurrencesRequest &Req,
+llvm::function_ref Callback) const {
+  log("findOccurrences is not implemented.");
+}
+
+std::shared_ptr>
+getSymbolsFromSlab(SymbolSlab Slab) {
   struct Snapshot {
 SymbolSlab Slab;
 std::vector Pointers;
@@ -81,17 +94,8 @@ std::unique_ptr MemIndex::b
   Snap->Slab = std::move(Slab);
   for (auto &Sym : Snap->Slab)
 Snap->Pointers.push_back(&Sym);
-  auto S = std::shared_ptr>(std::move(Snap),
-&Snap->Pointers);
-  auto MemIdx = llvm::make_unique();
-  MemIdx->build(std::move(S));
-  return std::move(MemIdx);
-}
-
-void MemIndex::findOccurrences(
-const OccurrencesRequest &Req,
-llvm::function_ref Callback) const {
-  log("findOccurrences is not implemented.");
+  return std::shared_ptr>(std::move(Snap),
+  &Snap->Pointers);
 }
 
 } // namespace clangd

Modified: clang-tools-extra/trunk/clangd/index/MemIndex.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/MemIndex.h?rev=340262&r1=340261&r2=340262&view=diff
==
--- clang-tools-extra/trunk/clangd/index/MemIndex.h (original)
+++ clang-tools-extra/trunk/clangd/index/MemIndex.h Tue Aug 21 03:32:27 2018
@@ -47,6 +47,11 @@ private:
   mutable std::mutex Mutex;
 };
 
+// Returns pointers to the symbols in given slab and bundles slab lifetime with
+// returned symbol pointers so that the pointers are never invalid.
+std::shared_ptr>
+getSymbolsFromSlab(SymbolSlab Slab);
+
 } // namespace clangd
 } // namespace clang
 

Modified: 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=340262&r1=340261&r2=340262&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp Tue Aug 21 03:32:27 
2018
@@ -69,6 +69,12 @@ void DexIndex::build(std::shared_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.

Modified: clang-tools-extra/trunk/clangd/index/dex/DexIndex.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/DexIndex.h?rev=340262&r1=340261&r2=340262&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/DexIndex.h (original)
+++ clang-tools-extra/trunk/clangd/index/dex/DexIndex.h Tue Aug 21 03:32:27 2018
@@ -43,6 +43,9 @@ public:
   /// 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;

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=340262&r1=340261&r2=340262&view

[PATCH] D50509: [analyzer][UninitializedObjectChecker] Refactoring p6.: Move dereferencing to a function

2018-08-21 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments.



Comment at: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:223
+  // int*).
+  while (auto Tmp = V.getAs()) {
+// We can't reason about symbolic regions, assume its initialized.

Szelethus wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > NoQ wrote:
> > > > Hmm, i still have concerns about things like `int *x = (int *)&x;`. Why 
> > > > not just check the type to terminate the loop? Type hierarchy is 
> > > > guaranteed to be finite.
> > > There actually is a testcase for that -- it would create a 
> > > nonloc::LocAsInteger, not a loc::MemRegionVal.
> > > 
> > > I'll add a TODO to revisit this loop condition (again :) ).
> > Ok, let's try with one more asterisk:
> > ```
> > 1 void test() {
> > 2   int **x = (int **)&x;
> > 3   int *y = *x;
> > 4   int z = *y;
> > 5 }
> > ```
> > 
> > Here's what i get in the Store:
> > ```
> > (x,0,direct) : &element{x,0 S64b,int *}
> > (y,0,direct) : &element{x,0 S64b,int *}
> > (z,0,direct) : &element{x,0 S64b,int *}
> > ```
> Sounds fun, I'll see how the checker behaves to these when I'm in the office.
Yup, you were correct, it ends up in an infinite loop. I'll add the testcase 
for it before commiting.


https://reviews.llvm.org/D50509



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


[clang-tools-extra] r340263 - [clangd] NFC: Fix broken build

2018-08-21 Thread Kirill Bobyrev via cfe-commits
Author: omtcyfz
Date: Tue Aug 21 03:40:19 2018
New Revision: 340263

URL: http://llvm.org/viewvc/llvm-project?rev=340263&view=rev
Log:
[clangd] NFC: Fix broken build

Modified:
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=340263&r1=340262&r2=340263&view=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Aug 21 03:40:19 2018
@@ -29,6 +29,11 @@
 using namespace clang;
 using namespace clang::clangd;
 
+static llvm::cl::opt
+UseDex("use-dex-index",
+   llvm::cl::desc("Use experimental Dex static index."),
+   llvm::cl::init(false), llvm::cl::Hidden);
+
 namespace {
 
 enum class PCHStorageFlag { Disk, Memory };
@@ -47,7 +52,7 @@ std::unique_ptr buildStatic
   for (auto Sym : Slab)
 SymsBuilder.insert(Sym);
 
-  return UseDex ? DexIndex::build(std::move(SymsBuilder).build())
+  return UseDex ? dex::DexIndex::build(std::move(SymsBuilder).build())
 : MemIndex::build(std::move(SymsBuilder).build());
 }
 
@@ -189,11 +194,6 @@ static llvm::cl::opt Co
 "'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) {


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


[PATCH] D50697: [clang-format] fix PR38557 - comments between "default" and ':' causes the case label to be treated as an identifier

2018-08-21 Thread Manuel Klimek via Phabricator via cfe-commits
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

lg


Repository:
  rC Clang

https://reviews.llvm.org/D50697



___
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.

2018-08-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 161683.
kadircet marked 3 inline comments as done.
kadircet added a comment.

- Put overrides through scoring and resolve nits.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50898

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

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

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

2018-08-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked an inline comment as done.
kadircet added inline comments.



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

hokein wrote:
> It seems that we treat it as a special case, the code path here runs out of 
> the `ranking` path.
Put override suggestions to ranking system.


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] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.

2018-08-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment.

In https://reviews.llvm.org/D50898#1205756, @hokein wrote:

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


drive-by: +1 to this.


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


r340265 - [analyzer][UninitializedObjectChecker] Refactoring p6.: Move dereferencing to a function

2018-08-21 Thread Kristof Umann via cfe-commits
Author: szelethus
Date: Tue Aug 21 03:45:21 2018
New Revision: 340265

URL: http://llvm.org/viewvc/llvm-project?rev=340265&view=rev
Log:
[analyzer][UninitializedObjectChecker] Refactoring p6.: Move dereferencing to a 
function

Now that it has it's own file, it makes little sense for
isPointerOrReferenceUninit to be this large, so I moved
dereferencing to a separate function.

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

Modified:

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

cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp?rev=340265&r1=340264&r2=340265&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
 (original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
 Tue Aug 21 03:45:21 2018
@@ -265,7 +265,8 @@ bool FindUninitializedFields::isNonUnion
   continue;
 }
 
-if (T->isAnyPointerType() || T->isReferenceType() || 
T->isBlockPointerType()) {
+if (T->isAnyPointerType() || T->isReferenceType() ||
+T->isBlockPointerType()) {
   if (isPointerOrReferenceUninit(FR, LocalChain))
 ContainsUninitField = true;
   continue;

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=340265&r1=340264&r2=340265&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
 (original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
 Tue Aug 21 03:45:21 2018
@@ -95,6 +95,12 @@ public:
 /// known, and thus FD can not be analyzed.
 static bool isVoidPointer(QualType T);
 
+/// Dereferences \p V and returns the value and dynamic type of the pointee, as
+/// well as wether \p FR needs to be casted back to that type. If for whatever
+/// reason dereferencing fails, returns with None.
+static llvm::Optional>
+dereference(ProgramStateRef State, const FieldRegion *FR);
+
 
//===--===//
 //   Methods for FindUninitializedFields.
 
//===--===//
@@ -126,67 +132,22 @@ bool FindUninitializedFields::isPointerO
 return false;
   }
 
-  assert(V.getAs() &&
- "At this point V must be loc::MemRegionVal!");
-  auto L = V.castAs();
-
-  // We can't reason about symbolic regions, assume its initialized.
-  // Note that this also avoids a potential infinite recursion, because
-  // constructors for list-like classes are checked without being called, and
-  // the Static Analyzer will construct a symbolic region for Node *next; or
-  // similar code snippets.
-  if (L.getRegion()->getSymbolicBase()) {
-IsAnyFieldInitialized = true;
-return false;
-  }
-
-  DynamicTypeInfo DynTInfo = getDynamicTypeInfo(State, L.getRegion());
-  if (!DynTInfo.isValid()) {
+  // At this point the pointer itself is initialized and points to a valid
+  // location, we'll now check the pointee.
+  llvm::Optional> DerefInfo =
+  dereference(State, FR);
+  if (!DerefInfo) {
 IsAnyFieldInitialized = true;
 return false;
   }
 
-  QualType DynT = DynTInfo.getType();
-
-  // If the static type of the field is a void pointer, we need to cast it back
-  // to the dynamic type before dereferencing.
-  bool NeedsCastBack = isVoidPointer(FR->getDecl()->getType());
-
-  if (isVoidPointer(DynT)) {
-IsAnyFieldInitialized = true;
-return false;
-  }
-
-  // At this point the pointer itself is initialized and points to a valid
-  // location, we'll now check the pointee.
-  SVal DerefdV = State->getSVal(V.castAs(), DynT);
-
-  // If DerefdV is still a pointer value, we'll dereference it again (e.g.:
-  // int** -> int*).
-  while (auto Tmp = DerefdV.getAs()) {
-if (Tmp->getRegion()->getSymbolicBase()) {
-  IsAnyFieldInitialized = true;
-  return false;
-}
-
-DynTInfo = getDynamicTypeInfo(State, Tmp->getRegion());
-if (!DynTInfo.isValid()) {
-  IsAnyFieldInitialized = true;
-  return false;
-}
-
-DynT = DynTInfo.getType();
-if (isVoidPointer(DynT)) {
-  IsAnyFieldInitialized = true;
-  return false;
-}
-
-DerefdV = State->getSVal(*Tmp, DynT);
-  }
+  V = std::get<0>(*DerefInfo);
+  QualType DynT = std::get<1>(*Deref

[PATCH] D50509: [analyzer][UninitializedObjectChecker] Refactoring p6.: Move dereferencing to a function

2018-08-21 Thread Umann Kristóf via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL340265: [analyzer][UninitializedObjectChecker] Refactoring 
p6.: Move dereferencing to a… (authored by Szelethus, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50509?vs=161209&id=161685#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50509

Files:
  
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
  
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
  cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp

Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -265,7 +265,8 @@
   continue;
 }
 
-if (T->isAnyPointerType() || T->isReferenceType() || T->isBlockPointerType()) {
+if (T->isAnyPointerType() || T->isReferenceType() ||
+T->isBlockPointerType()) {
   if (isPointerOrReferenceUninit(FR, LocalChain))
 ContainsUninitField = true;
   continue;
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
@@ -95,6 +95,12 @@
 /// known, and thus FD can not be analyzed.
 static bool isVoidPointer(QualType T);
 
+/// Dereferences \p V and returns the value and dynamic type of the pointee, as
+/// well as wether \p FR needs to be casted back to that type. If for whatever
+/// reason dereferencing fails, returns with None.
+static llvm::Optional>
+dereference(ProgramStateRef State, const FieldRegion *FR);
+
 //===--===//
 //   Methods for FindUninitializedFields.
 //===--===//
@@ -126,67 +132,22 @@
 return false;
   }
 
-  assert(V.getAs() &&
- "At this point V must be loc::MemRegionVal!");
-  auto L = V.castAs();
-
-  // We can't reason about symbolic regions, assume its initialized.
-  // Note that this also avoids a potential infinite recursion, because
-  // constructors for list-like classes are checked without being called, and
-  // the Static Analyzer will construct a symbolic region for Node *next; or
-  // similar code snippets.
-  if (L.getRegion()->getSymbolicBase()) {
-IsAnyFieldInitialized = true;
-return false;
-  }
-
-  DynamicTypeInfo DynTInfo = getDynamicTypeInfo(State, L.getRegion());
-  if (!DynTInfo.isValid()) {
+  // At this point the pointer itself is initialized and points to a valid
+  // location, we'll now check the pointee.
+  llvm::Optional> DerefInfo =
+  dereference(State, FR);
+  if (!DerefInfo) {
 IsAnyFieldInitialized = true;
 return false;
   }
 
-  QualType DynT = DynTInfo.getType();
-
-  // If the static type of the field is a void pointer, we need to cast it back
-  // to the dynamic type before dereferencing.
-  bool NeedsCastBack = isVoidPointer(FR->getDecl()->getType());
-
-  if (isVoidPointer(DynT)) {
-IsAnyFieldInitialized = true;
-return false;
-  }
-
-  // At this point the pointer itself is initialized and points to a valid
-  // location, we'll now check the pointee.
-  SVal DerefdV = State->getSVal(V.castAs(), DynT);
-
-  // If DerefdV is still a pointer value, we'll dereference it again (e.g.:
-  // int** -> int*).
-  while (auto Tmp = DerefdV.getAs()) {
-if (Tmp->getRegion()->getSymbolicBase()) {
-  IsAnyFieldInitialized = true;
-  return false;
-}
-
-DynTInfo = getDynamicTypeInfo(State, Tmp->getRegion());
-if (!DynTInfo.isValid()) {
-  IsAnyFieldInitialized = true;
-  return false;
-}
-
-DynT = DynTInfo.getType();
-if (isVoidPointer(DynT)) {
-  IsAnyFieldInitialized = true;
-  return false;
-}
-
-DerefdV = State->getSVal(*Tmp, DynT);
-  }
+  V = std::get<0>(*DerefInfo);
+  QualType DynT = std::get<1>(*DerefInfo);
+  bool NeedsCastBack = std::get<2>(*DerefInfo);
 
   // If FR is a pointer pointing to a non-primitive type.
   if (Optional RecordV =
-  DerefdV.getAs()) {
+  V.getAs()) {
 
 const TypedValueRegion *R = RecordV->getRegion();
 
@@ -220,7 +181,7 @@
  "At this point FR must either have a primitive dynamic type, or it "
  "must be a null, undefined, unknown or concrete pointer!");
 
-  if (isPrimitiveUninit(DerefdV)) {
+  if (isPrimitiveUninit(V)) {

[PATCH] D50904: [analyzer][UninitializedObjectChecker] Added documentation to the checker list

2018-08-21 Thread Umann Kristóf via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL340266: [analyzer][UninitializedObjectChecker] Added 
documentation to the checker list (authored by Szelethus, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50904?vs=161262&id=161686#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50904

Files:
  
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
  cfe/trunk/www/analyzer/alpha_checks.html

Index: cfe/trunk/www/analyzer/alpha_checks.html
===
--- cfe/trunk/www/analyzer/alpha_checks.html
+++ cfe/trunk/www/analyzer/alpha_checks.html
@@ -323,6 +323,118 @@
 };
 
 
+
+
+alpha.cplusplus.UninitializedObject
+(C++)
+This checker reports uninitialized fields in objects created
+after a constructor call. It doesn't only find direct uninitialized
+fields, but rather makes a deep inspection of the object,
+analyzing all of it's fields subfields. 
+The checker regards inherited fields as direct fields, so one
+will recieve warnings for uninitialized inherited data members
+as well. 
+
+It has several options:
+
+  
+"Pedantic" (boolean). If its not set or is set to false, the checker
+won't emit warnings for objects that don't have at least one initialized
+field. This may be set with 
+-analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true.
+  
+  
+"NotesAsWarnings" (boolean). If set to true, the checker will emit a
+warning for each uninitalized field, as opposed to emitting one warning
+per constructor call, and listing the uninitialized fields that belongs
+to it in notes. Defaults to false. 
+-analyzer-config alpha.cplusplus.UninitializedObject:NotesAsWarnings=true.
+  
+  
+"CheckPointeeInitialization" (boolean). If set to false, the checker will
+not analyze the pointee of pointer/reference fields, and will only check
+whether the object itself is initialized. Defaults to false. 
+-analyzer-config alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true.
+  
+
+
+
+// With Pedantic and CheckPointeeInitialization set to true
+
+struct A {
+  struct B {
+int x; // note: uninitialized field 'this->b.x'
+   // note: uninitialized field 'this->bptr->x'
+int y; // note: uninitialized field 'this->b.y'
+   // note: uninitialized field 'this->bptr->y'
+  };
+  int *iptr; // note: uninitialized pointer 'this->iptr'
+  B b;
+  B *bptr;
+  char *cptr; // note: uninitialized pointee 'this->cptr'
+
+  A (B *bptr, char *cptr) : bptr(bptr), cptr(cptr) {}
+};
+
+void f() {
+  A::B b;
+  char c;
+  A a(&b, &c); // warning: 6 uninitialized fields
+   //  after the constructor call
+}
+
+
+// With Pedantic set to false and
+// CheckPointeeInitialization set to true
+// (every field is uninitialized)
+
+struct A {
+  struct B {
+int x;
+int y;
+  };
+  int *iptr;
+  B b;
+  B *bptr;
+  char *cptr;
+
+  A (B *bptr, char *cptr) : bptr(bptr), cptr(cptr) {}
+};
+
+void f() {
+  A::B b;
+  char c;
+  A a(&b, &c); // no warning
+}
+
+
+// With Pedantic and CheckPointeeInitialization set to false
+// (pointees are regarded as initialized)
+
+struct A {
+  struct B {
+int x; // note: uninitialized field 'this->b.x'
+int y; // note: uninitialized field 'this->b.y'
+  };
+  int *iptr; // note: uninitialized pointer 'this->iptr'
+  B b;
+  B *bptr;
+  char *cptr;
+
+  A (B *bptr, char *cptr) : bptr(bptr), cptr(cptr) {}
+};
+
+void f() {
+  A::B b;
+  char c;
+  A a(&b, &c); // warning: 3 uninitialized fields
+   //  after the constructor call
+}
+
+
+
+
+
 
 
 
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -286,18 +286,6 @@
   }
 
   // Checking bases.
-  // FIXME: As of now, because of `willObjectBeAnalyzedLater`, objects whose
-  // type is a descendant of another type will emit warnings for uninitalized
-  // inherited members.
-  // This is not the only way to analyze bases of an object -- if we didn't
-  // filter them out, and didn't analyze the bases, this checker would run for
-  // each base of the object in order of base initailization and in theory would
-  // find every uninitalized field. This approach could also make handling
-  // diamond inheritances more easily.
-  //
-  // This rule (that a descendant type's cunstructor is responsible for
-  // initializing inherited data members) is not obvious, and should it should
-  // be.
   const auto *CXXRD = dyn_cast(RD);
   if (!CXXRD)
 return ContainsUninitField;
___
cf

r340266 - [analyzer][UninitializedObjectChecker] Added documentation to the checker list

2018-08-21 Thread Kristof Umann via cfe-commits
Author: szelethus
Date: Tue Aug 21 03:47:19 2018
New Revision: 340266

URL: http://llvm.org/viewvc/llvm-project?rev=340266&view=rev
Log:
[analyzer][UninitializedObjectChecker] Added documentation to the checker list

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

Modified:

cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
cfe/trunk/www/analyzer/alpha_checks.html

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp?rev=340266&r1=340265&r2=340266&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
 (original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
 Tue Aug 21 03:47:19 2018
@@ -286,18 +286,6 @@ bool FindUninitializedFields::isNonUnion
   }
 
   // Checking bases.
-  // FIXME: As of now, because of `willObjectBeAnalyzedLater`, objects whose
-  // type is a descendant of another type will emit warnings for uninitalized
-  // inherited members.
-  // This is not the only way to analyze bases of an object -- if we didn't
-  // filter them out, and didn't analyze the bases, this checker would run for
-  // each base of the object in order of base initailization and in theory 
would
-  // find every uninitalized field. This approach could also make handling
-  // diamond inheritances more easily.
-  //
-  // This rule (that a descendant type's cunstructor is responsible for
-  // initializing inherited data members) is not obvious, and should it should
-  // be.
   const auto *CXXRD = dyn_cast(RD);
   if (!CXXRD)
 return ContainsUninitField;

Modified: cfe/trunk/www/analyzer/alpha_checks.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/analyzer/alpha_checks.html?rev=340266&r1=340265&r2=340266&view=diff
==
--- cfe/trunk/www/analyzer/alpha_checks.html (original)
+++ cfe/trunk/www/analyzer/alpha_checks.html Tue Aug 21 03:47:19 2018
@@ -323,6 +323,118 @@ public:
 };
 
 
+
+
+alpha.cplusplus.UninitializedObject
+(C++)
+This checker reports uninitialized fields in objects created
+after a constructor call. It doesn't only find direct uninitialized
+fields, but rather makes a deep inspection of the object,
+analyzing all of it's fields subfields. 
+The checker regards inherited fields as direct fields, so one
+will recieve warnings for uninitialized inherited data members
+as well. 
+
+It has several options:
+
+  
+"Pedantic" (boolean). If its not set or is set to false, the 
checker
+won't emit warnings for objects that don't have at least one initialized
+field. This may be set with 
+-analyzer-config 
alpha.cplusplus.UninitializedObject:Pedantic=true.
+  
+  
+"NotesAsWarnings" (boolean). If set to true, the checker will 
emit a
+warning for each uninitalized field, as opposed to emitting one warning
+per constructor call, and listing the uninitialized fields that belongs
+to it in notes. Defaults to false. 
+-analyzer-config 
alpha.cplusplus.UninitializedObject:NotesAsWarnings=true.
+  
+  
+"CheckPointeeInitialization" (boolean). If set to false, the 
checker will
+not analyze the pointee of pointer/reference fields, and will only check
+whether the object itself is initialized. Defaults to false. 
+-analyzer-config 
alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true.
+  
+
+
+
+// With Pedantic and CheckPointeeInitialization set to true
+
+struct A {
+  struct B {
+int x; // note: uninitialized field 'this->b.x'
+   // note: uninitialized field 'this->bptr->x'
+int y; // note: uninitialized field 'this->b.y'
+   // note: uninitialized field 'this->bptr->y'
+  };
+  int *iptr; // note: uninitialized pointer 'this->iptr'
+  B b;
+  B *bptr;
+  char *cptr; // note: uninitialized pointee 'this->cptr'
+
+  A (B *bptr, char *cptr) : bptr(bptr), cptr(cptr) {}
+};
+
+void f() {
+  A::B b;
+  char c;
+  A a(&b, &c); // warning: 6 uninitialized fields
+   //  after the constructor call
+}
+
+
+// With Pedantic set to false and
+// CheckPointeeInitialization set to true
+// (every field is uninitialized)
+
+struct A {
+  struct B {
+int x;
+int y;
+  };
+  int *iptr;
+  B b;
+  B *bptr;
+  char *cptr;
+
+  A (B *bptr, char *cptr) : bptr(bptr), cptr(cptr) {}
+};
+
+void f() {
+  A::B b;
+  char c;
+  A a(&b, &c); // no warning
+}
+
+
+// With Pedantic and CheckPointeeInitialization set to false
+// (pointees are regarded as initialized)
+
+struct A {
+  struct B {
+int x; // note: uninitialized field 'this->b.x'
+int y; // note: uninitialized field 'this->b.y'
+  };
+  int *iptr; // note: uniniti

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

2018-08-21 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

1. Can we put the helper that executes the tasks asynchronously with a stack 
size into llvm's threading library (i.e. somewhere close to 
`llvm::execute_on_thread`?) All platform-specific code is already there, we can 
reuse most of the implementation too.
2. I wonder if we should expose the stack size as an option to clangd?




Comment at: clangd/Threading.cpp:57
+template 
+static void executeOnThreadAndDetach(UserFnPtr UserFn, Data UserData) {
+#if !defined(__APPLE__)

Maybe add a parameter for the stack size to this function? (And document it's 
only taken into account when pthread-based implementation is used)



Comment at: clangd/Threading.cpp:57
+template 
+static void executeOnThreadAndDetach(UserFnPtr UserFn, Data UserData) {
+#if !defined(__APPLE__)

ilya-biryukov wrote:
> Maybe add a parameter for the stack size to this function? (And document it's 
> only taken into account when pthread-based implementation is used)
Maybe accept a `llvm::unique_function` instead of `UserFn` and `Data`? 
This split is clearly implementation detail of pthreads-based solution. 
`std::thread` does not need it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50993



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


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

2018-08-21 Thread Stefan Maksimovic via Phabricator via cfe-commits
smaksimovic updated this revision to Diff 161690.
smaksimovic added a comment.

Expanded a piece of code using switch cases as suggested.


https://reviews.llvm.org/D50557

Files:
  include/clang/Basic/DiagnosticCommonKinds.td
  lib/Basic/Targets/Mips.cpp
  lib/Basic/Targets/Mips.h
  test/Preprocessor/init.c

Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -3442,7 +3442,7 @@
 // MIPS32BE:#define __mips 32
 // MIPS32BE:#define __mips__ 1
 // MIPS32BE:#define __mips_abicalls 1
-// MIPS32BE:#define __mips_fpr 32
+// MIPS32BE:#define __mips_fpr 0
 // MIPS32BE:#define __mips_hard_float 1
 // MIPS32BE:#define __mips_o32 1
 // MIPS32BE:#define _mips 1
@@ -3649,7 +3649,7 @@
 // MIPS32EL:#define __mips 32
 // MIPS32EL:#define __mips__ 1
 // MIPS32EL:#define __mips_abicalls 1
-// MIPS32EL:#define __mips_fpr 32
+// MIPS32EL:#define __mips_fpr 0
 // MIPS32EL:#define __mips_hard_float 1
 // MIPS32EL:#define __mips_o32 1
 // MIPS32EL:#define _mips 1
@@ -4900,6 +4900,41 @@
 // RUN:   | FileCheck -match-full-lines -check-prefix NOMIPS-ABS2008 %s
 // NOMIPS-ABS2008-NOT:#define __mips_abs2008 1
 //
+// RUN: %clang_cc1  \
+// RUN:   -E -dM -triple=mips-none-none < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefix MIPS32-NOFP %s
+// MIPS32-NOFP:#define __mips_fpr 0
+//
+// RUN: %clang_cc1 -target-feature +fpxx \
+// RUN:   -E -dM -triple=mips-none-none < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefix MIPS32-MFPXX %s
+// MIPS32-MFPXX:#define __mips_fpr 0
+//
+// RUN: %clang_cc1 -target-cpu mips32r6 -target-feature +fpxx \
+// RUN:   -E -dM -triple=mips-none-none < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefix MIPS32R6-MFPXX %s
+// MIPS32R6-MFPXX:#define __mips_fpr 0
+//
+// RUN: %clang_cc1  \
+// RUN:   -E -dM -triple=mips64-none-none < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefix MIPS64-NOFP %s
+// MIPS64-NOFP:#define __mips_fpr 64
+//
+// RUN: not %clang_cc1 -target-feature -fp64 \
+// RUN:   -E -dM -triple=mips64-none-none < /dev/null 2>&1 \
+// RUN:   | FileCheck -match-full-lines -check-prefix MIPS64-MFP32 %s
+// MIPS64-MFP32:error: option '-mfpxx' cannot be specified with 'mips64r2'
+//
+// RUN: not %clang_cc1 -target-feature +fpxx \
+// RUN:   -E -dM -triple=mips64-none-none < /dev/null 2>&1 \
+// RUN:   | FileCheck -match-full-lines -check-prefix MIPS64-MFPXX %s
+// MIPS64-MFPXX:error: '-mfpxx' can only be used with the 'o32' ABI
+//
+// RUN: not %clang_cc1 -target-cpu mips64r6 -target-feature +fpxx \
+// RUN:   -E -dM -triple=mips64-none-none < /dev/null 2>&1 \
+// RUN:   | FileCheck -match-full-lines -check-prefix MIPS64R6-MFPXX %s
+// MIPS64R6-MFPXX:error: '-mfpxx' can only be used with the 'o32' ABI
+//
 // RUN: %clang_cc1 -target-feature -fp64 \
 // RUN:   -E -dM -triple=mips-none-none < /dev/null \
 // RUN:   | FileCheck -match-full-lines -check-prefix MIPS32-MFP32 %s
@@ -4916,7 +4951,7 @@
 // RUN:   -E -dM -triple=mips-none-none < /dev/null \
 // RUN:   | FileCheck -match-full-lines -check-prefix MIPS32-MFP32SF %s
 // MIPS32-MFP32SF:#define _MIPS_FPSET 32
-// MIPS32-MFP32SF:#define __mips_fpr 32
+// MIPS32-MFP32SF:#define __mips_fpr 0
 //
 // RUN: %clang_cc1 -target-feature +fp64 \
 // RUN:   -E -dM -triple=mips64-none-none < /dev/null \
Index: lib/Basic/Targets/Mips.h
===
--- lib/Basic/Targets/Mips.h
+++ lib/Basic/Targets/Mips.h
@@ -57,16 +57,16 @@
   bool UseIndirectJumpHazard;
 
 protected:
-  bool HasFP64;
+  enum FPModeEnum { FPXX, FP32, FP64 } FPMode;
   std::string ABI;
 
 public:
   MipsTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
   : TargetInfo(Triple), IsMips16(false), IsMicromips(false),
 IsNan2008(false), IsAbs2008(false), IsSingleFloat(false),
 IsNoABICalls(false), CanUseBSDABICalls(false), FloatABI(HardFloat),
 DspRev(NoDSP), HasMSA(false), DisableMadd4(false),
-UseIndirectJumpHazard(false), HasFP64(false) {
+UseIndirectJumpHazard(false), FPMode(FPXX) {
 TheCXXABI.set(TargetCXXABI::GenericMIPS);
 
 setABI(getTriple().isMIPS32() ? "o32" : "n64");
@@ -181,6 +181,8 @@
 return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);
   }
 
+  unsigned getISARev() const;
+
   void getTargetDefines(const LangOptions &Opts,
 MacroBuilder &Builder) const override;
 
@@ -305,7 +307,7 @@
 IsSingleFloat = false;
 FloatABI = HardFloat;
 DspRev = NoDSP;
-HasFP64 = isFP64Default();
+FPMode = isFP64Default() ? FP64 : FPXX;
 
 for (const auto &Feature : Features) {
   if (Feature == "+single-float")
@@ -325,9 +327,11 @@
   else if (Feature == "+nomadd4")
 DisableMadd4 = true;
   else if (Feature == "+fp64")
-HasFP64 = true;
+FPMode = FP64;
   else if (Feature == "-fp64")
-HasFP64 = false;

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

2018-08-21 Thread Henry Wong via Phabricator via cfe-commits
MTC added a comment.

@xazax.hun What do you think?


https://reviews.llvm.org/D48027



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


[PATCH] D44539: [Sema][Objective-C] Add check to warn when property of objc type has assign attribute

2018-08-21 Thread Alfred Zien via Phabricator via cfe-commits
QF5690 updated this revision to Diff 161684.
QF5690 added a comment.

Using `isObjCARCImplicitlyUnretainedType` instead of `isObjCClassType`
Tests in `property-in-class-extension-1.m` relaxed to 
`-Wproperty-attribute-mismatch` (was `-Weverything`)


Repository:
  rC Clang

https://reviews.llvm.org/D44539

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaObjCProperty.cpp
  test/SemaObjC/property-assign-on-object-type.m
  test/SemaObjC/property-in-class-extension-1.m


Index: test/SemaObjC/property-in-class-extension-1.m
===
--- test/SemaObjC/property-in-class-extension-1.m
+++ test/SemaObjC/property-in-class-extension-1.m
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1  -fsyntax-only -triple x86_64-apple-darwin11 
-fobjc-runtime-has-weak -fobjc-weak -verify -Weverything %s
-// RUN: %clang_cc1 -x objective-c++ -triple x86_64-apple-darwin11 
-fobjc-runtime-has-weak -fobjc-weak -fsyntax-only -verify -Weverything %s
+// RUN: %clang_cc1  -fsyntax-only -triple x86_64-apple-darwin11 
-fobjc-runtime-has-weak -fobjc-weak -verify -Wproperty-attribute-mismatch %s
+// RUN: %clang_cc1 -x objective-c++ -triple x86_64-apple-darwin11 
-fobjc-runtime-has-weak -fobjc-weak -fsyntax-only -verify 
-Wproperty-attribute-mismatch %s
 // rdar://12103400
 
 @class NSString;
Index: test/SemaObjC/property-assign-on-object-type.m
===
--- /dev/null
+++ test/SemaObjC/property-assign-on-object-type.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wobjc-property-assign-on-object-type 
%s
+
+@interface Foo @end
+@protocol Prot @end
+
+@interface Bar
+@property(assign, readonly) Foo* o1; // expected-warning {{'assign' property 
of object type may become a dangling reference; consider using 
'unsafe_unretained'}}
+@property(unsafe_unretained, readonly) Foo* o2;
+
+@property(assign) Class classProperty;
+@property(assign) Class classWithProtocolProperty;
+@property(assign) int s1;
+@property(assign) int* s2;
+@end
+
+@interface Bar ()
+@property(readwrite) Foo* o1;
+@property(readwrite) Foo* o2;
+@end
Index: lib/Sema/SemaObjCProperty.cpp
===
--- lib/Sema/SemaObjCProperty.cpp
+++ lib/Sema/SemaObjCProperty.cpp
@@ -2547,6 +2547,14 @@
 PropertyDecl->setInvalidDecl();
   }
 
+  // Check for assign on object types.
+  if ((Attributes & ObjCDeclSpec::DQ_PR_assign) &&
+  !(Attributes & ObjCDeclSpec::DQ_PR_unsafe_unretained) &&
+  PropertyTy->isObjCRetainableType() &&
+  !PropertyTy->isObjCARCImplicitlyUnretainedType()) {
+Diag(Loc, diag::warn_objc_property_assign_on_object);
+  }
+
   // Check for more than one of { assign, copy, retain }.
   if (Attributes & ObjCDeclSpec::DQ_PR_assign) {
 if (Attributes & ObjCDeclSpec::DQ_PR_copy) {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -1014,6 +1014,9 @@
   "property attributes '%0' and '%1' are mutually exclusive">;
 def err_objc_property_requires_object : Error<
   "property with '%0' attribute must be of object type">;
+def warn_objc_property_assign_on_object : Warning<
+  "'assign' property of object type may become a dangling reference; consider 
using 'unsafe_unretained'">,
+  InGroup, DefaultIgnore;
 def warn_objc_property_no_assignment_attribute : Warning<
   "no 'assign', 'retain', or 'copy' attribute is specified - "
   "'assign' is assumed">,
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -367,6 +367,7 @@
 def BadFunctionCast : DiagGroup<"bad-function-cast">;
 def ObjCPropertyImpl : DiagGroup<"objc-property-implementation">;
 def ObjCPropertyNoAttribute : DiagGroup<"objc-property-no-attribute">;
+def ObjCPropertyAssignOnObjectType : 
DiagGroup<"objc-property-assign-on-object-type">;
 def ObjCProtocolQualifiers : DiagGroup<"objc-protocol-qualifiers">;
 def ObjCMissingSuperCalls : DiagGroup<"objc-missing-super-calls">;
 def ObjCDesignatedInit : DiagGroup<"objc-designated-initializers">;


Index: test/SemaObjC/property-in-class-extension-1.m
===
--- test/SemaObjC/property-in-class-extension-1.m
+++ test/SemaObjC/property-in-class-extension-1.m
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1  -fsyntax-only -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fobjc-weak -verify -Weverything %s
-// RUN: %clang_cc1 -x objective-c++ -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fobjc-weak -fsyntax-only -verify -Weverything %s
+// RUN: %clang_cc1  -fsyntax-only -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fobjc-weak -verify -Wproperty-attri

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

2018-08-21 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 161694.
ilya-biryukov marked 3 inline comments as done.
ilya-biryukov added a comment.

- Move DynamicIndex into ClangdServer.cpp
- Rename FileIdx to DynamicIdx
- Avoid modifying input args


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50889

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h

Index: clangd/index/FileIndex.h
===
--- clangd/index/FileIndex.h
+++ clangd/index/FileIndex.h
@@ -64,7 +64,11 @@
   /// nullptr, this removes all symbols in the file.
   /// If \p AST is not null, \p PP cannot be null and it should be the
   /// preprocessor that was used to build \p AST.
-  void update(PathRef Path, ASTContext *AST, std::shared_ptr PP);
+  /// If \p TopLevelDecls is set, only these decls are indexed. Otherwise, all
+  /// top level decls obtained from \p AST are indexed.
+  void
+  update(PathRef Path, ASTContext *AST, std::shared_ptr PP,
+ llvm::Optional> TopLevelDecls = llvm::None);
 
   bool
   fuzzyFind(const FuzzyFindRequest &Req,
@@ -86,8 +90,12 @@
 /// Retrieves namespace and class level symbols in \p AST.
 /// Exposed to assist in unit tests.
 /// If URISchemes is empty, the default schemes in SymbolCollector will be used.
-SymbolSlab indexAST(ASTContext &AST, std::shared_ptr PP,
-llvm::ArrayRef URISchemes = {});
+/// If \p TopLevelDecls is set, only these decls are indexed. Otherwise, all top
+/// level decls obtained from \p AST are indexed.
+SymbolSlab
+indexAST(ASTContext &AST, std::shared_ptr PP,
+ llvm::Optional> TopLevelDecls = llvm::None,
+ llvm::ArrayRef URISchemes = {});
 
 } // namespace clangd
 } // namespace clang
Index: clangd/index/FileIndex.cpp
===
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -8,15 +8,16 @@
 //===--===//
 
 #include "FileIndex.h"
-#include "SymbolCollector.h"
 #include "../Logger.h"
+#include "SymbolCollector.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Lex/Preprocessor.h"
 
 namespace clang {
 namespace clangd {
 
 SymbolSlab indexAST(ASTContext &AST, std::shared_ptr PP,
+llvm::Optional> TopLevelDecls,
 llvm::ArrayRef URISchemes) {
   SymbolCollector::Options CollectorOpts;
   // FIXME(ioeric): we might also want to collect include headers. We would need
@@ -38,10 +39,14 @@
   index::IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly;
   IndexOpts.IndexFunctionLocals = false;
 
-  std::vector TopLevelDecls(
-  AST.getTranslationUnitDecl()->decls().begin(),
-  AST.getTranslationUnitDecl()->decls().end());
-  index::indexTopLevelDecls(AST, TopLevelDecls, Collector, IndexOpts);
+  std::vector DeclsToIndex;
+  if (TopLevelDecls)
+DeclsToIndex.assign(TopLevelDecls->begin(), TopLevelDecls->end());
+  else
+DeclsToIndex.assign(AST.getTranslationUnitDecl()->decls().begin(),
+AST.getTranslationUnitDecl()->decls().end());
+
+  index::indexTopLevelDecls(AST, DeclsToIndex, Collector, IndexOpts);
 
   return Collector.takeSymbols();
 }
@@ -81,13 +86,14 @@
 }
 
 void FileIndex::update(PathRef Path, ASTContext *AST,
-   std::shared_ptr PP) {
+   std::shared_ptr PP,
+   llvm::Optional> TopLevelDecls) {
   if (!AST) {
 FSymbols.update(Path, nullptr);
   } else {
 assert(PP);
 auto Slab = llvm::make_unique();
-*Slab = indexAST(*AST, PP, URISchemes);
+*Slab = indexAST(*AST, PP, TopLevelDecls, URISchemes);
 FSymbols.update(Path, std::move(Slab));
   }
   auto Symbols = FSymbols.allSymbols();
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -99,6 +99,7 @@
   /// synchronize access to shared state.
   ClangdServer(GlobalCompilationDatabase &CDB, FileSystemProvider &FSProvider,
DiagnosticsConsumer &DiagConsumer, const Options &Opts);
+  ~ClangdServer();
 
   /// Set the root path of the workspace.
   void setRootPath(PathRef RootPath);
@@ -200,6 +201,7 @@
   formatCode(llvm::StringRef Code, PathRef File,
  ArrayRef Ranges);
 
+  class DynamicIndex;
   typedef uint64_t DocVersion;
 
   void consumeDiagnostics(PathRef File, DocVersion Version,
@@ -217,15 +219,14 @@
   Path ResourceDir;
   // The index used to look up symbols. This could be:
   //   - null (all index functionality is optional)
-  //   - the dynamic index owned by ClangdServer (FileIdx)
+  //   - the dynamic index owned by ClangdServer (DynamicIdx)
   //   - the static index passed to the constructor
   //   - a merged view of a static and dynamic index (MergedIndex)
   SymbolIndex *Index;
-  // If present, an up-to-date of

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

2018-08-21 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



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

ioeric wrote:
> Maybe provide noop implementations for `ParsingCallbacks::onPreambleAST()` 
> and `ParsingCallbacks::onMainAST` by default?
I'll address this in a parent revision. 



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.

ioeric wrote:
> nit: `s/FileIdx/DymIdx`? (also need to update the comment above)
Went with `DynamicIdx`.


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


r340271 - Add missing library dependency to fix build break after rC340247

2018-08-21 Thread Hiroshi Inoue via cfe-commits
Author: inouehrs
Date: Tue Aug 21 04:41:41 2018
New Revision: 340271

URL: http://llvm.org/viewvc/llvm-project?rev=340271&view=rev
Log:
Add missing library dependency to fix build break after rC340247

Modified:
cfe/trunk/lib/ARCMigrate/CMakeLists.txt

Modified: cfe/trunk/lib/ARCMigrate/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/CMakeLists.txt?rev=340271&r1=340270&r2=340271&view=diff
==
--- cfe/trunk/lib/ARCMigrate/CMakeLists.txt (original)
+++ cfe/trunk/lib/ARCMigrate/CMakeLists.txt Tue Aug 21 04:41:41 2018
@@ -35,4 +35,5 @@ add_clang_library(clangARCMigrate
   clangSema
   clangSerialization
   clangStaticAnalyzerCheckers
+  clangStaticAnalyzerCore
   )


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


[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like keys

2018-08-21 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment.

In https://reviews.llvm.org/D50488#1204655, @mgrang wrote:

> In https://reviews.llvm.org/D50488#1203876, @Szelethus wrote:
>
> >
>
>
> This is my first time with ASTMatchers and I am not sure how to get the 
> value_type from hasType (I don't see a matcher for value_types in 
> ASTMatchers.h). Would I end up using a visitor for that? If yes, then maybe 
> the entire check for pointer types needs to be done via visitors? Sorry for 
> the newbie questions :)


I played around for a bit, my `clang-query` is a little out of date, but here's 
what I found: You can match `typedef`s with `typedefDecl()`, and `using` by 
`typeAliasDecl()`, and then add `hasName("value_type")` as a parameter. As to 
how to check whether a type has any of these, I'm a little unsure myself, but 
you could use `hasDescendant`, and narrow down the matches.

I'm not sure whether this checks inherited type declarations, and it sure 
doesn't check template specializations of `std::iterator_traits`, but it is a 
good start :) I'll revisit this when I have a little more time on my hand.


https://reviews.llvm.org/D50488



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


[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization

2018-08-21 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 161698.
martong marked an inline comment as done.
martong added a comment.

- Change comments


Repository:
  rC Clang

https://reviews.llvm.org/D50451

Files:
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTImporter.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -2683,6 +2683,263 @@
   EXPECT_EQ(FromIndex, 3u);
 }
 
+TEST_P(ASTImporterTestBase, MergeFieldDeclsOfClassTemplateSpecialization) {
+  std::string ClassTemplate =
+  R"(
+  template 
+  struct X {
+  int a{0}; // FieldDecl with InitListExpr
+  X(char) : a(3) {} // (1)
+  X(int) {} // (2)
+  };
+  )";
+  Decl *ToTU = getToTuDecl(ClassTemplate +
+  R"(
+  void foo() {
+  // ClassTemplateSpec with ctor (1): FieldDecl without InitlistExpr
+  X xc('c');
+  }
+  )", Lang_CXX11);
+  auto *ToSpec = FirstDeclMatcher().match(
+  ToTU, classTemplateSpecializationDecl(hasName("X")));
+  // FieldDecl without InitlistExpr:
+  auto *ToField = *ToSpec->field_begin();
+  ASSERT_TRUE(ToField);
+  ASSERT_FALSE(ToField->getInClassInitializer());
+  Decl *FromTU = getTuDecl(ClassTemplate +
+  R"(
+  void bar() {
+  // ClassTemplateSpec with ctor (2): FieldDecl WITH InitlistExpr
+  X xc(1);
+  }
+  )", Lang_CXX11);
+  auto *FromSpec = FirstDeclMatcher().match(
+  FromTU, classTemplateSpecializationDecl(hasName("X")));
+  // FieldDecl with InitlistExpr:
+  auto *FromField = *FromSpec->field_begin();
+  ASSERT_TRUE(FromField);
+  ASSERT_TRUE(FromField->getInClassInitializer());
+
+  auto *ImportedSpec = Import(FromSpec, Lang_CXX11);
+  ASSERT_TRUE(ImportedSpec);
+  EXPECT_EQ(ImportedSpec, ToSpec);
+  // After the import, the FieldDecl has to be merged, thus it should have the
+  // InitListExpr.
+  EXPECT_TRUE(ToField->getInClassInitializer());
+}
+
+TEST_P(ASTImporterTestBase, MergeFunctionOfClassTemplateSpecialization) {
+  std::string ClassTemplate =
+  R"(
+  template 
+  struct X {
+void f() {}
+void g() {}
+  };
+  )";
+  Decl *ToTU = getToTuDecl(ClassTemplate +
+  R"(
+  void foo() {
+  X x;
+  x.f();
+  }
+  )", Lang_CXX11);
+  Decl *FromTU = getTuDecl(ClassTemplate +
+  R"(
+  void bar() {
+  X x;
+  x.g();
+  }
+  )", Lang_CXX11);
+  auto *FromSpec = FirstDeclMatcher().match(
+  FromTU, classTemplateSpecializationDecl(hasName("X")));
+  auto FunPattern = functionDecl(hasName("g"),
+ hasParent(classTemplateSpecializationDecl()));
+  auto *FromFun =
+  FirstDeclMatcher().match(FromTU, FunPattern);
+  auto *ToFun =
+  FirstDeclMatcher().match(ToTU, FunPattern);
+  ASSERT_TRUE(FromFun->hasBody());
+  ASSERT_FALSE(ToFun->hasBody());
+  auto *ImportedSpec = Import(FromSpec, Lang_CXX11);
+  ASSERT_TRUE(ImportedSpec);
+  auto *ToSpec = FirstDeclMatcher().match(
+  ToTU, classTemplateSpecializationDecl(hasName("X")));
+  EXPECT_EQ(ImportedSpec, ToSpec);
+  EXPECT_TRUE(ToFun->hasBody());
+}
+
+TEST_P(ASTImporterTestBase,
+   ODRViolationOfClassTemplateSpecializationsShouldBeReported) {
+  std::string ClassTemplate =
+  R"(
+  template 
+  struct X {};
+  )";
+  Decl *ToTU = getToTuDecl(ClassTemplate +
+   R"(
+  template <>
+  struct X {
+  int a;
+  };
+  void foo() {
+  X x;
+  }
+  )",
+   Lang_CXX11);
+  Decl *FromTU = getTuDecl(ClassTemplate +
+   R"(
+  template <>
+  struct X {
+  int b;
+  };
+  void foo() {
+  X x;
+  }
+  )",
+   Lang_CXX11);
+  auto *FromSpec = FirstDeclMatcher().match(
+  FromTU, classTemplateSpecializationDecl(hasName("X")));
+  auto *ImportedSpec = Import(FromSpec, Lang_CXX11);
+
+  // We expect one (ODR) warning during the import.
+  EXPECT_EQ(1u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
+
+  // The second specialization is different from the first, thus it violates
+  // ODR, consequently we expect to keep the first specialization only, which is
+  // already in the "To" context.
+  EXPECT_TRUE(ImportedSpec);
+  auto *ToSpec = FirstDeclMatcher().match(
+  ToTU, classTemplateSpecializationDecl(hasName("X")));
+  EXPECT_EQ(ImportedSpec, ToSpec);
+  EXPECT_EQ(1u, DeclCounter().match(
+ToTU, classTemplateSpecializationDecl()));
+}
+
+TEST_P(ASTImporterTestBase, MergeCtorOfClassTemplateSpecialization) {
+  std::string ClassTemplate =
+  R"(
+  template 
+  struct X {
+  X(char) {}
+  X(int) {}
+  };
+  )";
+  Decl *ToTU = getToTuDecl

[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization

2018-08-21 Thread Gabor Marton via Phabricator via cfe-commits
martong marked an inline comment as done.
martong added inline comments.



Comment at: lib/AST/ASTImporter.cpp:4550
+  // in the "From" context, but not in the "To" context.
+  for (auto *FromField : D->fields())
+Importer.Import(FromField);

a_sidorin wrote:
> martong wrote:
> > martong wrote:
> > > a_sidorin wrote:
> > > > Importing additional fields can change the layout of the 
> > > > specialization. For CSA, this usually results in strange assertions 
> > > > hard to debug. Could you please share the results of testing of this 
> > > > change?
> > > > This change also doesn't seem to have related tests in this patch.
> > > TLDR; We will not create additional fields.
> > > 
> > > By the time when we import the field, we already know that the existing 
> > > specialization is structurally equivalent with the new one. 
> > > Since a ClassTemplateSpecializationDecl is the descendant of RecordDecl, 
> > > the structural equivalence check ensures that they have the exact same 
> > > fields.
> > > When we import the field of the new spec and if there is an existing 
> > > FieldDecl in the "To" context, then no new FieldDecl will be created 
> > > (this is handled in `VisitFieldDecl` by first doing a lookup of existing 
> > > field with the same name and type).
> > > This patch extends `VisitFieldDecl` in a way that we add new initializer 
> > > expressions to the existing FieldDecl, if it didn't have and in the 
> > > "From" context it has.
> > > 
> > > For the record, I  added a new test to make sure that a new FieldDecl 
> > > will not be created during the merge.
> > This is the new test: 
> > `ODRViolationOfClassTemplateSpecializationsShouldBeReported`. It checks 
> > that it is not possible to add new fields to a specialization, rather an 
> > ODR violation is diagnosed.
> Thank you for the explanation. However, I find the comment very misleading. 
> It tells: 
> ```
>   // Check and merge those fields which have been instantiated
>   // in the "From" context, but not in the "To" context.
> ```
> Would it be correct to change it to "Import field initializers that are still 
> not instantiated", or do I still misunderstand something?
Yes, I agree that this comment is not precise and could be misleading, thank 
you for pointing this out. I changed this comment and others too.


Repository:
  rC Clang

https://reviews.llvm.org/D50451



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


[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization

2018-08-21 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 161699.
martong added a comment.

- Add one more TODO to import instantiated exception specifications


Repository:
  rC Clang

https://reviews.llvm.org/D50451

Files:
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTImporter.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -2683,6 +2683,263 @@
   EXPECT_EQ(FromIndex, 3u);
 }
 
+TEST_P(ASTImporterTestBase, MergeFieldDeclsOfClassTemplateSpecialization) {
+  std::string ClassTemplate =
+  R"(
+  template 
+  struct X {
+  int a{0}; // FieldDecl with InitListExpr
+  X(char) : a(3) {} // (1)
+  X(int) {} // (2)
+  };
+  )";
+  Decl *ToTU = getToTuDecl(ClassTemplate +
+  R"(
+  void foo() {
+  // ClassTemplateSpec with ctor (1): FieldDecl without InitlistExpr
+  X xc('c');
+  }
+  )", Lang_CXX11);
+  auto *ToSpec = FirstDeclMatcher().match(
+  ToTU, classTemplateSpecializationDecl(hasName("X")));
+  // FieldDecl without InitlistExpr:
+  auto *ToField = *ToSpec->field_begin();
+  ASSERT_TRUE(ToField);
+  ASSERT_FALSE(ToField->getInClassInitializer());
+  Decl *FromTU = getTuDecl(ClassTemplate +
+  R"(
+  void bar() {
+  // ClassTemplateSpec with ctor (2): FieldDecl WITH InitlistExpr
+  X xc(1);
+  }
+  )", Lang_CXX11);
+  auto *FromSpec = FirstDeclMatcher().match(
+  FromTU, classTemplateSpecializationDecl(hasName("X")));
+  // FieldDecl with InitlistExpr:
+  auto *FromField = *FromSpec->field_begin();
+  ASSERT_TRUE(FromField);
+  ASSERT_TRUE(FromField->getInClassInitializer());
+
+  auto *ImportedSpec = Import(FromSpec, Lang_CXX11);
+  ASSERT_TRUE(ImportedSpec);
+  EXPECT_EQ(ImportedSpec, ToSpec);
+  // After the import, the FieldDecl has to be merged, thus it should have the
+  // InitListExpr.
+  EXPECT_TRUE(ToField->getInClassInitializer());
+}
+
+TEST_P(ASTImporterTestBase, MergeFunctionOfClassTemplateSpecialization) {
+  std::string ClassTemplate =
+  R"(
+  template 
+  struct X {
+void f() {}
+void g() {}
+  };
+  )";
+  Decl *ToTU = getToTuDecl(ClassTemplate +
+  R"(
+  void foo() {
+  X x;
+  x.f();
+  }
+  )", Lang_CXX11);
+  Decl *FromTU = getTuDecl(ClassTemplate +
+  R"(
+  void bar() {
+  X x;
+  x.g();
+  }
+  )", Lang_CXX11);
+  auto *FromSpec = FirstDeclMatcher().match(
+  FromTU, classTemplateSpecializationDecl(hasName("X")));
+  auto FunPattern = functionDecl(hasName("g"),
+ hasParent(classTemplateSpecializationDecl()));
+  auto *FromFun =
+  FirstDeclMatcher().match(FromTU, FunPattern);
+  auto *ToFun =
+  FirstDeclMatcher().match(ToTU, FunPattern);
+  ASSERT_TRUE(FromFun->hasBody());
+  ASSERT_FALSE(ToFun->hasBody());
+  auto *ImportedSpec = Import(FromSpec, Lang_CXX11);
+  ASSERT_TRUE(ImportedSpec);
+  auto *ToSpec = FirstDeclMatcher().match(
+  ToTU, classTemplateSpecializationDecl(hasName("X")));
+  EXPECT_EQ(ImportedSpec, ToSpec);
+  EXPECT_TRUE(ToFun->hasBody());
+}
+
+TEST_P(ASTImporterTestBase,
+   ODRViolationOfClassTemplateSpecializationsShouldBeReported) {
+  std::string ClassTemplate =
+  R"(
+  template 
+  struct X {};
+  )";
+  Decl *ToTU = getToTuDecl(ClassTemplate +
+   R"(
+  template <>
+  struct X {
+  int a;
+  };
+  void foo() {
+  X x;
+  }
+  )",
+   Lang_CXX11);
+  Decl *FromTU = getTuDecl(ClassTemplate +
+   R"(
+  template <>
+  struct X {
+  int b;
+  };
+  void foo() {
+  X x;
+  }
+  )",
+   Lang_CXX11);
+  auto *FromSpec = FirstDeclMatcher().match(
+  FromTU, classTemplateSpecializationDecl(hasName("X")));
+  auto *ImportedSpec = Import(FromSpec, Lang_CXX11);
+
+  // We expect one (ODR) warning during the import.
+  EXPECT_EQ(1u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
+
+  // The second specialization is different from the first, thus it violates
+  // ODR, consequently we expect to keep the first specialization only, which is
+  // already in the "To" context.
+  EXPECT_TRUE(ImportedSpec);
+  auto *ToSpec = FirstDeclMatcher().match(
+  ToTU, classTemplateSpecializationDecl(hasName("X")));
+  EXPECT_EQ(ImportedSpec, ToSpec);
+  EXPECT_EQ(1u, DeclCounter().match(
+ToTU, classTemplateSpecializationDecl()));
+}
+
+TEST_P(ASTImporterTestBase, MergeCtorOfClassTemplateSpecialization) {
+  std::string ClassTemplate =
+  R"(
+  template 
+  struct X {
+  X(char) {}
+  X(int) {}
+  };
+  )";
+  Decl *ToTU = get

r340272 - [analyzer][UninitializedObjectChecker] Explicit namespace resolution for inherited data members

2018-08-21 Thread Kristof Umann via cfe-commits
Author: szelethus
Date: Tue Aug 21 05:16:59 2018
New Revision: 340272

URL: http://llvm.org/viewvc/llvm-project?rev=340272&view=rev
Log:
[analyzer][UninitializedObjectChecker] Explicit namespace resolution for 
inherited data members

For the following example:

  struct Base {
int x;
  };

  // In a different translation unit

  struct Derived : public Base {
Derived() {}
  };

For a call to Derived::Derived(), we'll receive a note that
this->x is uninitialized. Since x is not a direct field of Derived,
it could be a little confusing. This patch aims to fix this, as well
as the case when the derived object has a field that has the name as
an inherited uninitialized data member:

  struct Base {
int x; // note: uninitialized field 'this->Base::x'
  };

  struct Derived : public Base {
int x = 5;
Derived() {}
  };

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

Modified:

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

cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
cfe/trunk/test/Analysis/cxx-uninitialized-object-inheritance.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=340272&r1=340271&r2=340272&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h 
(original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h 
Tue Aug 21 05:16:59 2018
@@ -32,10 +32,10 @@ class FieldNode {
 protected:
   const FieldRegion *FR;
 
-  ~FieldNode() = default;
+  /* non-virtual */ ~FieldNode() = default;
 
 public:
-  FieldNode(const FieldRegion *FR) : FR(FR) { assert(FR); }
+  FieldNode(const FieldRegion *FR) : FR(FR) {}
 
   FieldNode() = delete;
   FieldNode(const FieldNode &) = delete;
@@ -47,11 +47,21 @@ public:
   /// FoldingSet.
   void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddPointer(this); }
 
-  bool operator<(const FieldNode &Other) const { return FR < Other.FR; }
-  bool isSameRegion(const FieldRegion *OtherFR) const { return FR == OtherFR; }
+  // Helper method for uniqueing.
+  bool isSameRegion(const FieldRegion *OtherFR) const {
+// Special FieldNode descendants may wrap nullpointers -- we wouldn't like
+// to unique these objects.
+if (FR == nullptr)
+  return false;
+
+return FR == OtherFR;
+  }
 
   const FieldRegion *getRegion() const { return FR; }
-  const FieldDecl *getDecl() const { return FR->getDecl(); }
+  const FieldDecl *getDecl() const {
+assert(FR);
+return FR->getDecl();
+  }
 
   // When a fieldchain is printed (a list of FieldNode objects), it will have
   // the following format:
@@ -71,6 +81,8 @@ public:
   /// Print the separator. For example, fields may be separated with '.' or
   /// "->".
   virtual void printSeparator(llvm::raw_ostream &Out) const = 0;
+
+  virtual bool isBase() const { return false; }
 };
 
 /// Returns with Field's name. This is a helper function to get the correct 
name
@@ -94,15 +106,24 @@ private:
   FieldChain::Factory &ChainFactory;
   FieldChain Chain;
 
+  FieldChainInfo(FieldChain::Factory &F, FieldChain NewChain)
+  : FieldChainInfo(F) {
+Chain = NewChain;
+  }
+
 public:
   FieldChainInfo() = delete;
   FieldChainInfo(FieldChain::Factory &F) : ChainFactory(F) {}
   FieldChainInfo(const FieldChainInfo &Other) = default;
 
   template  FieldChainInfo add(const FieldNodeT &FN);
+  template  FieldChainInfo replaceHead(const FieldNodeT &FN);
 
   bool contains(const FieldRegion *FR) const;
+  bool isEmpty() const { return Chain.isEmpty(); }
+
   const FieldRegion *getUninitRegion() const;
+  const FieldNode &getHead() { return Chain.getHead(); }
   void printNoteMsg(llvm::raw_ostream &Out) const;
 };
 
@@ -250,6 +271,12 @@ inline FieldChainInfo FieldChainInfo::ad
   return NewChain;
 }
 
+template 
+inline FieldChainInfo FieldChainInfo::replaceHead(const FieldNodeT &FN) {
+  FieldChainInfo NewChain(ChainFactory, Chain.getTail());
+  return NewChain.add(FN);
+}
+
 } // end of namespace ento
 } // end of namespace clang
 

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp?rev=340272&r1=340271&r2=340272&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
 (original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
 Tue Aug 21 05:16:59 2018
@@ -93,6 +93,33 @@ public:
   }
 };
 
+/// Represents that the FieldNode that comes after this is

[PATCH] D50905: [analyzer][UninitializedObjectChecker] Explicit namespace resolution for inherited data members

2018-08-21 Thread Umann Kristóf via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC340272: [analyzer][UninitializedObjectChecker] Explicit 
namespace resolution for… (authored by Szelethus, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50905?vs=161271&id=161700#toc

Repository:
  rC Clang

https://reviews.llvm.org/D50905

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

Index: test/Analysis/cxx-uninitialized-object-inheritance.cpp
===
--- test/Analysis/cxx-uninitialized-object-inheritance.cpp
+++ test/Analysis/cxx-uninitialized-object-inheritance.cpp
@@ -35,7 +35,7 @@
 }
 
 class NonPolymorphicBaseClass2 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->NonPolymorphicBaseClass2::x'}}
 protected:
   int y;
 
@@ -62,7 +62,7 @@
   int x;
 
 protected:
-  int y; // expected-note{{uninitialized field 'this->y'}}
+  int y; // expected-note{{uninitialized field 'this->NonPolymorphicBaseClass3::y'}}
 public:
   NonPolymorphicBaseClass3() = default;
   NonPolymorphicBaseClass3(int) : x(7) {}
@@ -140,7 +140,7 @@
 }
 
 class PolymorphicRight1 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->PolymorphicRight1::x'}}
 protected:
   int y;
 
@@ -168,7 +168,7 @@
   int x;
 
 protected:
-  int y; // expected-note{{uninitialized field 'this->y'}}
+  int y; // expected-note{{uninitialized field 'this->PolymorphicBaseClass3::y'}}
 public:
   virtual ~PolymorphicBaseClass3() = default;
   PolymorphicBaseClass3() = default;
@@ -248,7 +248,7 @@
 }
 
 class VirtualPolymorphicRight1 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->VirtualPolymorphicRight1::x'}}
 protected:
   int y;
 
@@ -276,7 +276,7 @@
   int x;
 
 protected:
-  int y; // expected-note{{uninitialized field 'this->y'}}
+  int y; // expected-note{{uninitialized field 'this->VirtualPolymorphicBaseClass3::y'}}
 public:
   virtual ~VirtualPolymorphicBaseClass3() = default;
   VirtualPolymorphicBaseClass3() = default;
@@ -358,7 +358,7 @@
   Left2(int) : x(36) {}
 };
 struct Right2 {
-  int y; // expected-note{{uninitialized field 'this->y'}}
+  int y; // expected-note{{uninitialized field 'this->Right2::y'}}
   Right2() = default;
   Right2(int) : y(37) {}
 };
@@ -378,7 +378,7 @@
 }
 
 struct Left3 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->Left3::x'}}
   Left3() = default;
   Left3(int) : x(39) {}
 };
@@ -433,7 +433,7 @@
   Left5(int) : x(44) {}
 };
 struct Right5 {
-  int y; // expected-note{{uninitialized field 'this->y'}}
+  int y; // expected-note{{uninitialized field 'this->Right5::y'}}
   Right5() = default;
   Right5(int) : y(45) {}
 };
@@ -514,7 +514,7 @@
 }
 
 struct NonVirtualBase2 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->NonVirtualBase2::x'}}
   NonVirtualBase2() = default;
   NonVirtualBase2(int) : x(52) {}
 };
@@ -542,7 +542,7 @@
 }
 
 struct NonVirtualBase3 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->NonVirtualBase3::x'}}
   NonVirtualBase3() = default;
   NonVirtualBase3(int) : x(54) {}
 };
@@ -570,8 +570,8 @@
 }
 
 struct NonVirtualBase4 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
-  // expected-note@-1{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->NonVirtualBase4::x'}}
+  // expected-note@-1{{uninitialized field 'this->NonVirtualBase4::x'}}
   NonVirtualBase4() = default;
   NonVirtualBase4(int) : x(56) {}
 };
@@ -626,7 +626,7 @@
 }
 
 struct NonVirtualBase6 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->NonVirtualBase6::x'}}
   NonVirtualBase6() = default;
   NonVirtualBase6(int) : x(59) {}
 };
@@ -712,7 +712,7 @@
 }
 
 struct VirtualBase2 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->VirtualBase2::x'}}
   VirtualBase2() = default;
   VirtualBase2(int) : x(63) {}
 };
@@ -751,7 +751,7 @@
 }
 
 struct VirtualBase3 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->VirtualBase3::x'}}
   VirtualBase3() = default;
   VirtualBase3(int) : x(66) {}
 };
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers

[PATCH] D50905: [analyzer][UninitializedObjectChecker] Explicit namespace resolution for inherited data members

2018-08-21 Thread Umann Kristóf via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL340272: [analyzer][UninitializedObjectChecker] Explicit 
namespace resolution for… (authored by Szelethus, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50905?vs=161271&id=161701#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50905

Files:
  
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
  
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
  cfe/trunk/test/Analysis/cxx-uninitialized-object-inheritance.cpp

Index: cfe/trunk/test/Analysis/cxx-uninitialized-object-inheritance.cpp
===
--- cfe/trunk/test/Analysis/cxx-uninitialized-object-inheritance.cpp
+++ cfe/trunk/test/Analysis/cxx-uninitialized-object-inheritance.cpp
@@ -35,7 +35,7 @@
 }
 
 class NonPolymorphicBaseClass2 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->NonPolymorphicBaseClass2::x'}}
 protected:
   int y;
 
@@ -62,7 +62,7 @@
   int x;
 
 protected:
-  int y; // expected-note{{uninitialized field 'this->y'}}
+  int y; // expected-note{{uninitialized field 'this->NonPolymorphicBaseClass3::y'}}
 public:
   NonPolymorphicBaseClass3() = default;
   NonPolymorphicBaseClass3(int) : x(7) {}
@@ -140,7 +140,7 @@
 }
 
 class PolymorphicRight1 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->PolymorphicRight1::x'}}
 protected:
   int y;
 
@@ -168,7 +168,7 @@
   int x;
 
 protected:
-  int y; // expected-note{{uninitialized field 'this->y'}}
+  int y; // expected-note{{uninitialized field 'this->PolymorphicBaseClass3::y'}}
 public:
   virtual ~PolymorphicBaseClass3() = default;
   PolymorphicBaseClass3() = default;
@@ -248,7 +248,7 @@
 }
 
 class VirtualPolymorphicRight1 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->VirtualPolymorphicRight1::x'}}
 protected:
   int y;
 
@@ -276,7 +276,7 @@
   int x;
 
 protected:
-  int y; // expected-note{{uninitialized field 'this->y'}}
+  int y; // expected-note{{uninitialized field 'this->VirtualPolymorphicBaseClass3::y'}}
 public:
   virtual ~VirtualPolymorphicBaseClass3() = default;
   VirtualPolymorphicBaseClass3() = default;
@@ -358,7 +358,7 @@
   Left2(int) : x(36) {}
 };
 struct Right2 {
-  int y; // expected-note{{uninitialized field 'this->y'}}
+  int y; // expected-note{{uninitialized field 'this->Right2::y'}}
   Right2() = default;
   Right2(int) : y(37) {}
 };
@@ -378,7 +378,7 @@
 }
 
 struct Left3 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->Left3::x'}}
   Left3() = default;
   Left3(int) : x(39) {}
 };
@@ -433,7 +433,7 @@
   Left5(int) : x(44) {}
 };
 struct Right5 {
-  int y; // expected-note{{uninitialized field 'this->y'}}
+  int y; // expected-note{{uninitialized field 'this->Right5::y'}}
   Right5() = default;
   Right5(int) : y(45) {}
 };
@@ -514,7 +514,7 @@
 }
 
 struct NonVirtualBase2 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->NonVirtualBase2::x'}}
   NonVirtualBase2() = default;
   NonVirtualBase2(int) : x(52) {}
 };
@@ -542,7 +542,7 @@
 }
 
 struct NonVirtualBase3 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->NonVirtualBase3::x'}}
   NonVirtualBase3() = default;
   NonVirtualBase3(int) : x(54) {}
 };
@@ -570,8 +570,8 @@
 }
 
 struct NonVirtualBase4 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
-  // expected-note@-1{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->NonVirtualBase4::x'}}
+  // expected-note@-1{{uninitialized field 'this->NonVirtualBase4::x'}}
   NonVirtualBase4() = default;
   NonVirtualBase4(int) : x(56) {}
 };
@@ -626,7 +626,7 @@
 }
 
 struct NonVirtualBase6 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->NonVirtualBase6::x'}}
   NonVirtualBase6() = default;
   NonVirtualBase6(int) : x(59) {}
 };
@@ -712,7 +712,7 @@
 }
 
 struct VirtualBase2 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->VirtualBase2::x'}}
   VirtualBase2() = default;
   VirtualBase2(int) : x(63) {}
 };
@@ -751,7 +751,7 @@
 }
 
 struct VirtualBase3 {
-  int x; // expected-note{{uninitialized field 'this->x'}}
+  int x; // expected-note{{uninitialized field 'this->VirtualBase3::x'}}
   VirtualBase3() = default;
   VirtualBase3(int) : x(66) {}
 };
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
--

[PATCH] D51001: [ASTImporter] Add test for CXXForRangeStmt

2018-08-21 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: rnkovacs.

LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D51001



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


[PATCH] D51026: [CodeGen] Implicitly set stackrealign on the main function, if custom stack alignment is used

2018-08-21 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: lib/CodeGen/CodeGenFunction.cpp:989
+CGM.getCodeGenOpts().StackAlignment)
+  Fn->addFnAttr("stackrealign");
+

Is there not an attribute name for this already in LLVM?  I guess I'm otherwise 
fine with this, but would want @craig.topper or @rnk to confirm that we're Ok 
just sending this string as an attribute.


Repository:
  rC Clang

https://reviews.llvm.org/D51026



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


[PATCH] D51026: [CodeGen] Implicitly set stackrealign on the main function, if custom stack alignment is used

2018-08-21 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added inline comments.



Comment at: lib/CodeGen/CodeGenFunction.cpp:989
+CGM.getCodeGenOpts().StackAlignment)
+  Fn->addFnAttr("stackrealign");
+

erichkeane wrote:
> Is there not an attribute name for this already in LLVM?  I guess I'm 
> otherwise fine with this, but would want @craig.topper or @rnk to confirm 
> that we're Ok just sending this string as an attribute.
I don't see one in llvm/include/llvm/IR/Attributes.td at least, and all other 
occurrances in clang just use the plain string.


Repository:
  rC Clang

https://reviews.llvm.org/D51026



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


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

2018-08-21 Thread Dmitry via Phabricator via cfe-commits
Dmitry.Kozhevnikov added a comment.

In https://reviews.llvm.org/D50993#1207357, @ilya-biryukov wrote:

> 1. Can we put the helper that executes the tasks asynchronously with a stack 
> size into llvm's threading library (i.e. somewhere close to 
> `llvm::execute_on_thread`?) All platform-specific code is already there, we 
> can reuse most of the implementation too.


Do you think it would be OK to have pthreads-only version there, with 
`std::thread` fallback on Windows? I'd like to avoid writing a Windows-only 
implementation myself, as it's a bit alien for me.

> 2. I wonder if we should expose the stack size as an option to clangd?

Do you expect it would be ever used?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50993



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


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

2018-08-21 Thread Simon Atanasyan via Phabricator via cfe-commits
atanasyan accepted this revision.
atanasyan added a comment.
This revision is now accepted and ready to land.

LGTM


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] D50564: Add support for SEH unwinding on Windows.

2018-08-21 Thread Kristina Brooks via Phabricator via cfe-commits
kristina added a comment.

In https://reviews.llvm.org/D50564#1206393, @mstorsjo wrote:

> In https://reviews.llvm.org/D50564#1206370, @cdavis5x wrote:
>
> > In https://reviews.llvm.org/D50564#1206302, @kristina wrote:
> >
> > > I'm all for this change except the core issue is that you're using 
> > > libunwind as a shim around the actual unwinding API provided by Windows. 
> > > It would be nice to have something that did not have to do that and was 
> > > capable of performing unwinding of SEH-style exceptions without needing 
> > > additional runtime support.
> >
> >
> > It would be nice, but that would require extra work. We'd have to implement 
> > reading and interpreting unwind codes, and calling any handlers present at 
> > each frame (which all have a different calling convention from Itanium 
> > handlers), and handling chained unwind info... Or we could use the 
> > implementation that MS provided to us for free--and which gets loaded into 
> > every process anyway by virtue of being in NTDLL, and which is extremely 
> > well tested. Given all that, I'm wondering what implementing all that 
> > ourselves would gain us. I suppose we could eventually do all that, but for 
> > now, I think this is outside the scope of my change.
>
>
> +1. I guess such a library would be very nice to have, but from the point of 
> view of implementing exception handling, using the underlying APIs probably 
> is the way to go. The other question, as posted before, is whether we want to 
> wrap the whole CONTEXT structs in the UnwindCursor class and expose it via 
> the unw_* set of APIs. It gives quite a significant amount of extra code here 
> compared to libgcc's unwind-seh.c which is <500 loc altogether, providing 
> only the _Unwind_* API, implementing it directly with the Windows Rtl* APIs 
> from ntdll. That would give only a partial libunwind, with only the higher 
> level API available, but that's the only part used for exception handling at 
> least.


That still makes the underlying assumption that SEH is exclusive to Windows 
(and by that virtue PE/COFF) which doesn't necessarily hold true. There is also 
the issue of generic names like `_LIBUNWIND_SUPPORT_SEH_UNWIND` to guard 
includes of Windows specific headers which ties into my previous point. Would 
it be possible to somehow abstract away the Windows runtime function call and 
Windows specific header includes, even if it's via CMake options.

I'm raising this issue as there are other implementations of SEH that share the 
same (or extremely similar, depending on SDK versions) structures, but would 
not have the Windows headers available when compiling libunwind, and while the 
runtime function call to the `Rtl*` API is easy to change later on, separating 
rest of it from Windows headers is slightly messier.

Would it, at very least, be possible to decouple most of it from a dependency 
on Windows headers?

Would be much appreciated, as I'm going through a backlog of patches I'd like 
to put up for review one of which includes SEH for x86_64 Linux (ELF) 
implemented using RT signals and additional compiler runtime glue code, 
currently residing within compiler-rt builtins (one of the issues I want to 
address before). It's quite a wonky ABI since I tried to keep as much 
compatible with 64-bit Windows SEH (`.xdata` variation) in terms of frame 
lowering etc, but run-time mechanisms differ vastly for obvious reasons.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564



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


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

2018-08-21 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

In https://reviews.llvm.org/D50993#1207464, @Dmitry.Kozhevnikov wrote:

> Do you think it would be OK to have pthreads-only version there, with 
> `std::thread` fallback on Windows? I'd like to avoid writing a Windows-only 
> implementation myself, as it's a bit alien for me.


Yeah, we should probably do both Unix and Windows there. My guess is that the 
Windows implementation shouldn't be too hard, since most of the code is already 
there.
In any way, it's fine to start with a Unix-only version and a stub in Windows 
and figuring out the implementation during the review. Happy to look into 
Windows myself, it shouldn't be too hard :-)

>> 2. I wonder if we should expose the stack size as an option to clangd?
> 
> Do you expect it would be ever used?

Not frequently, but I don't see how we can figure out good defaults there. It's 
one of those things where the guideline is "as small as possible, as long as it 
does not crash clangd".
An option could provide a simple workaround for cases when clangd is actually 
crashing.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50993



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


[PATCH] D51020: MultiSource/Benchmarks/DOE-ProxyApps-C++/CLAMP: Fix build with newer libstdc++

2018-08-21 Thread Brian Homerding via Phabricator via cfe-commits
homerdin accepted this revision.
homerdin added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks for fixing this


Repository:
  rT test-suite

https://reviews.llvm.org/D51020



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


[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier

2018-08-21 Thread Marco Elver via Phabricator via cfe-commits
melver created this revision.
Herald added a subscriber: cfe-commits.

This fixes formatting namespaces with preceding 'inline' and 'export' (Modules 
TS) specifiers.

This change fixes namespaces not being identified as such with preceding 
'inline' or 'export' specifiers.

Motivation: I was experimenting with the Modules TS (-fmodules-ts) and found it 
would be useful if clang-format would correctly format 'export namespace'. 
While making the changes, I noticed that similar issues still exist with 
'inline namespace', and addressed them as well.


Repository:
  rC Clang

https://reviews.llvm.org/D51036

Files:
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -200,6 +200,24 @@
"inti;\n"
"}",
getGoogleStyle()));
+  EXPECT_EQ("inline namespace N {\n"
+"\n"
+"int i;\n"
+"}",
+format("inline namespace N {\n"
+   "\n"
+   "inti;\n"
+   "}",
+   getGoogleStyle()));
+  EXPECT_EQ("export namespace N {\n"
+"\n"
+"int i;\n"
+"}",
+format("export namespace N {\n"
+   "\n"
+   "inti;\n"
+   "}",
+   getGoogleStyle()));
   EXPECT_EQ("extern /**/ \"C\" /**/ {\n"
 "\n"
 "int i;\n"
@@ -1573,6 +1591,11 @@
"void f() { f(); }\n"
"}",
LLVMWithNoNamespaceFix);
+  verifyFormat("export namespace X {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
   verifyFormat("using namespace some_namespace;\n"
"class A {};\n"
"void f() { f(); }",
@@ -7556,6 +7579,9 @@
   verifyFormat("inline namespace Foo\n"
"{};",
Style);
+  verifyFormat("export namespace Foo\n"
+   "{};",
+   Style);
   verifyFormat("namespace Foo\n"
"{\n"
"void Bar();\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1063,6 +1063,13 @@
   parseJavaScriptEs6ImportExport();
   return;
 }
+if (Style.isCpp()) {
+  nextToken();
+  if (FormatTok->Tok.is(tok::kw_namespace)) {
+parseNamespace();
+return;
+  }
+}
 break;
   case tok::identifier:
 if (FormatTok->is(TT_ForEachMacro)) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -529,7 +529,10 @@
 Tok->SpacesRequiredBefore = 0;
 Tok->CanBreakBefore = true;
 return 1;
-  } else if (Limit != 0 && !Line.startsWith(tok::kw_namespace) &&
+  } else if (Limit != 0 &&
+ !(Line.startsWith(tok::kw_namespace) ||
+   Line.startsWith(tok::kw_inline, tok::kw_namespace) ||
+   Line.startsWith(tok::kw_export, tok::kw_namespace)) &&
  !startsExternCBlock(Line)) {
 // We don't merge short records.
 FormatToken *RecordTok = Line.First;
@@ -1154,7 +1157,9 @@
   // Remove empty lines after "{".
   if (!Style.KeepEmptyLinesAtTheStartOfBlocks && PreviousLine &&
   PreviousLine->Last->is(tok::l_brace) &&
-  PreviousLine->First->isNot(tok::kw_namespace) &&
+  !(PreviousLine->startsWith(tok::kw_namespace) ||
+PreviousLine->startsWith(tok::kw_inline, tok::kw_namespace) ||
+PreviousLine->startsWith(tok::kw_export, tok::kw_namespace)) &&
   !startsExternCBlock(*PreviousLine))
 Newlines = 1;
 
Index: lib/Format/NamespaceEndCommentsFixer.cpp
===
--- lib/Format/NamespaceEndCommentsFixer.cpp
+++ lib/Format/NamespaceEndCommentsFixer.cpp
@@ -125,8 +125,8 @@
 if (StartLineIndex > 0)
   NamespaceTok = AnnotatedLines[StartLineIndex - 1]->First;
   }
-  // Detect "(inline)? namespace" in the beginning of a line.
-  if (NamespaceTok->is(tok::kw_inline))
+  // Detect "(inline|export)? namespace" in the beginning of a line.
+  if (NamespaceTok->is(tok::kw_inline) || NamespaceTok->is(tok::kw_export))
 NamespaceTok = NamespaceTok->getNextNonComment();
   if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace))
 return nullptr;
Index: lib/Format/FormatToken.h
==

[PATCH] D47814: Teach libc++ to use native NetBSD's max_align_t

2018-08-21 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments.



Comment at: include/stddef.h:55
 
 // Re-use the compiler's  max_align_t where possible.
 #if !defined(__CLANG_MAX_ALIGN_T_DEFINED) && !defined(_GCC_MAX_ALIGN_T) && \

chandlerc wrote:
> Unrelated to your patch, but this comment is now amazingly out of place.
Nope, based on `git blame` I think it's still where it should be.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47814



___
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

2018-08-21 Thread Hugo Gonzalez via Phabricator via cfe-commits
hugoeg updated this revision to Diff 161714.
hugoeg marked an inline comment as done.
hugoeg added a comment.

All comments have been address and documentation has been added to matcher


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/external-file.h
  test/clang-tidy/Inputs/absl/strings/internal-file.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,39 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t,  -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-internal-deps' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:1:23: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil [abseil-no-internal-deps]
+
+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/internal-file.h
===
--- test/clang-tidy/Inputs/absl/strings/internal-file.h
+++ test/clang-tidy/Inputs/absl/strings/internal-file.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: test/clang-tidy/Inputs/absl/external-file.h
===
--- test/clang-tidy/Inputs/absl/external-file.h
+++ test/clang-tidy/Inputs/absl/external-file.h
@@ -0,0 +1 @@
+void DirectAcess2() { absl::strings_internal::InternalFunction(); }
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
+===
+
+Warns 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://abse

r340277 - [ASTImporter] Adding some friend function related unittests.

2018-08-21 Thread Balazs Keri via cfe-commits
Author: balazske
Date: Tue Aug 21 07:32:21 2018
New Revision: 340277

URL: http://llvm.org/viewvc/llvm-project?rev=340277&view=rev
Log:
[ASTImporter] Adding some friend function related unittests.

Reviewers: a.sidorin, a_sidorin

Reviewed By: a_sidorin

Subscribers: a_sidorin, martong, cfe-commits

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

Modified:
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=340277&r1=340276&r2=340277&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Tue Aug 21 07:32:21 2018
@@ -2274,6 +2274,211 @@ TEST_P(ImportFriendFunctions, ImportFrie
   EXPECT_EQ(ImportedD1->getPreviousDecl(), ImportedD);
 }
 
+TEST_P(ImportFriendFunctions, Lookup) {
+  auto FunctionPattern = functionDecl(hasName("f"));
+  auto ClassPattern = cxxRecordDecl(hasName("X"));
+
+  TranslationUnitDecl *FromTU =
+  getTuDecl("struct X { friend void f(); };", Lang_CXX, "input0.cc");
+  auto *FromD = FirstDeclMatcher().match(FromTU, 
FunctionPattern);
+  ASSERT_TRUE(FromD->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  ASSERT_FALSE(FromD->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  {
+auto FromName = FromD->getDeclName();
+auto *Class = FirstDeclMatcher().match(FromTU, 
ClassPattern);
+auto LookupRes = Class->noload_lookup(FromName);
+ASSERT_EQ(LookupRes.size(), 0u);
+LookupRes = FromTU->noload_lookup(FromName);
+ASSERT_EQ(LookupRes.size(), 1u);
+  }
+
+  auto *ToD = cast(Import(FromD, Lang_CXX));
+  auto ToName = ToD->getDeclName();
+
+  TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  auto *Class = FirstDeclMatcher().match(ToTU, ClassPattern);
+  auto LookupRes = Class->noload_lookup(ToName);
+  EXPECT_EQ(LookupRes.size(), 0u);
+  LookupRes = ToTU->noload_lookup(ToName);
+  EXPECT_EQ(LookupRes.size(), 1u);
+
+  EXPECT_EQ(DeclCounter().match(ToTU, FunctionPattern), 1u);
+  auto *To0 = FirstDeclMatcher().match(ToTU, FunctionPattern);
+  EXPECT_TRUE(To0->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  EXPECT_FALSE(To0->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+}
+
+TEST_P(ImportFriendFunctions, DISABLED_LookupWithProtoAfter) {
+  auto FunctionPattern = functionDecl(hasName("f"));
+  auto ClassPattern = cxxRecordDecl(hasName("X"));
+
+  TranslationUnitDecl *FromTU = getTuDecl(
+  "struct X { friend void f(); };"
+  // This proto decl makes f available to normal
+  // lookup, otherwise it is hidden.
+  // Normal C++ lookup (implemented in
+  // `clang::Sema::CppLookupName()` and in `LookupDirect()`)
+  // returns the found `NamedDecl` only if the set IDNS is matched
+  "void f();",
+  Lang_CXX, "input0.cc");
+  auto *FromFriend =
+  FirstDeclMatcher().match(FromTU, FunctionPattern);
+  auto *FromNormal =
+  LastDeclMatcher().match(FromTU, FunctionPattern);
+  ASSERT_TRUE(FromFriend->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  ASSERT_FALSE(FromFriend->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  ASSERT_FALSE(FromNormal->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  ASSERT_TRUE(FromNormal->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+
+  auto FromName = FromFriend->getDeclName();
+  auto *FromClass =
+  FirstDeclMatcher().match(FromTU, ClassPattern);
+  auto LookupRes = FromClass->noload_lookup(FromName);
+  ASSERT_EQ(LookupRes.size(), 0u);
+  LookupRes = FromTU->noload_lookup(FromName);
+  ASSERT_EQ(LookupRes.size(), 1u);
+
+  auto *ToFriend = cast(Import(FromFriend, Lang_CXX));
+  auto ToName = ToFriend->getDeclName();
+
+  TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  auto *ToClass = FirstDeclMatcher().match(ToTU, ClassPattern);
+  LookupRes = ToClass->noload_lookup(ToName);
+  EXPECT_EQ(LookupRes.size(), 0u);
+  LookupRes = ToTU->noload_lookup(ToName);
+  // Test is disabled because this result is 2.
+  EXPECT_EQ(LookupRes.size(), 1u);
+
+  ASSERT_EQ(DeclCounter().match(ToTU, FunctionPattern), 2u);
+  ToFriend = FirstDeclMatcher().match(ToTU, FunctionPattern);
+  auto *ToNormal = LastDeclMatcher().match(ToTU, 
FunctionPattern);
+  EXPECT_TRUE(ToFriend->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  EXPECT_FALSE(ToFriend->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  EXPECT_FALSE(ToNormal->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  EXPECT_TRUE(ToNormal->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+}
+
+TEST_P(ImportFriendFunctions, LookupWithProtoBefore) {
+  auto FunctionPattern = functionDecl(hasName("f"));
+  auto ClassPattern = cxxRecordDecl(hasName("X"));
+
+  TranslationUnitDecl *FromTU = getTuDecl(
+  "void f();"
+  "struct X { friend void f(); };",
+  Lang_CXX, "input0.cc");
+  auto 

[PATCH] D49798: [ASTImporter] Adding some friend function related unittests.

2018-08-21 Thread Balázs Kéri via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC340277: [ASTImporter] Adding some friend function related 
unittests. (authored by balazske, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D49798?vs=160972&id=161715#toc

Repository:
  rC Clang

https://reviews.llvm.org/D49798

Files:
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -2274,6 +2274,211 @@
   EXPECT_EQ(ImportedD1->getPreviousDecl(), ImportedD);
 }
 
+TEST_P(ImportFriendFunctions, Lookup) {
+  auto FunctionPattern = functionDecl(hasName("f"));
+  auto ClassPattern = cxxRecordDecl(hasName("X"));
+
+  TranslationUnitDecl *FromTU =
+  getTuDecl("struct X { friend void f(); };", Lang_CXX, "input0.cc");
+  auto *FromD = FirstDeclMatcher().match(FromTU, FunctionPattern);
+  ASSERT_TRUE(FromD->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  ASSERT_FALSE(FromD->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  {
+auto FromName = FromD->getDeclName();
+auto *Class = FirstDeclMatcher().match(FromTU, ClassPattern);
+auto LookupRes = Class->noload_lookup(FromName);
+ASSERT_EQ(LookupRes.size(), 0u);
+LookupRes = FromTU->noload_lookup(FromName);
+ASSERT_EQ(LookupRes.size(), 1u);
+  }
+
+  auto *ToD = cast(Import(FromD, Lang_CXX));
+  auto ToName = ToD->getDeclName();
+
+  TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  auto *Class = FirstDeclMatcher().match(ToTU, ClassPattern);
+  auto LookupRes = Class->noload_lookup(ToName);
+  EXPECT_EQ(LookupRes.size(), 0u);
+  LookupRes = ToTU->noload_lookup(ToName);
+  EXPECT_EQ(LookupRes.size(), 1u);
+
+  EXPECT_EQ(DeclCounter().match(ToTU, FunctionPattern), 1u);
+  auto *To0 = FirstDeclMatcher().match(ToTU, FunctionPattern);
+  EXPECT_TRUE(To0->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  EXPECT_FALSE(To0->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+}
+
+TEST_P(ImportFriendFunctions, DISABLED_LookupWithProtoAfter) {
+  auto FunctionPattern = functionDecl(hasName("f"));
+  auto ClassPattern = cxxRecordDecl(hasName("X"));
+
+  TranslationUnitDecl *FromTU = getTuDecl(
+  "struct X { friend void f(); };"
+  // This proto decl makes f available to normal
+  // lookup, otherwise it is hidden.
+  // Normal C++ lookup (implemented in
+  // `clang::Sema::CppLookupName()` and in `LookupDirect()`)
+  // returns the found `NamedDecl` only if the set IDNS is matched
+  "void f();",
+  Lang_CXX, "input0.cc");
+  auto *FromFriend =
+  FirstDeclMatcher().match(FromTU, FunctionPattern);
+  auto *FromNormal =
+  LastDeclMatcher().match(FromTU, FunctionPattern);
+  ASSERT_TRUE(FromFriend->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  ASSERT_FALSE(FromFriend->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  ASSERT_FALSE(FromNormal->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  ASSERT_TRUE(FromNormal->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+
+  auto FromName = FromFriend->getDeclName();
+  auto *FromClass =
+  FirstDeclMatcher().match(FromTU, ClassPattern);
+  auto LookupRes = FromClass->noload_lookup(FromName);
+  ASSERT_EQ(LookupRes.size(), 0u);
+  LookupRes = FromTU->noload_lookup(FromName);
+  ASSERT_EQ(LookupRes.size(), 1u);
+
+  auto *ToFriend = cast(Import(FromFriend, Lang_CXX));
+  auto ToName = ToFriend->getDeclName();
+
+  TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  auto *ToClass = FirstDeclMatcher().match(ToTU, ClassPattern);
+  LookupRes = ToClass->noload_lookup(ToName);
+  EXPECT_EQ(LookupRes.size(), 0u);
+  LookupRes = ToTU->noload_lookup(ToName);
+  // Test is disabled because this result is 2.
+  EXPECT_EQ(LookupRes.size(), 1u);
+
+  ASSERT_EQ(DeclCounter().match(ToTU, FunctionPattern), 2u);
+  ToFriend = FirstDeclMatcher().match(ToTU, FunctionPattern);
+  auto *ToNormal = LastDeclMatcher().match(ToTU, FunctionPattern);
+  EXPECT_TRUE(ToFriend->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  EXPECT_FALSE(ToFriend->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  EXPECT_FALSE(ToNormal->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  EXPECT_TRUE(ToNormal->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+}
+
+TEST_P(ImportFriendFunctions, LookupWithProtoBefore) {
+  auto FunctionPattern = functionDecl(hasName("f"));
+  auto ClassPattern = cxxRecordDecl(hasName("X"));
+
+  TranslationUnitDecl *FromTU = getTuDecl(
+  "void f();"
+  "struct X { friend void f(); };",
+  Lang_CXX, "input0.cc");
+  auto *FromNormal =
+  FirstDeclMatcher().match(FromTU, FunctionPattern);
+  auto *FromFriend =
+  LastDeclMatcher().match(FromTU, FunctionPattern);
+  ASSERT_FALSE(FromNormal->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  ASSERT_TRUE(FromNormal->isInIdentifierNamesp

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

2018-08-21 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 161717.
ilya-biryukov added a comment.

- Provide noop callbacks implementation by default.
- Update docs.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50847

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  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;
@@ -44,8 +45,7 @@
 
 TEST_F(TUSchedulerTests, MissingFiles) {
   TUScheduler S(getDefaultAsyncThreadsCount(),
-/*StorePreamblesInMemory=*/true,
-/*PreambleParsedCallback=*/nullptr,
+/*StorePreamblesInMemory=*/true, noopParsingCallbacks(),
 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
 ASTRetentionPolicy());
 
@@ -101,8 +101,7 @@
 Notification Ready;
 TUScheduler S(
 getDefaultAsyncThreadsCount(),
-/*StorePreamblesInMemory=*/true,
-/*PreambleParsedCallback=*/nullptr,
+/*StorePreamblesInMemory=*/true, noopParsingCallbacks(),
 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
 ASTRetentionPolicy());
 auto Path = testPath("foo.cpp");
@@ -130,8 +129,7 @@
   std::atomic CallbackCount(0);
   {
 TUScheduler S(getDefaultAsyncThreadsCount(),
-  /*StorePreamblesInMemory=*/true,
-  /*PreambleParsedCallback=*/nullptr,
+  /*StorePreamblesInMemory=*/true, noopParsingCallbacks(),
   /*UpdateDebounce=*/std::chrono::seconds(1),
   ASTRetentionPolicy());
 // FIXME: we could probably use timeouts lower than 1 second here.
@@ -162,8 +160,7 @@
   // Run TUScheduler and collect some stats.
   {
 TUScheduler S(getDefaultAsyncThreadsCount(),
-  /*StorePreamblesInMemory=*/true,
-  /*PreambleParsedCallback=*/nullptr,
+  /*StorePreamblesInMemory=*/true, noopParsingCallbacks(),
   /*UpdateDebounce=*/std::chrono::milliseconds(50),
   ASTRetentionPolicy());
 
@@ -261,7 +258,7 @@
   Policy.MaxRetainedASTs = 2;
   TUScheduler S(
   /*AsyncThreadsCount=*/1, /*StorePreambleInMemory=*/true,
-  PreambleParsedCallback(),
+  noopParsingCallbacks(),
   /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), Policy);
 
   llvm::StringLiteral SourceContents = R"cpp(
@@ -313,7 +310,7 @@
   // the same time. All reads should get the same non-null preamble.
   TUScheduler S(
   /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
-  PreambleParsedCallback(),
+  noopParsingCallbacks(),
   /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
   ASTRetentionPolicy());
   auto Foo = testPath("foo.cpp");
@@ -346,7 +343,7 @@
 TEST_F(TUSchedulerTests, NoopOnEmptyChanges) {
   TUScheduler S(
   /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
-  /*StorePreambleInMemory=*/true, PreambleParsedCallback(),
+  /*StorePreambleInMemory=*/true, noopParsingCallbacks(),
   /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
   ASTRetentionPolicy());
 
@@ -399,7 +396,7 @@
 TEST_F(TUSchedulerTests, NoChangeDiags) {
   TUScheduler S(
   /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
-  /*StorePreambleInMemory=*/true, PreambleParsedCallback(),
+  /*StorePreambleInMemory=*/true, noopParsingCallbacks(),
   /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
   ASTRetentionPolicy());
 
@@ -430,5 +427,6 @@
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1)));
 }
 
+} // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -51,6 +51,30 @@
   unsigned MaxRetainedASTs = 3;
 };
 
+class ParsingCallbacks {
+public:
+  virtual ~ParsingCallbacks() = default;
+
+  /// Called on the AST that was built for emitting the preamble. The built AST
+  /// contains only AST nodes from the #include directives at the start of the
+  /// file. AST node in the current file should be observed on onMainAST call.
+  virtual void onPreambleAST(PathRef Path, ASTContext &Ctx,
+ std::shared_ptr PP) {}
+  /// Called on the AST built for the file itself. Note that preamble AST nodes
+  /// are not deserialized and should be processed in the onPreambleAST call
+  /// instead.
+  /// The \p AST always contains all AST nodes for the main file itself, an

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

2018-08-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 161716.
kadircet marked 4 inline comments as done.
kadircet added a comment.

- Get rid of CancellationToken && Resolve discussions.


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,74 @@
+#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 = createTaskHandle();
+  WithContext ContextWithCancellation(setCurrentTask(TH));
+  EXPECT_FALSE(isCancelled());
+  TH->cancel();
+  EXPECT_TRUE(isCancelled());
+}
+
+TEST(CancellationTest, TaskTestHandleDiesContextLives) {
+  llvm::Optional ContextWithCancellation;
+  {
+TaskHandle TH = createTaskHandle();
+ContextWithCancellation.emplace(setCurrentTask(TH));
+EXPECT_FALSE(isCancelled());
+TH->cancel();
+EXPECT_TRUE(isCancelled());
+  }
+  EXPECT_TRUE(isCancelled());
+}
+
+TEST(CancellationTest, TaskContextDiesHandleLives) {
+  TaskHandle TH = createTaskHandle();
+  {
+WithContext ContextWithCancellation(setCurrentTask(TH));
+EXPECT_FALSE(isCancelled());
+TH->cancel();
+EXPECT_TRUE(isCancelled());
+  }
+  // Still should be able to cancel without any problems.
+  TH->cancel();
+}
+
+TEST(CancellationTest, CancellationToken) {
+  TaskHandle TH = createTaskHandle();
+  WithContext ContextWithCancellation(setCurrentTask(TH));
+  const auto &CT = getCurrentTask();
+  EXPECT_FALSE(CT.isCancelled());
+  TH->cancel();
+  EXPECT_TRUE(CT.isCancelled());
+}
+
+TEST(CancellationTest, AsynCancellationTest) {
+  std::atomic HasCancelled(false);
+  Notification Cancelled;
+  auto TaskToBeCancelled = [&](ConstTaskHandle CT) {
+WithContext ContextGuard(setCurrentTask(std::move(CT)));
+Cancelled.wait();
+HasCancelled = isCancelled();
+  };
+  TaskHandle TH = createTaskHandle();
+  std::thread AsyncTask(TaskToBeCancelled, TH);
+  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
+  /// out and always use a string.
+  std::string ID;
+};
+llvm::json::Value toJSON(const CancelParams &);
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const CancelParams &);
+bool fromJSON(const llvm::json::Value &, CancelParams &);
+
 } // namespace clangd
 } // namespace clang
 
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -615,5 +615,30 @@

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

2018-08-21 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



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

hokein wrote:
> 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?
I suggest we add an extra method to `DynamicIndex` that we call when the file 
is closed. I don't think it's intentional that we don't clean up the index for 
the closed files.
Not sure what's the best way to handle invalid ASTs, but we're never calling 
the current callback with `nullptr` anyway, so I suggest we keep it a reference 
to better reflect that callbacks don't actually handle any errors for us.


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


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

2018-08-21 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 161718.
ilya-biryukov added a comment.

- Use noop callbacks from the parent revision


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50889

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h

Index: clangd/index/FileIndex.h
===
--- clangd/index/FileIndex.h
+++ clangd/index/FileIndex.h
@@ -64,7 +64,11 @@
   /// nullptr, this removes all symbols in the file.
   /// If \p AST is not null, \p PP cannot be null and it should be the
   /// preprocessor that was used to build \p AST.
-  void update(PathRef Path, ASTContext *AST, std::shared_ptr PP);
+  /// If \p TopLevelDecls is set, only these decls are indexed. Otherwise, all
+  /// top level decls obtained from \p AST are indexed.
+  void
+  update(PathRef Path, ASTContext *AST, std::shared_ptr PP,
+ llvm::Optional> TopLevelDecls = llvm::None);
 
   bool
   fuzzyFind(const FuzzyFindRequest &Req,
@@ -86,8 +90,12 @@
 /// Retrieves namespace and class level symbols in \p AST.
 /// Exposed to assist in unit tests.
 /// If URISchemes is empty, the default schemes in SymbolCollector will be used.
-SymbolSlab indexAST(ASTContext &AST, std::shared_ptr PP,
-llvm::ArrayRef URISchemes = {});
+/// If \p TopLevelDecls is set, only these decls are indexed. Otherwise, all top
+/// level decls obtained from \p AST are indexed.
+SymbolSlab
+indexAST(ASTContext &AST, std::shared_ptr PP,
+ llvm::Optional> TopLevelDecls = llvm::None,
+ llvm::ArrayRef URISchemes = {});
 
 } // namespace clangd
 } // namespace clang
Index: clangd/index/FileIndex.cpp
===
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -8,15 +8,16 @@
 //===--===//
 
 #include "FileIndex.h"
-#include "SymbolCollector.h"
 #include "../Logger.h"
+#include "SymbolCollector.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Lex/Preprocessor.h"
 
 namespace clang {
 namespace clangd {
 
 SymbolSlab indexAST(ASTContext &AST, std::shared_ptr PP,
+llvm::Optional> TopLevelDecls,
 llvm::ArrayRef URISchemes) {
   SymbolCollector::Options CollectorOpts;
   // FIXME(ioeric): we might also want to collect include headers. We would need
@@ -38,10 +39,14 @@
   index::IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly;
   IndexOpts.IndexFunctionLocals = false;
 
-  std::vector TopLevelDecls(
-  AST.getTranslationUnitDecl()->decls().begin(),
-  AST.getTranslationUnitDecl()->decls().end());
-  index::indexTopLevelDecls(AST, TopLevelDecls, Collector, IndexOpts);
+  std::vector DeclsToIndex;
+  if (TopLevelDecls)
+DeclsToIndex.assign(TopLevelDecls->begin(), TopLevelDecls->end());
+  else
+DeclsToIndex.assign(AST.getTranslationUnitDecl()->decls().begin(),
+AST.getTranslationUnitDecl()->decls().end());
+
+  index::indexTopLevelDecls(AST, DeclsToIndex, Collector, IndexOpts);
 
   return Collector.takeSymbols();
 }
@@ -81,13 +86,14 @@
 }
 
 void FileIndex::update(PathRef Path, ASTContext *AST,
-   std::shared_ptr PP) {
+   std::shared_ptr PP,
+   llvm::Optional> TopLevelDecls) {
   if (!AST) {
 FSymbols.update(Path, nullptr);
   } else {
 assert(PP);
 auto Slab = llvm::make_unique();
-*Slab = indexAST(*AST, PP, URISchemes);
+*Slab = indexAST(*AST, PP, TopLevelDecls, URISchemes);
 FSymbols.update(Path, std::move(Slab));
   }
   auto Symbols = FSymbols.allSymbols();
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -99,6 +99,7 @@
   /// synchronize access to shared state.
   ClangdServer(GlobalCompilationDatabase &CDB, FileSystemProvider &FSProvider,
DiagnosticsConsumer &DiagConsumer, const Options &Opts);
+  ~ClangdServer();
 
   /// Set the root path of the workspace.
   void setRootPath(PathRef RootPath);
@@ -200,6 +201,7 @@
   formatCode(llvm::StringRef Code, PathRef File,
  ArrayRef Ranges);
 
+  class DynamicIndex;
   typedef uint64_t DocVersion;
 
   void consumeDiagnostics(PathRef File, DocVersion Version,
@@ -217,15 +219,14 @@
   Path ResourceDir;
   // The index used to look up symbols. This could be:
   //   - null (all index functionality is optional)
-  //   - the dynamic index owned by ClangdServer (FileIdx)
+  //   - the dynamic index owned by ClangdServer (DynamicIdx)
   //   - the static index passed to the constructor
   //   - a merged view of a static and dynamic index (MergedIndex)
   SymbolIndex *Index;
-  // If present, an up-to-date of symbols in open files. Read via Index.
-  std::unique_ptr FileIdx;
-  /// Callbacks responsible for upd

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

2018-08-21 Thread Anastasia Stulova via cfe-commits
If there are no objections I would like to revert this old commit that coverts 
error about implicit function declaration into a warning.


We have decided to generate an error for this https://reviews.llvm.org/D31745 
because for OpenCL variadic prototypes are disallowed (section 6.9.e, 
https://www.khronos.org/registry/OpenCL/specs/opencl-2.0-openclc.pdf) and the 
implicit prototype requires variadic support. As most vendors that support 
OpenCL don't support variadic functions it was decided to restrict this 
explicitly in the spec (section s6.9.u). There is a little bit of more history 
in https://reviews.llvm.org/D17438.


Currently the code that can't run correctly on most OpenCL targets compiles 
successfully. The problem can't be easily seen by the OpenCL developers since 
it's not very common to retrieve the compilation warning log during online 
compilation. Also generated IR doesn't seem to be correct if I compare with the 
similar code in C.

Example:
 1 typedef long long16 __attribute__((ext_vector_type(16)));
 2 void test_somefunc( __global int *d, __global void *s )
 3 {
 4   int i = get_global_id(0);
 5   d[i] = somefunc((( __global long16 *)s)[i]);
 6 }

Is generated to:

%call1 = call i32 (<16 x i64>*, ...) bitcast (i32 ()* @somefunc to i32 (<16 x 
i64>*, ...)*)(<16 x i64>* byval nonnull align 128 %indirect-arg-temp) #2
...

declare i32 @somefunc() local_unnamed_addr #1

Equivalent C code at least generates variadic function prototype correctly:

%call1 = call i32 (<16 x i64>*, ...) bitcast (i32 (...)* @somefunc to i32 (<16 
x i64>*, ...)*)(<16 x i64>* byval align 128 %indirect-arg-temp)
...
declare i32 @somefunc(...)

Anastasia

From: cfe-commits  on behalf of Richard 
Smith via cfe-commits 
Sent: 04 October 2017 02:58
To: cfe-commits@lists.llvm.org
Subject: r314872 - We allow implicit function declarations as an extension in 
all C dialects. Remove OpenCL special case.

Author: rsmith
Date: Tue Oct  3 18:58:22 2017
New Revision: 314872

URL: http://llvm.org/viewvc/llvm-project?rev=314872&view=rev
Log:
We allow implicit function declarations as an extension in all C dialects. 
Remove OpenCL special case.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/SemaOpenCL/clang-builtin-version.cl
cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=314872&r1=314871&r2=314872&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Oct  3 18:58:22 
2017
@@ -355,7 +355,7 @@ def warn_implicit_function_decl : Warnin
   "implicit declaration of function %0">,
   InGroup, DefaultIgnore;
 def ext_implicit_function_decl : ExtWarn<
-  "implicit declaration of function %0 is invalid in C99">,
+  "implicit declaration of function %0 is invalid in %select{C99|OpenCL}1">,
   InGroup;
 def note_function_suggestion : Note<"did you mean %0?">;

@@ -8449,8 +8449,6 @@ def err_opencl_scalar_type_rank_greater_
 "element. (%0 and %1)">;
 def err_bad_kernel_param_type : Error<
   "%0 cannot be used as the type of a kernel parameter">;
-def err_opencl_implicit_function_decl : Error<
-  "implicit declaration of function %0 is invalid in OpenCL">;
 def err_record_with_pointers_kernel_param : Error<
   "%select{struct|union}0 kernel parameters may not contain pointers">;
 def note_within_field_of_type : Note<

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=314872&r1=314871&r2=314872&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Oct  3 18:58:22 2017
@@ -12642,17 +12642,15 @@ NamedDecl *Sema::ImplicitlyDefineFunctio
   }

   // Extension in C99.  Legal in C90, but warn about it.
+  // OpenCL v2.0 s6.9.u - Implicit function declaration is not supported.
   unsigned diag_id;
   if (II.getName().startswith("__builtin_"))
 diag_id = diag::warn_builtin_unknown;
-  // OpenCL v2.0 s6.9.u - Implicit function declaration is not supported.
-  else if (getLangOpts().OpenCL)
-diag_id = diag::err_opencl_implicit_function_decl;
-  else if (getLangOpts().C99)
+  else if (getLangOpts().C99 || getLangOpts().OpenCL)
 diag_id = diag::ext_implicit_function_decl;
   else
 diag_id = diag::warn_implicit_function_decl;
-  Diag(Loc, diag_id) << &II;
+  Diag(Loc, diag_id) << &II << getLangOpts().OpenCL;

   // If we found a prior declaration of this function, don't bother building
   // another one. We've already pushed that one into scope, so there's nothing

Modified: cfe/trunk/test/SemaOpenCL/clang-builtin-ver

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

2018-08-21 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.

All the issues should be addressed at this point




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

ilya-biryukov wrote:
> ioeric wrote:
> > Maybe provide noop implementations for `ParsingCallbacks::onPreambleAST()` 
> > and `ParsingCallbacks::onMainAST` by default?
> I'll address this in a parent revision. 
Done


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] D50889: [clangd] Make FileIndex aware of the main file

2018-08-21 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.



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

hokein wrote:
> 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`).  
The only reason is to keep the changes to `FileIndex` minimal in this change. 
We'll have to change more code to cover those differences, but I'm not sure I 
have enough context on what's going to be different for this change, a change 
that will plug xrefs into clangd is probably a better candidate for those.


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] D50542: [clang-tidy] Add abseil-no-internal-deps check

2018-08-21 Thread Hugo Gonzalez via Phabricator via cfe-commits
hugoeg added a comment.

Ok , so I've clean some stuff up, I'm currently working with @deannagarcia to 
get https://reviews.llvm.org/D50580 submitted with the match I implemented for 
us first, then will adjust this as necessary


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] D50580: [clang-tidy] Abseil: no namespace check

2018-08-21 Thread Deanna Garcia via Phabricator via cfe-commits
deannagarcia updated this revision to Diff 161720.
deannagarcia marked 12 inline comments as done.

https://reviews.llvm.org/D50580

Files:
  clang-tidy/abseil/AbseilMatcher.h
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoNamespaceCheck.cpp
  clang-tidy/abseil/NoNamespaceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-namespace.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/external-file.h
  test/clang-tidy/Inputs/absl/strings/internal-file.h
  test/clang-tidy/abseil-no-namespace.cpp

Index: test/clang-tidy/abseil-no-namespace.cpp
===
--- test/clang-tidy/abseil-no-namespace.cpp
+++ test/clang-tidy/abseil-no-namespace.cpp
@@ -0,0 +1,26 @@
+// RUN: %check_clang_tidy %s abseil-no-namespace %t -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-namespace' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+// Warning will not be triggered on internal Abseil code that is included.
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+// Warning will be triggered on code that is not internal that is included.
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:1:11: warning: namespace 'absl' is reserved
+// for implementation of the Abseil library and should not be opened in
+// user code [abseil-no-namespace]
+
+namespace absl {}
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: namespace 'absl' is reserved for
+// implementation of the Abseil library and should not be opened in the user
+// code [abseil-no-namespace]
+
+namespace absl {
+int i = 5;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: namespace 'absl' 
+
+// Things that shouldn't trigger the check
+int i = 5;
+namespace std {}
Index: test/clang-tidy/Inputs/absl/strings/internal-file.h
===
--- test/clang-tidy/Inputs/absl/strings/internal-file.h
+++ test/clang-tidy/Inputs/absl/strings/internal-file.h
@@ -0,0 +1 @@
+namespace absl {}
Index: test/clang-tidy/Inputs/absl/external-file.h
===
--- test/clang-tidy/Inputs/absl/external-file.h
+++ test/clang-tidy/Inputs/absl/external-file.h
@@ -0,0 +1 @@
+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-namespace
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-no-namespace.rst
===
--- docs/clang-tidy/checks/abseil-no-namespace.rst
+++ docs/clang-tidy/checks/abseil-no-namespace.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - abseil-no-namespace
+
+abseil-no-namespace
+===
+
+This check gives users a warning if they attempt to open ``namespace absl``.
+Users should not open ``namespace absl`` as that conflicts with abseil's
+compatibility guidelines and may result in breakage.
+
+Any user that uses:
+
+.. code-block:: c++
+
+ namespace absl {
+  ...
+ }
+
+will be prompted with a warning.
+
+See `the full Abseil compatibility guidelines `_ for more information.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,7 +57,11 @@
 Improvements to clang-tidy
 --
 
-The improvements are...
+- New :doc:`abseil-no-namespace
+  ` check.
+
+  Checks to ensure code does not open `namespace absl` as that
+  violates Abseil's compatibility guidelines.
 
 Improvements to include-fixer
 -
Index: clang-tidy/abseil/NoNamespaceCheck.h
===
--- clang-tidy/abseil/NoNamespaceCheck.h
+++ clang-tidy/abseil/NoNamespaceCheck.h
@@ -0,0 +1,36 @@
+//===--- NoNamespaceCheck.h - clang-tidy-*- 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_CLANG_TIDY_ABSEIL_NONAMESPACECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NONAMESPACECHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// This test ensures users don't open namespace absl, as that violates
+/// our compatibility guidelines.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-no-namespace.html
+class NoNamespaceCheck : public ClangTidyCheck {
+public:
+  NoNamespaceCheck(StringRef Name, ClangTid

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

2018-08-21 Thread Deanna Garcia via Phabricator via cfe-commits
deannagarcia added inline comments.



Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+  auto Filename = FileEntry->getName();
+  llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
+ "synchronization|types|utiliy)");

lebedev.ri wrote:
> hokein wrote:
> > lebedev.ri wrote:
> > > hokein wrote:
> > > > The regex seems incomplete, we are missing `algorithm`, `time` 
> > > > subdirectory. 
> > > So what happens if open the namespace in a file that is located in my 
> > > personal `absl/base` directory?
> > > It will be false-negatively not reported?
> >  Yes, I'd say this is a known limitation.
> Similarly, the check will break (start producing false-positives) as soon as 
> a new directory is added to abseil proper.
> I don't have any reliable idea on how to do this better, but the current 
> solution seems rather suboptimal..
We are aware that there will be a false-negative if code opens namespace in the 
absl directories, however we think that is pretty rare and that users shouldn't 
be doing that anyway. No matter how we do this there will be a way for users to 
circumvent the check, and we will allow those users to do it because it will be 
their code that breaks in the end.



Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+  auto Filename = FileEntry->getName();
+  llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
+ "synchronization|types|utiliy)");

deannagarcia wrote:
> lebedev.ri wrote:
> > hokein wrote:
> > > lebedev.ri wrote:
> > > > hokein wrote:
> > > > > The regex seems incomplete, we are missing `algorithm`, `time` 
> > > > > subdirectory. 
> > > > So what happens if open the namespace in a file that is located in my 
> > > > personal `absl/base` directory?
> > > > It will be false-negatively not reported?
> > >  Yes, I'd say this is a known limitation.
> > Similarly, the check will break (start producing false-positives) as soon 
> > as a new directory is added to abseil proper.
> > I don't have any reliable idea on how to do this better, but the current 
> > solution seems rather suboptimal..
> We are aware that there will be a false-negative if code opens namespace in 
> the absl directories, however we think that is pretty rare and that users 
> shouldn't be doing that anyway. No matter how we do this there will be a way 
> for users to circumvent the check, and we will allow those users to do it 
> because it will be their code that breaks in the end.
The false-positive with new directories is something we thought about, but new 
directories aren't added to absl very often so we thought it wouldn't be too 
hard to add them to this regex when they are.


https://reviews.llvm.org/D50580



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


[PATCH] D50970: [clangd] Implement BOOST iterator

2018-08-21 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Looking at this one. (to make sure we don't clash with @ioeric)




Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:111
 
+  const static unsigned DEFAULT_BOOST_SCORE;
+

Maybe make it `constexpr` and put the value into the header?


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] D51038: [clangd] Make sure codecompletion is called for calls even when inside a token.

2018-08-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision.
kadircet added reviewers: ilya-biryukov, ioeric, hokein.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay.

Currently CodeCompleteCall only gets called after a comma or parantheses. This
patch makes sure it is called even at the cases like:

  foo(1^);


Repository:
  rC Clang

https://reviews.llvm.org/D51038

Files:
  include/clang/Parse/Parser.h
  lib/Parse/ParseExpr.cpp


Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -1648,6 +1648,10 @@
 Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs);
  })) {
 (void)Actions.CorrectDelayedTyposInExpr(LHS);
+if (PP.isCodeCompletionReached() && !CalledOverloadCompletion) {
+  Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs);
+  CalledOverloadCompletion = true;
+}
 LHS = ExprError();
   } else if (LHS.isInvalid()) {
 for (auto &E : ArgExprs)
@@ -2813,6 +2817,7 @@
 Completer();
   else
 Actions.CodeCompleteOrdinaryName(getCurScope(), Sema::PCC_Expression);
+  CalledOverloadCompletion = true;
   cutOffParsing();
   return true;
 }
Index: include/clang/Parse/Parser.h
===
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -2965,6 +2965,10 @@
   void CodeCompleteMacroArgument(IdentifierInfo *Macro, MacroInfo *MacroInfo,
  unsigned ArgumentIndex) override;
   void CodeCompleteNaturalLanguage() override;
+
+  /// Gets set to true after calling CodeCompleteCall, it is for a hack to make
+  /// signature help working even when it is triggered inside a token.
+  bool CalledOverloadCompletion = false;
 };
 
 }  // end namespace clang


Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -1648,6 +1648,10 @@
 Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs);
  })) {
 (void)Actions.CorrectDelayedTyposInExpr(LHS);
+if (PP.isCodeCompletionReached() && !CalledOverloadCompletion) {
+  Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs);
+  CalledOverloadCompletion = true;
+}
 LHS = ExprError();
   } else if (LHS.isInvalid()) {
 for (auto &E : ArgExprs)
@@ -2813,6 +2817,7 @@
 Completer();
   else
 Actions.CodeCompleteOrdinaryName(getCurScope(), Sema::PCC_Expression);
+  CalledOverloadCompletion = true;
   cutOffParsing();
   return true;
 }
Index: include/clang/Parse/Parser.h
===
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -2965,6 +2965,10 @@
   void CodeCompleteMacroArgument(IdentifierInfo *Macro, MacroInfo *MacroInfo,
  unsigned ArgumentIndex) override;
   void CodeCompleteNaturalLanguage() override;
+
+  /// Gets set to true after calling CodeCompleteCall, it is for a hack to make
+  /// signature help working even when it is triggered inside a token.
+  bool CalledOverloadCompletion = false;
 };
 
 }  // end namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D51039: [clangd] Add unittests for D51038

2018-08-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision.
kadircet added reviewers: ilya-biryukov, ioeric, hokein.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51039

Files:
  unittests/clangd/CodeCompleteTests.cpp


Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1590,6 +1590,56 @@
   ElementsAre(Sig("foo(T, U) -> void", {"T", "U"})));
 }
 
+TEST(SignatureHelpTest, InsideArgument) {
+  {
+const auto Results = signatures(R"cpp(
+  void foo(int x);
+  void foo(int x, int y);
+  int main() { foo(1+^); }
+)cpp");
+EXPECT_THAT(
+Results.signatures,
+ElementsAre(Sig("foo(int x) -> void", {"int x"}),
+Sig("foo(int x, int y) -> void", {"int x", "int y"})));
+EXPECT_EQ(0, Results.activeParameter);
+  }
+  {
+const auto Results = signatures(R"cpp(
+  void foo(int x);
+  void foo(int x, int y);
+  int main() { foo(1^); }
+)cpp");
+EXPECT_THAT(
+Results.signatures,
+ElementsAre(Sig("foo(int x) -> void", {"int x"}),
+Sig("foo(int x, int y) -> void", {"int x", "int y"})));
+EXPECT_EQ(0, Results.activeParameter);
+  }
+  {
+const auto Results = signatures(R"cpp(
+  void foo(int x);
+  void foo(int x, int y);
+  int main() { foo(1^0); }
+)cpp");
+EXPECT_THAT(
+Results.signatures,
+ElementsAre(Sig("foo(int x) -> void", {"int x"}),
+Sig("foo(int x, int y) -> void", {"int x", "int y"})));
+EXPECT_EQ(0, Results.activeParameter);
+  }
+  {
+const auto Results = signatures(R"cpp(
+  void foo(int x);
+  void foo(int x, int y);
+  int bar(int x, int y);
+  int main() { bar(foo(2, 3^)); }
+)cpp");
+EXPECT_THAT(Results.signatures, ElementsAre(Sig("foo(int x, int y) -> 
void",
+{"int x", "int y"})));
+EXPECT_EQ(1, Results.activeParameter);
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang


Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1590,6 +1590,56 @@
   ElementsAre(Sig("foo(T, U) -> void", {"T", "U"})));
 }
 
+TEST(SignatureHelpTest, InsideArgument) {
+  {
+const auto Results = signatures(R"cpp(
+  void foo(int x);
+  void foo(int x, int y);
+  int main() { foo(1+^); }
+)cpp");
+EXPECT_THAT(
+Results.signatures,
+ElementsAre(Sig("foo(int x) -> void", {"int x"}),
+Sig("foo(int x, int y) -> void", {"int x", "int y"})));
+EXPECT_EQ(0, Results.activeParameter);
+  }
+  {
+const auto Results = signatures(R"cpp(
+  void foo(int x);
+  void foo(int x, int y);
+  int main() { foo(1^); }
+)cpp");
+EXPECT_THAT(
+Results.signatures,
+ElementsAre(Sig("foo(int x) -> void", {"int x"}),
+Sig("foo(int x, int y) -> void", {"int x", "int y"})));
+EXPECT_EQ(0, Results.activeParameter);
+  }
+  {
+const auto Results = signatures(R"cpp(
+  void foo(int x);
+  void foo(int x, int y);
+  int main() { foo(1^0); }
+)cpp");
+EXPECT_THAT(
+Results.signatures,
+ElementsAre(Sig("foo(int x) -> void", {"int x"}),
+Sig("foo(int x, int y) -> void", {"int x", "int y"})));
+EXPECT_EQ(0, Results.activeParameter);
+  }
+  {
+const auto Results = signatures(R"cpp(
+  void foo(int x);
+  void foo(int x, int y);
+  int bar(int x, int y);
+  int main() { bar(foo(2, 3^)); }
+)cpp");
+EXPECT_THAT(Results.signatures, ElementsAre(Sig("foo(int x, int y) -> void",
+{"int x", "int y"})));
+EXPECT_EQ(1, Results.activeParameter);
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2018-08-21 Thread Hugo Gonzalez via Phabricator via cfe-commits
hugoeg added inline comments.



Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+  auto Filename = FileEntry->getName();
+  llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
+ "synchronization|types|utiliy)");

deannagarcia wrote:
> deannagarcia wrote:
> > lebedev.ri wrote:
> > > hokein wrote:
> > > > lebedev.ri wrote:
> > > > > hokein wrote:
> > > > > > The regex seems incomplete, we are missing `algorithm`, `time` 
> > > > > > subdirectory. 
> > > > > So what happens if open the namespace in a file that is located in my 
> > > > > personal `absl/base` directory?
> > > > > It will be false-negatively not reported?
> > > >  Yes, I'd say this is a known limitation.
> > > Similarly, the check will break (start producing false-positives) as soon 
> > > as a new directory is added to abseil proper.
> > > I don't have any reliable idea on how to do this better, but the current 
> > > solution seems rather suboptimal..
> > We are aware that there will be a false-negative if code opens namespace in 
> > the absl directories, however we think that is pretty rare and that users 
> > shouldn't be doing that anyway. No matter how we do this there will be a 
> > way for users to circumvent the check, and we will allow those users to do 
> > it because it will be their code that breaks in the end.
> The false-positive with new directories is something we thought about, but 
> new directories aren't added to absl very often so we thought it wouldn't be 
> too hard to add them to this regex when they are.
Members of the Abseil team can also update this check as new directories are 
release


https://reviews.llvm.org/D50580



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


r340280 - [analyzer] Correctly marked a virtual function 'override'

2018-08-21 Thread Kristof Umann via cfe-commits
Author: szelethus
Date: Tue Aug 21 08:09:22 2018
New Revision: 340280

URL: http://llvm.org/viewvc/llvm-project?rev=340280&view=rev
Log:
[analyzer] Correctly marked a virtual function 'override'

Modified:

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

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp?rev=340280&r1=340279&r2=340280&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
 (original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
 Tue Aug 21 08:09:22 2018
@@ -117,7 +117,7 @@ public:
 
   virtual void printSeparator(llvm::raw_ostream &Out) const override {}
 
-  virtual bool isBase() const { return true; }
+  virtual bool isBase() const override { return true; }
 };
 
 } // end of anonymous namespace


___
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.

2018-08-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

After today's offline discussion I suppose we are not going to implement it 
within Sema. And also I think getVirtualNonOverridenMethods is a pretty generic 
function that has no ties to clangd, so it can be easily moved into Sema. 
Should we still move it into a separate file or leave it there?


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


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

2018-08-21 Thread aleksey.ba...@gmail.com via cfe-commits
Hi Anastasia,

Richard and I discussed the way to enforce the error w/o customizing
the diagnostic for OpenCL off-list and Richard proposed to use
"-Werror=implicit-function-declaration" command line option if error
is required. Will it work for you?

Thanks,
Alexey
On Tue, Aug 21, 2018 at 5:41 PM Anastasia Stulova via cfe-commits
 wrote:
>
> If there are no objections I would like to revert this old commit that 
> coverts error about implicit function declaration into a warning.
>
>
> We have decided to generate an error for this https://reviews.llvm.org/D31745 
> because for OpenCL variadic prototypes are disallowed (section 6.9.e, 
> https://www.khronos.org/registry/OpenCL/specs/opencl-2.0-openclc.pdf) and the 
> implicit prototype requires variadic support. As most vendors that support 
> OpenCL don't support variadic functions it was decided to restrict this 
> explicitly in the spec (section s6.9.u). There is a little bit of more 
> history in https://reviews.llvm.org/D17438.
>
> Currently the code that can't run correctly on most OpenCL targets compiles 
> successfully. The problem can't be easily seen by the OpenCL developers since 
> it's not very common to retrieve the compilation warning log during online 
> compilation. Also generated IR doesn't seem to be correct if I compare with 
> the similar code in C.
>
> Example:
>  1 typedef long long16 __attribute__((ext_vector_type(16)));
>  2 void test_somefunc( __global int *d, __global void *s )
>  3 {
>  4   int i = get_global_id(0);
>  5   d[i] = somefunc((( __global long16 *)s)[i]);
>  6 }
>
> Is generated to:
>
> %call1 = call i32 (<16 x i64>*, ...) bitcast (i32 ()* @somefunc to i32 (<16 x 
> i64>*, ...)*)(<16 x i64>* byval nonnull align 128 %indirect-arg-temp) #2
> ...
>
> declare i32 @somefunc() local_unnamed_addr #1
>
> Equivalent C code at least generates variadic function prototype correctly:
>
> %call1 = call i32 (<16 x i64>*, ...) bitcast (i32 (...)* @somefunc to i32 
> (<16 x i64>*, ...)*)(<16 x i64>* byval align 128 %indirect-arg-temp)
> ...
> declare i32 @somefunc(...)
>
> Anastasia
> 
> From: cfe-commits  on behalf of Richard 
> Smith via cfe-commits 
> Sent: 04 October 2017 02:58
> To: cfe-commits@lists.llvm.org
> Subject: r314872 - We allow implicit function declarations as an extension in 
> all C dialects. Remove OpenCL special case.
>
> Author: rsmith
> Date: Tue Oct  3 18:58:22 2017
> New Revision: 314872
>
> URL: http://llvm.org/viewvc/llvm-project?rev=314872&view=rev
> Log:
> We allow implicit function declarations as an extension in all C dialects. 
> Remove OpenCL special case.
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/test/SemaOpenCL/clang-builtin-version.cl
> cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=314872&r1=314871&r2=314872&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Oct  3 18:58:22 
> 2017
> @@ -355,7 +355,7 @@ def warn_implicit_function_decl : Warnin
>"implicit declaration of function %0">,
>InGroup, DefaultIgnore;
>  def ext_implicit_function_decl : ExtWarn<
> -  "implicit declaration of function %0 is invalid in C99">,
> +  "implicit declaration of function %0 is invalid in %select{C99|OpenCL}1">,
>InGroup;
>  def note_function_suggestion : Note<"did you mean %0?">;
>
> @@ -8449,8 +8449,6 @@ def err_opencl_scalar_type_rank_greater_
>  "element. (%0 and %1)">;
>  def err_bad_kernel_param_type : Error<
>"%0 cannot be used as the type of a kernel parameter">;
> -def err_opencl_implicit_function_decl : Error<
> -  "implicit declaration of function %0 is invalid in OpenCL">;
>  def err_record_with_pointers_kernel_param : Error<
>"%select{struct|union}0 kernel parameters may not contain pointers">;
>  def note_within_field_of_type : Note<
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=314872&r1=314871&r2=314872&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Oct  3 18:58:22 2017
> @@ -12642,17 +12642,15 @@ NamedDecl *Sema::ImplicitlyDefineFunctio
>}
>
>// Extension in C99.  Legal in C90, but warn about it.
> +  // OpenCL v2.0 s6.9.u - Implicit function declaration is not supported.
>unsigned diag_id;
>if (II.getName().startswith("__builtin_"))
>  diag_id = diag::warn_builtin_unknown;
> -  // OpenCL v2.0 s6.9.u - Implicit function declaration is not supported.
> -  else if (getLangOpts().OpenCL)
> -

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

2018-08-21 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision.
xazax.hun added a comment.

Sorry for the delay, I think this is OK to commit.
As a possible improvement, I can imagine making it slightly stricter, e.g. you 
could only skip QualifiedNames for inline namespaces and the beginning. Maybe 
add support for staring with `""` or `::` to even disable skipping namespaces 
at the beginning?
But these are just nice to have features, I am perfectly ok with not having 
them or doing it in a followup patch.


https://reviews.llvm.org/D48027



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


[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

2018-08-21 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

Oh, and thanks for working on this, this improvement was long overdue, but 
everybody was busy with something else :)


https://reviews.llvm.org/D48027



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


[PATCH] D51041: [clang-tidy] Don't run misc-unused-using-decls check in C++17.

2018-08-21 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added a subscriber: xazax.hun.

It introduces some false positives which are non-trivial to fix.
Ignore running the check in C++17.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51041

Files:
  clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  test/clang-tidy/misc-unused-using-decls-cxx17.cpp


Index: test/clang-tidy/misc-unused-using-decls-cxx17.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-unused-using-decls-cxx17.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- -- 
-fno-delayed-template-parsing -std=gnu++17
+
+namespace ns {
+template 
+class KV {
+public:
+  KV(K, V);
+};
+}
+
+using ns::KV;
+
+void f() {
+  KV(1, 2);
+}
Index: clang-tidy/misc/UnusedUsingDeclsCheck.cpp
===
--- clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -29,6 +29,12 @@
 }
 
 void UnusedUsingDeclsCheck::registerMatchers(MatchFinder *Finder) {
+  // FIXME: make the check support C++17 or later. The check relies on the fact
+  // that the used declarations are visited after the "using" declaration, but
+  // it is not ture in C++17's template argument deduction.
+  if (!getLangOpts().CPlusPlus || getLangOpts().CPlusPlus17 ||
+  getLangOpts().CPlusPlus2a)
+return;
   Finder->addMatcher(usingDecl(isExpansionInMainFile()).bind("using"), this);
   auto DeclMatcher = hasDeclaration(namedDecl().bind("used"));
   Finder->addMatcher(loc(enumType(DeclMatcher)), this);


Index: test/clang-tidy/misc-unused-using-decls-cxx17.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-unused-using-decls-cxx17.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- -- -fno-delayed-template-parsing -std=gnu++17
+
+namespace ns {
+template 
+class KV {
+public:
+  KV(K, V);
+};
+}
+
+using ns::KV;
+
+void f() {
+  KV(1, 2);
+}
Index: clang-tidy/misc/UnusedUsingDeclsCheck.cpp
===
--- clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -29,6 +29,12 @@
 }
 
 void UnusedUsingDeclsCheck::registerMatchers(MatchFinder *Finder) {
+  // FIXME: make the check support C++17 or later. The check relies on the fact
+  // that the used declarations are visited after the "using" declaration, but
+  // it is not ture in C++17's template argument deduction.
+  if (!getLangOpts().CPlusPlus || getLangOpts().CPlusPlus17 ||
+  getLangOpts().CPlusPlus2a)
+return;
   Finder->addMatcher(usingDecl(isExpansionInMainFile()).bind("using"), this);
   auto DeclMatcher = hasDeclaration(namedDecl().bind("used"));
   Finder->addMatcher(loc(enumType(DeclMatcher)), this);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D51041: [clang-tidy] Don't run misc-unused-using-decls check in C++17.

2018-08-21 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments.



Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:35
+  // it is not ture in C++17's template argument deduction.
+  if (!getLangOpts().CPlusPlus || getLangOpts().CPlusPlus17 ||
+  getLangOpts().CPlusPlus2a)

Isn't the check for `getLangOpts().CPlusPlus2a` redundant? Shouldn't it imply 
the C++17 flag?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51041



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


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

2018-08-21 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

Thanks for the updates. Looks mostly good, a few nits.




Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+
+AST_POLYMORPHIC_MATCHER(isExpansionInAbseilHeader,
+AST_POLYMORPHIC_SUPPORTED_TYPES(

nit: this matcher name now should be `isInAbseilFile`.



Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:26
+  Finder->addMatcher(
+  namespaceDecl(hasName("absl"), unless(isExpansionInAbseilHeader()))
+  .bind("abslNamespace"),

Does the `absl` namespace is always the top level namespace? If so, I think it 
would be safer to use qualified name "::absl" here.



Comment at: clang-tidy/abseil/NoNamespaceCheck.h:19
+
+/// This test ensures users don't open namespace absl, as that violates
+/// our compatibility guidelines.

nit: test => check.



Comment at: clang-tidy/abseil/NoNamespaceCheck.h:20
+/// This test ensures users don't open namespace absl, as that violates
+/// our compatibility guidelines.
+///

nit: our = abseil?



Comment at: test/clang-tidy/abseil-no-namespace.cpp:22
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: namespace 'absl' 
+

nit: put it immediately after the above `name absl {`.


https://reviews.llvm.org/D50580



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


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

2018-08-21 Thread Anastasia Stulova via cfe-commits
Hi Alexey,


I understand that avoiding adding extra diagnostics is desirable, but I am not 
sure it's a good idea to accept and generate code that is not going to work. 
Variadic functions are currently not expected to be supported by all OpenCL 
targets hence we disallow this in the spec explicitly.


I would be inclined to explicitly reject the code by default, unless there is a 
valid way to handle the generated bitcast of variadic prototype to non-variadic 
one? Then we could look at updating the spec and explain that this is the 
difference in implicit function prototypes for OpenCL.


One compromise could be to try to set "-Werror=implicit-function-declaration" 
by default in the OpenCL mode? Not sure if it's easier/better than an extra 
diagnostic though.


Cheers,

Anastasia



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

Hi Anastasia,

Richard and I discussed the way to enforce the error w/o customizing
the diagnostic for OpenCL off-list and Richard proposed to use
"-Werror=implicit-function-declaration" command line option if error
is required. Will it work for you?

Thanks,
Alexey
On Tue, Aug 21, 2018 at 5:41 PM Anastasia Stulova via cfe-commits
 wrote:
>
> If there are no objections I would like to revert this old commit that 
> coverts error about implicit function declaration into a warning.
>
>
> We have decided to generate an error for this https://reviews.llvm.org/D31745 
> because for OpenCL variadic prototypes are disallowed (section 6.9.e, 
> https://www.khronos.org/registry/OpenCL/specs/opencl-2.0-openclc.pdf) and the 
> implicit prototype requires variadic support. As most vendors that support 
> OpenCL don't support variadic functions it was decided to restrict this 
> explicitly in the spec (section s6.9.u). There is a little bit of more 
> history in https://reviews.llvm.org/D17438.
>
> Currently the code that can't run correctly on most OpenCL targets compiles 
> successfully. The problem can't be easily seen by the OpenCL developers since 
> it's not very common to retrieve the compilation warning log during online 
> compilation. Also generated IR doesn't seem to be correct if I compare with 
> the similar code in C.
>
> Example:
>  1 typedef long long16 __attribute__((ext_vector_type(16)));
>  2 void test_somefunc( __global int *d, __global void *s )
>  3 {
>  4   int i = get_global_id(0);
>  5   d[i] = somefunc((( __global long16 *)s)[i]);
>  6 }
>
> Is generated to:
>
> %call1 = call i32 (<16 x i64>*, ...) bitcast (i32 ()* @somefunc to i32 (<16 x 
> i64>*, ...)*)(<16 x i64>* byval nonnull align 128 %indirect-arg-temp) #2
> ...
>
> declare i32 @somefunc() local_unnamed_addr #1
>
> Equivalent C code at least generates variadic function prototype correctly:
>
> %call1 = call i32 (<16 x i64>*, ...) bitcast (i32 (...)* @somefunc to i32 
> (<16 x i64>*, ...)*)(<16 x i64>* byval align 128 %indirect-arg-temp)
> ...
> declare i32 @somefunc(...)
>
> Anastasia
> 
> From: cfe-commits  on behalf of Richard 
> Smith via cfe-commits 
> Sent: 04 October 2017 02:58
> To: cfe-commits@lists.llvm.org
> Subject: r314872 - We allow implicit function declarations as an extension in 
> all C dialects. Remove OpenCL special case.
>
> Author: rsmith
> Date: Tue Oct  3 18:58:22 2017
> New Revision: 314872
>
> URL: http://llvm.org/viewvc/llvm-project?rev=314872&view=rev
> Log:
> We allow implicit function declarations as an extension in all C dialects. 
> Remove OpenCL special case.
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/test/SemaOpenCL/clang-builtin-version.cl
> cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=314872&r1=314871&r2=314872&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Oct  3 18:58:22 
> 2017
> @@ -355,7 +355,7 @@ def warn_implicit_function_decl : Warnin
>"implicit declaration of function %0">,
>InGroup, DefaultIgnore;
>  def ext_implicit_function_decl : ExtWarn<
> -  "implicit declaration of function %0 is invalid in C99">,
> +  "implicit declaration of function %0 is invalid in %select{C99|OpenCL}1">,
>InGroup;
>  def note_function_suggestion : Note<"did you mean %0?">;
>
> @@ -8449,8 +8449,6 @@ def err_opencl_scalar_type_rank_greater_
>  "element. (%0 and %1)">;
>  def err_bad_kernel_param_type : Error<
>"%0 cannot be used as the type of a kernel parameter">;
> -def err_opencl_impl

[PATCH] D51043: [clang][NFC] Fix typo in the name of a note

2018-08-21 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision.
ldionne added a reviewer: ahatanak.
Herald added subscribers: cfe-commits, dexonsmith.

r306722 introduced a new note called 
note_silence_unligned_allocation_unavailable
where I believe what was meant is note_silence_aligned_allocation_unavailable.


Repository:
  rCXX libc++

https://reviews.llvm.org/D51043

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExprCXX.cpp


Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -1744,7 +1744,7 @@
 S.Diag(Loc, diag::err_aligned_allocation_unavailable)
 << IsDelete << FD.getType().getAsString() << OSName
 << alignedAllocMinVersion(T.getOS()).getAsString();
-S.Diag(Loc, diag::note_silence_unligned_allocation_unavailable);
+S.Diag(Loc, diag::note_silence_aligned_allocation_unavailable);
   }
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6468,7 +6468,7 @@
 def err_aligned_allocation_unavailable : Error<
   "aligned %select{allocation|deallocation}0 function of type '%1' is only "
   "available on %2 %3 or newer">;
-def note_silence_unligned_allocation_unavailable : Note<
+def note_silence_aligned_allocation_unavailable : Note<
   "if you supply your own aligned allocation functions, use "
   "-faligned-allocation to silence this diagnostic">;
 


Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -1744,7 +1744,7 @@
 S.Diag(Loc, diag::err_aligned_allocation_unavailable)
 << IsDelete << FD.getType().getAsString() << OSName
 << alignedAllocMinVersion(T.getOS()).getAsString();
-S.Diag(Loc, diag::note_silence_unligned_allocation_unavailable);
+S.Diag(Loc, diag::note_silence_aligned_allocation_unavailable);
   }
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6468,7 +6468,7 @@
 def err_aligned_allocation_unavailable : Error<
   "aligned %select{allocation|deallocation}0 function of type '%1' is only "
   "available on %2 %3 or newer">;
-def note_silence_unligned_allocation_unavailable : Note<
+def note_silence_aligned_allocation_unavailable : Note<
   "if you supply your own aligned allocation functions, use "
   "-faligned-allocation to silence this diagnostic">;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D51043: [clang][NFC] Fix typo in the name of a note

2018-08-21 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak accepted this revision.
ahatanak added a comment.
This revision is now accepted and ready to land.

Thanks. Yes, it's a typo.


Repository:
  rCXX libc++

https://reviews.llvm.org/D51043



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


r340288 - [clang][NFC] Fix typo in the name of a note

2018-08-21 Thread Louis Dionne via cfe-commits
Author: ldionne
Date: Tue Aug 21 08:54:24 2018
New Revision: 340288

URL: http://llvm.org/viewvc/llvm-project?rev=340288&view=rev
Log:
[clang][NFC] Fix typo in the name of a note

Summary:
r306722 introduced a new note called 
note_silence_unligned_allocation_unavailable
where I believe what was meant is note_silence_aligned_allocation_unavailable.

Reviewers: ahatanak

Subscribers: dexonsmith, cfe-commits

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaExprCXX.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=340288&r1=340287&r2=340288&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Aug 21 08:54:24 
2018
@@ -6468,7 +6468,7 @@ def warn_overaligned_type : Warning<
 def err_aligned_allocation_unavailable : Error<
   "aligned %select{allocation|deallocation}0 function of type '%1' is only "
   "available on %2 %3 or newer">;
-def note_silence_unligned_allocation_unavailable : Note<
+def note_silence_aligned_allocation_unavailable : Note<
   "if you supply your own aligned allocation functions, use "
   "-faligned-allocation to silence this diagnostic">;
 

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=340288&r1=340287&r2=340288&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Aug 21 08:54:24 2018
@@ -1744,7 +1744,7 @@ static void diagnoseUnavailableAlignedAl
 S.Diag(Loc, diag::err_aligned_allocation_unavailable)
 << IsDelete << FD.getType().getAsString() << OSName
 << alignedAllocMinVersion(T.getOS()).getAsString();
-S.Diag(Loc, diag::note_silence_unligned_allocation_unavailable);
+S.Diag(Loc, diag::note_silence_aligned_allocation_unavailable);
   }
 }
 


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


[PATCH] D51043: [clang][NFC] Fix typo in the name of a note

2018-08-21 Thread Louis Dionne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL340288: [clang][NFC] Fix typo in the name of a note 
(authored by ldionne, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51043?vs=161737&id=161739#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51043

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Sema/SemaExprCXX.cpp


Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6468,7 +6468,7 @@
 def err_aligned_allocation_unavailable : Error<
   "aligned %select{allocation|deallocation}0 function of type '%1' is only "
   "available on %2 %3 or newer">;
-def note_silence_unligned_allocation_unavailable : Note<
+def note_silence_aligned_allocation_unavailable : Note<
   "if you supply your own aligned allocation functions, use "
   "-faligned-allocation to silence this diagnostic">;
 
Index: cfe/trunk/lib/Sema/SemaExprCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp
@@ -1744,7 +1744,7 @@
 S.Diag(Loc, diag::err_aligned_allocation_unavailable)
 << IsDelete << FD.getType().getAsString() << OSName
 << alignedAllocMinVersion(T.getOS()).getAsString();
-S.Diag(Loc, diag::note_silence_unligned_allocation_unavailable);
+S.Diag(Loc, diag::note_silence_aligned_allocation_unavailable);
   }
 }
 


Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6468,7 +6468,7 @@
 def err_aligned_allocation_unavailable : Error<
   "aligned %select{allocation|deallocation}0 function of type '%1' is only "
   "available on %2 %3 or newer">;
-def note_silence_unligned_allocation_unavailable : Note<
+def note_silence_aligned_allocation_unavailable : Note<
   "if you supply your own aligned allocation functions, use "
   "-faligned-allocation to silence this diagnostic">;
 
Index: cfe/trunk/lib/Sema/SemaExprCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp
@@ -1744,7 +1744,7 @@
 S.Diag(Loc, diag::err_aligned_allocation_unavailable)
 << IsDelete << FD.getType().getAsString() << OSName
 << alignedAllocMinVersion(T.getOS()).getAsString();
-S.Diag(Loc, diag::note_silence_unligned_allocation_unavailable);
+S.Diag(Loc, diag::note_silence_aligned_allocation_unavailable);
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r340048 - Make __shiftleft128 / __shiftright128 real compiler built-ins.

2018-08-21 Thread Hans Wennborg via cfe-commits
Merged to 7.0 in r340289.

On Fri, Aug 17, 2018 at 10:19 AM, Nico Weber via cfe-commits
 wrote:
> Author: nico
> Date: Fri Aug 17 10:19:06 2018
> New Revision: 340048
>
> URL: http://llvm.org/viewvc/llvm-project?rev=340048&view=rev
> Log:
> Make __shiftleft128 / __shiftright128 real compiler built-ins.
>
> r337619 added __shiftleft128 / __shiftright128 as functions in intrin.h.
> Microsoft's STL plans on using these functions, and they're using intrin0.h
> which just has declarations of built-ins to not pull in the huge intrin.h
> header in the standard library headers. That requires that these functions are
> real built-ins.
>
> https://reviews.llvm.org/D50907
>
> Modified:
> cfe/trunk/include/clang/Basic/BuiltinsX86_64.def
> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> cfe/trunk/lib/Headers/intrin.h
> cfe/trunk/test/CodeGen/ms-x86-intrinsics.c
> cfe/trunk/test/Headers/ms-intrin.cpp
>
> Modified: cfe/trunk/include/clang/Basic/BuiltinsX86_64.def
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86_64.def?rev=340048&r1=340047&r2=340048&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/BuiltinsX86_64.def (original)
> +++ cfe/trunk/include/clang/Basic/BuiltinsX86_64.def Fri Aug 17 10:19:06 2018
> @@ -31,6 +31,8 @@ TARGET_HEADER_BUILTIN(_mul128, "LLiLLiLL
>  TARGET_HEADER_BUILTIN(_umul128, "ULLiULLiULLiULLi*", "nch",   "intrin.h", 
> ALL_MS_LANGUAGES, "")
>
>  TARGET_HEADER_BUILTIN(__faststorefence, "v", "nh", "intrin.h", 
> ALL_MS_LANGUAGES, "")
> +TARGET_HEADER_BUILTIN(__shiftleft128, "ULLiULLiULLiUc", "nch", "intrin.h", 
> ALL_MS_LANGUAGES, "")
> +TARGET_HEADER_BUILTIN(__shiftright128, "ULLiULLiULLiUc", "nch", "intrin.h", 
> ALL_MS_LANGUAGES, "")
>
>  TARGET_HEADER_BUILTIN(_InterlockedAnd64, "LLiLLiD*LLi", "nh", 
> "intrin.h", ALL_MS_LANGUAGES, "")
>  TARGET_HEADER_BUILTIN(_InterlockedDecrement64,   "LLiLLiD*","nh", 
> "intrin.h", ALL_MS_LANGUAGES, "")
>
> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=340048&r1=340047&r2=340048&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Aug 17 10:19:06 2018
> @@ -10440,6 +10440,27 @@ Value *CodeGenFunction::EmitX86BuiltinEx
>  return Builder.CreateFence(llvm::AtomicOrdering::SequentiallyConsistent,
> llvm::SyncScope::System);
>}
> +  case X86::BI__shiftleft128:
> +  case X86::BI__shiftright128: {
> +// FIXME: Once fshl/fshr no longer add an unneeded and and cmov, do this:
> +// llvm::Function *F = CGM.getIntrinsic(
> +//   BuiltinID == X86::BI__shiftleft128 ? Intrinsic::fshl : 
> Intrinsic::fshr,
> +//   Int64Ty);
> +// Ops[2] = Builder.CreateZExt(Ops[2], Int64Ty);
> +// return Builder.CreateCall(F, Ops);
> +llvm::Type *Int128Ty = Builder.getInt128Ty();
> +Value *Val = Builder.CreateOr(
> +Builder.CreateShl(Builder.CreateZExt(Ops[1], Int128Ty), 64),
> +Builder.CreateZExt(Ops[0], Int128Ty));
> +Value *Amt = Builder.CreateAnd(Builder.CreateZExt(Ops[2], Int128Ty),
> +   llvm::ConstantInt::get(Int128Ty, 0x3f));
> +Value *Res;
> +if (BuiltinID == X86::BI__shiftleft128)
> +  Res = Builder.CreateLShr(Builder.CreateShl(Val, Amt), 64);
> +else
> +  Res = Builder.CreateLShr(Val, Amt);
> +return Builder.CreateTrunc(Res, Int64Ty);
> +  }
>case X86::BI_ReadWriteBarrier:
>case X86::BI_ReadBarrier:
>case X86::BI_WriteBarrier: {
>
> Modified: cfe/trunk/lib/Headers/intrin.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/intrin.h?rev=340048&r1=340047&r2=340048&view=diff
> ==
> --- cfe/trunk/lib/Headers/intrin.h (original)
> +++ cfe/trunk/lib/Headers/intrin.h Fri Aug 17 10:19:06 2018
> @@ -863,20 +863,6 @@ __nop(void) {
>__asm__ volatile ("nop");
>  }
>  #endif
> -#if defined(__x86_64__)
> -static __inline__ unsigned __int64 __DEFAULT_FN_ATTRS
> -__shiftleft128(unsigned __int64 __l, unsigned __int64 __h, unsigned char 
> __d) {
> -  unsigned __int128 __val = ((unsigned __int128)__h << 64) | __l;
> -  unsigned __int128 __res = __val << (__d & 63);
> -  return (unsigned __int64)(__res >> 64);
> -}
> -static __inline__ unsigned __int64 __DEFAULT_FN_ATTRS
> -__shiftright128(unsigned __int64 __l, unsigned __int64 __h, unsigned char 
> __d) {
> -  unsigned __int128 __val = ((unsigned __int128)__h << 64) | __l;
> -  unsigned __int128 __res = __val >> (__d & 63);
> -  return (unsigned __int64)__res;
> -}
> -#endif
>
>  
> /**\
>  |* Privileged intrinsics
>
> Modified: cfe/trunk/test/CodeGen/ms-x86-intrinsic

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

2018-08-21 Thread Reka Kovacs via Phabricator via cfe-commits
rnkovacs accepted this revision.
rnkovacs added a comment.

In https://reviews.llvm.org/D48027#1203944, @MTC wrote:

> However this approach has limit. Given the code below, we cannot distinguish 
> whether the `basic_string` is user-defined struct or namespace. That's means 
> when the user provide {"std", "basic_string", "append"}, we can only know the 
> qualified name of the call sequentially contains `std`, `basic_string`, 
> `append`. We don't know if these names come from `RecordDecl` or 
> `NamespaceDecl`.
>
>   namespace  std {
> namespace basic_string {
>   struct A {
> void append() {}
>   };
> }
>   }
>  
>   void foo() {
> std::basic_string::A a;
> a.append(); // Match
>   }
>
>
> @rnkovacs What do you think? Can this approach meet `InnerPointerChecker`'s 
> needs?


I guess it is highly unlikely for someone to write namespaces like that, and if 
they do, I think they deserve to have them treated as a `std::string`.
Thanks, Henry, this is great!


https://reviews.llvm.org/D48027



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


r340292 - AMDGPU: Move target code into TargetParser

2018-08-21 Thread Matt Arsenault via cfe-commits
Author: arsenm
Date: Tue Aug 21 09:13:29 2018
New Revision: 340292

URL: http://llvm.org/viewvc/llvm-project?rev=340292&view=rev
Log:
AMDGPU: Move target code into TargetParser

Modified:
cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
cfe/trunk/lib/Basic/Targets/AMDGPU.h

Modified: cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AMDGPU.cpp?rev=340292&r1=340291&r2=340292&view=diff
==
--- cfe/trunk/lib/Basic/Targets/AMDGPU.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.cpp Tue Aug 21 09:13:29 2018
@@ -127,12 +127,14 @@ bool AMDGPUTargetInfo::initFeatureMap(
 llvm::StringMap &Features, DiagnosticsEngine &Diags, StringRef CPU,
 const std::vector &FeatureVec) const {
 
+  using namespace llvm::AMDGPU;
+
   // XXX - What does the member GPU mean if device name string passed here?
   if (isAMDGCN(getTriple())) {
 if (CPU.empty())
   CPU = "gfx600";
 
-switch (parseAMDGCNName(CPU).Kind) {
+switch (llvm::AMDGPU::parseArchAMDGCN(CPU)) {
 case GK_GFX906:
   Features["dl-insts"] = true;
   LLVM_FALLTHROUGH;
@@ -169,7 +171,7 @@ bool AMDGPUTargetInfo::initFeatureMap(
 if (CPU.empty())
   CPU = "r600";
 
-switch (parseR600Name(CPU).Kind) {
+switch (llvm::AMDGPU::parseArchR600(CPU)) {
 case GK_CAYMAN:
 case GK_CYPRESS:
 case GK_RV770:
@@ -201,7 +203,7 @@ void AMDGPUTargetInfo::adjustTargetOptio
TargetOptions &TargetOpts) const {
   bool hasFP32Denormals = false;
   bool hasFP64Denormals = false;
-  GPUInfo CGOptsGPU = parseGPUName(TargetOpts.CPU);
+
   for (auto &I : TargetOpts.FeaturesAsWritten) {
 if (I == "+fp32-denormals" || I == "-fp32-denormals")
   hasFP32Denormals = true;
@@ -210,54 +212,20 @@ void AMDGPUTargetInfo::adjustTargetOptio
   }
   if (!hasFP32Denormals)
 TargetOpts.Features.push_back(
-(Twine(CGOptsGPU.HasFastFMAF && CGOptsGPU.HasFullRateF32Denorms &&
-   !CGOpts.FlushDenorm
-   ? '+'
-   : '-') +
- Twine("fp32-denormals"))
+  (Twine(hasFastFMAF() && hasFullRateDenormalsF32() && !CGOpts.FlushDenorm
+ ? '+' : '-') + Twine("fp32-denormals"))
 .str());
   // Always do not flush fp64 or fp16 denorms.
-  if (!hasFP64Denormals && CGOptsGPU.HasFP64)
+  if (!hasFP64Denormals && hasFP64())
 TargetOpts.Features.push_back("+fp64-fp16-denormals");
 }
 
-constexpr AMDGPUTargetInfo::GPUInfo AMDGPUTargetInfo::InvalidGPU;
-constexpr AMDGPUTargetInfo::GPUInfo AMDGPUTargetInfo::R600GPUs[];
-constexpr AMDGPUTargetInfo::GPUInfo AMDGPUTargetInfo::AMDGCNGPUs[];
-
-AMDGPUTargetInfo::GPUInfo AMDGPUTargetInfo::parseR600Name(StringRef Name) {
-  const auto *Result = llvm::find_if(
-  R600GPUs, [Name](const GPUInfo &GPU) { return GPU.Name == Name; });
-
-  if (Result == std::end(R600GPUs))
-return InvalidGPU;
-  return *Result;
-}
-
-AMDGPUTargetInfo::GPUInfo AMDGPUTargetInfo::parseAMDGCNName(StringRef Name) {
-  const auto *Result = llvm::find_if(
-  AMDGCNGPUs, [Name](const GPUInfo &GPU) { return GPU.Name == Name; });
-
-  if (Result == std::end(AMDGCNGPUs))
-return InvalidGPU;
-  return *Result;
-}
-
-AMDGPUTargetInfo::GPUInfo AMDGPUTargetInfo::parseGPUName(StringRef Name) const 
{
-  if (isAMDGCN(getTriple()))
-return parseAMDGCNName(Name);
-  else
-return parseR600Name(Name);
-}
-
 void AMDGPUTargetInfo::fillValidCPUList(
 SmallVectorImpl &Values) const {
   if (isAMDGCN(getTriple()))
-llvm::for_each(AMDGCNGPUs, [&Values](const GPUInfo &GPU) {
-   Values.emplace_back(GPU.Name);});
+llvm::AMDGPU::fillValidArchListAMDGCN(Values);
   else
-llvm::for_each(R600GPUs, [&Values](const GPUInfo &GPU) {
-   Values.emplace_back(GPU.Name);});
+llvm::AMDGPU::fillValidArchListR600(Values);
 }
 
 void AMDGPUTargetInfo::setAddressSpaceMap(bool DefaultIsPrivate) {
@@ -267,7 +235,12 @@ void AMDGPUTargetInfo::setAddressSpaceMa
 AMDGPUTargetInfo::AMDGPUTargetInfo(const llvm::Triple &Triple,
const TargetOptions &Opts)
 : TargetInfo(Triple),
-  GPU(isAMDGCN(Triple) ? AMDGCNGPUs[0] : parseR600Name(Opts.CPU)) {
+  GPUKind(isAMDGCN(Triple) ?
+  llvm::AMDGPU::parseArchAMDGCN(Opts.CPU) :
+  llvm::AMDGPU::parseArchR600(Opts.CPU)),
+  GPUFeatures(isAMDGCN(Triple) ?
+  llvm::AMDGPU::getArchAttrAMDGCN(GPUKind) :
+  llvm::AMDGPU::getArchAttrR600(GPUKind)) {
   resetDataLayout(isAMDGCN(getTriple()) ? DataLayoutStringAMDGCN
 : DataLayoutStringR600);
   assert(DataLayout->getAllocaAddrSpace() == Private);
@@ -312,19 +285,22 @@ void AMDGPUTargetInfo::getTargetDefines(
   else
 Builder.defineMacro("__R600__");
 
-  if (GPU.Kind != GK_NONE)
-Builder.defineMacro(Twine("__") + Twine(GPU.C

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

2018-08-21 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm closed this revision.
arsenm added a comment.

r340292


https://reviews.llvm.org/D50984



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


r340297 - [ASTImporter] Add test for CXXForRangeStmt

2018-08-21 Thread Raphael Isemann via cfe-commits
Author: teemperor
Date: Tue Aug 21 09:36:49 2018
New Revision: 340297

URL: http://llvm.org/viewvc/llvm-project?rev=340297&view=rev
Log:
[ASTImporter] Add test for CXXForRangeStmt

Reviewers: a.sidorin, martong

Reviewed By: martong

Subscribers: rnkovacs, martong, cfe-commits

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

Added:
cfe/trunk/test/Import/cxx-for-range/
cfe/trunk/test/Import/cxx-for-range/Inputs/
cfe/trunk/test/Import/cxx-for-range/Inputs/F.cpp
cfe/trunk/test/Import/cxx-for-range/test.cpp

Added: cfe/trunk/test/Import/cxx-for-range/Inputs/F.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/cxx-for-range/Inputs/F.cpp?rev=340297&view=auto
==
--- cfe/trunk/test/Import/cxx-for-range/Inputs/F.cpp (added)
+++ cfe/trunk/test/Import/cxx-for-range/Inputs/F.cpp Tue Aug 21 09:36:49 2018
@@ -0,0 +1,11 @@
+struct Container {
+  int *begin();
+  int *end();
+};
+
+void f() {
+  Container c;
+  for (int varname : c) {
+return;
+  }
+}

Added: cfe/trunk/test/Import/cxx-for-range/test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/cxx-for-range/test.cpp?rev=340297&view=auto
==
--- cfe/trunk/test/Import/cxx-for-range/test.cpp (added)
+++ cfe/trunk/test/Import/cxx-for-range/test.cpp Tue Aug 21 09:36:49 2018
@@ -0,0 +1,53 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | 
FileCheck %s
+
+// CHECK: CXXForRangeStmt
+
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: 'c'
+// CHECK-SAME: Container
+
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl
+// CHECK-NEXT: CXXMemberCallExpr
+// CHECK-SAME: 'int *'
+// CHECK-NEXT: MemberExpr
+// CHECK-SAME: .begin
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: '__range1'
+// CHECK-SAME: Container
+
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl
+// CHECK-NEXT: CXXMemberCallExpr
+// CHECK-SAME: 'int *'
+// CHECK-NEXT: MemberExpr
+// CHECK-SAME: .end
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: '__range1'
+// CHECK-SAME: Container
+
+// CHECK-NEXT: BinaryOperator
+// CHECK-SAME: '!='
+// CHECK-NEXT: ImplicitCastExpr
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: '__begin1'
+// CHECK-NEXT: ImplicitCastExpr
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: '__end1'
+
+// CHECK-NEXT: UnaryOperator
+// CHECK-SAME: '++'
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: '__begin1'
+
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl
+// CHECK-SAME: varname
+
+// CHECK: ReturnStmt
+
+void expr() {
+  f();
+}


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


[PATCH] D51001: [ASTImporter] Add test for CXXForRangeStmt

2018-08-21 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC340297: [ASTImporter] Add test for CXXForRangeStmt (authored 
by teemperor, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D51001

Files:
  test/Import/cxx-for-range/Inputs/F.cpp
  test/Import/cxx-for-range/test.cpp


Index: test/Import/cxx-for-range/Inputs/F.cpp
===
--- test/Import/cxx-for-range/Inputs/F.cpp
+++ test/Import/cxx-for-range/Inputs/F.cpp
@@ -0,0 +1,11 @@
+struct Container {
+  int *begin();
+  int *end();
+};
+
+void f() {
+  Container c;
+  for (int varname : c) {
+return;
+  }
+}
Index: test/Import/cxx-for-range/test.cpp
===
--- test/Import/cxx-for-range/test.cpp
+++ test/Import/cxx-for-range/test.cpp
@@ -0,0 +1,53 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | 
FileCheck %s
+
+// CHECK: CXXForRangeStmt
+
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: 'c'
+// CHECK-SAME: Container
+
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl
+// CHECK-NEXT: CXXMemberCallExpr
+// CHECK-SAME: 'int *'
+// CHECK-NEXT: MemberExpr
+// CHECK-SAME: .begin
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: '__range1'
+// CHECK-SAME: Container
+
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl
+// CHECK-NEXT: CXXMemberCallExpr
+// CHECK-SAME: 'int *'
+// CHECK-NEXT: MemberExpr
+// CHECK-SAME: .end
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: '__range1'
+// CHECK-SAME: Container
+
+// CHECK-NEXT: BinaryOperator
+// CHECK-SAME: '!='
+// CHECK-NEXT: ImplicitCastExpr
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: '__begin1'
+// CHECK-NEXT: ImplicitCastExpr
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: '__end1'
+
+// CHECK-NEXT: UnaryOperator
+// CHECK-SAME: '++'
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: '__begin1'
+
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl
+// CHECK-SAME: varname
+
+// CHECK: ReturnStmt
+
+void expr() {
+  f();
+}


Index: test/Import/cxx-for-range/Inputs/F.cpp
===
--- test/Import/cxx-for-range/Inputs/F.cpp
+++ test/Import/cxx-for-range/Inputs/F.cpp
@@ -0,0 +1,11 @@
+struct Container {
+  int *begin();
+  int *end();
+};
+
+void f() {
+  Container c;
+  for (int varname : c) {
+return;
+  }
+}
Index: test/Import/cxx-for-range/test.cpp
===
--- test/Import/cxx-for-range/test.cpp
+++ test/Import/cxx-for-range/test.cpp
@@ -0,0 +1,53 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | FileCheck %s
+
+// CHECK: CXXForRangeStmt
+
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: 'c'
+// CHECK-SAME: Container
+
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl
+// CHECK-NEXT: CXXMemberCallExpr
+// CHECK-SAME: 'int *'
+// CHECK-NEXT: MemberExpr
+// CHECK-SAME: .begin
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: '__range1'
+// CHECK-SAME: Container
+
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl
+// CHECK-NEXT: CXXMemberCallExpr
+// CHECK-SAME: 'int *'
+// CHECK-NEXT: MemberExpr
+// CHECK-SAME: .end
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: '__range1'
+// CHECK-SAME: Container
+
+// CHECK-NEXT: BinaryOperator
+// CHECK-SAME: '!='
+// CHECK-NEXT: ImplicitCastExpr
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: '__begin1'
+// CHECK-NEXT: ImplicitCastExpr
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: '__end1'
+
+// CHECK-NEXT: UnaryOperator
+// CHECK-SAME: '++'
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: '__begin1'
+
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl
+// CHECK-SAME: varname
+
+// CHECK: ReturnStmt
+
+void expr() {
+  f();
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2018-08-21 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment.

Isn't this duplicating code in lib/Support/Unix/Threading.inc with a different 
implementation?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50993



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


r340301 - [Parser] Support alternative operator token keyword args in Objective-C++

2018-08-21 Thread Erik Pilkington via cfe-commits
Author: epilk
Date: Tue Aug 21 09:47:04 2018
New Revision: 340301

URL: http://llvm.org/viewvc/llvm-project?rev=340301&view=rev
Log:
[Parser] Support alternative operator token keyword args in Objective-C++

rdar://30741878

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

Added:
cfe/trunk/test/Parser/message-expr-alt-op.mm
Modified:
cfe/trunk/lib/Parse/ParseExpr.cpp

Modified: cfe/trunk/lib/Parse/ParseExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=340301&r1=340300&r2=340301&view=diff
==
--- cfe/trunk/lib/Parse/ParseExpr.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExpr.cpp Tue Aug 21 09:47:04 2018
@@ -315,6 +315,19 @@ Parser::ParseRHSOfBinaryExpression(ExprR
   return LHS;
 }
 
+// In Objective-C++, alternative operator tokens can be used as keyword 
args
+// in message expressions. Unconsume the token so that it can reinterpreted
+// as an identifier in ParseObjCMessageExpressionBody. i.e., we support:
+//   [foo meth:0 and:0];
+//   [foo not_eq];
+if (getLangOpts().ObjC1 && getLangOpts().CPlusPlus &&
+Tok.isOneOf(tok::colon, tok::r_square) &&
+OpToken.getIdentifierInfo() != nullptr) {
+  PP.EnterToken(Tok);
+  Tok = OpToken;
+  return LHS;
+}
+
 // Special case handling for the ternary operator.
 ExprResult TernaryMiddle(true);
 if (NextTokPrec == prec::Conditional) {

Added: cfe/trunk/test/Parser/message-expr-alt-op.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/message-expr-alt-op.mm?rev=340301&view=auto
==
--- cfe/trunk/test/Parser/message-expr-alt-op.mm (added)
+++ cfe/trunk/test/Parser/message-expr-alt-op.mm Tue Aug 21 09:47:04 2018
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+@interface WeirdInterface
+-(void)allOfThem:(int)a
+ and:(int)b
+  and_eq:(int)c
+  bitand:(int)d
+   bitor:(int)e
+   compl:(int)f
+ not:(int)g
+  not_eq:(int)h
+  or:(int)i
+   or_eq:(int)j
+ xor:(int)k
+  xor_eq:(int)l;
+
+-(void)justAnd:(int)x and:(int)y;
+-(void)and;
+-(void)and:(int)x;
+@end
+
+void call_it(WeirdInterface *x) {
+  [x allOfThem:0
+   and:0
+and_eq:0
+bitand:0
+ bitor:0
+ compl:0
+   not:0
+not_eq:0
+or:0
+ or_eq:0
+   xor:0
+xor_eq:0];
+
+  [x and];
+  [x and:0];
+  [x &&:0]; // expected-error{{expected expression}};
+  [x justAnd:0 and:1];
+  [x and: 0 ? : 1];
+}


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


[PATCH] D50527: [Parser] Support alternative operator token keyword args in Objective-C++

2018-08-21 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC340301: [Parser] Support alternative operator token keyword 
args in Objective-C++ (authored by epilk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50527?vs=159995&id=161748#toc

Repository:
  rC Clang

https://reviews.llvm.org/D50527

Files:
  lib/Parse/ParseExpr.cpp
  test/Parser/message-expr-alt-op.mm


Index: test/Parser/message-expr-alt-op.mm
===
--- test/Parser/message-expr-alt-op.mm
+++ test/Parser/message-expr-alt-op.mm
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+@interface WeirdInterface
+-(void)allOfThem:(int)a
+ and:(int)b
+  and_eq:(int)c
+  bitand:(int)d
+   bitor:(int)e
+   compl:(int)f
+ not:(int)g
+  not_eq:(int)h
+  or:(int)i
+   or_eq:(int)j
+ xor:(int)k
+  xor_eq:(int)l;
+
+-(void)justAnd:(int)x and:(int)y;
+-(void)and;
+-(void)and:(int)x;
+@end
+
+void call_it(WeirdInterface *x) {
+  [x allOfThem:0
+   and:0
+and_eq:0
+bitand:0
+ bitor:0
+ compl:0
+   not:0
+not_eq:0
+or:0
+ or_eq:0
+   xor:0
+xor_eq:0];
+
+  [x and];
+  [x and:0];
+  [x &&:0]; // expected-error{{expected expression}};
+  [x justAnd:0 and:1];
+  [x and: 0 ? : 1];
+}
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -315,6 +315,19 @@
   return LHS;
 }
 
+// In Objective-C++, alternative operator tokens can be used as keyword 
args
+// in message expressions. Unconsume the token so that it can reinterpreted
+// as an identifier in ParseObjCMessageExpressionBody. i.e., we support:
+//   [foo meth:0 and:0];
+//   [foo not_eq];
+if (getLangOpts().ObjC1 && getLangOpts().CPlusPlus &&
+Tok.isOneOf(tok::colon, tok::r_square) &&
+OpToken.getIdentifierInfo() != nullptr) {
+  PP.EnterToken(Tok);
+  Tok = OpToken;
+  return LHS;
+}
+
 // Special case handling for the ternary operator.
 ExprResult TernaryMiddle(true);
 if (NextTokPrec == prec::Conditional) {


Index: test/Parser/message-expr-alt-op.mm
===
--- test/Parser/message-expr-alt-op.mm
+++ test/Parser/message-expr-alt-op.mm
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+@interface WeirdInterface
+-(void)allOfThem:(int)a
+ and:(int)b
+  and_eq:(int)c
+  bitand:(int)d
+   bitor:(int)e
+   compl:(int)f
+ not:(int)g
+  not_eq:(int)h
+  or:(int)i
+   or_eq:(int)j
+ xor:(int)k
+  xor_eq:(int)l;
+
+-(void)justAnd:(int)x and:(int)y;
+-(void)and;
+-(void)and:(int)x;
+@end
+
+void call_it(WeirdInterface *x) {
+  [x allOfThem:0
+   and:0
+and_eq:0
+bitand:0
+ bitor:0
+ compl:0
+   not:0
+not_eq:0
+or:0
+ or_eq:0
+   xor:0
+xor_eq:0];
+
+  [x and];
+  [x and:0];
+  [x &&:0]; // expected-error{{expected expression}};
+  [x justAnd:0 and:1];
+  [x and: 0 ? : 1];
+}
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -315,6 +315,19 @@
   return LHS;
 }
 
+// In Objective-C++, alternative operator tokens can be used as keyword args
+// in message expressions. Unconsume the token so that it can reinterpreted
+// as an identifier in ParseObjCMessageExpressionBody. i.e., we support:
+//   [foo meth:0 and:0];
+//   [foo not_eq];
+if (getLangOpts().ObjC1 && getLangOpts().CPlusPlus &&
+Tok.isOneOf(tok::colon, tok::r_square) &&
+OpToken.getIdentifierInfo() != nullptr) {
+  PP.EnterToken(Tok);
+  Tok = OpToken;
+  return LHS;
+}
+
 // Special case handling for the ternary operator.
 ExprResult TernaryMiddle(true);
 if (NextTokPrec == prec::Conditional) {
___
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

2018-08-21 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:92
+  /// function, the parameter is needed to propagate through the tree.
+  virtual unsigned boost(DocID ID) const = 0;
 

Maybe add a note to the comment on why an `ID` parameter actually is here?
IIUC, we need to because various iterators in the tree may point to different 
elements and we need to know which one we've actually matched.



Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:129
+std::vector>
+consumeAndBoost(Iterator &It,
+size_t Limit = std::numeric_limits::max());

Could we describe the rationale for keeping both `consume` and 
`consumeAndBoost` somewhere in the comments?

From the offline conversation, it seems `consumeAndBoost` is more expensive, 
but our clients will all use it at some point in the future.
The idea of paying for boosting without actually using it seems bad, so keeping 
this function separate makes sense.



Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:164
+std::unique_ptr createBoost(std::unique_ptr Child,
+  unsigned Factor);
+

Maybe use `float` scores to align with the scoring code we have for completion?


https://reviews.llvm.org/D50970



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


  1   2   >