[PATCH] D41031: [clangd] (Attempt to) read clang-format file for document formatting

[Sam McCall via Phabricator via cfe-commits]

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

I'm not actually sure we're doing the user a service with this error handling.
LSP errors aren't surfaced to the user with a decent UI in practice.
Plus it adds a bunch of internal complexity.

I'd strongly consider just falling back to LLVM style on any error, inside 
ClangdServer::formatCode. I think the UX will be better, and the code will be 
simpler. But up to you.


https://reviews.llvm.org/D41031



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

[PATCH] D38831: [libcxx] P0604, invoke_result and is_invocable

[Benjamin Buch via Phabricator via cfe-commits]

bebuch accepted this revision.
bebuch added a comment.

Please commit!


https://reviews.llvm.org/D38831



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

[PATCH] D40860: [clangd] Fix diagnostic positions

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Hmm, I looked into the empty diagnostic ranges, and it overlaps here a lot.

Feel free to go ahead and land this and I can merge, otherwise I can include 
this fix there.
Regardless of how it gets landed, thanks for finding and fixing this, this 
explains a lot of weird behavior!




Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:124
+/// Convert a clang::SourceLocation to a clangd::Position
+Position SourceLocToClangdPosition(const SourceManager &SrcMgr,
+   SourceLocation Location) {

sammccall wrote:
> nit: functions are `lowerCamelCase`.
> Here and below.
nit: consider just `toPosition`, with the location as the first argument

And similarly below. The shorter names and putting the main param first make 
the callsites easier to read I think.



Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:200
 public:
-  StoreDiagsConsumer(std::vector &Output) : Output(Output) {}
+  StoreDiagsConsumer(const LangOptions &Options,
+ std::vector &Output)

Actually I think the DiagnosticConsumer interface should pass you the language 
options itself, if you override the lifecycle methods.


https://reviews.llvm.org/D40860



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

[PATCH] D41056: [clang-tidy] New check misc-uniqueptr-release-unused-retval

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

> What do you think about making this check fully configurable through the 
> check options? The options could list the functions to check. The 
> documentation could suggest some candidate functions from std lib to 
> configure checks for.

That sound reasonable. A similar approach has been used for 
`cppcoreguidelines-no-malloc` already.


Repository:
  rL LLVM

https://reviews.llvm.org/D41056



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

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/Context.h:169
+  struct ContextData {
+// We need to make sure Parent outlives the Value, so the order of members
+// is important. We do that to allow classes stored in Context's child

sammccall wrote:
> Is this comment still true/relevant?
> I thought the motivating case was Span, but Span now stores a copy of the 
> parent pointer (and ContextData isn't accessible by it).
I'd argue we still want to keep this invariant, it gives a natural order of 
destruction.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40485



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

Phabricator will be down for ~30 mins

[Eric Liu via cfe-commits]

Upgrading Phabricator to a newer version. Sorry for the short notice!

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

Re: Phabricator will be down for ~30 mins

[Eric Liu via cfe-commits]

It was faster than I thought. Phabricator is up again.

The most noticeable new feature is the ability to hide inline comments:[image:
xkhNjkAc2ma.png]

Cheers,
Eric

On Tue, Dec 12, 2017 at 11:32 AM Eric Liu  wrote:

> Upgrading Phabricator to a newer version. Sorry for the short notice!
>
> - Eric
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 126515.
ilya-biryukov marked 4 inline comments as done.
ilya-biryukov added a comment.

- Change derive() signature to accept a value instead of the variadic arguments.
- Rewrite a while loop into a for loop.
- Return a value from Context::empty() instead of a reference.
- Renamed ContextData to Data.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40485

Files:
  clangd/CMakeLists.txt
  clangd/Context.cpp
  clangd/Context.h
  unittests/clangd/CMakeLists.txt
  unittests/clangd/ContextTests.cpp

Index: unittests/clangd/ContextTests.cpp
===
--- /dev/null
+++ unittests/clangd/ContextTests.cpp
@@ -0,0 +1,57 @@
+//===-- ContextTests.cpp - Context tests *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Context.h"
+
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+
+TEST(ContextTests, Simple) {
+  Key IntParam;
+  Key ExtraIntParam;
+
+  Context Ctx = Context::empty().derive(IntParam, 10).derive(ExtraIntParam, 20);
+
+  EXPECT_EQ(*Ctx.get(IntParam), 10);
+  EXPECT_EQ(*Ctx.get(ExtraIntParam), 20);
+}
+
+TEST(ContextTests, MoveOps) {
+  Key> Param;
+
+  Context Ctx = Context::empty().derive(Param, llvm::make_unique(10));
+  EXPECT_EQ(**Ctx.get(Param), 10);
+
+  Context NewCtx = std::move(Ctx);
+  EXPECT_EQ(**NewCtx.get(Param), 10);
+}
+
+TEST(ContextTests, Builders) {
+  Key ParentParam;
+  Key ParentAndChildParam;
+  Key ChildParam;
+
+  Context ParentCtx =
+  Context::empty().derive(ParentParam, 10).derive(ParentAndChildParam, 20);
+  Context ChildCtx =
+  ParentCtx.derive(ParentAndChildParam, 30).derive(ChildParam, 40);
+
+  EXPECT_EQ(*ParentCtx.get(ParentParam), 10);
+  EXPECT_EQ(*ParentCtx.get(ParentAndChildParam), 20);
+  EXPECT_EQ(ParentCtx.get(ChildParam), nullptr);
+
+  EXPECT_EQ(*ChildCtx.get(ParentParam), 10);
+  EXPECT_EQ(*ChildCtx.get(ParentAndChildParam), 30);
+  EXPECT_EQ(*ChildCtx.get(ChildParam), 40);
+}
+
+} // namespace clangd
+} // namespace clang
Index: unittests/clangd/CMakeLists.txt
===
--- unittests/clangd/CMakeLists.txt
+++ unittests/clangd/CMakeLists.txt
@@ -11,6 +11,7 @@
 add_extra_unittest(ClangdTests
   ClangdTests.cpp
   CodeCompleteTests.cpp
+  ContextTests.cpp
   FuzzyMatchTests.cpp
   JSONExprTests.cpp
   TestFS.cpp
Index: clangd/Context.h
===
--- /dev/null
+++ clangd/Context.h
@@ -0,0 +1,186 @@
+//===--- Context.h - Mechanism for passing implicit data *- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Context for storing and retrieving implicit data. Useful for passing implicit
+// parameters on a per-request basis.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONTEXT_H_
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONTEXT_H_
+
+#include "llvm/ADT/STLExtras.h"
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+
+/// A key for a value of type \p Type, stored inside a context. Keys are
+/// non-movable and non-copyable. See documentation of the Context class for
+/// more details and usage examples.
+template  class Key {
+public:
+  static_assert(!std::is_reference::value,
+"Reference arguments to Key<> are not allowed");
+
+  Key() = default;
+
+  Key(Key const &) = delete;
+  Key &operator=(Key const &) = delete;
+  Key(Key &&) = delete;
+  Key &operator=(Key &&) = delete;
+};
+
+/// A context is an immutable container for per-request data that must be
+/// propagated through layers that don't care about it. An example is a request
+/// ID that we may want to use when logging.
+///
+/// Conceptually, a context is a heterogeneous map, T>. Each key has
+/// an associated value type, which allows the map to be typesafe.
+///
+/// You can't add data to an existing context, instead you create a new
+/// immutable context derived from it with extra data added. When you retrieve
+/// data, the context will walk up the parent chain until the key is found.
+///
+/// Contexts should be:
+///  - passed by reference when calling synchronous functions
+///  - passed by value (move) when calling asynchronous functions. The result
+///callback of async operations will receive the context again.
+///  - cloned only when 'forking' an asynchronous computation that we don't wait
+///for.
+///
+/// Copy ope

[PATCH] D40562: [Sema] Ignore decls in namespaces when global decls are not wanted.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Ping?


Repository:
  rC Clang

https://reviews.llvm.org/D40562



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

[PATCH] D41102: Setup clang-doc frontend framework

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Hi julie,

i like the new approach for doc generator. I added my thoughts on the code :)




Comment at: tools/clang-doc/ClangDoc.cpp:81
+  llvm::StringRef InFile) {
+  return std::unique_ptr(
+  new ClangDocConsumer(&Compiler.getASTContext(), Reporter));

`llvm::make_unique` is cleaner here.



Comment at: tools/clang-doc/ClangDoc.h:30
+  // Which format to emit representation in.
+  std::string EmitFormat;
+};

Is this a string for a discrete set of configurations? If so maybe a `enum 
class` would be a better fit.



Comment at: tools/clang-doc/ClangDocReporter.cpp:91
+ClangDocReporter::ClangDocReporter(std::vector SourcePathList) {
+  for (std::string Path : SourcePathList)
+AddFile(Path);

That loop will copy the string in every iteration. Is this intended?

Same with the function: It will copy the whole list. I think at least one of 
both copies is not necessary.



Comment at: tools/clang-doc/ClangDocReporter.cpp:96
+void ClangDocReporter::AddComment(StringRef Filename, CommentInfo &CI) {
+  FileRecords[Filename].UnattachedComments.push_back(std::move(CI));
+}

That move will move from the reference. That has the effect, that the call site 
can not use `CI` anymore because its moved from. (same in `AddDecl`). Is this 
intended? Even if it is, that might be very subtle. Taking by value might be a 
better option here.



Comment at: tools/clang-doc/ClangDocReporter.cpp:106
+  FI.Filename = Filename;
+  FileRecords.insert(std::pair(Filename, 
std::move(FI)));
+}

`std::make_pair(FileName, std::move(FI))` would be shorter.



Comment at: tools/clang-doc/ClangDocReporter.cpp:205
+  ConstCommentVisitor::visit(C);
+  for (comments::Comment::child_iterator I = C->child_begin(),
+ E = C->child_end();

`comments::Comment` could get a `childs()` method returning a view to iterate 
with nice range based loops.



Comment at: tools/clang-doc/ClangDocReporter.cpp:236
+
+bool ClangDocReporter::isWhitespaceOnly(std::string S) {
+  return S.find_first_not_of(" \t\n\v\f\r") == std::string::npos || S.empty();

will copy `S`. using a `const&` removes the potential allocation



Comment at: tools/clang-doc/tool/ClangDocMain.cpp:69
+  llvm::outs() << "Writing docs...\n";
+  Reporter.Serialize(EmitFormat, llvm::outs());
+}

final `return 0;` for main is missing.


https://reviews.llvm.org/D41102



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

[PATCH] D33776: [libcxx] LWG2221: No formatted output operator for nullptr

[Zhihao Yuan via Phabricator via cfe-commits]

lichray added inline comments.



Comment at: include/ostream:225
+basic_ostream& operator<<(nullptr_t)
+{ return *this << (const void*)0; }
+

Oh, common, I persuaded the committee to allow you to print a `(null)`  and you 
don't do it...


https://reviews.llvm.org/D33776



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

[clang-tools-extra] r320468 - [clangd] Introduced a Context that stores implicit data

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue Dec 12 03:16:45 2017
New Revision: 320468

URL: http://llvm.org/viewvc/llvm-project?rev=320468&view=rev
Log:
[clangd] Introduced a Context that stores implicit data

Summary:
It will be used to pass around things like Logger and Tracer throughout
clangd classes.

Reviewers: sammccall, ioeric, hokein, bkramer

Reviewed By: sammccall

Subscribers: klimek, bkramer, mgorny, cfe-commits

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

Added:
clang-tools-extra/trunk/clangd/Context.cpp
clang-tools-extra/trunk/clangd/Context.h
clang-tools-extra/trunk/unittests/clangd/ContextTests.cpp
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=320468&r1=320467&r2=320468&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue Dec 12 03:16:45 2017
@@ -8,6 +8,7 @@ add_clang_library(clangDaemon
   ClangdUnit.cpp
   ClangdUnitStore.cpp
   CodeComplete.cpp
+  Context.cpp
   Compiler.cpp
   DraftStore.cpp
   FuzzyMatch.cpp

Added: clang-tools-extra/trunk/clangd/Context.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Context.cpp?rev=320468&view=auto
==
--- clang-tools-extra/trunk/clangd/Context.cpp (added)
+++ clang-tools-extra/trunk/clangd/Context.cpp Tue Dec 12 03:16:45 2017
@@ -0,0 +1,24 @@
+//===--- Context.cpp -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===-===//
+
+#include "Context.h"
+#include 
+
+namespace clang {
+namespace clangd {
+
+Context Context::empty() { return Context(/*Data=*/nullptr); }
+
+Context::Context(std::shared_ptr DataPtr)
+: DataPtr(std::move(DataPtr)) {}
+
+Context Context::clone() const { return Context(DataPtr); }
+
+} // namespace clangd
+} // namespace clang

Added: clang-tools-extra/trunk/clangd/Context.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Context.h?rev=320468&view=auto
==
--- clang-tools-extra/trunk/clangd/Context.h (added)
+++ clang-tools-extra/trunk/clangd/Context.h Tue Dec 12 03:16:45 2017
@@ -0,0 +1,186 @@
+//===--- Context.h - Mechanism for passing implicit data *- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Context for storing and retrieving implicit data. Useful for passing 
implicit
+// parameters on a per-request basis.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONTEXT_H_
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONTEXT_H_
+
+#include "llvm/ADT/STLExtras.h"
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+
+/// A key for a value of type \p Type, stored inside a context. Keys are
+/// non-movable and non-copyable. See documentation of the Context class for
+/// more details and usage examples.
+template  class Key {
+public:
+  static_assert(!std::is_reference::value,
+"Reference arguments to Key<> are not allowed");
+
+  Key() = default;
+
+  Key(Key const &) = delete;
+  Key &operator=(Key const &) = delete;
+  Key(Key &&) = delete;
+  Key &operator=(Key &&) = delete;
+};
+
+/// A context is an immutable container for per-request data that must be
+/// propagated through layers that don't care about it. An example is a request
+/// ID that we may want to use when logging.
+///
+/// Conceptually, a context is a heterogeneous map, T>. Each key has
+/// an associated value type, which allows the map to be typesafe.
+///
+/// You can't add data to an existing context, instead you create a new
+/// immutable context derived from it with extra data added. When you retrieve
+/// data, the context will walk up the parent chain until the key is found.
+///
+/// Contexts should be:
+///  - passed by reference when calling synchronous functions
+///  - passed by value (move) when calling asynchronous functions. The result
+///callback of async operations will receive the context again.
+///  - cloned only when 'forking' an asynchronous computation that we don't 
wait
+///for.
+///
+/// Copy operations for this class are deleted, use an explici

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

[Phabricator 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 rCTE320468: [clangd] Introduced a Context that stores implicit 
data (authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D40485?vs=126515&id=126518#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40485

Files:
  clangd/CMakeLists.txt
  clangd/Context.cpp
  clangd/Context.h
  unittests/clangd/CMakeLists.txt
  unittests/clangd/ContextTests.cpp

Index: unittests/clangd/CMakeLists.txt
===
--- unittests/clangd/CMakeLists.txt
+++ unittests/clangd/CMakeLists.txt
@@ -11,6 +11,7 @@
 add_extra_unittest(ClangdTests
   ClangdTests.cpp
   CodeCompleteTests.cpp
+  ContextTests.cpp
   FuzzyMatchTests.cpp
   JSONExprTests.cpp
   TestFS.cpp
Index: unittests/clangd/ContextTests.cpp
===
--- unittests/clangd/ContextTests.cpp
+++ unittests/clangd/ContextTests.cpp
@@ -0,0 +1,57 @@
+//===-- ContextTests.cpp - Context tests *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Context.h"
+
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+
+TEST(ContextTests, Simple) {
+  Key IntParam;
+  Key ExtraIntParam;
+
+  Context Ctx = Context::empty().derive(IntParam, 10).derive(ExtraIntParam, 20);
+
+  EXPECT_EQ(*Ctx.get(IntParam), 10);
+  EXPECT_EQ(*Ctx.get(ExtraIntParam), 20);
+}
+
+TEST(ContextTests, MoveOps) {
+  Key> Param;
+
+  Context Ctx = Context::empty().derive(Param, llvm::make_unique(10));
+  EXPECT_EQ(**Ctx.get(Param), 10);
+
+  Context NewCtx = std::move(Ctx);
+  EXPECT_EQ(**NewCtx.get(Param), 10);
+}
+
+TEST(ContextTests, Builders) {
+  Key ParentParam;
+  Key ParentAndChildParam;
+  Key ChildParam;
+
+  Context ParentCtx =
+  Context::empty().derive(ParentParam, 10).derive(ParentAndChildParam, 20);
+  Context ChildCtx =
+  ParentCtx.derive(ParentAndChildParam, 30).derive(ChildParam, 40);
+
+  EXPECT_EQ(*ParentCtx.get(ParentParam), 10);
+  EXPECT_EQ(*ParentCtx.get(ParentAndChildParam), 20);
+  EXPECT_EQ(ParentCtx.get(ChildParam), nullptr);
+
+  EXPECT_EQ(*ChildCtx.get(ParentParam), 10);
+  EXPECT_EQ(*ChildCtx.get(ParentAndChildParam), 30);
+  EXPECT_EQ(*ChildCtx.get(ChildParam), 40);
+}
+
+} // namespace clangd
+} // namespace clang
Index: clangd/CMakeLists.txt
===
--- clangd/CMakeLists.txt
+++ clangd/CMakeLists.txt
@@ -8,6 +8,7 @@
   ClangdUnit.cpp
   ClangdUnitStore.cpp
   CodeComplete.cpp
+  Context.cpp
   Compiler.cpp
   DraftStore.cpp
   FuzzyMatch.cpp
Index: clangd/Context.h
===
--- clangd/Context.h
+++ clangd/Context.h
@@ -0,0 +1,186 @@
+//===--- Context.h - Mechanism for passing implicit data *- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Context for storing and retrieving implicit data. Useful for passing implicit
+// parameters on a per-request basis.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONTEXT_H_
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONTEXT_H_
+
+#include "llvm/ADT/STLExtras.h"
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+
+/// A key for a value of type \p Type, stored inside a context. Keys are
+/// non-movable and non-copyable. See documentation of the Context class for
+/// more details and usage examples.
+template  class Key {
+public:
+  static_assert(!std::is_reference::value,
+"Reference arguments to Key<> are not allowed");
+
+  Key() = default;
+
+  Key(Key const &) = delete;
+  Key &operator=(Key const &) = delete;
+  Key(Key &&) = delete;
+  Key &operator=(Key &&) = delete;
+};
+
+/// A context is an immutable container for per-request data that must be
+/// propagated through layers that don't care about it. An example is a request
+/// ID that we may want to use when logging.
+///
+/// Conceptually, a context is a heterogeneous map, T>. Each key has
+/// an associated value type, which allows the map to be typesafe.
+///
+/// You can't add data to an existing context, instead you create a new
+/// immutable context derived from it with extra data added. When you retrieve
+/// data, the context will walk up the parent chain until the key is found.
+///
+/// C

[PATCH] D40897: [clangd] Introduce a "Symbol" class.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 126519.
hokein marked 4 inline comments as done.
hokein added a comment.

Address remaining comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40897

Files:
  clangd/CMakeLists.txt
  clangd/index/CMakeLists.txt
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/CMakeLists.txt
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- /dev/null
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -0,0 +1,110 @@
+//===-- SymbolCollectorTests.cpp  ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "index/SymbolCollector.h"
+#include "clang/Index/IndexingAction.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "gtest/gtest.h"
+#include "gmock/gmock.h"
+
+#include 
+#include 
+
+using testing::UnorderedElementsAre;
+using testing::Eq;
+using testing::Field;
+
+// GMock helpers for matching Symbol.
+MATCHER_P(QName, Name, "") { return arg.second.QualifiedName == Name; }
+
+namespace clang {
+namespace clangd {
+
+namespace {
+class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
+ public:
+  SymbolIndexActionFactory() = default;
+
+  clang::FrontendAction *create() override {
+index::IndexingOptions IndexOpts;
+IndexOpts.SystemSymbolFilter =
+index::IndexingOptions::SystemSymbolFilterKind::All;
+IndexOpts.IndexFunctionLocals = false;
+Collector = std::make_shared();
+FrontendAction *Action =
+index::createIndexingAction(Collector, IndexOpts, nullptr).release();
+return Action;
+  }
+
+  std::shared_ptr Collector;
+};
+
+class SymbolCollectorTest : public ::testing::Test {
+public:
+  bool runSymbolCollector(StringRef HeaderCode, StringRef MainCode) {
+llvm::IntrusiveRefCntPtr InMemoryFileSystem(
+new vfs::InMemoryFileSystem);
+llvm::IntrusiveRefCntPtr Files(
+new FileManager(FileSystemOptions(), InMemoryFileSystem));
+
+const std::string FileName = "symbol.cc";
+const std::string HeaderName = "symbols.h";
+auto Factory = llvm::make_unique();
+
+tooling::ToolInvocation Invocation(
+{"symbol_collector", "-fsyntax-only", "-std=c++11", FileName},
+Factory->create(), Files.get(),
+std::make_shared());
+
+InMemoryFileSystem->addFile(HeaderName, 0,
+llvm::MemoryBuffer::getMemBuffer(HeaderCode));
+
+std::string Content = "#include\"" + std::string(HeaderName) + "\"";
+Content += "\n" + MainCode.str();
+InMemoryFileSystem->addFile(FileName, 0,
+llvm::MemoryBuffer::getMemBuffer(Content));
+Invocation.run();
+Symbols = Factory->Collector->takeSymbols();
+return true;
+  }
+
+protected:
+  SymbolSlab Symbols;
+};
+
+TEST_F(SymbolCollectorTest, CollectSymbol) {
+  const std::string Header = R"(
+class Foo {
+  void f();
+};
+void f1();
+inline void f2() {}
+  )";
+  const std::string Main = R"(
+namespace {
+void ff() {} // ignore
+}
+void f1() {}
+  )";
+  runSymbolCollector(Header, Main);
+  EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Foo::f"),
+QName("f1"), QName("f2")));
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: unittests/clangd/CMakeLists.txt
===
--- unittests/clangd/CMakeLists.txt
+++ unittests/clangd/CMakeLists.txt
@@ -15,12 +15,14 @@
   JSONExprTests.cpp
   TestFS.cpp
   TraceTests.cpp
+  SymbolCollectorTests.cpp
   )
 
 target_link_libraries(ClangdTests
   PRIVATE
   clangBasic
   clangDaemon
+  clangdIndex
   clangFormat
   clangFrontend
   clangSema
Index: clangd/index/SymbolCollector.h
===
--- /dev/null
+++ clangd/index/SymbolCollector.h
@@ -0,0 +1,43 @@
+//===--- SymbolCollector.h ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Index.h"
+
+#

[PATCH] D38831: [libcxx] P0604, invoke_result and is_invocable

[Zhihao Yuan via Phabricator via cfe-commits]

lichray added a comment.

  [...]test/std/utilities/meta/meta.rel/is_nothrow_invocable.pass.cpp:118:9: 
error: static_assert failed due to requirement 
'!std::is_nothrow_invocable_r_v' ""
  static_assert(!std::is_nothrow_invocable_r_v, "");
  ^ ~~~
  1 error generated.
  --

with Clang-5.0 and trunk.  Can you reproduce?


https://reviews.llvm.org/D38831



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

[PATCH] D40897: [clangd] Introduce a "Symbol" class.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clangd/index/Index.cpp:34
+
+SymbolSlab::const_iterator SymbolSlab::find(const SymbolID& SymID) const {
+  return Symbols.find(SymID);

sammccall wrote:
> assert frozen? (and in begin())
We may not do the assert "frozen" in these getter methods -- as writers may 
also have needs to access these APIs (e.g. checking whether SymbolID is already 
in the slab before constructing and inserting a new Symbol).



Comment at: clangd/index/SymbolCollector.h:35
+
+  StringRef getFilename() const {
+return Filename;

sammccall wrote:
> What's this for? Seems like we should be able to handle multiple TUs with one 
> collector?
We don't have particular usage for this method except for testing. Removed it.  


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40897



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

r320471 - [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Tue Dec 12 03:35:46 2017
New Revision: 320471

URL: http://llvm.org/viewvc/llvm-project?rev=320471&view=rev
Log:
[SemaCodeComplete] Allow passing out scope specifiers in qualified-id 
completions via completion context.

Reviewers: ilya-biryukov, arphaman

Reviewed By: arphaman

Subscribers: nik, cfe-commits

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

Modified:
cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
cfe/trunk/lib/Sema/SemaCodeComplete.cpp

Modified: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h?rev=320471&r1=320470&r2=320471&view=diff
==
--- cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h (original)
+++ cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h Tue Dec 12 03:35:46 2017
@@ -18,6 +18,7 @@
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/Type.h"
 #include "clang/Sema/CodeCompleteOptions.h"
+#include "clang/Sema/DeclSpec.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
@@ -280,6 +281,10 @@ private:
   /// \brief The identifiers for Objective-C selector parts.
   ArrayRef SelIdents;
 
+  /// \brief The scope specifier that comes before the completion token e.g.
+  /// "a::b::"
+  llvm::Optional ScopeSpecifier;
+
 public:
   /// \brief Construct a new code-completion context of the given kind.
   CodeCompletionContext(enum Kind Kind) : Kind(Kind), SelIdents(None) { }
@@ -315,8 +320,20 @@ public:
   /// \brief Determines whether we want C++ constructors as results within this
   /// context.
   bool wantConstructorResults() const;
-};
 
+  /// \brief Sets the scope specifier that comes before the completion token.
+  /// This is expected to be set in code completions on qualfied specifiers
+  /// (e.g. "a::b::").
+  void setCXXScopeSpecifier(CXXScopeSpec SS) {
+this->ScopeSpecifier = std::move(SS);
+  }
+
+  llvm::Optional getCXXScopeSpecifier() {
+if (ScopeSpecifier)
+  return ScopeSpecifier.getPointer();
+return llvm::None;
+  }
+};
 
 /// \brief A "string" used to describe how code completion can
 /// be performed for an entity.

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=320471&r1=320470&r2=320471&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Tue Dec 12 03:35:46 2017
@@ -4603,9 +4603,19 @@ void Sema::CodeCompleteAssignmentRHS(Sco
 
 void Sema::CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
bool EnteringContext) {
-  if (!SS.getScopeRep() || !CodeCompleter)
+  if (SS.isEmpty() || !CodeCompleter)
 return;
 
+  // We want to keep the scope specifier even if it's invalid (e.g. the scope
+  // "a::b::" is not corresponding to any context/namespace in the AST), since
+  // it can be useful for global code completion which have information about
+  // contexts/symbols that are not in the AST.
+  if (SS.isInvalid()) {
+CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
+CC.setCXXScopeSpecifier(SS);
+HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);
+return;
+  }
   // Always pretend to enter a context to ensure that a dependent type
   // resolves to a dependent record.
   DeclContext *Ctx = computeDeclContext(SS, /*EnteringContext=*/true);
@@ -4621,7 +4631,7 @@ void Sema::CodeCompleteQualifiedId(Scope
 CodeCompleter->getCodeCompletionTUInfo(),
 CodeCompletionContext::CCC_Name);
   Results.EnterNewScope();
-  
+
   // The "template" keyword can follow "::" in the grammar, but only
   // put it into the grammar if the nested-name-specifier is dependent.
   NestedNameSpecifier *NNS = SS.getScopeRep();
@@ -4635,16 +4645,18 @@ void Sema::CodeCompleteQualifiedId(Scope
   // qualified-id completions.
   if (!EnteringContext)
 MaybeAddOverrideCalls(*this, Ctx, Results);
-  Results.ExitScope();  
-  
+  Results.ExitScope();
+
   CodeCompletionDeclConsumer Consumer(Results, CurContext);
   LookupVisibleDecls(Ctx, LookupOrdinaryName, Consumer,
  /*IncludeGlobalScope=*/true,
  /*IncludeDependentBases=*/true);
 
-  HandleCodeCompleteResults(this, CodeCompleter, 
-Results.getCompletionContext(),
-Results.data(),Results.size());
+  auto CC = Results.getCompletionContext();
+  CC.setCXXScopeSpecifier(SS);
+
+  HandleCodeCompleteResults(this, CodeCompleter, CC, Results.data(),
+Results.size());
 }
 
 void Sema::CodeCompleteUsing(Scope *S) {


___
cfe-commits mailing list
cfe-commits@lists.ll

[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320471: [SemaCodeComplete] Allow passing out scope 
specifiers in qualified-id… (authored by ioeric, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D40563

Files:
  cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
  cfe/trunk/lib/Sema/SemaCodeComplete.cpp

Index: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
===
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp
@@ -4603,9 +4603,19 @@
 
 void Sema::CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
bool EnteringContext) {
-  if (!SS.getScopeRep() || !CodeCompleter)
+  if (SS.isEmpty() || !CodeCompleter)
 return;
 
+  // We want to keep the scope specifier even if it's invalid (e.g. the scope
+  // "a::b::" is not corresponding to any context/namespace in the AST), since
+  // it can be useful for global code completion which have information about
+  // contexts/symbols that are not in the AST.
+  if (SS.isInvalid()) {
+CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
+CC.setCXXScopeSpecifier(SS);
+HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);
+return;
+  }
   // Always pretend to enter a context to ensure that a dependent type
   // resolves to a dependent record.
   DeclContext *Ctx = computeDeclContext(SS, /*EnteringContext=*/true);
@@ -4621,7 +4631,7 @@
 CodeCompleter->getCodeCompletionTUInfo(),
 CodeCompletionContext::CCC_Name);
   Results.EnterNewScope();
-  
+
   // The "template" keyword can follow "::" in the grammar, but only
   // put it into the grammar if the nested-name-specifier is dependent.
   NestedNameSpecifier *NNS = SS.getScopeRep();
@@ -4635,16 +4645,18 @@
   // qualified-id completions.
   if (!EnteringContext)
 MaybeAddOverrideCalls(*this, Ctx, Results);
-  Results.ExitScope();  
-  
+  Results.ExitScope();
+
   CodeCompletionDeclConsumer Consumer(Results, CurContext);
   LookupVisibleDecls(Ctx, LookupOrdinaryName, Consumer,
  /*IncludeGlobalScope=*/true,
  /*IncludeDependentBases=*/true);
 
-  HandleCodeCompleteResults(this, CodeCompleter, 
-Results.getCompletionContext(),
-Results.data(),Results.size());
+  auto CC = Results.getCompletionContext();
+  CC.setCXXScopeSpecifier(SS);
+
+  HandleCodeCompleteResults(this, CodeCompleter, CC, Results.data(),
+Results.size());
 }
 
 void Sema::CodeCompleteUsing(Scope *S) {
Index: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
===
--- cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
+++ cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
@@ -18,6 +18,7 @@
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/Type.h"
 #include "clang/Sema/CodeCompleteOptions.h"
+#include "clang/Sema/DeclSpec.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
@@ -280,6 +281,10 @@
   /// \brief The identifiers for Objective-C selector parts.
   ArrayRef SelIdents;
 
+  /// \brief The scope specifier that comes before the completion token e.g.
+  /// "a::b::"
+  llvm::Optional ScopeSpecifier;
+
 public:
   /// \brief Construct a new code-completion context of the given kind.
   CodeCompletionContext(enum Kind Kind) : Kind(Kind), SelIdents(None) { }
@@ -315,8 +320,20 @@
   /// \brief Determines whether we want C++ constructors as results within this
   /// context.
   bool wantConstructorResults() const;
-};
 
+  /// \brief Sets the scope specifier that comes before the completion token.
+  /// This is expected to be set in code completions on qualfied specifiers
+  /// (e.g. "a::b::").
+  void setCXXScopeSpecifier(CXXScopeSpec SS) {
+this->ScopeSpecifier = std::move(SS);
+  }
+
+  llvm::Optional getCXXScopeSpecifier() {
+if (ScopeSpecifier)
+  return ScopeSpecifier.getPointer();
+return llvm::None;
+  }
+};
 
 /// \brief A "string" used to describe how code completion can
 /// be performed for an entity.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40731: Integrate CHash into CLang

[Christian Dietrich via Phabricator via cfe-commits]

stettberger updated this revision to Diff 126523.
stettberger added a comment.

I removed the magic constants and added the number of children for various AST 
nodes. This should avoid hash collisions.


Repository:
  rC Clang

https://reviews.llvm.org/D40731

Files:
  include/clang/AST/AttrDataCollectors.td
  include/clang/AST/CHashVisitor.h
  include/clang/AST/CMakeLists.txt
  include/clang/AST/DataCollection.h
  include/clang/AST/DeclDataCollectors.td
  include/clang/AST/StmtDataCollectors.td
  include/clang/AST/TypeDataCollectors.td
  unittests/AST/CHashTest.cpp
  unittests/AST/CMakeLists.txt

Index: unittests/AST/CMakeLists.txt
===
--- unittests/AST/CMakeLists.txt
+++ unittests/AST/CMakeLists.txt
@@ -18,6 +18,7 @@
   PostOrderASTVisitor.cpp
   SourceLocationTest.cpp
   StmtPrinterTest.cpp
+  CHashTest.cpp
   )
 
 target_link_libraries(ASTTests
Index: unittests/AST/CHashTest.cpp
===
--- /dev/null
+++ unittests/AST/CHashTest.cpp
@@ -0,0 +1,91 @@
+//===- unittests/AST/DataCollectionTest.cpp ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file contains tests for the DataCollection module.
+//
+// They work by hashing the collected data of two nodes and asserting that the
+// hash values are equal iff the nodes are considered equal.
+//
+//===--===//
+
+#include "clang/AST/CHashVisitor.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace clang;
+using namespace tooling;
+
+
+class CHashConsumer : public ASTConsumer {
+CompilerInstance &CI;
+llvm::MD5::MD5Result *ASTHash;
+
+public:
+
+CHashConsumer(CompilerInstance &CI, llvm::MD5::MD5Result *ASTHash)
+: CI(CI), ASTHash(ASTHash){}
+
+virtual void HandleTranslationUnit(clang::ASTContext &Context) override {
+TranslationUnitDecl *TU = Context.getTranslationUnitDecl();
+
+// Traversing the translation unit decl via a RecursiveASTVisitor
+// will visit all nodes in the AST.
+CHashVisitor<> Visitor(Context);
+Visitor.TraverseDecl(TU);
+// Copy Away the resulting hash
+*ASTHash = *Visitor.getHash(TU);
+
+}
+
+~CHashConsumer() override {}
+};
+
+struct CHashAction : public ASTFrontendAction {
+llvm::MD5::MD5Result *Hash;
+
+CHashAction(llvm::MD5::MD5Result *Hash) : Hash(Hash) {}
+
+std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
+   StringRef) override {
+return std::unique_ptr(new CHashConsumer(CI, Hash));
+}
+};
+
+static testing::AssertionResult
+isASTHashEqual(StringRef Code1, StringRef Code2) {
+llvm::MD5::MD5Result Hash1, Hash2;
+if (!runToolOnCode(new CHashAction(&Hash1), Code1)) {
+return testing::AssertionFailure()
+<< "Parsing error in (A)\"" << Code1.str() << "\"";
+}
+if (!runToolOnCode(new CHashAction(&Hash2), Code2)) {
+return testing::AssertionFailure()
+<< "Parsing error in (B) \"" << Code2.str() << "\"";
+}
+return testing::AssertionResult(Hash1 == Hash2);
+}
+
+TEST(CHashVisitor, TestRecordTypes) {
+ASSERT_TRUE(isASTHashEqual( // Unused struct
+ "struct foobar { int a0; char a1; unsigned long a2; };",
+ "struct foobar { int a0; char a1;};"
+ ));
+
+}
+
+TEST(CHashVisitor, TestSourceStructure) {
+ASSERT_FALSE(isASTHashEqual(
+ "void foo() { int c; if (0) { c = 1; } }",
+ "void foo() { int c; if (0) { } c = 1; }"));
+
+ASSERT_FALSE(isASTHashEqual(
+ "void f1() {} void f2() {   }",
+ "void f1() {} void f2() { f1(); }"));
+}
Index: include/clang/AST/TypeDataCollectors.td
===
--- include/clang/AST/TypeDataCollectors.td
+++ include/clang/AST/TypeDataCollectors.td
@@ -0,0 +1,78 @@
+//--- Types ---//
+
+class Type {
+  code Code = [{
+ addData(llvm::hash_value(S->getTypeClass()));
+  }];
+}
+
+class BuiltinType {
+   code Code = [{
+  addData(S->getKind());
+   }];
+}
+
+class ArrayType  {
+   code Code = [{
+  addData(S->getSizeModifier());
+  addData(S->getIndexTypeCVRQualifiers());
+   }];
+}
+
+class ConstantArrayType {
+   code Code = [{
+  addData(S->getSize().getZExtValue());
+   }];
+}
+
+class VectorType {
+   code Code = [{
+  addData(S->getNumElements());
+  addData(S->getVectorKind());
+   }];
+}
+
+class FunctionTy

[PATCH] D41108: [clang-format] Improve ObjC headers detection.

[Jacek Olesiak via Phabricator via cfe-commits]

jolesiak created this revision.
jolesiak added a reviewer: krasimir.
Herald added a subscriber: klimek.

This patch improves detection of ObjC header files.
Right now many ObjC headers, especially short ones, are categorized as C/C++.

Way of filtering still isn't the best, as most likely it should be token-based.


Repository:
  rC Clang

https://reviews.llvm.org/D41108

Files:
  lib/Format/Format.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -79,6 +79,17 @@
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
+  Style = getStyle("LLVM", "a.h", "none", "@interface\n"
+  "@end\n"
+  "//comment");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+  Style = getStyle("LLVM", "a.h", "none", "@interface\n"
+  "@end //comment");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
   // No recognizable ObjC.
   Style = getStyle("LLVM", "a.h", "none", "void f() {}");
   ASSERT_TRUE((bool)Style);
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -2129,7 +2129,9 @@
   // should be improved over time and probably be done on tokens, not one the
   // bare content of the file.
   if (Style.Language == FormatStyle::LK_Cpp && FileName.endswith(".h") &&
-  (Code.contains("\n- (") || Code.contains("\n+ (")))
+  (Code.contains("\n- (") || Code.contains("\n+ (") ||
+   Code.contains("\n@end\n") || Code.contains("\n@end ") ||
+   Code.endswith("@end")))
 Style.Language = FormatStyle::LK_ObjC;
 
   FormatStyle FallbackStyle = getNoStyle();


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -79,6 +79,17 @@
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
+  Style = getStyle("LLVM", "a.h", "none", "@interface\n"
+  "@end\n"
+  "//comment");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+  Style = getStyle("LLVM", "a.h", "none", "@interface\n"
+  "@end //comment");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
   // No recognizable ObjC.
   Style = getStyle("LLVM", "a.h", "none", "void f() {}");
   ASSERT_TRUE((bool)Style);
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -2129,7 +2129,9 @@
   // should be improved over time and probably be done on tokens, not one the
   // bare content of the file.
   if (Style.Language == FormatStyle::LK_Cpp && FileName.endswith(".h") &&
-  (Code.contains("\n- (") || Code.contains("\n+ (")))
+  (Code.contains("\n- (") || Code.contains("\n+ (") ||
+   Code.contains("\n@end\n") || Code.contains("\n@end ") ||
+   Code.endswith("@end")))
 Style.Language = FormatStyle::LK_ObjC;
 
   FormatStyle FallbackStyle = getNoStyle();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38425: [clangd] Document highlights for clangd

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM minor a slight tweak to fix compilation. I'll land this today.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38425



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

[PATCH] D38425: [clangd] Document highlights for clangd

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320474: [clangd] Document highlights for clangd (authored by 
ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D38425?vs=126371&id=126529#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38425

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/ClangdLSPServer.h
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.h
  clang-tools-extra/trunk/clangd/ClangdUnit.cpp
  clang-tools-extra/trunk/clangd/ClangdUnit.h
  clang-tools-extra/trunk/clangd/Protocol.cpp
  clang-tools-extra/trunk/clangd/Protocol.h
  clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
  clang-tools-extra/trunk/clangd/ProtocolHandlers.h
  clang-tools-extra/trunk/test/clangd/documenthighlight.test
  clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test
  clang-tools-extra/trunk/test/clangd/initialize-params.test

Index: clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
===
--- clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
+++ clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
@@ -72,4 +72,6 @@
   Register("textDocument/rename", &ProtocolCallbacks::onRename);
   Register("workspace/didChangeWatchedFiles", &ProtocolCallbacks::onFileEvent);
   Register("workspace/executeCommand", &ProtocolCallbacks::onCommand);
+  Register("textDocument/documentHighlight",
+   &ProtocolCallbacks::onDocumentHighlight);
 }
Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp
@@ -509,6 +509,39 @@
   return llvm::None;
 }
 
+llvm::Expected>>
+ClangdServer::findDocumentHighlights(PathRef File, Position Pos) {
+  auto FileContents = DraftMgr.getDraft(File);
+  if (!FileContents.Draft)
+return llvm::make_error(
+"findDocumentHighlights called on non-added file",
+llvm::errc::invalid_argument);
+
+  auto TaggedFS = FSProvider.getTaggedFileSystem(File);
+
+  std::shared_ptr Resources = Units.getFile(File);
+  if (!Resources)
+return llvm::make_error(
+"findDocumentHighlights called on non-added file",
+llvm::errc::invalid_argument);
+
+  std::vector Result;
+  llvm::Optional Err;
+  Resources->getAST().get()->runUnderLock([Pos, &Result, &Err,
+   this](ParsedAST *AST) {
+if (!AST) {
+  Err = llvm::make_error("Invalid AST",
+llvm::errc::invalid_argument);
+  return;
+}
+Result = clangd::findDocumentHighlights(*AST, Pos, Logger);
+  });
+
+  if (Err)
+return std::move(*Err);
+  return make_tagged(Result, TaggedFS.Tag);
+}
+
 std::future ClangdServer::scheduleReparseAndDiags(
 PathRef File, VersionedDraft Contents, std::shared_ptr Resources,
 Tagged> TaggedFS) {
Index: clang-tools-extra/trunk/clangd/Protocol.h
===
--- clang-tools-extra/trunk/clangd/Protocol.h
+++ clang-tools-extra/trunk/clangd/Protocol.h
@@ -557,6 +557,36 @@
 };
 bool fromJSON(const json::Expr &, RenameParams &);
 
+enum class DocumentHighlightKind { Text = 1, Read = 2, Write = 3 };
+
+/// A document highlight is a range inside a text document which deserves
+/// special attention. Usually a document highlight is visualized by changing
+/// the background color of its range.
+
+struct DocumentHighlight {
+
+  /// The range this highlight applies to.
+
+  Range range;
+
+  /// The highlight kind, default is DocumentHighlightKind.Text.
+
+  DocumentHighlightKind kind = DocumentHighlightKind::Text;
+
+  friend bool operator<(const DocumentHighlight &LHS,
+const DocumentHighlight &RHS) {
+int LHSKind = static_cast(LHS.kind);
+int RHSKind = static_cast(RHS.kind);
+return std::tie(LHS.range, LHSKind) < std::tie(RHS.range, RHSKind);
+  }
+
+  friend bool operator==(const DocumentHighlight &LHS,
+ const DocumentHighlight &RHS) {
+return LHS.kind == RHS.kind && LHS.range == RHS.range;
+  }
+};
+json::Expr toJSON(const DocumentHighlight &DH);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -57,6 +57,7 @@
  {"triggerCharacters", {"(", ","}},
  }},
 {"definitionProvider", true},
+{"documentHighlightProvider", true},
 {"renameProvider", true},
 {"executeCommandProvider",
  json::obj{
@@ -235,6 +236,22 @@
   C.reply(Result ? URI::fromFile(*Resul

[clang-tools-extra] r320474 - [clangd] Document highlights for clangd

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue Dec 12 04:27:47 2017
New Revision: 320474

URL: http://llvm.org/viewvc/llvm-project?rev=320474&view=rev
Log:
[clangd] Document highlights for clangd

Summary: Implementation of Document Highlights Request as described in
LSP.

Contributed by William Enright (nebiroth).

Reviewers: malaperle, krasimir, bkramer, ilya-biryukov

Reviewed By: malaperle

Subscribers: mgrang, sammccall, klimek, ioeric, rwols, cfe-commits, arphaman, 
ilya-biryukov

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

Added:
clang-tools-extra/trunk/test/clangd/documenthighlight.test
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
clang-tools-extra/trunk/clangd/ProtocolHandlers.h
clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test
clang-tools-extra/trunk/test/clangd/initialize-params.test

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=320474&r1=320473&r2=320474&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue Dec 12 04:27:47 2017
@@ -57,6 +57,7 @@ void ClangdLSPServer::onInitialize(Ctx C
  {"triggerCharacters", {"(", ","}},
  }},
 {"definitionProvider", true},
+{"documentHighlightProvider", true},
 {"renameProvider", true},
 {"executeCommandProvider",
  json::obj{
@@ -235,6 +236,22 @@ void ClangdLSPServer::onSwitchSourceHead
   C.reply(Result ? URI::fromFile(*Result).uri : "");
 }
 
+void ClangdLSPServer::onDocumentHighlight(Ctx C,
+  TextDocumentPositionParams &Params) {
+
+  auto Highlights = Server.findDocumentHighlights(
+  Params.textDocument.uri.file,
+  Position{Params.position.line, Params.position.character});
+
+  if (!Highlights) {
+C.replyError(ErrorCode::InternalError,
+ llvm::toString(Highlights.takeError()));
+return;
+  }
+
+  C.reply(json::ary(Highlights->Value));
+}
+
 ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, unsigned AsyncThreadsCount,
  bool StorePreamblesInMemory,
  const clangd::CodeCompleteOptions &CCOpts,

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=320474&r1=320473&r2=320474&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Tue Dec 12 04:27:47 2017
@@ -69,6 +69,7 @@ private:
   void onSignatureHelp(Ctx C, TextDocumentPositionParams &Params) override;
   void onGoToDefinition(Ctx C, TextDocumentPositionParams &Params) override;
   void onSwitchSourceHeader(Ctx C, TextDocumentIdentifier &Params) override;
+  void onDocumentHighlight(Ctx C, TextDocumentPositionParams &Params) override;
   void onFileEvent(Ctx C, DidChangeWatchedFilesParams &Params) override;
   void onCommand(Ctx C, ExecuteCommandParams &Params) override;
   void onRename(Ctx C, RenameParams &Parames) override;

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=320474&r1=320473&r2=320474&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Dec 12 04:27:47 2017
@@ -509,6 +509,39 @@ llvm::Optional ClangdServer::switc
   return llvm::None;
 }
 
+llvm::Expected>>
+ClangdServer::findDocumentHighlights(PathRef File, Position Pos) {
+  auto FileContents = DraftMgr.getDraft(File);
+  if (!FileContents.Draft)
+return llvm::make_error(
+"findDocumentHighlights called on non-added file",
+llvm::errc::invalid_argument);
+
+  auto TaggedFS = FSProvider.getTaggedFileSystem(File);
+
+  std::shared_ptr Resources = Units.getFile(File);
+  if (!Resources)
+return llvm::make_error(
+"findDocumentHighlights called on non-added file",
+llvm::errc::invalid_argument);
+
+  std::vector Result;
+  llvm::Optional Err;
+  Resources->getAST().get()->runUnderLock([Pos, &Result, &Err,
+   this](ParsedAST *AST)

[PATCH] D41102: Setup clang-doc frontend framework

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere requested changes to this revision.
JDevlieghere added inline comments.
This revision now requires changes to proceed.



Comment at: tools/clang-doc/ClangDocReporter.cpp:106
+  FI.Filename = Filename;
+  FileRecords.insert(std::pair(Filename, 
std::move(FI)));
+}

JonasToth wrote:
> `std::make_pair(FileName, std::move(FI))` would be shorter.
Also I don't think there's a point in moving StringRefs as they are intended to 
be light weight already.



Comment at: tools/clang-doc/ClangDocReporter.cpp:136
+  }
+  C->isSelfClosing() ? CurrentCI->SelfClosing = true
+ : CurrentCI->SelfClosing = false;

Why not `CurrentCI->SelfClosing = C->isSelfClosing()`?



Comment at: tools/clang-doc/ClangDocReporter.cpp:154
+  ParamCommandComment::getDirectionAsString(C->getDirection());
+  C->isDirectionExplicit() ? CurrentCI->Explicit = true
+   : CurrentCI->Explicit = false;

Same here.



Comment at: tools/clang-doc/ClangDocReporter.cpp:205
+  ConstCommentVisitor::visit(C);
+  for (comments::Comment::child_iterator I = C->child_begin(),
+ E = C->child_end();

JonasToth wrote:
> `comments::Comment` could get a `childs()` method returning a view to iterate 
> with nice range based loops.
Yup, you can use `llvm::make_range` for this.



Comment at: tools/clang-doc/ClangDocReporter.h:45
+  bool Explicit = false;
+  std::vector Args;
+  std::vector Attrs;

Would this be a good use case for `llvm::SmallVector`?



Comment at: tools/clang-doc/ClangDocReporter.h:46
+  std::vector Args;
+  std::vector Attrs;
+  std::vector Position;

Why not use `llvm::StringMap` here? I'm guessing you went with a `StringPair` 
because of the YAMP serialization, but that should work with StringMap. 



Comment at: tools/clang-doc/ClangDocReporter.h:90
+
+  std::set GetFilesInThisTU() const { return FilesInThisTU; }
+  bool HasFile(StringRef Filename) const;

You probably want to return a const-ref here, rather than a copy. 


https://reviews.llvm.org/D41102



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

[clang-tools-extra] r320476 - [clangd] clang-format the code. NFC

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue Dec 12 04:56:46 2017
New Revision: 320476

URL: http://llvm.org/viewvc/llvm-project?rev=320476&view=rev
Log:
[clangd] clang-format the code. NFC

Modified:
clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp?rev=320476&r1=320475&r2=320476&view=diff
==
--- clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp (original)
+++ clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp Tue Dec 12 04:56:46 
2017
@@ -13,8 +13,8 @@
 ///
 
//===--===//
 
-#include "CodeComplete.h"
 #include "ClangdLSPServer.h"
+#include "CodeComplete.h"
 #include 
 
 extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {

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=320476&r1=320475&r2=320476&view=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Dec 12 04:56:46 2017
@@ -172,8 +172,7 @@ int main(int argc, char *argv[]) {
   CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(Out, WorkerThreadsCount, StorePreamblesInMemory,
-CCOpts, ResourceDirRef,
-CompileCommandsDirPath);
+CCOpts, ResourceDirRef, CompileCommandsDirPath);
   constexpr int NoShutdownRequestErrorCode = 1;
   llvm::set_thread_name("clangd.main");
   return LSPServer.run(std::cin) ? 0 : NoShutdownRequestErrorCode;

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=320476&r1=320475&r2=320476&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Tue Dec 12 
04:56:46 2017
@@ -231,8 +231,9 @@ void TestGlobalScopeCompletion(clangd::C
   // A macro.
   EXPECT_IFF(Opts.IncludeMacros, Results.items, Has("MACRO"));
   // Local items. Must be present always.
-  EXPECT_THAT(Results.items, AllOf(Has("local_var"), Has("LocalClass"),
- Contains(Kind(CompletionItemKind::Snippet;
+  EXPECT_THAT(Results.items,
+  AllOf(Has("local_var"), Has("LocalClass"),
+Contains(Kind(CompletionItemKind::Snippet;
   // Check documentation.
   EXPECT_IFF(Opts.IncludeBriefComments, Results.items,
  Contains(IsDocumented()));


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

[PATCH] D38831: [libcxx] P0604, invoke_result and is_invocable

[Agustín Bergé via Phabricator via cfe-commits]

K-ballo added a comment.

In https://reviews.llvm.org/D38831#952228, @lichray wrote:

>   [...]test/std/utilities/meta/meta.rel/is_nothrow_invocable.pass.cpp:118:9: 
> error: static_assert failed due to requirement 
> '!std::is_nothrow_invocable_r_v' ""
>   static_assert(!std::is_nothrow_invocable_r_v, "");
>   ^ ~~~
>   1 error generated.
>   --
>
>
> with Clang-5.0 and trunk.  Can you reproduce?


I can reproduce.

It seems the intention was to match the tests from the previous section, so an 
extra `int` argument would be missing. I'll take care of it.


https://reviews.llvm.org/D38831



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

[PATCH] D41108: [clang-format] Improve ObjC headers detection.

[Krasimir Georgiev via Phabricator via cfe-commits]

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

Thank you! I'll go ahead and commit this for you!


Repository:
  rC Clang

https://reviews.llvm.org/D41108



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

r320479 - [clang-format] Improve ObjC headers detection.

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Tue Dec 12 05:43:59 2017
New Revision: 320479

URL: http://llvm.org/viewvc/llvm-project?rev=320479&view=rev
Log:
[clang-format] Improve ObjC headers detection.

This patch improves detection of ObjC header files.
Right now many ObjC headers, especially short ones, are categorized as C/C++.

Way of filtering still isn't the best, as most likely it should be token-based.

Contributed by jolesiak!

Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/FormatTestObjC.cpp

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=320479&r1=320478&r2=320479&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Tue Dec 12 05:43:59 2017
@@ -2129,7 +2129,9 @@ llvm::Expected getStyle(Str
   // should be improved over time and probably be done on tokens, not one the
   // bare content of the file.
   if (Style.Language == FormatStyle::LK_Cpp && FileName.endswith(".h") &&
-  (Code.contains("\n- (") || Code.contains("\n+ (")))
+  (Code.contains("\n- (") || Code.contains("\n+ (") ||
+   Code.contains("\n@end\n") || Code.contains("\n@end ") ||
+   Code.endswith("@end")))
 Style.Language = FormatStyle::LK_ObjC;
 
   FormatStyle FallbackStyle = getNoStyle();

Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=320479&r1=320478&r2=320479&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Tue Dec 12 05:43:59 2017
@@ -79,6 +79,17 @@ TEST(FormatTestObjCStyle, DetectsObjCInH
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
+  Style = getStyle("LLVM", "a.h", "none", "@interface\n"
+  "@end\n"
+  "//comment");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+  Style = getStyle("LLVM", "a.h", "none", "@interface\n"
+  "@end //comment");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
   // No recognizable ObjC.
   Style = getStyle("LLVM", "a.h", "none", "void f() {}");
   ASSERT_TRUE((bool)Style);


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

[PATCH] D36952: [libclang] Add support for checking abstractness of records

[Johann Klähn via Phabricator via cfe-commits]

jklaehn added a comment.

In https://reviews.llvm.org/D36952#951616, @arphaman wrote:

> LGTM


Thanks! Can you submit this for me? (I do not have commit access yet.)


https://reviews.llvm.org/D36952



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

[PATCH] D40897: [clangd] Introduce a "Symbol" class.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/index/CMakeLists.txt:5
+
+add_clang_library(clangdIndex
+  Index.cpp

hmm, I'm not sure whether we actually want this to be a separate library. This 
means we can't depend on anything from elsewhere in clangd, and may force us to 
create more components.

e.g. if we want to pass contexts into the index, or if we want to reuse LSP 
data models from protocol.h.

Maybe we should make this part of the main clangd lib, what do you think?



Comment at: clangd/index/Index.cpp:34
+
+SymbolSlab::const_iterator SymbolSlab::find(const SymbolID& SymID) const {
+  return Symbols.find(SymID);

hokein wrote:
> sammccall wrote:
> > assert frozen? (and in begin())
> We may not do the assert "frozen" in these getter methods -- as writers may 
> also have needs to access these APIs (e.g. checking whether SymbolID is 
> already in the slab before constructing and inserting a new Symbol).
Right, I'd specifically like not to allow that if possible - it makes it very 
difficult to understand what the invariants are and what operations are allowed.

I think there's no such need now, right? If we want to add one in future, we 
can relax this check, or add a builder, or similar - at least we should 
explicitly consider it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40897



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

[PATCH] D40897: [clangd] Introduce a "Symbol" class.

[Haojian Wu via Phabricator via cfe-commits]

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

- Get rid of clangdIndex library, using the existing clangDaemon library.
- Remove the getID() method.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40897

Files:
  clangd/CMakeLists.txt
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/CMakeLists.txt
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- /dev/null
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -0,0 +1,110 @@
+//===-- SymbolCollectorTests.cpp  ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "index/SymbolCollector.h"
+#include "clang/Index/IndexingAction.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "gtest/gtest.h"
+#include "gmock/gmock.h"
+
+#include 
+#include 
+
+using testing::UnorderedElementsAre;
+using testing::Eq;
+using testing::Field;
+
+// GMock helpers for matching Symbol.
+MATCHER_P(QName, Name, "") { return arg.second.QualifiedName == Name; }
+
+namespace clang {
+namespace clangd {
+
+namespace {
+class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
+ public:
+  SymbolIndexActionFactory() = default;
+
+  clang::FrontendAction *create() override {
+index::IndexingOptions IndexOpts;
+IndexOpts.SystemSymbolFilter =
+index::IndexingOptions::SystemSymbolFilterKind::All;
+IndexOpts.IndexFunctionLocals = false;
+Collector = std::make_shared();
+FrontendAction *Action =
+index::createIndexingAction(Collector, IndexOpts, nullptr).release();
+return Action;
+  }
+
+  std::shared_ptr Collector;
+};
+
+class SymbolCollectorTest : public ::testing::Test {
+public:
+  bool runSymbolCollector(StringRef HeaderCode, StringRef MainCode) {
+llvm::IntrusiveRefCntPtr InMemoryFileSystem(
+new vfs::InMemoryFileSystem);
+llvm::IntrusiveRefCntPtr Files(
+new FileManager(FileSystemOptions(), InMemoryFileSystem));
+
+const std::string FileName = "symbol.cc";
+const std::string HeaderName = "symbols.h";
+auto Factory = llvm::make_unique();
+
+tooling::ToolInvocation Invocation(
+{"symbol_collector", "-fsyntax-only", "-std=c++11", FileName},
+Factory->create(), Files.get(),
+std::make_shared());
+
+InMemoryFileSystem->addFile(HeaderName, 0,
+llvm::MemoryBuffer::getMemBuffer(HeaderCode));
+
+std::string Content = "#include\"" + std::string(HeaderName) + "\"";
+Content += "\n" + MainCode.str();
+InMemoryFileSystem->addFile(FileName, 0,
+llvm::MemoryBuffer::getMemBuffer(Content));
+Invocation.run();
+Symbols = Factory->Collector->takeSymbols();
+return true;
+  }
+
+protected:
+  SymbolSlab Symbols;
+};
+
+TEST_F(SymbolCollectorTest, CollectSymbol) {
+  const std::string Header = R"(
+class Foo {
+  void f();
+};
+void f1();
+inline void f2() {}
+  )";
+  const std::string Main = R"(
+namespace {
+void ff() {} // ignore
+}
+void f1() {}
+  )";
+  runSymbolCollector(Header, Main);
+  EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Foo::f"),
+QName("f1"), QName("f2")));
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: unittests/clangd/CMakeLists.txt
===
--- unittests/clangd/CMakeLists.txt
+++ unittests/clangd/CMakeLists.txt
@@ -15,6 +15,7 @@
   JSONExprTests.cpp
   TestFS.cpp
   TraceTests.cpp
+  SymbolCollectorTests.cpp
   )
 
 target_link_libraries(ClangdTests
Index: clangd/index/SymbolCollector.h
===
--- /dev/null
+++ clangd/index/SymbolCollector.h
@@ -0,0 +1,43 @@
+//===--- SymbolCollector.h ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Index.h"
+
+#include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexSymbol.h"
+
+namespace clang

[clang-tools-extra] r320482 - [clangd] Removed unused variable. NFC

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue Dec 12 06:15:01 2017
New Revision: 320482

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

Modified:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=320482&r1=320481&r2=320482&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Tue Dec 12 06:15:01 2017
@@ -328,7 +328,7 @@ public:
 std::find(Decls.begin(), Decls.end(), D) == Decls.end()) {
   return true;
 }
-SourceLocation Begin, End;
+SourceLocation End;
 const LangOptions &LangOpts = AST.getLangOpts();
 SourceLocation StartOfFileLoc = SourceMgr.getLocForStartOfFile(FID);
 SourceLocation HightlightStartLoc = 
StartOfFileLoc.getLocWithOffset(Offset);


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

[PATCH] D40897: [clangd] Introduce a "Symbol" class.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clangd/index/CMakeLists.txt:5
+
+add_clang_library(clangdIndex
+  Index.cpp

sammccall wrote:
> hmm, I'm not sure whether we actually want this to be a separate library. 
> This means we can't depend on anything from elsewhere in clangd, and may 
> force us to create more components.
> 
> e.g. if we want to pass contexts into the index, or if we want to reuse LSP 
> data models from protocol.h.
> 
> Maybe we should make this part of the main clangd lib, what do you think?
As discussed offline, made it to the main clangd library.



Comment at: clangd/index/Index.cpp:34
+
+SymbolSlab::const_iterator SymbolSlab::find(const SymbolID& SymID) const {
+  return Symbols.find(SymID);

sammccall wrote:
> hokein wrote:
> > sammccall wrote:
> > > assert frozen? (and in begin())
> > We may not do the assert "frozen" in these getter methods -- as writers may 
> > also have needs to access these APIs (e.g. checking whether SymbolID is 
> > already in the slab before constructing and inserting a new Symbol).
> Right, I'd specifically like not to allow that if possible - it makes it very 
> difficult to understand what the invariants are and what operations are 
> allowed.
> 
> I think there's no such need now, right? If we want to add one in future, we 
> can relax this check, or add a builder, or similar - at least we should 
> explicitly consider it.
hmm, SymbolCollector has such need at the moment -- ` if (Symbols.find(ID) != 
Symbols.end()) return true;` to avoid creating a duplicated Symbol.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40897



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

[PATCH] D41030: [libcxx] [test] Fix MSVC warnings, null pointer deref.

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

> Avoid MSVC "warning C4293: '<<': shift count negative or too big, undefined 
> behavior". 
>  MSVC sees (1ULL << N) and warns - being guarded by const bool canFit is 
> insufficient. A small change to the code

Your compiler needs to either (a) get much smarter about its warnings, or (b) 
stop warning about this.  It's just WRONG here.


https://reviews.llvm.org/D41030



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

[PATCH] D41030: [libcxx] [test] Fix MSVC warnings, null pointer deref.

[Marshall Clow via Phabricator via cfe-commits]

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

These look fine to me.  Thanks for the attention to detail.


https://reviews.llvm.org/D41030



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

[PATCH] D40486: [clangd] Implemented logging using Context

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 126544.
ilya-biryukov marked 7 inline comments as done.
ilya-biryukov added a comment.

- Added a helper that runs functions in destructor.
- Fixed compilation after rebase.
- Got rid of FulfilContextPromiseGuard
- Added a FIXME to remove the Ctx typedef.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40486

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/ClangdUnitStore.cpp
  clangd/ClangdUnitStore.h
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Function.h
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  clangd/JSONRPCDispatcher.cpp
  clangd/JSONRPCDispatcher.h
  clangd/Logger.cpp
  clangd/Logger.h
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -9,6 +9,7 @@
 #include "ClangdServer.h"
 #include "Compiler.h"
 #include "Matchers.h"
+#include "Context.h"
 #include "Protocol.h"
 #include "TestFS.h"
 #include "gmock/gmock.h"
@@ -38,6 +39,10 @@
 using ::testing::ElementsAre;
 using ::testing::Not;
 
+/// Creates an empty context that is used for running async methods inside the
+/// testing code.
+Context testCtx() { return Context::empty().clone(); }
+
 class IgnoreDiagnostics : public DiagnosticsConsumer {
   void onDiagnosticsReady(
   PathRef File, Tagged> Diagnostics) override {}
@@ -92,12 +97,13 @@
   MockCompilationDatabase CDB;
   IgnoreDiagnostics DiagConsumer;
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-  /*StorePreamblesInMemory=*/true,
-  EmptyLogger::getInstance());
+  /*StorePreamblesInMemory=*/true);
   auto File = getVirtualTestFilePath("foo.cpp");
   auto Test = parseTextMarker(Text);
-  Server.addDocument(File, Test.Text);
-  return Server.codeComplete(File, Test.MarkerPos, Opts).get().Value;
+  Server.addDocument(File, Test.Text, testCtx());
+  return Server.codeComplete(File, Test.MarkerPos, Opts, testCtx())
+  .get()
+  .first.Value;
 }
 
 TEST(CompletionTest, Limit) {
@@ -127,7 +133,6 @@
   int Qux;
 };
   )cpp";
-
   EXPECT_THAT(completions(Body + "int main() { S().Foba^ }").items,
   AllOf(Has("FooBar"), Has("FooBaz"), Not(Has("Qux";
 
@@ -269,18 +274,17 @@
   IgnoreDiagnostics DiagConsumer;
   MockCompilationDatabase CDB;
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-  /*StorePreamblesInMemory=*/true,
-  EmptyLogger::getInstance());
+  /*StorePreamblesInMemory=*/true);
   auto File = getVirtualTestFilePath("foo.cpp");
-  Server.addDocument(File, "ignored text!");
+  Server.addDocument(File, "ignored text!", testCtx());
 
   auto Example = parseTextMarker("int cbc; int b = ^;");
   auto Results =
   Server
   .codeComplete(File, Example.MarkerPos, clangd::CodeCompleteOptions(),
-StringRef(Example.Text))
+testCtx(), StringRef(Example.Text))
   .get()
-  .Value;
+  .first.Value;
   EXPECT_THAT(Results.items, Contains(Named("cbc")));
 }
 
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -9,7 +9,7 @@
 
 #include "ClangdLSPServer.h"
 #include "ClangdServer.h"
-#include "Logger.h"
+#include "Context.h"
 #include "TestFS.h"
 #include "clang/Config/config.h"
 #include "llvm/ADT/SmallVector.h"
@@ -28,8 +28,13 @@
 
 namespace clang {
 namespace clangd {
+
 namespace {
 
+/// Creates an empty context that is used for running async methods inside the
+/// testing code.
+Context testCtx() { return Context::empty().clone(); }
+
 // Don't wait for async ops in clangd test more than that to avoid blocking
 // indefinitely in case of bugs.
 static const std::chrono::seconds DefaultFutureTimeout =
@@ -123,8 +128,7 @@
 ErrorCheckingDiagConsumer DiagConsumer;
 MockCompilationDatabase CDB;
 ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-/*StorePreamblesInMemory=*/true,
-EmptyLogger::getInstance());
+/*StorePreamblesInMemory=*/true);
 for (const auto &FileWithContents : ExtraFiles)
   FS.Files[getVirtualTestFilePath(FileWithContents.first)] =
   FileWithContents.second;
@@ -135,7 +139,8 @@
 
 // Have to sync reparses because requests are processed on the calling
 // thread.
-auto AddDocFuture = Server.addDocumen

[PATCH] D40486: [clangd] Implemented logging using Context

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I think all the comments are addressed now and this change is ready for another 
round of review.
Let me know if I missed something.




Comment at: clangd/ClangdServer.cpp:36
 
-  ~FulfillPromiseGuard() { Promise.set_value(); }
+  ~FulfillContextPromiseGuard() { Promise.set_value(std::move(Ctx)); }
 

sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > Yikes, I can see how we got here, but we really don't get to move out of 
> > > something we received an lvalue-reference to...
> > > 
> > > I think a safer idiom here is turning a whole lambda into a destructor:
> > > 
> > >auto Fulfil = MakeDestructor([&]{
> > >   DonePromise.set_value(std::move(Ctx));
> > >});
> > > 
> > > I'm not sure if LLVM has a shared utility for this, I've seen it in other 
> > > codebases. Either way we could just define it here.
> > > 
> > > (Or we could copy the context to avoid the awkwardness)
> > Thanks for suggestion. I totally agree, callbacks on scope exit are better.
> > 
> > I couldn't find an utility like that in LLVM, but I think it's a useful 
> > one. I'll add this locally to this file, but maybe we should make this 
> > helper public and put it into clangd?
> > What are you thoughts on that?
> Sounds good. I'm wary of adding lots of tiny utility headers - maybe if we 
> squint it could go in Function.h?
Then Function.h it is :-)



Comment at: clangd/ProtocolHandlers.h:32
 public:
-  using Ctx = RequestContext;
+  using Ctx = Context;
   virtual ~ProtocolCallbacks() = default;

sammccall wrote:
> We should use one name or the other. Fine to do this to keep the patch small, 
> but leave a FIXME?
Added a FIXME, will send a follow-up change that removes it after this change 
lands.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40486



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

[PATCH] D38831: [libcxx] P0604, invoke_result and is_invocable

[Agustín Bergé via Phabricator via cfe-commits]

K-ballo updated this revision to Diff 126545.
K-ballo added a comment.

Fixed incorrect test case.


https://reviews.llvm.org/D38831

Files:
  include/type_traits
  include/variant
  test/std/utilities/function.objects/func.not_fn/not_fn.pass.cpp
  test/std/utilities/function.objects/unord.hash/non_enum.pass.cpp
  test/std/utilities/meta/meta.rel/is_callable.pass.cpp
  test/std/utilities/meta/meta.rel/is_invocable.pass.cpp
  test/std/utilities/meta/meta.rel/is_nothrow_callable.pass.cpp
  test/std/utilities/meta/meta.rel/is_nothrow_invocable.pass.cpp
  test/std/utilities/meta/meta.trans/meta.trans.other/result_of.pass.cpp
  test/std/utilities/meta/meta.trans/meta.trans.other/result_of11.pass.cpp

Index: test/std/utilities/meta/meta.trans/meta.trans.other/result_of11.pass.cpp
===
--- test/std/utilities/meta/meta.trans/meta.trans.other/result_of11.pass.cpp
+++ test/std/utilities/meta/meta.trans/meta.trans.other/result_of11.pass.cpp
@@ -27,16 +27,32 @@
 struct F {};
 struct FD : public F {};
 
+#if TEST_STD_VER > 14
+template 
+struct test_invoke_result;
+
+template 
+struct test_invoke_result
+{
+static void call()
+{
+static_assert(std::is_invocable::value, "");
+static_assert(std::is_invocable_r::value, "");
+static_assert((std::is_same::type, Ret>::value), "");
+static_assert((std::is_same, Ret>::value), "");
+}
+};
+#endif
+
 template 
 void test_result_of_imp()
 {
 static_assert((std::is_same::type, U>::value), "");
 #if TEST_STD_VER > 11
 static_assert((std::is_same, U>::value), "");
 #endif
 #if TEST_STD_VER > 14
-static_assert(std::is_callable::value, "");
-static_assert(std::is_callable::value, "");
+test_invoke_result::call();
 #endif
 }
 
Index: test/std/utilities/meta/meta.trans/meta.trans.other/result_of.pass.cpp
===
--- test/std/utilities/meta/meta.trans/meta.trans.other/result_of.pass.cpp
+++ test/std/utilities/meta/meta.trans/meta.trans.other/result_of.pass.cpp
@@ -42,24 +42,54 @@
 template 
 struct HasType::type> : std::true_type {};
 
+#if TEST_STD_VER > 14
+template 
+struct test_invoke_result;
+
+template 
+struct test_invoke_result
+{
+static void call()
+{
+static_assert(std::is_invocable::value, "");
+static_assert(std::is_invocable_r::value, "");
+static_assert((std::is_same::type, Ret>::value), "");
+}
+};
+#endif
+
 template 
 void test_result_of()
 {
+static_assert((std::is_same::type, U>::value), "");
 #if TEST_STD_VER > 14
-static_assert(std::is_callable::value, "");
-static_assert(std::is_callable::value, "");
+test_invoke_result::call();
 #endif
-static_assert((std::is_same::type, U>::value), "");
 }
 
+#if TEST_STD_VER > 14
+template 
+struct test_invoke_no_result;
+
+template 
+struct test_invoke_no_result
+{
+static void call()
+{
+static_assert(std::is_invocable::value == false, "");
+static_assert((!HasType >::value), "");
+}
+};
+#endif
+
 template 
 void test_no_result()
 {
 #if TEST_STD_VER >= 11
 static_assert((!HasType >::value), "");
 #endif
 #if TEST_STD_VER > 14
-static_assert(std::is_callable::value == false, "");
+test_invoke_no_result::call();
 #endif
 }
 
Index: test/std/utilities/meta/meta.rel/is_nothrow_invocable.pass.cpp
===
--- /dev/null
+++ test/std/utilities/meta/meta.rel/is_nothrow_invocable.pass.cpp
@@ -0,0 +1,121 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
+// type_traits
+
+// is_nothrow_invocable
+
+#include 
+#include 
+
+#include "test_macros.h"
+
+struct Tag {};
+
+struct Implicit {
+  Implicit(int) noexcept {}
+};
+
+struct ThrowsImplicit {
+  ThrowsImplicit(int) {}
+};
+
+struct Explicit {
+  explicit Explicit(int) noexcept {}
+};
+
+template 
+struct CallObject {
+  Ret operator()(Args&&...) const noexcept(IsNoexcept);
+};
+
+template 
+constexpr bool throws_invocable() {
+return std::is_invocable::value &&
+!std::is_nothrow_invocable::value;
+}
+
+template 
+constexpr bool throws_invocable_r() {
+return std::is_invocable_r::value &&
+!std::is_nothrow_invocable_r::value;
+}
+
+// FIXME(EricWF) Don't test the where noexcept is *not* part of the type system
+// once implementations have caught up.
+void test_noexcept_function_pointers()
+{
+struct Dummy { void foo() noexcept {} static void bar() noexcept {} };
+#if !defined(__cpp_noexcept_function_type)
+{
+// Check that PMF's and function pointers 

[PATCH] D40897: [clangd] Introduce a "Symbol" class.

[Haojian Wu via Phabricator via cfe-commits]

hokein marked an inline comment as done.
hokein added a comment.

I'm going to submit this patch to unblock the stuff in 
https://reviews.llvm.org/D40548. Would be happy to address any further comments 
afterwards.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40897



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

[PATCH] D40897: [clangd] Introduce a "Symbol" class.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE320486: [clangd] Introduce a "Symbol" class. 
(authored by hokein, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D40897?vs=126538&id=126548#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40897

Files:
  clangd/CMakeLists.txt
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/CMakeLists.txt
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/CMakeLists.txt
===
--- unittests/clangd/CMakeLists.txt
+++ unittests/clangd/CMakeLists.txt
@@ -16,6 +16,7 @@
   JSONExprTests.cpp
   TestFS.cpp
   TraceTests.cpp
+  SymbolCollectorTests.cpp
   )
 
 target_link_libraries(ClangdTests
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -0,0 +1,110 @@
+//===-- SymbolCollectorTests.cpp  ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "index/SymbolCollector.h"
+#include "clang/Index/IndexingAction.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "gtest/gtest.h"
+#include "gmock/gmock.h"
+
+#include 
+#include 
+
+using testing::UnorderedElementsAre;
+using testing::Eq;
+using testing::Field;
+
+// GMock helpers for matching Symbol.
+MATCHER_P(QName, Name, "") { return arg.second.QualifiedName == Name; }
+
+namespace clang {
+namespace clangd {
+
+namespace {
+class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
+ public:
+  SymbolIndexActionFactory() = default;
+
+  clang::FrontendAction *create() override {
+index::IndexingOptions IndexOpts;
+IndexOpts.SystemSymbolFilter =
+index::IndexingOptions::SystemSymbolFilterKind::All;
+IndexOpts.IndexFunctionLocals = false;
+Collector = std::make_shared();
+FrontendAction *Action =
+index::createIndexingAction(Collector, IndexOpts, nullptr).release();
+return Action;
+  }
+
+  std::shared_ptr Collector;
+};
+
+class SymbolCollectorTest : public ::testing::Test {
+public:
+  bool runSymbolCollector(StringRef HeaderCode, StringRef MainCode) {
+llvm::IntrusiveRefCntPtr InMemoryFileSystem(
+new vfs::InMemoryFileSystem);
+llvm::IntrusiveRefCntPtr Files(
+new FileManager(FileSystemOptions(), InMemoryFileSystem));
+
+const std::string FileName = "symbol.cc";
+const std::string HeaderName = "symbols.h";
+auto Factory = llvm::make_unique();
+
+tooling::ToolInvocation Invocation(
+{"symbol_collector", "-fsyntax-only", "-std=c++11", FileName},
+Factory->create(), Files.get(),
+std::make_shared());
+
+InMemoryFileSystem->addFile(HeaderName, 0,
+llvm::MemoryBuffer::getMemBuffer(HeaderCode));
+
+std::string Content = "#include\"" + std::string(HeaderName) + "\"";
+Content += "\n" + MainCode.str();
+InMemoryFileSystem->addFile(FileName, 0,
+llvm::MemoryBuffer::getMemBuffer(Content));
+Invocation.run();
+Symbols = Factory->Collector->takeSymbols();
+return true;
+  }
+
+protected:
+  SymbolSlab Symbols;
+};
+
+TEST_F(SymbolCollectorTest, CollectSymbol) {
+  const std::string Header = R"(
+class Foo {
+  void f();
+};
+void f1();
+inline void f2() {}
+  )";
+  const std::string Main = R"(
+namespace {
+void ff() {} // ignore
+}
+void f1() {}
+  )";
+  runSymbolCollector(Header, Main);
+  EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Foo::f"),
+QName("f1"), QName("f2")));
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -0,0 +1,102 @@
+//===--- SymbolCollector.cpp -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===

[clang-tools-extra] r320486 - [clangd] Introduce a "Symbol" class.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Tue Dec 12 07:42:10 2017
New Revision: 320486

URL: http://llvm.org/viewvc/llvm-project?rev=320486&view=rev
Log:
[clangd] Introduce a "Symbol" class.

Summary:
* The "Symbol" class represents a C++ symbol in the codebase, containing all the
  information of a C++ symbol needed by clangd. clangd will use it in clangd's
  AST/dynamic index and global/static index (code completion and code
  navigation).
* The SymbolCollector (another IndexAction) will be used to recollect the
  symbols when the source file is changed (for ASTIndex), or to generate
  all C++ symbols for the whole project.

In the long term (when index-while-building is ready), clangd should share a
same "Symbol" structure and IndexAction with index-while-building, but
for now we want to have some stuff working in clangd.

Reviewers: ioeric, sammccall, ilya-biryukov, malaperle

Reviewed By: sammccall

Subscribers: malaperle, klimek, mgorny, cfe-commits

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

Added:
clang-tools-extra/trunk/clangd/index/
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.h
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=320486&r1=320485&r2=320486&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue Dec 12 07:42:10 2017
@@ -19,6 +19,8 @@ add_clang_library(clangDaemon
   Protocol.cpp
   ProtocolHandlers.cpp
   Trace.cpp
+  index/Index.cpp
+  index/SymbolCollector.cpp
 
   LINK_LIBS
   clangAST

Added: clang-tools-extra/trunk/clangd/index/Index.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=320486&view=auto
==
--- clang-tools-extra/trunk/clangd/index/Index.cpp (added)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Tue Dec 12 07:42:10 2017
@@ -0,0 +1,49 @@
+//===--- Index.cpp ---*- 
C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Index.h"
+
+#include "llvm/Support/SHA1.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+ArrayRef toArrayRef(StringRef S) {
+  return {reinterpret_cast(S.data()), S.size()};
+}
+} // namespace
+
+SymbolID::SymbolID(llvm::StringRef USR)
+: HashValue(llvm::SHA1::hash(toArrayRef(USR))) {}
+
+SymbolSlab::const_iterator SymbolSlab::begin() const {
+  return Symbols.begin();
+}
+
+SymbolSlab::const_iterator SymbolSlab::end() const {
+  return Symbols.end();
+}
+
+SymbolSlab::const_iterator SymbolSlab::find(const SymbolID& SymID) const {
+  return Symbols.find(SymID);
+}
+
+void SymbolSlab::freeze() {
+  Frozen = true;
+}
+
+void SymbolSlab::insert(Symbol S) {
+  assert(!Frozen &&
+ "Can't insert a symbol after the slab has been frozen!");
+  Symbols[S.ID] = std::move(S);
+}
+
+} // namespace clangd
+} // namespace clang

Added: clang-tools-extra/trunk/clangd/index/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=320486&view=auto
==
--- clang-tools-extra/trunk/clangd/index/Index.h (added)
+++ clang-tools-extra/trunk/clangd/index/Index.h Tue Dec 12 07:42:10 2017
@@ -0,0 +1,136 @@
+//===--- Symbol.h ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===-===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_INDEX_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_INDEX_H
+
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringExtras.h"
+
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+
+struct SymbolLocation {
+  // The absolute path of the source file where a symbol occurs.
+  std::string FilePath;
+  // The 0-based offset to the first character of the symbol from the beginning
+  // of the source file.
+  unsigned StartOffset;
+  // The 0-based offset to the last character of the symbol from the beginning
+

[PATCH] D40486: [clangd] Implemented logging using Context

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Make Context the first parameter everywhere.
- Add a SFINAE check to UniqueFunction's constructor to avoid creating it from 
non-callable objects.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40486

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/ClangdUnitStore.cpp
  clangd/ClangdUnitStore.h
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Function.h
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  clangd/JSONRPCDispatcher.cpp
  clangd/JSONRPCDispatcher.h
  clangd/Logger.cpp
  clangd/Logger.h
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -9,6 +9,7 @@
 #include "ClangdServer.h"
 #include "Compiler.h"
 #include "Matchers.h"
+#include "Context.h"
 #include "Protocol.h"
 #include "TestFS.h"
 #include "gmock/gmock.h"
@@ -92,12 +93,13 @@
   MockCompilationDatabase CDB;
   IgnoreDiagnostics DiagConsumer;
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-  /*StorePreamblesInMemory=*/true,
-  EmptyLogger::getInstance());
+  /*StorePreamblesInMemory=*/true);
   auto File = getVirtualTestFilePath("foo.cpp");
   auto Test = parseTextMarker(Text);
-  Server.addDocument(File, Test.Text);
-  return Server.codeComplete(File, Test.MarkerPos, Opts).get().Value;
+  Server.addDocument(Context::empty(), File, Test.Text);
+  return Server.codeComplete(Context::empty(), File, Test.MarkerPos, Opts)
+  .get()
+  .first.Value;
 }
 
 TEST(CompletionTest, Limit) {
@@ -127,7 +129,6 @@
   int Qux;
 };
   )cpp";
-
   EXPECT_THAT(completions(Body + "int main() { S().Foba^ }").items,
   AllOf(Has("FooBar"), Has("FooBaz"), Not(Has("Qux";
 
@@ -269,18 +270,17 @@
   IgnoreDiagnostics DiagConsumer;
   MockCompilationDatabase CDB;
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-  /*StorePreamblesInMemory=*/true,
-  EmptyLogger::getInstance());
+  /*StorePreamblesInMemory=*/true);
   auto File = getVirtualTestFilePath("foo.cpp");
-  Server.addDocument(File, "ignored text!");
+  Server.addDocument(Context::empty(), File, "ignored text!");
 
   auto Example = parseTextMarker("int cbc; int b = ^;");
   auto Results =
   Server
-  .codeComplete(File, Example.MarkerPos, clangd::CodeCompleteOptions(),
-StringRef(Example.Text))
+  .codeComplete(Context::empty(), File, Example.MarkerPos,
+clangd::CodeCompleteOptions(), StringRef(Example.Text))
   .get()
-  .Value;
+  .first.Value;
   EXPECT_THAT(Results.items, Contains(Named("cbc")));
 }
 
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -9,7 +9,7 @@
 
 #include "ClangdLSPServer.h"
 #include "ClangdServer.h"
-#include "Logger.h"
+#include "Context.h"
 #include "TestFS.h"
 #include "clang/Config/config.h"
 #include "llvm/ADT/SmallVector.h"
@@ -28,6 +28,7 @@
 
 namespace clang {
 namespace clangd {
+
 namespace {
 
 // Don't wait for async ops in clangd test more than that to avoid blocking
@@ -123,8 +124,7 @@
 ErrorCheckingDiagConsumer DiagConsumer;
 MockCompilationDatabase CDB;
 ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-/*StorePreamblesInMemory=*/true,
-EmptyLogger::getInstance());
+/*StorePreamblesInMemory=*/true);
 for (const auto &FileWithContents : ExtraFiles)
   FS.Files[getVirtualTestFilePath(FileWithContents.first)] =
   FileWithContents.second;
@@ -135,7 +135,8 @@
 
 // Have to sync reparses because requests are processed on the calling
 // thread.
-auto AddDocFuture = Server.addDocument(SourceFilename, SourceContents);
+auto AddDocFuture =
+Server.addDocument(Context::empty(), SourceFilename, SourceContents);
 
 auto Result = dumpASTWithoutMemoryLocs(Server, SourceFilename);
 
@@ -187,8 +188,7 @@
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB;
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-  /*StorePreamblesInMemory=*/true,
-  EmptyLogger::getInstance());
+  /*StorePreamblesInMemory=*/true);
 
   const auto SourceContent

r320489 - Fix ICE when __has_unqiue_object_representations called with invalid decl

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Tue Dec 12 08:02:06 2017
New Revision: 320489

URL: http://llvm.org/viewvc/llvm-project?rev=320489&view=rev
Log:
Fix ICE when __has_unqiue_object_representations called with invalid decl

Modified:
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/test/SemaCXX/type-traits.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=320489&r1=320488&r2=320489&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Tue Dec 12 08:02:06 2017
@@ -2279,6 +2279,9 @@ bool ASTContext::hasUniqueObjectRepresen
   if (Ty->isRecordType()) {
 const RecordDecl *Record = Ty->getAs()->getDecl();
 
+if (Record->isInvalidDecl())
+  return false;
+
 if (Record->isUnion())
   return unionHasUniqueObjectRepresentations(*this, Record);
 

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=320489&r1=320488&r2=320489&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Dec 12 08:02:06 2017
@@ -4958,7 +4958,7 @@ static bool EvaluateBinaryTypeTrait(Sema
 EnterExpressionEvaluationContext Unevaluated(
 Self, Sema::ExpressionEvaluationContext::Unevaluated);
 Sema::SFINAETrap SFINAE(Self, /*AccessCheckingSFINAE=*/true);
-Sema::ContextRAII TUContext(Self, Self.Context.getTranslationUnitDecl());
+Sema::ContextRAII TUContext(Self, Self.Context.getTranslationUnitDecl()); {
 ExprResult Result = Self.BuildBinOp(/*S=*/nullptr, KeyLoc, BO_Assign, &Lhs,
 &Rhs);
 if (Result.isInvalid() || SFINAE.hasErrorOccurred())
@@ -4981,6 +4981,7 @@ static bool EvaluateBinaryTypeTrait(Sema
 
 llvm_unreachable("unhandled type trait");
 return false;
+}
   }
 default: llvm_unreachable("not a BTT");
   }

Modified: cfe/trunk/test/SemaCXX/type-traits.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/type-traits.cpp?rev=320489&r1=320488&r2=320489&view=diff
==
--- cfe/trunk/test/SemaCXX/type-traits.cpp (original)
+++ cfe/trunk/test/SemaCXX/type-traits.cpp Tue Dec 12 08:02:06 2017
@@ -2661,3 +2661,11 @@ static_assert(sizeof(CanBeUniqueIfNoPadd
   has_unique_object_representations::value,
   "inherit from std layout");
 
+namespace ErrorType {
+  struct S; //expected-note{{forward declaration of 'ErrorType::S'}}
+
+  struct T {
+S t; //expected-error{{field has incomplete type 'ErrorType::S'}}
+  };
+  bool b = __has_unique_object_representations(T);
+};


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

[PATCH] D40486: [clangd] Implemented logging using Context

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

This is now ready for review.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40486



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

[PATCH] D40486: [clangd] Implemented logging using Context

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/Function.h:44
+/// A sfinae-check that Callable can be called with Args...
+class = decltype(std::declval()(std::declval()...),
+ void())>

We need this check to make `ClangdServer::codeComplete` overloads work. Without 
it, overloading thinks `UniqueFunction` can be constructed from any type, so we 
end up getting ambig between the two overloads when calling it in a following 
way: `codeComplete(Ctx, File, Pos, CCOpts, OverridenContents)`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40486



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

Re: r320489 - Fix ICE when __has_unqiue_object_representations called with invalid decl

[Richard Smith via cfe-commits]

On 12 Dec 2017 16:02, "Erich Keane via cfe-commits" <
cfe-commits@lists.llvm.org> wrote:

Author: erichkeane
Date: Tue Dec 12 08:02:06 2017
New Revision: 320489

URL: http://llvm.org/viewvc/llvm-project?rev=320489&view=rev
Log:
Fix ICE when __has_unqiue_object_representations called with invalid decl

Modified:
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/test/SemaCXX/type-traits.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/
ASTContext.cpp?rev=320489&r1=320488&r2=320489&view=diff

==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Tue Dec 12 08:02:06 2017
@@ -2279,6 +2279,9 @@ bool ASTContext::hasUniqueObjectRepresen
   if (Ty->isRecordType()) {
 const RecordDecl *Record = Ty->getAs()->getDecl();

+if (Record->isInvalidDecl())
+  return false;
+
 if (Record->isUnion())
   return unionHasUniqueObjectRepresentations(*this, Record);


Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
SemaExprCXX.cpp?rev=320489&r1=320488&r2=320489&view=diff

==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Dec 12 08:02:06 2017
@@ -4958,7 +4958,7 @@ static bool EvaluateBinaryTypeTrait(Sema
 EnterExpressionEvaluationContext Unevaluated(
 Self, Sema::ExpressionEvaluationContext::Unevaluated);
 Sema::SFINAETrap SFINAE(Self, /*AccessCheckingSFINAE=*/true);
-Sema::ContextRAII TUContext(Self, Self.Context.
getTranslationUnitDecl());
+Sema::ContextRAII TUContext(Self, Self.Context.getTranslationUnitDecl());
{
 ExprResult Result = Self.BuildBinOp(/*S=*/nullptr, KeyLoc, BO_Assign,
&Lhs,
 &Rhs);
 if (Result.isInvalid() || SFINAE.hasErrorOccurred())
@@ -4981,6 +4981,7 @@ static bool EvaluateBinaryTypeTrait(Sema

 llvm_unreachable("unhandled type trait");
 return false;
+}


Was this change intentional?

   }
 default: llvm_unreachable("not a BTT");
   }

Modified: cfe/trunk/test/SemaCXX/type-traits.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
SemaCXX/type-traits.cpp?rev=320489&r1=320488&r2=320489&view=diff

==
--- cfe/trunk/test/SemaCXX/type-traits.cpp (original)
+++ cfe/trunk/test/SemaCXX/type-traits.cpp Tue Dec 12 08:02:06 2017
@@ -2661,3 +2661,11 @@ static_assert(sizeof(CanBeUniqueIfNoPadd
   has_unique_object_representations::
value,
   "inherit from std layout");

+namespace ErrorType {
+  struct S; //expected-note{{forward declaration of 'ErrorType::S'}}
+
+  struct T {
+S t; //expected-error{{field has incomplete type 'ErrorType::S'}}
+  };
+  bool b = __has_unique_object_representations(T);
+};


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

[PATCH] D40486: [clangd] Implemented logging using Context

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Also check that return type of Callable is correct.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40486

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/ClangdUnitStore.cpp
  clangd/ClangdUnitStore.h
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Function.h
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  clangd/JSONRPCDispatcher.cpp
  clangd/JSONRPCDispatcher.h
  clangd/Logger.cpp
  clangd/Logger.h
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -9,6 +9,7 @@
 #include "ClangdServer.h"
 #include "Compiler.h"
 #include "Matchers.h"
+#include "Context.h"
 #include "Protocol.h"
 #include "TestFS.h"
 #include "gmock/gmock.h"
@@ -92,12 +93,13 @@
   MockCompilationDatabase CDB;
   IgnoreDiagnostics DiagConsumer;
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-  /*StorePreamblesInMemory=*/true,
-  EmptyLogger::getInstance());
+  /*StorePreamblesInMemory=*/true);
   auto File = getVirtualTestFilePath("foo.cpp");
   auto Test = parseTextMarker(Text);
-  Server.addDocument(File, Test.Text);
-  return Server.codeComplete(File, Test.MarkerPos, Opts).get().Value;
+  Server.addDocument(Context::empty(), File, Test.Text);
+  return Server.codeComplete(Context::empty(), File, Test.MarkerPos, Opts)
+  .get()
+  .first.Value;
 }
 
 TEST(CompletionTest, Limit) {
@@ -127,7 +129,6 @@
   int Qux;
 };
   )cpp";
-
   EXPECT_THAT(completions(Body + "int main() { S().Foba^ }").items,
   AllOf(Has("FooBar"), Has("FooBaz"), Not(Has("Qux";
 
@@ -269,18 +270,17 @@
   IgnoreDiagnostics DiagConsumer;
   MockCompilationDatabase CDB;
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-  /*StorePreamblesInMemory=*/true,
-  EmptyLogger::getInstance());
+  /*StorePreamblesInMemory=*/true);
   auto File = getVirtualTestFilePath("foo.cpp");
-  Server.addDocument(File, "ignored text!");
+  Server.addDocument(Context::empty(), File, "ignored text!");
 
   auto Example = parseTextMarker("int cbc; int b = ^;");
   auto Results =
   Server
-  .codeComplete(File, Example.MarkerPos, clangd::CodeCompleteOptions(),
-StringRef(Example.Text))
+  .codeComplete(Context::empty(), File, Example.MarkerPos,
+clangd::CodeCompleteOptions(), StringRef(Example.Text))
   .get()
-  .Value;
+  .first.Value;
   EXPECT_THAT(Results.items, Contains(Named("cbc")));
 }
 
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -9,7 +9,7 @@
 
 #include "ClangdLSPServer.h"
 #include "ClangdServer.h"
-#include "Logger.h"
+#include "Context.h"
 #include "TestFS.h"
 #include "clang/Config/config.h"
 #include "llvm/ADT/SmallVector.h"
@@ -28,6 +28,7 @@
 
 namespace clang {
 namespace clangd {
+
 namespace {
 
 // Don't wait for async ops in clangd test more than that to avoid blocking
@@ -123,8 +124,7 @@
 ErrorCheckingDiagConsumer DiagConsumer;
 MockCompilationDatabase CDB;
 ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-/*StorePreamblesInMemory=*/true,
-EmptyLogger::getInstance());
+/*StorePreamblesInMemory=*/true);
 for (const auto &FileWithContents : ExtraFiles)
   FS.Files[getVirtualTestFilePath(FileWithContents.first)] =
   FileWithContents.second;
@@ -135,7 +135,8 @@
 
 // Have to sync reparses because requests are processed on the calling
 // thread.
-auto AddDocFuture = Server.addDocument(SourceFilename, SourceContents);
+auto AddDocFuture =
+Server.addDocument(Context::empty(), SourceFilename, SourceContents);
 
 auto Result = dumpASTWithoutMemoryLocs(Server, SourceFilename);
 
@@ -187,8 +188,7 @@
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB;
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-  /*StorePreamblesInMemory=*/true,
-  EmptyLogger::getInstance());
+  /*StorePreamblesInMemory=*/true);
 
   const auto SourceContents = R"cpp(
 #include "foo.h"
@@ -203,21 +203,21 @@
   FS.ExpectedFile = FooCpp;
 
   // To sync

RE: r320489 - Fix ICE when __has_unqiue_object_representations called with invalid decl

[Keane, Erich via cfe-commits]

Gah, no it wasn’t.  I’ll revert that now.

From: Richard Smith [mailto:rich...@metafoo.co.uk]
Sent: Tuesday, December 12, 2017 8:07 AM
To: Keane, Erich 
Cc: cfe-commits 
Subject: Re: r320489 - Fix ICE when __has_unqiue_object_representations called 
with invalid decl

On 12 Dec 2017 16:02, "Erich Keane via cfe-commits" 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Tue Dec 12 08:02:06 2017
New Revision: 320489

URL: http://llvm.org/viewvc/llvm-project?rev=320489&view=rev
Log:
Fix ICE when __has_unqiue_object_representations called with invalid decl

Modified:
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/test/SemaCXX/type-traits.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=320489&r1=320488&r2=320489&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Tue Dec 12 08:02:06 2017
@@ -2279,6 +2279,9 @@ bool ASTContext::hasUniqueObjectRepresen
   if (Ty->isRecordType()) {
 const RecordDecl *Record = Ty->getAs()->getDecl();

+if (Record->isInvalidDecl())
+  return false;
+
 if (Record->isUnion())
   return unionHasUniqueObjectRepresentations(*this, Record);


Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=320489&r1=320488&r2=320489&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Dec 12 08:02:06 2017
@@ -4958,7 +4958,7 @@ static bool EvaluateBinaryTypeTrait(Sema
 EnterExpressionEvaluationContext Unevaluated(
 Self, Sema::ExpressionEvaluationContext::Unevaluated);
 Sema::SFINAETrap SFINAE(Self, /*AccessCheckingSFINAE=*/true);
-Sema::ContextRAII TUContext(Self, Self.Context.getTranslationUnitDecl());
+Sema::ContextRAII TUContext(Self, Self.Context.getTranslationUnitDecl()); {
 ExprResult Result = Self.BuildBinOp(/*S=*/nullptr, KeyLoc, BO_Assign, &Lhs,
 &Rhs);
 if (Result.isInvalid() || SFINAE.hasErrorOccurred())
@@ -4981,6 +4981,7 @@ static bool EvaluateBinaryTypeTrait(Sema

 llvm_unreachable("unhandled type trait");
 return false;
+}

Was this change intentional?

   }
 default: llvm_unreachable("not a BTT");
   }

Modified: cfe/trunk/test/SemaCXX/type-traits.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/type-traits.cpp?rev=320489&r1=320488&r2=320489&view=diff
==
--- cfe/trunk/test/SemaCXX/type-traits.cpp (original)
+++ cfe/trunk/test/SemaCXX/type-traits.cpp Tue Dec 12 08:02:06 2017
@@ -2661,3 +2661,11 @@ static_assert(sizeof(CanBeUniqueIfNoPadd
   has_unique_object_representations::value,
   "inherit from std layout");

+namespace ErrorType {
+  struct S; //expected-note{{forward declaration of 'ErrorType::S'}}
+
+  struct T {
+S t; //expected-error{{field has incomplete type 'ErrorType::S'}}
+  };
+  bool b = __has_unique_object_representations(T);
+};


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

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

r320493 - Revert a part of 320489 that was submitted unintentionally.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Tue Dec 12 08:22:31 2017
New Revision: 320493

URL: http://llvm.org/viewvc/llvm-project?rev=320493&view=rev
Log:
Revert a part of 320489 that was submitted unintentionally.

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

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=320493&r1=320492&r2=320493&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Dec 12 08:22:31 2017
@@ -4958,7 +4958,7 @@ static bool EvaluateBinaryTypeTrait(Sema
 EnterExpressionEvaluationContext Unevaluated(
 Self, Sema::ExpressionEvaluationContext::Unevaluated);
 Sema::SFINAETrap SFINAE(Self, /*AccessCheckingSFINAE=*/true);
-Sema::ContextRAII TUContext(Self, Self.Context.getTranslationUnitDecl()); {
+Sema::ContextRAII TUContext(Self, Self.Context.getTranslationUnitDecl());
 ExprResult Result = Self.BuildBinOp(/*S=*/nullptr, KeyLoc, BO_Assign, &Lhs,
 &Rhs);
 if (Result.isInvalid() || SFINAE.hasErrorOccurred())
@@ -4981,7 +4981,6 @@ static bool EvaluateBinaryTypeTrait(Sema
 
 llvm_unreachable("unhandled type trait");
 return false;
-}
   }
 default: llvm_unreachable("not a BTT");
   }


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

RE: r320489 - Fix ICE when __has_unqiue_object_representations called with invalid decl

[Keane, Erich via cfe-commits]

Fixed in 320493.  Thanks for catching that.

From: Richard Smith [mailto:rich...@metafoo.co.uk]
Sent: Tuesday, December 12, 2017 8:07 AM
To: Keane, Erich 
Cc: cfe-commits 
Subject: Re: r320489 - Fix ICE when __has_unqiue_object_representations called 
with invalid decl

On 12 Dec 2017 16:02, "Erich Keane via cfe-commits" 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Tue Dec 12 08:02:06 2017
New Revision: 320489

URL: http://llvm.org/viewvc/llvm-project?rev=320489&view=rev
Log:
Fix ICE when __has_unqiue_object_representations called with invalid decl

Modified:
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/test/SemaCXX/type-traits.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=320489&r1=320488&r2=320489&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Tue Dec 12 08:02:06 2017
@@ -2279,6 +2279,9 @@ bool ASTContext::hasUniqueObjectRepresen
   if (Ty->isRecordType()) {
 const RecordDecl *Record = Ty->getAs()->getDecl();

+if (Record->isInvalidDecl())
+  return false;
+
 if (Record->isUnion())
   return unionHasUniqueObjectRepresentations(*this, Record);


Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=320489&r1=320488&r2=320489&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Dec 12 08:02:06 2017
@@ -4958,7 +4958,7 @@ static bool EvaluateBinaryTypeTrait(Sema
 EnterExpressionEvaluationContext Unevaluated(
 Self, Sema::ExpressionEvaluationContext::Unevaluated);
 Sema::SFINAETrap SFINAE(Self, /*AccessCheckingSFINAE=*/true);
-Sema::ContextRAII TUContext(Self, Self.Context.getTranslationUnitDecl());
+Sema::ContextRAII TUContext(Self, Self.Context.getTranslationUnitDecl()); {
 ExprResult Result = Self.BuildBinOp(/*S=*/nullptr, KeyLoc, BO_Assign, &Lhs,
 &Rhs);
 if (Result.isInvalid() || SFINAE.hasErrorOccurred())
@@ -4981,6 +4981,7 @@ static bool EvaluateBinaryTypeTrait(Sema

 llvm_unreachable("unhandled type trait");
 return false;
+}

Was this change intentional?

   }
 default: llvm_unreachable("not a BTT");
   }

Modified: cfe/trunk/test/SemaCXX/type-traits.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/type-traits.cpp?rev=320489&r1=320488&r2=320489&view=diff
==
--- cfe/trunk/test/SemaCXX/type-traits.cpp (original)
+++ cfe/trunk/test/SemaCXX/type-traits.cpp Tue Dec 12 08:02:06 2017
@@ -2661,3 +2661,11 @@ static_assert(sizeof(CanBeUniqueIfNoPadd
   has_unique_object_representations::value,
   "inherit from std layout");

+namespace ErrorType {
+  struct S; //expected-note{{forward declaration of 'ErrorType::S'}}
+
+  struct T {
+S t; //expected-error{{field has incomplete type 'ErrorType::S'}}
+  };
+  bool b = __has_unique_object_representations(T);
+};


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

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

[PATCH] D40897: [clangd] Introduce a "Symbol" class.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/index/Index.h:52
+private:
+  friend class llvm::DenseMapInfo;
+

warning: class 'DenseMapInfo' was previously declared as a struct 
[-Wmismatched-tags]


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40897



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

[PATCH] D41056: [clang-tidy] New check misc-uniqueptr-release-unused-retval

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D41056#951507, @khuttun wrote:

> P0600R1 seems to suggest quite conservative approach (and rightly so, IMO) in 
> taking `[[nodiscard]]` in use in std lib. For example it proposes *not* to 
> use it with `unique_ptr::release`. I think there could still be room for 
> static unused ret val checking for parts of std lib not covered by P0600R1, 
> but also for boost, POSIX APIs etc.
>
> What do you think about making this check fully configurable through the 
> check options? The options could list the functions to check. The 
> documentation could suggest some candidate functions from std lib to 
> configure checks for.


Agreed; that's largely what I was suggesting with my previous comment. I think 
this should be a generic, configurable check that is prepopulated with some 
APIs that make sense. You can use P0600R1 as a starting point, but I'd 
encourage you to find a pretty decent (but still conservative) list of APIs.


Repository:
  rL LLVM

https://reviews.llvm.org/D41056



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

Re: [PATCH] D41081: Fix clang Lexer Windows line-ending bug

[Benoit Belley via cfe-commits]

Thanks Zachary.

Maybe, we can approach the issue through another, hopefully less controversial 
angle. Clang currently has a limited number of tests verifying that it 
correctly handles all types of line endings. IMHO,  Zhen's solution (dos2unix & 
unix2dos conversions) provides an handy mechanism to expand these tests without 
having to duplicate test cases.

For reference, here's the current list of clang test files which seems to 
explicitly test various types of line endings (CRLF instead of LF only):

clang/test/CXX/lex/lex.literal/lex.string/p4.cpp
clang/test/FixIt/fixit-newline-style.c
clang/test/Frontend/system-header-line-directive-ms-lineendings.c

And here's the list of remaining clang test files with CRLF line endings, 
although AFAICT, that seems to be unintentional:

clang/test/CXX/expr/expr.prim/expr.prim.lambda/p15-star-this-capture.cpp
clang/test/CodeGenCXX/attr-x86-no_caller_saved_registers.cpp
clang/test/CodeGenOpenCL/no-signed-zeros.cl
clang/test/Driver/ps4-analyzer-defaults.cpp
clang/test/Frontend/Inputs/rewrite-includes-bom.h
clang/test/Index/preamble-reparse-with-BOM.m
clang/test/Misc/ast-dump-c-attr.c
clang/test/OpenMP/simd_codegen.cpp
clang/test/Preprocessor/macro_vaopt_check.cpp
clang/test/Preprocessor/macro_vaopt_expand.cpp
clang/test/SemaCXX/attr-non-x86-no_caller_saved_registers.cpp
clang/test/SemaCXX/attr-x86-no_caller_saved_registers.cpp
clang/test/SemaOpenCL/array-init.cl

I am of course opened to other solutions to improve the current test coverage.

Cheers,
Benoit


From: Zhen Cao mailto:zhen@autodesk.com>>
Date: lundi 11 décembre 2017 à 16:23
To: Zachary Turner mailto:ztur...@google.com>>
Cc: Benoit Belley 
mailto:benoit.bel...@autodesk.com>>, 
"reviews+d41081+public+5a71b504a12c1...@reviews.llvm.org"
 
mailto:reviews+d41081+public+5a71b504a12c1...@reviews.llvm.org>>,
 "r...@google.com" 
mailto:r...@google.com>>, 
"cfe-commits@lists.llvm.org" 
mailto:cfe-commits@lists.llvm.org>>
Subject: RE: [PATCH] D41081: Fix clang Lexer Windows line-ending bug

Thank you very much, Zachary :)

From: Zachary Turner [mailto:ztur...@google.com]
Sent: Monday, December 11, 2017 4:21 PM
To: Zhen Cao mailto:zhen@autodesk.com>>
Cc: Benoit Belley 
mailto:benoit.bel...@autodesk.com>>; 
reviews+d41081+public+5a71b504a12c1...@reviews.llvm.org;
 r...@google.com; 
cfe-commits@lists.llvm.org
Subject: Re: [PATCH] D41081: Fix clang Lexer Windows line-ending bug

I'll let rnk@ weigh in (he's on vacation today though).

I don't feel comfortable lgtm'ing any change where "don't use 
core.autocrlf=true" is an alternative solution, but if other people want to 
disagree with me and lgtm this, then that suggests I'm in the minority, which 
is fine.  :)

On Mon, Dec 11, 2017 at 1:13 PM Zhen Cao 
mailto:zhen@autodesk.com>> wrote:
I think adding two lit substitutions is not a significant issue since dos2unix 
and unix2dos are common Unix utilities. This solution also has the advantage of 
testing both styles of line-endings with only one test case, which I think 
actually reduces complexity and maintenance burden.

From: Zachary Turner [mailto:ztur...@google.com]
Sent: Monday, December 11, 2017 3:42 PM
To: Benoit Belley 
mailto:benoit.bel...@autodesk.com>>
Cc: 
reviews+d41081+public+5a71b504a12c1...@reviews.llvm.org;
 Zhen Cao mailto:zhen@autodesk.com>>; 
r...@google.com

Subject: Re: [PATCH] D41081: Fix clang Lexer Windows line-ending bug

To be honest, I'm not a fan of testing code in a way that tries to work around 
issues related to source control.  I think we had a similar discussion before 
on a previous patch, and my suggested fix was to simply set core.autocrlf=false 
in your git config.  This solves all of these problems and does not come with 
any real downsides that I'm aware of.

It also yields better tests IMO, because the fewer steps of translation that an 
input has to go through before being fed to the test program the better.  If 
you want to test the behavior of a file with \r\n, just check in an input file 
with \r\n and you're ready to go.  Furthermore, by not doing this we are adding 
complexity (and hence, technical debt) *to the code* to deal with issues that 
could easily be solved using an appropriate developer configuration.  I'm not 
saying that we should not be testing the case of \r\n newlines, because we 
can't control what perforce outputs and the tool itself needs to work with 
either style of newline.  But we can control how we configure our git clones, 
and if doing so leads to better tests with less testing infrastructure hacks 
and workarounds, then I think we should do that.

On Mon, Dec 11, 2017 at

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun updated this revision to Diff 126543.
xazax.hun added a comment.

- Further improvements to python script part.


https://reviews.llvm.org/D30691

Files:
  include/clang/AST/ASTContext.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/CallEvent.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  lib/StaticAnalyzer/Frontend/CMakeLists.txt
  test/Analysis/Inputs/ctu-chain.cpp
  test/Analysis/Inputs/ctu-other.cpp
  test/Analysis/Inputs/externalFnMap.txt
  test/Analysis/ctu-main.cpp
  tools/scan-build-py/libscanbuild/__init__.py
  tools/scan-build-py/libscanbuild/analyze.py
  tools/scan-build-py/libscanbuild/arguments.py
  tools/scan-build-py/libscanbuild/clang.py
  tools/scan-build-py/libscanbuild/report.py
  tools/scan-build-py/tests/unit/test_analyze.py
  tools/scan-build-py/tests/unit/test_clang.py

Index: tools/scan-build-py/tests/unit/test_clang.py
===
--- tools/scan-build-py/tests/unit/test_clang.py
+++ tools/scan-build-py/tests/unit/test_clang.py
@@ -92,3 +92,15 @@
 self.assertEqual('Checker One description', result.get('checker.one'))
 self.assertTrue('checker.two' in result)
 self.assertEqual('Checker Two description', result.get('checker.two'))
+
+
+class ClangIsCtuCapableTest(unittest.TestCase):
+def test_ctu_not_found(self):
+is_ctu = sut.is_ctu_capable('not-found-clang-func-mapping')
+self.assertFalse(is_ctu)
+
+
+class ClangGetTripleArchTest(unittest.TestCase):
+def test_arch_is_not_empty(self):
+arch = sut.get_triple_arch(['clang', '-E', '-'], '.')
+self.assertTrue(len(arch) > 0)
Index: tools/scan-build-py/tests/unit/test_analyze.py
===
--- tools/scan-build-py/tests/unit/test_analyze.py
+++ tools/scan-build-py/tests/unit/test_analyze.py
@@ -4,12 +4,12 @@
 # This file is distributed under the University of Illinois Open Source
 # License. See LICENSE.TXT for details.
 
-import libear
-import libscanbuild.analyze as sut
 import unittest
 import re
 import os
 import os.path
+import libear
+import libscanbuild.analyze as sut
 
 
 class ReportDirectoryTest(unittest.TestCase):
@@ -333,3 +333,83 @@
 
 def test_method_exception_not_caught(self):
 self.assertRaises(Exception, method_exception_from_inside, dict())
+
+
+class PrefixWithTest(unittest.TestCase):
+
+def test_gives_empty_on_empty(self):
+res = sut.prefix_with(0, [])
+self.assertFalse(res)
+
+def test_interleaves_prefix(self):
+res = sut.prefix_with(0, [1, 2, 3])
+self.assertListEqual([0, 1, 0, 2, 0, 3], res)
+
+
+class MergeCtuMapTest(unittest.TestCase):
+
+def test_no_map_gives_empty(self):
+pairs = sut.create_global_ctu_function_map([])
+self.assertFalse(pairs)
+
+def test_multiple_maps_merged(self):
+concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
+  'c:@F@fun2#I# ast/fun2.c.ast',
+  'c:@F@fun3#I# ast/fun3.c.ast']
+pairs = sut.create_global_ctu_function_map(concat_map)
+self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
+self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
+self.assertTrue(('c:@F@fun3#I#', 'ast/fun3.c.ast') in pairs)
+self.assertEqual(3, len(pairs))
+
+def test_not_unique_func_left_out(self):
+concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
+  'c:@F@fun2#I# ast/fun2.c.ast',
+  'c:@F@fun1#I# ast/fun7.c.ast']
+pairs = sut.create_global_ctu_function_map(concat_map)
+self.assertFalse(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
+self.assertFalse(('c:@F@fun1#I#', 'ast/fun7.c.ast') in pairs)
+self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
+self.assertEqual(1, len(pairs))
+
+def test_duplicates_are_kept(self):
+concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
+  'c:@F@fun2#I# ast/fun2.c.ast',
+  'c:@F@fun1#I# ast/fun1.c.ast']
+pairs = sut.create_global_ctu_function_map(concat_map)
+self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
+self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
+self.assertEqual(2, len(pairs))
+
+def test_space_handled_in_source(self):
+concat_map = ['c:@F@fun1#I# ast/f un.c.ast']
+pairs = sut.create_global_ctu_function_map(concat_map)
+self.assertTrue(('c:@F@fun1#I#', 'ast/f un.c.ast') in pairs)
+self.assertEqual(1, len(pairs))
+
+
+class FuncMapSrcToAstTest(unittest.TestCase):
+
+def test_empty_gives_empty(self):
+fun

r320494 - [debuginfo-tests] Add support for moving debuginfo-tests from clang/test to llvm/projects or monorepo.

[Don Hinton via cfe-commits]

Author: dhinton
Date: Tue Dec 12 08:48:35 2017
New Revision: 320494

URL: http://llvm.org/viewvc/llvm-project?rev=320494&view=rev
Log:
[debuginfo-tests] Add support for moving debuginfo-tests from clang/test to 
llvm/projects or monorepo.

Summary:
The new version of debuginfo-tests will have it's own
lit.cfg.py file which is incompatible with the one in clang/test.
This change supports both the old and new versions, and can be used
until the bots actually move debuginfo-tests to either clang/test or
the monorepo.

This is a prerequisite for D40971.

Reviewers: zturner, aprantl

Subscribers: mgorny, JDevlieghere, llvm-commits, cfe-commits

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

Modified:
cfe/trunk/test/CMakeLists.txt
cfe/trunk/test/lit.cfg.py

Modified: cfe/trunk/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CMakeLists.txt?rev=320494&r1=320493&r2=320494&view=diff
==
--- cfe/trunk/test/CMakeLists.txt (original)
+++ cfe/trunk/test/CMakeLists.txt Tue Dec 12 08:48:35 2017
@@ -131,3 +131,12 @@ add_lit_testsuites(CLANG ${CMAKE_CURRENT
 add_custom_target(clang-test)
 add_dependencies(clang-test check-clang)
 set_target_properties(clang-test PROPERTIES FOLDER "Clang tests")
+
+# FIXME: This logic can be removed once all buildbots have moved
+# debuginfo-test from clang/test to llvm/projects or monorepo.
+if(EXISTS debuginfo-tests)
+  message(WARNING "Including debuginfo-tests in clang/test is deprecated.  
Move to llvm/projects or use monorepo.")
+  if(EXISTS debuginfo-tests/CMakeLists.txt)
+add_subdirectory(debuginfo-tests)
+  endif()
+endif()

Modified: cfe/trunk/test/lit.cfg.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/lit.cfg.py?rev=320494&r1=320493&r2=320494&view=diff
==
--- cfe/trunk/test/lit.cfg.py (original)
+++ cfe/trunk/test/lit.cfg.py Tue Dec 12 08:48:35 2017
@@ -58,12 +58,20 @@ tool_dirs = [config.clang_tools_dir, con
 
 tools = [
 'c-index-test', 'clang-check', 'clang-diff', 'clang-format', 'opt',
-ToolSubst('%test_debuginfo', command=os.path.join(
-config.llvm_src_root, 'utils', 'test_debuginfo.pl')),
 ToolSubst('%clang_func_map', command=FindTool(
 'clang-func-mapping'), unresolved='ignore'),
 ]
 
+# FIXME: This logic can be removed once all buildbots have moved
+# debuginfo-test from clang/test to llvm/projects or monorepo.
+if os.path.exists(os.path.join(config.test_source_root, 'debuginfo-tests')):
+  if os.path.isfile(
+  os.path.join(config.test_source_root, 'debuginfo-tests', 'lit.cfg.py')):
+config.excludes.append('debuginfo-tests')
+  else:
+tools.append(ToolSubst('%test_debuginfo', command=os.path.join(
+  config.llvm_src_root, 'utils', 'test_debuginfo.pl')))
+
 if config.clang_examples:
 tools.append('clang-interpreter')
 


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

[PATCH] D41055: [debuginfo-tests] Add support for moving debuginfo-tests from clang/test to llvm/projects or monorepo.

[Phabricator 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 rL320494: [debuginfo-tests] Add support for moving 
debuginfo-tests from clang/test to… (authored by dhinton, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D41055

Files:
  cfe/trunk/test/CMakeLists.txt
  cfe/trunk/test/lit.cfg.py


Index: cfe/trunk/test/lit.cfg.py
===
--- cfe/trunk/test/lit.cfg.py
+++ cfe/trunk/test/lit.cfg.py
@@ -58,12 +58,20 @@
 
 tools = [
 'c-index-test', 'clang-check', 'clang-diff', 'clang-format', 'opt',
-ToolSubst('%test_debuginfo', command=os.path.join(
-config.llvm_src_root, 'utils', 'test_debuginfo.pl')),
 ToolSubst('%clang_func_map', command=FindTool(
 'clang-func-mapping'), unresolved='ignore'),
 ]
 
+# FIXME: This logic can be removed once all buildbots have moved
+# debuginfo-test from clang/test to llvm/projects or monorepo.
+if os.path.exists(os.path.join(config.test_source_root, 'debuginfo-tests')):
+  if os.path.isfile(
+  os.path.join(config.test_source_root, 'debuginfo-tests', 'lit.cfg.py')):
+config.excludes.append('debuginfo-tests')
+  else:
+tools.append(ToolSubst('%test_debuginfo', command=os.path.join(
+  config.llvm_src_root, 'utils', 'test_debuginfo.pl')))
+
 if config.clang_examples:
 tools.append('clang-interpreter')
 
Index: cfe/trunk/test/CMakeLists.txt
===
--- cfe/trunk/test/CMakeLists.txt
+++ cfe/trunk/test/CMakeLists.txt
@@ -131,3 +131,12 @@
 add_custom_target(clang-test)
 add_dependencies(clang-test check-clang)
 set_target_properties(clang-test PROPERTIES FOLDER "Clang tests")
+
+# FIXME: This logic can be removed once all buildbots have moved
+# debuginfo-test from clang/test to llvm/projects or monorepo.
+if(EXISTS debuginfo-tests)
+  message(WARNING "Including debuginfo-tests in clang/test is deprecated.  
Move to llvm/projects or use monorepo.")
+  if(EXISTS debuginfo-tests/CMakeLists.txt)
+add_subdirectory(debuginfo-tests)
+  endif()
+endif()


Index: cfe/trunk/test/lit.cfg.py
===
--- cfe/trunk/test/lit.cfg.py
+++ cfe/trunk/test/lit.cfg.py
@@ -58,12 +58,20 @@
 
 tools = [
 'c-index-test', 'clang-check', 'clang-diff', 'clang-format', 'opt',
-ToolSubst('%test_debuginfo', command=os.path.join(
-config.llvm_src_root, 'utils', 'test_debuginfo.pl')),
 ToolSubst('%clang_func_map', command=FindTool(
 'clang-func-mapping'), unresolved='ignore'),
 ]
 
+# FIXME: This logic can be removed once all buildbots have moved
+# debuginfo-test from clang/test to llvm/projects or monorepo.
+if os.path.exists(os.path.join(config.test_source_root, 'debuginfo-tests')):
+  if os.path.isfile(
+  os.path.join(config.test_source_root, 'debuginfo-tests', 'lit.cfg.py')):
+config.excludes.append('debuginfo-tests')
+  else:
+tools.append(ToolSubst('%test_debuginfo', command=os.path.join(
+  config.llvm_src_root, 'utils', 'test_debuginfo.pl')))
+
 if config.clang_examples:
 tools.append('clang-interpreter')
 
Index: cfe/trunk/test/CMakeLists.txt
===
--- cfe/trunk/test/CMakeLists.txt
+++ cfe/trunk/test/CMakeLists.txt
@@ -131,3 +131,12 @@
 add_custom_target(clang-test)
 add_dependencies(clang-test check-clang)
 set_target_properties(clang-test PROPERTIES FOLDER "Clang tests")
+
+# FIXME: This logic can be removed once all buildbots have moved
+# debuginfo-test from clang/test to llvm/projects or monorepo.
+if(EXISTS debuginfo-tests)
+  message(WARNING "Including debuginfo-tests in clang/test is deprecated.  Move to llvm/projects or use monorepo.")
+  if(EXISTS debuginfo-tests/CMakeLists.txt)
+add_subdirectory(debuginfo-tests)
+  endif()
+endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40897: [clangd] Introduce a "Symbol" class.

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added inline comments.



Comment at: clangd/Symbol.h:37
+// The class presents a C++ symbol, e.g. class, function.
+struct Symbol {
+  // The symbol identifier, using USR.

sammccall wrote:
> malaperle wrote:
> > malaperle wrote:
> > > sammccall wrote:
> > > > hokein wrote:
> > > > > malaperle wrote:
> > > > > > sammccall wrote:
> > > > > > > hokein wrote:
> > > > > > > > malaperle wrote:
> > > > > > > > > I think it would be nice to have methods as an interface to 
> > > > > > > > > get this data instead of storing them directly. So that an 
> > > > > > > > > index-on-disk could go fetch the data. Especially the 
> > > > > > > > > occurrences which can take a lot of memory (I'm working on a 
> > > > > > > > > branch that does that). But perhaps defining that interface 
> > > > > > > > > is not within the scope of this patch and could be better 
> > > > > > > > > discussed in D40548 ?
> > > > > > > > I agree. We can't load all the symbol occurrences into the 
> > > > > > > > memory since they are too large. We need to design interface 
> > > > > > > > for the symbol occurrences. 
> > > > > > > > 
> > > > > > > > We could discuss the interface here, but CodeCompletion is the 
> > > > > > > > main thing which this patch focuses on. 
> > > > > > > > We can't load all the symbol occurrences into the memory since 
> > > > > > > > they are too large
> > > > > > > 
> > > > > > > I've heard this often, but never backed up by data :-)
> > > > > > > 
> > > > > > > Naively an array of references for a symbol could be doc ID + 
> > > > > > > offset + length, let's say 16 bytes.
> > > > > > > 
> > > > > > > If a source file consisted entirely of references to 1-character 
> > > > > > > symbols separated by punctuation (1 reference per 2 bytes) then 
> > > > > > > the total size of these references would be 8x the size of the 
> > > > > > > source file - in practice much less. That's not very big.
> > > > > > > 
> > > > > > > (Maybe there are edge cases with macros/templates, but we can 
> > > > > > > keep them under control)
> > > > > > I'd have to break down how much memory it used by what, I'll come 
> > > > > > back to you on that. Indexing llvm with ClangdIndexDataStorage, 
> > > > > > which is pretty packed is about 200MB. That's already a lot 
> > > > > > considering we want to index code bases many times bigger. But I'll 
> > > > > > try to come up with more precise numbers. I'm open to different 
> > > > > > strategies.
> > > > > > 
> > > > > I can see two points here:
> > > > > 
> > > > > 1. For all symbol occurrences of a TU, it is not quite large, and we 
> > > > > can keep them in memory.
> > > > > 2. For all symbol occurrences of a whole project, it's not a good 
> > > > > idea to load all of them into memory (For LLVM project, the size of 
> > > > > YAML dataset is ~1.2G).  
> > > > (This is still a sidebar - not asking for any changes)
> > > > 
> > > > The YAML dataset is not a good proxy for how big the data is (at least 
> > > > without an effort to estimate constant factor).
> > > > And "it's not a good idea" isn't an assertion that can hold without 
> > > > reasons, assumptions, and data.
> > > > If the size turns out to be, say, 120MB for LLVM, and we want to scale 
> > > > to 10x that, and we're spending 500MB/file for ASTs, then it might well 
> > > > be a good trade.
> > > > The YAML dataset is not a good proxy for how big the data is (at least 
> > > > without an effort to estimate constant factor).
> > > 
> > > Indeed. I'll try to come up with more realistic numbers. There are other 
> > > things not accounted for in the 16 bytes mentioned above, like storing 
> > > roles and relations.
> > > 
> > > > 500MB/file for ASTs
> > > 
> > > What do you mean? 500MB worth of occurrences per file? Or Preambles 
> > > perhaps?
> > > What do you mean? 500MB worth of occurrences per file? Or Preambles 
> > > perhaps?
> > 
> > Oh I see, the AST must be in memory for fast reparse. I just tried opening 
> > 3 files at the same time I it was already around 500MB. Hmm, that's a bit 
> > alarming.
> Right, just that we have to consider RAM usage for the index in the context 
> of clangd's overall requirements - if other parts of clangd use 1G of ram for 
> typical work on a large project, then we shouldn't rule out spending a couple 
> of 100MB on the index if it adds a lot of value.
Agreed we have to consider the overall requirements. I think over 1GB of RAM is 
not good for our use case, whether is comes from the AST or index. I think it's 
perfectly normal if we have different requirements but we can see discuss how 
to design things so there are options to use either more RAM or disk space. It 
seems the AST would be the most important factor for now so perhaps it's 
something we should start investigating/discussing.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40897



___
cfe-commits mailing list
cfe

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter updated this revision to Diff 126559.
CaseyCarter added a comment.

Better/simpler workaround from Zhihao.


https://reviews.llvm.org/D41048

Files:
  include/tuple


Index: include/tuple
===
--- include/tuple
+++ include/tuple
@@ -1012,10 +1012,10 @@
 
 template 
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+static constexpr bool __matches[sizeof...(_Args)] = {is_same<_T1, 
_Args>::value...};
 static constexpr size_t value = __find_detail::__find_idx(0, __matches);
-static_assert (value != __not_found, "type not found in type list" );
-static_assert(value != __ambiguous,"type occurs more than once in type 
list");
+static_assert(value != __not_found, "type not found in type list" );
+static_assert(value != __ambiguous, "type occurs more than once in type 
list");
 };
 
 template 


Index: include/tuple
===
--- include/tuple
+++ include/tuple
@@ -1012,10 +1012,10 @@
 
 template 
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+static constexpr bool __matches[sizeof...(_Args)] = {is_same<_T1, _Args>::value...};
 static constexpr size_t value = __find_detail::__find_idx(0, __matches);
-static_assert (value != __not_found, "type not found in type list" );
-static_assert(value != __ambiguous,"type occurs more than once in type list");
+static_assert(value != __not_found, "type not found in type list" );
+static_assert(value != __ambiguous, "type occurs more than once in type list");
 };
 
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter added a comment.

This new-and-improved workaround - thanks to @lichray - is unintrusive enough 
that I can't imagine it being unnacceptable to anyone. I'm going to go ahead 
and check this in.


https://reviews.llvm.org/D41048



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

[PATCH] D41118: [clangd] Emit ranges for clangd diagnostics, and fix off-by-one positions

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added reviewers: rwols, hokein.
Herald added subscribers: cfe-commits, ilya-biryukov, klimek.

- when the diagnostic has an explicit range, we prefer that
- if the diagnostic has a fixit, its RemoveRange is our next choice
- otherwise we try to expand the diagnostic location into a whole token. 
(inspired by VSCode, which does this client-side when given an empty range)
- if all else fails, we return the zero-width range as now. (clients react in 
different ways to this, highlighting a token or a char)
- this includes the off-by-one fix from https://reviews.llvm.org/D40860, and 
borrows heavily from it


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41118

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  test/clangd/diagnostics.test
  test/clangd/execute-command.test
  test/clangd/extra-flags.test
  test/clangd/fixits.test

Index: test/clangd/fixits.test
===
--- test/clangd/fixits.test
+++ test/clangd/fixits.test
@@ -15,11 +15,11 @@
 # CHECK-NEXT:"message": "using the result of an assignment as a condition without parentheses",
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
-# CHECK-NEXT:"character": 35,
+# CHECK-NEXT:"character": 37,
 # CHECK-NEXT:"line": 0
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "start": {
-# CHECK-NEXT:"character": 35,
+# CHECK-NEXT:"character": 32,
 # CHECK-NEXT:"line": 0
 # CHECK-NEXT:  }
 # CHECK-NEXT:},
@@ -33,7 +33,7 @@
 # CHECK-NEXT:"line": 0
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "start": {
-# CHECK-NEXT:"character": 35,
+# CHECK-NEXT:"character": 34,
 # CHECK-NEXT:"line": 0
 # CHECK-NEXT:  }
 # CHECK-NEXT:},
@@ -47,7 +47,7 @@
 # CHECK-NEXT:"line": 0
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "start": {
-# CHECK-NEXT:"character": 35,
+# CHECK-NEXT:"character": 34,
 # CHECK-NEXT:"line": 0
 # CHECK-NEXT:  }
 # CHECK-NEXT:},
@@ -58,7 +58,7 @@
 # CHECK-NEXT:  }
 Content-Length: 746
 
-{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
 #  CHECK:  "id": 2,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": [
@@ -128,7 +128,7 @@
 # CHECK-NEXT:  ]
 Content-Length: 771
 
-{"jsonrpc":"2.0","id":3,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":2,"code":"1","source":"foo","message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
+{"jsonrpc":"2.0","id":3,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "cha

[libcxx] r320500 - workaround PR 28385 in __find_exactly_one_checked

[Casey Carter via cfe-commits]

Author: caseycarter
Date: Tue Dec 12 09:22:24 2017
New Revision: 320500

URL: http://llvm.org/viewvc/llvm-project?rev=320500&view=rev
Log:
workaround PR 28385 in __find_exactly_one_checked

Fixes #35578.

Differential Revision: D41048

Modified:
libcxx/trunk/include/tuple

Modified: libcxx/trunk/include/tuple
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/tuple?rev=320500&r1=320499&r2=320500&view=diff
==
--- libcxx/trunk/include/tuple (original)
+++ libcxx/trunk/include/tuple Tue Dec 12 09:22:24 2017
@@ -1012,10 +1012,10 @@ constexpr size_t __find_idx(size_t __i,
 
 template 
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+static constexpr bool __matches[sizeof...(_Args)] = {is_same<_T1, 
_Args>::value...};
 static constexpr size_t value = __find_detail::__find_idx(0, __matches);
-static_assert (value != __not_found, "type not found in type list" );
-static_assert(value != __ambiguous,"type occurs more than once in type 
list");
+static_assert(value != __not_found, "type not found in type list" );
+static_assert(value != __ambiguous, "type occurs more than once in type 
list");
 };
 
 template 


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

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Casey Carter via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320500: workaround PR 28385 in __find_exactly_one_checked 
(authored by CaseyCarter, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41048?vs=126559&id=126564#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D41048

Files:
  libcxx/trunk/include/tuple


Index: libcxx/trunk/include/tuple
===
--- libcxx/trunk/include/tuple
+++ libcxx/trunk/include/tuple
@@ -1012,10 +1012,10 @@
 
 template 
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+static constexpr bool __matches[sizeof...(_Args)] = {is_same<_T1, 
_Args>::value...};
 static constexpr size_t value = __find_detail::__find_idx(0, __matches);
-static_assert (value != __not_found, "type not found in type list" );
-static_assert(value != __ambiguous,"type occurs more than once in type 
list");
+static_assert(value != __not_found, "type not found in type list" );
+static_assert(value != __ambiguous, "type occurs more than once in type 
list");
 };
 
 template 


Index: libcxx/trunk/include/tuple
===
--- libcxx/trunk/include/tuple
+++ libcxx/trunk/include/tuple
@@ -1012,10 +1012,10 @@
 
 template 
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+static constexpr bool __matches[sizeof...(_Args)] = {is_same<_T1, _Args>::value...};
 static constexpr size_t value = __find_detail::__find_idx(0, __matches);
-static_assert (value != __not_found, "type not found in type list" );
-static_assert(value != __ambiguous,"type occurs more than once in type list");
+static_assert(value != __not_found, "type not found in type list" );
+static_assert(value != __ambiguous, "type occurs more than once in type list");
 };
 
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40860: [clangd] Fix diagnostic positions

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

https://reviews.llvm.org/D41118 adds real (non-empty) ranges for diagnostics.
I've copied all the changes you've made here with some slight tweaks, as they 
were required for that too.

If you like it as-is, probably easiest to land that instead.
But I'm also happy for you to land this and I'll merge. Up to you.


https://reviews.llvm.org/D40860



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

[PATCH] D34848: Driver: Don't mix system tools with devtoolset tools on RHEL

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

Ping.


https://reviews.llvm.org/D34848



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

r320506 - Add --cuda-path to mock a CUDA Toolkit installation to avoid

[Kelvin Li via cfe-commits]

Author: kli
Date: Tue Dec 12 10:33:39 2017
New Revision: 320506

URL: http://llvm.org/viewvc/llvm-project?rev=320506&view=rev
Log:
Add --cuda-path to mock a CUDA Toolkit installation to avoid
unexpected error messages for incompatibility between the
default SM level and the support in the installed toolkit.

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

Modified:
cfe/trunk/test/Driver/unknown-std.cpp

Modified: cfe/trunk/test/Driver/unknown-std.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/unknown-std.cpp?rev=320506&r1=320505&r2=320506&view=diff
==
--- cfe/trunk/test/Driver/unknown-std.cpp (original)
+++ cfe/trunk/test/Driver/unknown-std.cpp Tue Dec 12 10:33:39 2017
@@ -4,7 +4,7 @@
 
 // RUN: not %clang %s -std=foobar -c 2>&1 | FileCheck --match-full-lines %s
 // RUN: not %clang -x objective-c++ %s -std=foobar -c 2>&1 | FileCheck 
--match-full-lines %s
-// RUN: not %clang -x cuda -nocudainc -nocudalib %s -std=foobar -c 2>&1 | 
FileCheck --match-full-lines --check-prefix=CHECK --check-prefix=CUDA %s
+// RUN: not %clang -x cuda -nocudainc -nocudalib 
--cuda-path=%S/Inputs/CUDA/usr/local/cuda %s -std=foobar -c 2>&1 | FileCheck 
--match-full-lines --check-prefix=CHECK --check-prefix=CUDA %s
 
 // CHECK: error: invalid value 'foobar' in '-std=foobar'
 // CHECK-NEXT: note: use 'c++98' or 'c++03' for 'ISO C++ 1998 with amendments' 
standard


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

[PATCH] D40996: Add --no-cuda-version-check in unknown-std.cpp

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC320506: Add --cuda-path to mock a CUDA Toolkit installation 
to avoid (authored by kli, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D40996

Files:
  test/Driver/unknown-std.cpp


Index: test/Driver/unknown-std.cpp
===
--- test/Driver/unknown-std.cpp
+++ test/Driver/unknown-std.cpp
@@ -4,7 +4,7 @@
 
 // RUN: not %clang %s -std=foobar -c 2>&1 | FileCheck --match-full-lines %s
 // RUN: not %clang -x objective-c++ %s -std=foobar -c 2>&1 | FileCheck 
--match-full-lines %s
-// RUN: not %clang -x cuda -nocudainc -nocudalib %s -std=foobar -c 2>&1 | 
FileCheck --match-full-lines --check-prefix=CHECK --check-prefix=CUDA %s
+// RUN: not %clang -x cuda -nocudainc -nocudalib 
--cuda-path=%S/Inputs/CUDA/usr/local/cuda %s -std=foobar -c 2>&1 | FileCheck 
--match-full-lines --check-prefix=CHECK --check-prefix=CUDA %s
 
 // CHECK: error: invalid value 'foobar' in '-std=foobar'
 // CHECK-NEXT: note: use 'c++98' or 'c++03' for 'ISO C++ 1998 with amendments' 
standard


Index: test/Driver/unknown-std.cpp
===
--- test/Driver/unknown-std.cpp
+++ test/Driver/unknown-std.cpp
@@ -4,7 +4,7 @@
 
 // RUN: not %clang %s -std=foobar -c 2>&1 | FileCheck --match-full-lines %s
 // RUN: not %clang -x objective-c++ %s -std=foobar -c 2>&1 | FileCheck --match-full-lines %s
-// RUN: not %clang -x cuda -nocudainc -nocudalib %s -std=foobar -c 2>&1 | FileCheck --match-full-lines --check-prefix=CHECK --check-prefix=CUDA %s
+// RUN: not %clang -x cuda -nocudainc -nocudalib --cuda-path=%S/Inputs/CUDA/usr/local/cuda %s -std=foobar -c 2>&1 | FileCheck --match-full-lines --check-prefix=CHECK --check-prefix=CUDA %s
 
 // CHECK: error: invalid value 'foobar' in '-std=foobar'
 // CHECK-NEXT: note: use 'c++98' or 'c++03' for 'ISO C++ 1998 with amendments' standard
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r320509 - [libcxx] P0604, invoke_result and is_invocable

[Zhihao Yuan via cfe-commits]

Author: lichray
Date: Tue Dec 12 10:42:04 2017
New Revision: 320509

URL: http://llvm.org/viewvc/llvm-project?rev=320509&view=rev
Log:
[libcxx] P0604, invoke_result and is_invocable

Summary:
Introduce a new form of `result_of` without function type encoding.

Rename and split `is_callable/is_nothrow_callable` into 
`is_invocable/is_nothrow_invocable/is_invocable_r/is_nothrow_invocable_r` (and 
associated types accordingly)

Change function type encoding of previous `is_callable/is_nothrow_callable` 
traits to conventional template type parameter lists.


Reviewers: EricWF, mclow.lists, bebuch

Reviewed By: EricWF, bebuch

Subscribers: lichray, bebuch, cfe-commits

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

Added:
libcxx/trunk/test/std/utilities/meta/meta.rel/is_invocable.pass.cpp
libcxx/trunk/test/std/utilities/meta/meta.rel/is_nothrow_invocable.pass.cpp
Removed:
libcxx/trunk/test/std/utilities/meta/meta.rel/is_callable.pass.cpp
libcxx/trunk/test/std/utilities/meta/meta.rel/is_nothrow_callable.pass.cpp
Modified:
libcxx/trunk/include/type_traits
libcxx/trunk/include/variant
libcxx/trunk/test/std/utilities/function.objects/func.not_fn/not_fn.pass.cpp

libcxx/trunk/test/std/utilities/function.objects/unord.hash/non_enum.pass.cpp

libcxx/trunk/test/std/utilities/meta/meta.trans/meta.trans.other/result_of.pass.cpp

libcxx/trunk/test/std/utilities/meta/meta.trans/meta.trans.other/result_of11.pass.cpp

Modified: libcxx/trunk/include/type_traits
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/type_traits?rev=320509&r1=320508&r2=320509&view=diff
==
--- libcxx/trunk/include/type_traits (original)
+++ libcxx/trunk/include/type_traits Tue Dec 12 10:42:04 2017
@@ -137,13 +137,11 @@ namespace std
 template  struct is_base_of;
 template  struct is_convertible;
 
-template  struct is_callable; // not defined
-template 
-  struct is_callable;
-
-template  struct is_nothrow_callable; // not defined
-template 
-  struct is_nothrow_callable;
+template  struct is_invocable;
+template  struct is_invocable_r;
+
+template  struct is_nothrow_invocable;
+template  struct 
is_nothrow_invocable_r;
 
 // Alignment properties and transformations:
 template  struct alignment_of;
@@ -157,6 +155,7 @@ namespace std
 template  struct underlying_type;
 template  class result_of; // undefined
 template  class result_of;
+template  struct invoke_result;  // C++17
 
 // const-volatile modifications:
 template 
@@ -215,8 +214,10 @@ namespace std
   using common_type_t = typename common_type::type;  // C++14
 template 
   using underlying_type_t = typename underlying_type::type;  // C++14
-template 
-  using result_of_t   = typename result_of::type;  // 
C++14
+template 
+  using result_of_t   = typename result_of::type;  // C++14
+template 
+  using invoke_result_t   = typename invoke_result::type; 
 // C++17
 
 template 
   using void_t = void;   // C++17
@@ -370,10 +371,14 @@ namespace std
 = is_base_of::value;  // 
C++17
   template  constexpr bool is_convertible_v
 = is_convertible::value;   // 
C++17
-  template  constexpr bool is_callable_v
-= is_callable::value;  // 
C++17
-  template  constexpr bool is_nothrow_callable_v
-= is_nothrow_callable::value;  // 
C++17
+  template  constexpr bool is_invocable_v
+= is_invocable::value;  // 
C++17
+  template  constexpr bool 
is_invocable_r_v
+= is_invocable_r::value; // 
C++17
+  template  constexpr bool 
is_nothrow_invocable_v
+= is_nothrow_invocable::value;  // 
C++17
+  template  constexpr bool 
is_nothrow_invocable_r_v
+= is_nothrow_invocable_r::value; // 
C++17
 
   // [meta.logical], logical operator traits:
   template struct conjunction;   // 
C++17
@@ -4402,6 +4407,13 @@ using __nothrow_invokable_r =
 >;
 
 template 
+using __nothrow_invokable =
+__nothrow_invokable_r_imp<
+__invokable<_Fp, _Args...>::value,
+true, void, _Fp, _Args...
+>;
+
+template 
 struct __invoke_of
 : public enable_if<
 __invokable<_Fp, _Args...>::value,
@@ -4423,30 +4435,48 @@ template  using result_of_t =
 
 #if _LIBCPP_STD_VER > 14
 
-// is_callable
+// invoke_result
 
-template 
-struct _LIBCPP_TEMPLATE_VIS is_callable;
+template 
+struct _LIBCPP_TEMPLATE_VIS invoke_result
+: __invoke_of<_Fn, _Args...>
+{
+};
+
+template 
+using invoke_result_t = typename invoke_result<_Fn, _Args...>::type;
+
+// is_invocable
 
-template 
-struct _LIBCPP_TEMPLATE_VIS is_callable<_Fn(_Args..

[PATCH] D41087: [Preprocessor] Implement __is_target_{arch|vendor|os|environment} function-like builtin macros

[Alex Lorenz via Phabricator via cfe-commits]

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



Comment at: lib/Lex/PPMacroExpansion.cpp:1923
+  Tok, *this, diag::err_feature_check_malformed);
+  return II ? getTargetInfo().getTriple().getArchName().equals_lower(
+  II->getName())

compnerd wrote:
> Hmm, the one thing to consider here is the canonicalized vs spelt target.  
> e.g. `armv7-windows` will map to `thumbv7-unknown-windows-msvc`.
I think it's ok to only allow "thumb" check to succeed instead of "arm", 
otherwise how would we differentiate between the two? However, we should take 
the sub arch into account, so when arch is "thumbv7", these checks should 
succeed:

```
__is_target_arch(thumb)
__is_target_arch(thumbv7)
```

but this one should fail:

```
__is_target_arch(thumbv6)
```

I fixed this in the updated patch.


Repository:
  rC Clang

https://reviews.llvm.org/D41087



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

[PATCH] D38831: [libcxx] P0604, invoke_result and is_invocable

[Zhihao Yuan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320509: [libcxx] P0604, invoke_result and is_invocable 
(authored by lichray, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D38831?vs=126545&id=126580#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38831

Files:
  libcxx/trunk/include/type_traits
  libcxx/trunk/include/variant
  libcxx/trunk/test/std/utilities/function.objects/func.not_fn/not_fn.pass.cpp
  libcxx/trunk/test/std/utilities/function.objects/unord.hash/non_enum.pass.cpp
  libcxx/trunk/test/std/utilities/meta/meta.rel/is_callable.pass.cpp
  libcxx/trunk/test/std/utilities/meta/meta.rel/is_invocable.pass.cpp
  libcxx/trunk/test/std/utilities/meta/meta.rel/is_nothrow_callable.pass.cpp
  libcxx/trunk/test/std/utilities/meta/meta.rel/is_nothrow_invocable.pass.cpp
  
libcxx/trunk/test/std/utilities/meta/meta.trans/meta.trans.other/result_of.pass.cpp
  
libcxx/trunk/test/std/utilities/meta/meta.trans/meta.trans.other/result_of11.pass.cpp

Index: libcxx/trunk/include/type_traits
===
--- libcxx/trunk/include/type_traits
+++ libcxx/trunk/include/type_traits
@@ -137,13 +137,11 @@
 template  struct is_base_of;
 template  struct is_convertible;
 
-template  struct is_callable; // not defined
-template 
-  struct is_callable;
-
-template  struct is_nothrow_callable; // not defined
-template 
-  struct is_nothrow_callable;
+template  struct is_invocable;
+template  struct is_invocable_r;
+
+template  struct is_nothrow_invocable;
+template  struct is_nothrow_invocable_r;
 
 // Alignment properties and transformations:
 template  struct alignment_of;
@@ -157,6 +155,7 @@
 template  struct underlying_type;
 template  class result_of; // undefined
 template  class result_of;
+template  struct invoke_result;  // C++17
 
 // const-volatile modifications:
 template 
@@ -215,8 +214,10 @@
   using common_type_t = typename common_type::type;  // C++14
 template 
   using underlying_type_t = typename underlying_type::type;  // C++14
-template 
-  using result_of_t   = typename result_of::type;  // C++14
+template 
+  using result_of_t   = typename result_of::type;  // C++14
+template 
+  using invoke_result_t   = typename invoke_result::type;  // C++17
 
 template 
   using void_t = void;   // C++17
@@ -370,10 +371,14 @@
 = is_base_of::value;  // C++17
   template  constexpr bool is_convertible_v
 = is_convertible::value;   // C++17
-  template  constexpr bool is_callable_v
-= is_callable::value;  // C++17
-  template  constexpr bool is_nothrow_callable_v
-= is_nothrow_callable::value;  // C++17
+  template  constexpr bool is_invocable_v
+= is_invocable::value;  // C++17
+  template  constexpr bool is_invocable_r_v
+= is_invocable_r::value; // C++17
+  template  constexpr bool is_nothrow_invocable_v
+= is_nothrow_invocable::value;  // C++17
+  template  constexpr bool is_nothrow_invocable_r_v
+= is_nothrow_invocable_r::value; // C++17
 
   // [meta.logical], logical operator traits:
   template struct conjunction;   // C++17
@@ -4402,6 +4407,13 @@
 >;
 
 template 
+using __nothrow_invokable =
+__nothrow_invokable_r_imp<
+__invokable<_Fp, _Args...>::value,
+true, void, _Fp, _Args...
+>;
+
+template 
 struct __invoke_of
 : public enable_if<
 __invokable<_Fp, _Args...>::value,
@@ -4423,30 +4435,48 @@
 
 #if _LIBCPP_STD_VER > 14
 
-// is_callable
+// invoke_result
 
-template 
-struct _LIBCPP_TEMPLATE_VIS is_callable;
+template 
+struct _LIBCPP_TEMPLATE_VIS invoke_result
+: __invoke_of<_Fn, _Args...>
+{
+};
+
+template 
+using invoke_result_t = typename invoke_result<_Fn, _Args...>::type;
+
+// is_invocable
 
-template 
-struct _LIBCPP_TEMPLATE_VIS is_callable<_Fn(_Args...), _Ret>
+template 
+struct _LIBCPP_TEMPLATE_VIS is_invocable
+: integral_constant::value> {};
+
+template 
+struct _LIBCPP_TEMPLATE_VIS is_invocable_r
 : integral_constant::value> {};
 
-template 
-constexpr bool is_callable_v = is_callable<_Fn, _Ret>::value;
+template 
+constexpr bool is_invocable_v = is_invocable<_Fn, _Args...>::value;
+
+template 
+constexpr bool is_invocable_r_v = is_invocable_r<_Ret, _Fn, _Args...>::value;
 
 // is_nothrow_callable
 
-template 
-struct _LIBCPP_TEMPLATE_VIS is_nothrow_callable;
+template 
+struct _LIBCPP_TEMPLATE_VIS is_nothrow_invocable
+: integral_constant::value> {};
+
+template 
+struct _LIBCPP_TEMPLATE_VIS is_nothrow_invocable_r
+: integral_constant::value> {};
 
-templ

[PATCH] D41087: [Preprocessor] Implement __is_target_{arch|vendor|os|environment} function-like builtin macros

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 126581.
arphaman added a comment.
Herald added a subscriber: javed.absar.

- Change error message.
- Take sub arch into account.


https://reviews.llvm.org/D41087

Files:
  include/clang/Lex/Preprocessor.h
  lib/Lex/PPMacroExpansion.cpp
  test/Preprocessor/is_target.c
  test/Preprocessor/is_target_arm.c
  test/Preprocessor/is_target_os_darwin.c
  test/Preprocessor/is_target_unknown.c

Index: test/Preprocessor/is_target_unknown.c
===
--- /dev/null
+++ test/Preprocessor/is_target_unknown.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fsyntax-only -triple i686-unknown-unknown -verify %s
+// RUN: %clang_cc1 -fsyntax-only -triple i686-- -verify %s
+// expected-no-diagnostics
+
+#if __is_target_arch(unknown)
+  #error "mismatching arch"
+#endif
+
+// Unknown vendor is allowed.
+#if !__is_target_vendor(unknown)
+  #error "mismatching vendor"
+#endif
+
+// Unknown OS is allowed.
+#if !__is_target_os(unknown)
+  #error "mismatching OS"
+#endif
+
+// Unknown environment is allowed.
+#if !__is_target_environment(unknown)
+  #error "mismatching environment"
+#endif
Index: test/Preprocessor/is_target_os_darwin.c
===
--- /dev/null
+++ test/Preprocessor/is_target_os_darwin.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macos -DMAC -verify %s
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-ios -verify %s
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-tvos -verify %s
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-watchos -verify %s
+// expected-no-diagnostics
+
+#if !__is_target_os(darwin)
+  #error "mismatching os"
+#endif
+
+// macOS matches both macOS and macOSX.
+#ifdef MAC
+
+#if !__is_target_os(macos)
+  #error "mismatching os"
+#endif
+
+#if !__is_target_os(macosx)
+  #error "mismatching os"
+#endif
+
+#if __is_target_os(ios)
+  #error "mismatching os"
+#endif
+
+#endif
Index: test/Preprocessor/is_target_arm.c
===
--- /dev/null
+++ test/Preprocessor/is_target_arm.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fsyntax-only -triple thumbv7--windows-msvc19.11.0 -verify %s
+// expected-no-diagnostics
+
+// ARM does not match thumb.
+#if __is_target_arch(arm) || __is_target_arch(armv7)
+  #error "mismatching arch"
+#endif
+
+// Allow checking for precise arch + subarch.
+#if !__is_target_arch(thumbv7)
+  #error "mismatching arch"
+#endif
+
+// But also allow checking for the arch without subarch.
+#if !__is_target_arch(thumb)
+  #error "mismatching arch"
+#endif
+
+// Same arch with a different subarch doesn't match.
+#if __is_target_arch(thumbv6)
+  #error "mismatching arch"
+#endif
Index: test/Preprocessor/is_target.c
===
--- /dev/null
+++ test/Preprocessor/is_target.c
@@ -0,0 +1,67 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-darwin-simulator -verify %s
+
+#if !__is_target_arch(x86_64) || !__is_target_arch(X86_64)
+  #error "mismatching arch"
+#endif
+
+#if __is_target_arch(arm64)
+  #error "mismatching arch"
+#endif
+
+// Silently ignore invalid archs. This will ensure that older compilers will
+// accept headers that support new arches/vendors/os variants.
+#if __is_target_arch(foo)
+  #error "invalid arch"
+#endif
+
+#if !__is_target_vendor(apple) || !__is_target_vendor(APPLE)
+  #error "mismatching vendor"
+#endif
+
+#if __is_target_vendor(unknown)
+  #error "mismatching vendor"
+#endif
+
+#if __is_target_vendor(foo)
+  #error "invalid vendor"
+#endif
+
+#if !__is_target_os(darwin) || !__is_target_os(DARWIN)
+  #error "mismatching os"
+#endif
+
+#if __is_target_os(ios)
+  #error "mismatching os"
+#endif
+
+#if __is_target_os(foo)
+  #error "invalid os"
+#endif
+
+#if !__is_target_environment(simulator) || !__is_target_environment(SIMULATOR)
+  #error "mismatching environment"
+#endif
+
+#if __is_target_environment(unknown)
+  #error "mismatching environment"
+#endif
+
+#if __is_target_environment(foo)
+  #error "invalid environment"
+#endif
+
+#if !__has_builtin(__is_target_arch) || !__has_builtin(__is_target_os) || !__has_builtin(__is_target_vendor) || !__has_builtin(__is_target_environment)
+  #error "has builtin doesn't work"
+#endif
+
+#if __is_target_arch(11) // expected-error {{builtin feature check macro requires a parenthesized identifier}}
+  #error "invalid arch"
+#endif
+
+#if __is_target_arch x86 // expected-error{{missing '(' after '__is_target_arch'}}
+  #error "invalid arch"
+#endif
+
+#if __is_target_arch ( x86  // expected-error {{unterminated function-like macro invocation}}
+  #error "invalid arch"
+#endif // expected-error@-2 {{expected value in expression}}
Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -375,6 +375,11 @@
   Ident__h

Re: [PATCH] D41039: Add support for attribute "trivial_abi"

[David Blaikie via cfe-commits]

On Mon, Dec 11, 2017 at 5:38 PM John McCall  wrote:

> On Mon, Dec 11, 2017 at 6:19 PM, David Blaikie  wrote:
>
>> On Mon, Dec 11, 2017 at 3:16 PM John McCall via Phabricator <
>> revi...@reviews.llvm.org> wrote:
>>
>>> rjmccall added a comment.
>>>
>>> In https://reviews.llvm.org/D41039#951648, @ahatanak wrote:
>>>
>>> > I had a discussion with Duncan today and he pointed out that perhaps
>>> we shouldn't allow users to annotate a struct with "trivial_abi" if one of
>>> its subobjects is non-trivial and is not annotated with "trivial_abi" since
>>> that gives users too much power.
>>> >
>>> > Should we error out or drop "trivial_abi" from struct Outer when the
>>> following code is compiled?
>>> >
>>> >   struct Inner1 {
>>> > ~Inner1(); // non-trivial
>>> > int x;
>>> >   };
>>> >
>>> >   struct __attribute__((trivial_abi)) Outer {
>>> > ~Outer();
>>> > Inner1 x;
>>> >   };
>>> >
>>> >
>>> > The current patch doesn't error out or drop the attribute, but the
>>> patch would probably be much simpler if we didn't allow it.
>>>
>>>
>>> I think it makes sense to emit an error if there is provably a
>>> non-trivial-ABI component.  However, for class temploids I think that
>>> diagnostic should only fire on the definition, not on instantiations; for
>>> example:
>>>
>>>   template  struct __attribute__((trivial_abi)) holder {
>>>  T value;
>>>  ~holder() {}
>>>   };
>>>   holder hs; // this instantiation should be legal despite
>>> the fact that holder cannot be trivial-ABI.
>>>
>>> But we should still be able to emit the diagnostic in template
>>> definitions, e.g.:
>>>
>>>   template  struct __attribute__((trivial_abi)) named_holder {
>>>  std::string name; // there are no instantiations of this template
>>> that could ever be trivial-ABI
>>>  T value;
>>>  ~named_holder() {}
>>>   };
>>>
>>> The wording should be something akin to the standard template rule that
>>> a template is ill-formed if it has no valid instantiations, no diagnostic
>>> required.
>>>
>>> I would definitely like to open the conversation about the name of the
>>> attribute.  I don't think we've used "abi" in an existing attribute name;
>>> usually it's more descriptive.  And "trivial" is a weighty word in the
>>> standard.  I'm not sure I have a great counter-proposal off the top of my
>>> head, though.
>>>
>>
>> Agreed on both counts (would love a better name, don't have any stand-out
>> candidates off the top of my head).
>>
>> I feel like a more descriptive term about the property of the object
>> would make me happier - something like "address_independent_identity"
>> (s/identity/value/?) though, yeah, that's not spectacular by any stretch.
>>
>
> Incidentally, your comments are not showing up on Phabricator for some
> reason.
>

Yeah, Phab doesn't do a great job looking on the mailing list for
interesting replies - I forget, maybe it only catches bottom post or top
post but not infix replies or something...


>
> The term "trivially movable" suggests itself, with two caveats:
>   - What we're talking about is trivial *destructive* movability, i.e.
> that the combination of moving the value to a new object and not destroying
> the old object can be done trivially, which is not quite the same as
> trivial movability in the normal C++ sense, which I guess could be a
> property that someone theoretically might care about (if the type is
> trivially destructed, but it isn't copyable for semantic reasons?).
>   - Trivial destructive movability is a really common property, and it's
> something that a compiler would really like to optimize based on even in
> cases where trivial_abi can't be adopted for binary-compatibility reasons.
> Stealing the term for the stronger property that the type is trivially
> destructively movable *and its ABI should reflect that in a specific way*
> would be unfortunate.
>

Fair point.


> "trivially_movable" is a long attribute anyway, and
> "trivially_destructively_movable" is even worse.
>
> Maybe that second point is telling us that this isn't purely descriptive —
> it's inherently talking about the ABI, not just the semantics of the type.
> I might be talking myself into accepting trivial_abi if we don't end up
> with a better suggestion.
>

*nod* I think if you want to slice this that way (ensuring there's space to
support describing a similar property for use only in non-ABI-breaking
contexts as well) it seems like ABI is the term to use somewhere in the
name.


> Random thing that occurred to me: is it actually reasonable to enforce
> trivial_abi correctness in a non-template context?  Templates aren't the
> only case where a user could reasonably want to add trivial_abi and just
> have it be suppressed if it's wrong.  Imagine if some stdlib made
> std::string trivial_abi; someone might reasonably want to make my
> named_holder example above trivial_abi as well, with the expectation that
> it would only have an effect on targets where std::string was trivial_abi.
> A

[PATCH] D38831: [libcxx] P0604, invoke_result and is_invocable

[Zhihao Yuan via Phabricator via cfe-commits]

lichray added a comment.

That commit message generated from `arc` wasn't all that friendly :/
Thanks for the patch, and @K-ballo you should ask Chris for a commit bit :)


Repository:
  rL LLVM

https://reviews.llvm.org/D38831



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

[PATCH] D41123: [OpenMP] Add function attribute for triggering data sharing.

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

gtbercea created this revision.
gtbercea added reviewers: hfinkel, Hahnfeld, ABataev, carlo.bertolli, caomhin.
Herald added a subscriber: jholewinski.

The backend should only emit data sharing code for the cases where it is needed.
A new function attribute is used by Clang to enable data sharing only for the 
cases where OpenMP semantics require it and there are variables that need to be 
shared.


Repository:
  rL LLVM

https://reviews.llvm.org/D41123

Files:
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  test/OpenMP/nvptx_data_sharing.cpp


Index: test/OpenMP/nvptx_data_sharing.cpp
===
--- test/OpenMP/nvptx_data_sharing.cpp
+++ test/OpenMP/nvptx_data_sharing.cpp
@@ -22,15 +22,15 @@
 
 /// = In the worker function = ///
 
-// CK1: define internal void 
@__omp_offloading_{{.*}}test_ds{{.*}}worker(){{.*}}{
+// CK1: define internal void @__omp_offloading_{{.*}}test_ds{{.*}}worker() #0
 // CK1: [[SHAREDARGS:%.+]] = alloca i8**
 // CK1: call i1 @__kmpc_kernel_parallel(i8** %work_fn, i8*** [[SHAREDARGS]])
 // CK1: [[SHARGSTMP:%.+]] = load i8**, i8*** [[SHAREDARGS]]
 // CK1: call void @__omp_outlined___wrapper{{.*}}({{.*}}, i8** [[SHARGSTMP]])
 
 /// = In the kernel function = ///
 
-// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}()
+// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}() #1
 // CK1: [[SHAREDARGS1:%.+]] = alloca i8**
 // CK1: call void @__kmpc_kernel_prepare_parallel({{.*}}, i8*** 
[[SHAREDARGS1]], i32 1)
 // CK1: [[SHARGSTMP1:%.+]] = load i8**, i8*** [[SHAREDARGS1]]
@@ -40,13 +40,18 @@
 
 /// = In the data sharing wrapper function = ///
 
-// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**){{.*}}{
+// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**) #0
 // CK1: [[SHAREDARGS2:%.+]] = alloca i8**
 // CK1: store i8** %2, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP3:%.+]] = load i8**, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP4:%.+]] = getelementptr inbounds i8*, i8** [[SHARGSTMP3]]
 // CK1: [[SHARGSTMP5:%.+]] = bitcast i8** [[SHARGSTMP4]] to i32**
 // CK1: [[SHARGSTMP6:%.+]] = load i32*, i32** [[SHARGSTMP5]]
 // CK1: call void @__omp_outlined__({{.*}}, i32* [[SHARGSTMP6]])
 
+/// = Attributes = ///
+
+// CK1-NOT: attributes #0 = { {{.*}}"has-nvptx-shared-depot"{{.*}} }
+// CK1: attributes #1 = { {{.*}}"has-nvptx-shared-depot"{{.*}} }
+
 #endif
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -942,6 +942,8 @@
 llvm::Value *ID = Bld.CreateBitOrPointerCast(WFn, CGM.Int8PtrTy);
 
 if (!CapturedVars.empty()) {
+  // There's somehting to share, add the attribute
+  CGF.CurFn->addFnAttr("has-nvptx-shared-depot");
   // Prepare for parallel region. Indicate the outlined function.
   Address SharedArgs =
   CGF.CreateDefaultAlignTempAlloca(CGF.VoidPtrPtrTy,


Index: test/OpenMP/nvptx_data_sharing.cpp
===
--- test/OpenMP/nvptx_data_sharing.cpp
+++ test/OpenMP/nvptx_data_sharing.cpp
@@ -22,15 +22,15 @@
 
 /// = In the worker function = ///
 
-// CK1: define internal void @__omp_offloading_{{.*}}test_ds{{.*}}worker(){{.*}}{
+// CK1: define internal void @__omp_offloading_{{.*}}test_ds{{.*}}worker() #0
 // CK1: [[SHAREDARGS:%.+]] = alloca i8**
 // CK1: call i1 @__kmpc_kernel_parallel(i8** %work_fn, i8*** [[SHAREDARGS]])
 // CK1: [[SHARGSTMP:%.+]] = load i8**, i8*** [[SHAREDARGS]]
 // CK1: call void @__omp_outlined___wrapper{{.*}}({{.*}}, i8** [[SHARGSTMP]])
 
 /// = In the kernel function = ///
 
-// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}()
+// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}() #1
 // CK1: [[SHAREDARGS1:%.+]] = alloca i8**
 // CK1: call void @__kmpc_kernel_prepare_parallel({{.*}}, i8*** [[SHAREDARGS1]], i32 1)
 // CK1: [[SHARGSTMP1:%.+]] = load i8**, i8*** [[SHAREDARGS1]]
@@ -40,13 +40,18 @@
 
 /// = In the data sharing wrapper function = ///
 
-// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**){{.*}}{
+// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**) #0
 // CK1: [[SHAREDARGS2:%.+]] = alloca i8**
 // CK1: store i8** %2, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP3:%.+]] = load i8**, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP4:%.+]] = getelementptr inbounds i8*, i8** [[SHARGSTMP3]]
 // CK1: [[SHARGSTMP5:%.+]] = bitcast i8** [[SHARGSTMP4]] to i32**
 // CK1: [[SHARGSTMP6:%.+]] = load i32*, i32** [[SHARGSTMP5]]
 // CK1: call void @__omp_outlined__({{.*}}, i32* [[SHARGSTMP6]])
 
+/// = Attributes = ///
+
+// CK1-NOT: attributes #0 = { {{.*}}"has-nvptx-shared-depot"{{.*}} }
+// CK1: attributes #1 = { {{.*}}"has-nvptx-shared-depot"{{.*}} 

[PATCH] D41123: [OpenMP] Add function attribute for triggering data sharing.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: test/OpenMP/nvptx_data_sharing.cpp:25-43
+// CK1: define internal void @__omp_offloading_{{.*}}test_ds{{.*}}worker() #0
 // CK1: [[SHAREDARGS:%.+]] = alloca i8**
 // CK1: call i1 @__kmpc_kernel_parallel(i8** %work_fn, i8*** [[SHAREDARGS]])
 // CK1: [[SHARGSTMP:%.+]] = load i8**, i8*** [[SHAREDARGS]]
 // CK1: call void @__omp_outlined___wrapper{{.*}}({{.*}}, i8** [[SHARGSTMP]])
 
 /// = In the kernel function = ///

Not a good idea to use exact values as attributes, use macros instead.


Repository:
  rL LLVM

https://reviews.llvm.org/D41123



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

[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Alternative #1 is actually the right option — only it shouldn't be an 
objc_retain, it should be an objc_retainAutoreleasedReturnValue.


Repository:
  rC Clang

https://reviews.llvm.org/D41050



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

[PATCH] D41123: [OpenMP] Add function attribute for triggering data sharing.

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

gtbercea updated this revision to Diff 126594.
gtbercea added a comment.

Fix test.


Repository:
  rL LLVM

https://reviews.llvm.org/D41123

Files:
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  test/OpenMP/nvptx_data_sharing.cpp


Index: test/OpenMP/nvptx_data_sharing.cpp
===
--- test/OpenMP/nvptx_data_sharing.cpp
+++ test/OpenMP/nvptx_data_sharing.cpp
@@ -22,15 +22,15 @@
 
 /// = In the worker function = ///
 
-// CK1: define internal void 
@__omp_offloading_{{.*}}test_ds{{.*}}worker(){{.*}}{
+// CK1: define internal void @__omp_offloading_{{.*}}test_ds{{.*}}worker() 
[[ATTR1:#.*]] {
 // CK1: [[SHAREDARGS:%.+]] = alloca i8**
 // CK1: call i1 @__kmpc_kernel_parallel(i8** %work_fn, i8*** [[SHAREDARGS]])
 // CK1: [[SHARGSTMP:%.+]] = load i8**, i8*** [[SHAREDARGS]]
 // CK1: call void @__omp_outlined___wrapper{{.*}}({{.*}}, i8** [[SHARGSTMP]])
 
 /// = In the kernel function = ///
 
-// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}()
+// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}() [[ATTR2:#.*]] 
{
 // CK1: [[SHAREDARGS1:%.+]] = alloca i8**
 // CK1: call void @__kmpc_kernel_prepare_parallel({{.*}}, i8*** 
[[SHAREDARGS1]], i32 1)
 // CK1: [[SHARGSTMP1:%.+]] = load i8**, i8*** [[SHAREDARGS1]]
@@ -40,13 +40,18 @@
 
 /// = In the data sharing wrapper function = ///
 
-// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**){{.*}}{
+// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**) 
[[ATTR1]] {
 // CK1: [[SHAREDARGS2:%.+]] = alloca i8**
 // CK1: store i8** %2, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP3:%.+]] = load i8**, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP4:%.+]] = getelementptr inbounds i8*, i8** [[SHARGSTMP3]]
 // CK1: [[SHARGSTMP5:%.+]] = bitcast i8** [[SHARGSTMP4]] to i32**
 // CK1: [[SHARGSTMP6:%.+]] = load i32*, i32** [[SHARGSTMP5]]
 // CK1: call void @__omp_outlined__({{.*}}, i32* [[SHARGSTMP6]])
 
+/// = Attributes = ///
+
+// CK1-NOT: attributes [[ATTR1]] = { {{.*}}"has-nvptx-shared-depot"{{.*}} }
+// CK1: attributes [[ATTR2]] = { {{.*}}"has-nvptx-shared-depot"{{.*}} }
+
 #endif
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -942,6 +942,8 @@
 llvm::Value *ID = Bld.CreateBitOrPointerCast(WFn, CGM.Int8PtrTy);
 
 if (!CapturedVars.empty()) {
+  // There's somehting to share, add the attribute
+  CGF.CurFn->addFnAttr("has-nvptx-shared-depot");
   // Prepare for parallel region. Indicate the outlined function.
   Address SharedArgs =
   CGF.CreateDefaultAlignTempAlloca(CGF.VoidPtrPtrTy,


Index: test/OpenMP/nvptx_data_sharing.cpp
===
--- test/OpenMP/nvptx_data_sharing.cpp
+++ test/OpenMP/nvptx_data_sharing.cpp
@@ -22,15 +22,15 @@
 
 /// = In the worker function = ///
 
-// CK1: define internal void @__omp_offloading_{{.*}}test_ds{{.*}}worker(){{.*}}{
+// CK1: define internal void @__omp_offloading_{{.*}}test_ds{{.*}}worker() [[ATTR1:#.*]] {
 // CK1: [[SHAREDARGS:%.+]] = alloca i8**
 // CK1: call i1 @__kmpc_kernel_parallel(i8** %work_fn, i8*** [[SHAREDARGS]])
 // CK1: [[SHARGSTMP:%.+]] = load i8**, i8*** [[SHAREDARGS]]
 // CK1: call void @__omp_outlined___wrapper{{.*}}({{.*}}, i8** [[SHARGSTMP]])
 
 /// = In the kernel function = ///
 
-// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}()
+// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}() [[ATTR2:#.*]] {
 // CK1: [[SHAREDARGS1:%.+]] = alloca i8**
 // CK1: call void @__kmpc_kernel_prepare_parallel({{.*}}, i8*** [[SHAREDARGS1]], i32 1)
 // CK1: [[SHARGSTMP1:%.+]] = load i8**, i8*** [[SHAREDARGS1]]
@@ -40,13 +40,18 @@
 
 /// = In the data sharing wrapper function = ///
 
-// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**){{.*}}{
+// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**) [[ATTR1]] {
 // CK1: [[SHAREDARGS2:%.+]] = alloca i8**
 // CK1: store i8** %2, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP3:%.+]] = load i8**, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP4:%.+]] = getelementptr inbounds i8*, i8** [[SHARGSTMP3]]
 // CK1: [[SHARGSTMP5:%.+]] = bitcast i8** [[SHARGSTMP4]] to i32**
 // CK1: [[SHARGSTMP6:%.+]] = load i32*, i32** [[SHARGSTMP5]]
 // CK1: call void @__omp_outlined__({{.*}}, i32* [[SHARGSTMP6]])
 
+/// = Attributes = ///
+
+// CK1-NOT: attributes [[ATTR1]] = { {{.*}}"has-nvptx-shared-depot"{{.*}} }
+// CK1: attributes [[ATTR2]] = { {{.*}}"has-nvptx-shared-depot"{{.*}} }
+
 #endif
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -942,6 

[PATCH] D41123: [OpenMP] Add function attribute for triggering data sharing.

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rL LLVM

https://reviews.llvm.org/D41123



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

r320519 - [cmake] Follow-up to rL320494.

[Don Hinton via cfe-commits]

Author: dhinton
Date: Tue Dec 12 11:47:40 2017
New Revision: 320519

URL: http://llvm.org/viewvc/llvm-project?rev=320519&view=rev
Log:
[cmake] Follow-up to rL320494.

EXISTS requires full paths.

Modified:
cfe/trunk/test/CMakeLists.txt

Modified: cfe/trunk/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CMakeLists.txt?rev=320519&r1=320518&r2=320519&view=diff
==
--- cfe/trunk/test/CMakeLists.txt (original)
+++ cfe/trunk/test/CMakeLists.txt Tue Dec 12 11:47:40 2017
@@ -134,9 +134,9 @@ set_target_properties(clang-test PROPERT
 
 # FIXME: This logic can be removed once all buildbots have moved
 # debuginfo-test from clang/test to llvm/projects or monorepo.
-if(EXISTS debuginfo-tests)
+if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/debuginfo-tests)
   message(WARNING "Including debuginfo-tests in clang/test is deprecated.  
Move to llvm/projects or use monorepo.")
-  if(EXISTS debuginfo-tests/CMakeLists.txt)
+  if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/debuginfo-tests/CMakeLists.txt)
 add_subdirectory(debuginfo-tests)
   endif()
 endif()


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

[PATCH] D41118: [clangd] Emit ranges for clangd diagnostics, and fix off-by-one positions

[Raoul Wols via Phabricator via cfe-commits]

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

LGTM, you can go ahead and land this, I'll drop 
https://reviews.llvm.org/D40860. I'm happy we got to the bottom of this :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41118



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

[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block

[Dan Zimmerman via Phabricator via cfe-commits]

danzimm added a comment.

@rjmccall ah right, good point -- I think it's ok to elide that 
objc_retainAutoreleasedReturnValue/objc_autoreleaseReturnValue pair in this 
case though, since all of this is generated within clang itself (and thus we 
can make the guarentee that the object will properly live until the return of 
the enclosing block) -- it reduces retain count churn this way too


Repository:
  rC Clang

https://reviews.llvm.org/D41050



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

[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block

[Dan Zimmerman via Phabricator via cfe-commits]

danzimm added a comment.

Or do we need to abide by those semantics strictly here? Could you expand on 
why that is, if that's the case?


Repository:
  rC Clang

https://reviews.llvm.org/D41050



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

[PATCH] D39050: Add index-while-building support to Clang

[Nathan Hawes via Phabricator via cfe-commits]

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

Thanks for taking another look @ioeric – I'll work through your comments and 
update.


https://reviews.llvm.org/D39050



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

r320521 - [OpenMP] Diagnose function name on the link clause

[Kelvin Li via cfe-commits]

Author: kli
Date: Tue Dec 12 12:08:12 2017
New Revision: 320521

URL: http://llvm.org/viewvc/llvm-project?rev=320521&view=rev
Log:
[OpenMP] Diagnose function name on the link clause

This patch is to add diagnose when a function name is
specified on the link clause. According to the  OpenMP
spec, only the list items that exclude the function 
name are allowed on the link clause.

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/declare_target_ast_print.cpp
cfe/trunk/test/OpenMP/declare_target_messages.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=320521&r1=320520&r2=320521&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Dec 12 12:08:12 
2017
@@ -8710,6 +8710,8 @@ def err_omp_declare_target_to_and_link :
 def warn_omp_not_in_target_context : Warning<
   "declaration is not declared in any declare target region">,
   InGroup;
+def err_omp_function_in_link_clause : Error<
+  "function name is not allowed in 'link' clause">;
 def err_omp_aligned_expected_array_or_ptr : Error<
   "argument of aligned clause should be array"
   "%select{ or pointer|, pointer, reference to array or reference to pointer}1"

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=320521&r1=320520&r2=320521&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Dec 12 12:08:12 2017
@@ -8656,7 +8656,8 @@ public:
 OMPDeclareTargetDeclAttr::MapTypeTy MT,
 NamedDeclSetType &SameDirectiveDecls);
   /// Check declaration inside target region.
-  void checkDeclIsAllowedInOpenMPTarget(Expr *E, Decl *D);
+  void checkDeclIsAllowedInOpenMPTarget(Expr *E, Decl *D,
+SourceLocation IdLoc = 
SourceLocation());
   /// Return true inside OpenMP declare target region.
   bool isInOpenMPDeclareTargetContext() const {
 return IsInOpenMPDeclareTargetContext;

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=320521&r1=320520&r2=320521&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Tue Dec 12 12:08:12 2017
@@ -12600,7 +12600,7 @@ void Sema::ActOnOpenMPDeclareTargetName(
   ND->addAttr(A);
   if (ASTMutationListener *ML = Context.getASTMutationListener())
 ML->DeclarationMarkedOpenMPDeclareTarget(ND, A);
-  checkDeclIsAllowedInOpenMPTarget(nullptr, ND);
+  checkDeclIsAllowedInOpenMPTarget(nullptr, ND, Id.getLoc());
 } else if (ND->getAttr()->getMapType() != MT) {
   Diag(Id.getLoc(), diag::err_omp_declare_target_to_and_link)
   << Id.getName();
@@ -12689,7 +12689,8 @@ static bool checkValueDeclInTarget(Sourc
   return true;
 }
 
-void Sema::checkDeclIsAllowedInOpenMPTarget(Expr *E, Decl *D) {
+void Sema::checkDeclIsAllowedInOpenMPTarget(Expr *E, Decl *D,
+SourceLocation IdLoc) {
   if (!D || D->isInvalidDecl())
 return;
   SourceRange SR = E ? E->getSourceRange() : D->getSourceRange();
@@ -12718,6 +12719,16 @@ void Sema::checkDeclIsAllowedInOpenMPTar
   return;
 }
   }
+  if (FunctionDecl *FD = dyn_cast(D)) {
+if (FD->hasAttr() &&
+(FD->getAttr()->getMapType() ==
+ OMPDeclareTargetDeclAttr::MT_Link)) {
+  assert(IdLoc.isValid() && "Source location is expected");
+  Diag(IdLoc, diag::err_omp_function_in_link_clause);
+  Diag(FD->getLocation(), diag::note_defined_here) << FD;
+  return;
+}
+  }
   if (!E) {
 // Checking declaration inside declare target region.
 if (!D->hasAttr() &&

Modified: cfe/trunk/test/OpenMP/declare_target_ast_print.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_target_ast_print.cpp?rev=320521&r1=320520&r2=320521&view=diff
==
--- cfe/trunk/test/OpenMP/declare_target_ast_print.cpp (original)
+++ cfe/trunk/test/OpenMP/declare_target_ast_print.cpp Tue Dec 12 12:08:12 2017
@@ -108,9 +108,7 @@ void f2() {
 // CHECK: #pragma omp end declare target
 
 int c1, c2, c3;
-void f3() {
-}
-#pragma omp declare target link(c1) link(c2), link(c3, f3)
+#pragma omp declare target link(c1) link(c2), link(c3)
 // CHECK: #pragma omp de

[PATCH] D40968: [OpenMP] Diagnose function name on the link clause

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320521: [OpenMP] Diagnose function name on the link clause 
(authored by kli, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D40968?vs=126217&id=126600#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40968

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/Sema/SemaOpenMP.cpp
  cfe/trunk/test/OpenMP/declare_target_ast_print.cpp
  cfe/trunk/test/OpenMP/declare_target_messages.cpp

Index: cfe/trunk/lib/Sema/SemaOpenMP.cpp
===
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp
@@ -12600,7 +12600,7 @@
   ND->addAttr(A);
   if (ASTMutationListener *ML = Context.getASTMutationListener())
 ML->DeclarationMarkedOpenMPDeclareTarget(ND, A);
-  checkDeclIsAllowedInOpenMPTarget(nullptr, ND);
+  checkDeclIsAllowedInOpenMPTarget(nullptr, ND, Id.getLoc());
 } else if (ND->getAttr()->getMapType() != MT) {
   Diag(Id.getLoc(), diag::err_omp_declare_target_to_and_link)
   << Id.getName();
@@ -12689,7 +12689,8 @@
   return true;
 }
 
-void Sema::checkDeclIsAllowedInOpenMPTarget(Expr *E, Decl *D) {
+void Sema::checkDeclIsAllowedInOpenMPTarget(Expr *E, Decl *D,
+SourceLocation IdLoc) {
   if (!D || D->isInvalidDecl())
 return;
   SourceRange SR = E ? E->getSourceRange() : D->getSourceRange();
@@ -12718,6 +12719,16 @@
   return;
 }
   }
+  if (FunctionDecl *FD = dyn_cast(D)) {
+if (FD->hasAttr() &&
+(FD->getAttr()->getMapType() ==
+ OMPDeclareTargetDeclAttr::MT_Link)) {
+  assert(IdLoc.isValid() && "Source location is expected");
+  Diag(IdLoc, diag::err_omp_function_in_link_clause);
+  Diag(FD->getLocation(), diag::note_defined_here) << FD;
+  return;
+}
+  }
   if (!E) {
 // Checking declaration inside declare target region.
 if (!D->hasAttr() &&
Index: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -8656,7 +8656,8 @@
 OMPDeclareTargetDeclAttr::MapTypeTy MT,
 NamedDeclSetType &SameDirectiveDecls);
   /// Check declaration inside target region.
-  void checkDeclIsAllowedInOpenMPTarget(Expr *E, Decl *D);
+  void checkDeclIsAllowedInOpenMPTarget(Expr *E, Decl *D,
+SourceLocation IdLoc = SourceLocation());
   /// Return true inside OpenMP declare target region.
   bool isInOpenMPDeclareTargetContext() const {
 return IsInOpenMPDeclareTargetContext;
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8710,6 +8710,8 @@
 def warn_omp_not_in_target_context : Warning<
   "declaration is not declared in any declare target region">,
   InGroup;
+def err_omp_function_in_link_clause : Error<
+  "function name is not allowed in 'link' clause">;
 def err_omp_aligned_expected_array_or_ptr : Error<
   "argument of aligned clause should be array"
   "%select{ or pointer|, pointer, reference to array or reference to pointer}1"
Index: cfe/trunk/test/OpenMP/declare_target_messages.cpp
===
--- cfe/trunk/test/OpenMP/declare_target_messages.cpp
+++ cfe/trunk/test/OpenMP/declare_target_messages.cpp
@@ -19,6 +19,10 @@
 
 void c(); // expected-warning {{declaration is not declared in any declare target region}}
 
+void func() {} // expected-note {{'func' defined here}}
+
+#pragma omp declare target link(func) // expected-error {{function name is not allowed in 'link' clause}}
+
 extern int b;
 
 struct NonT {
Index: cfe/trunk/test/OpenMP/declare_target_ast_print.cpp
===
--- cfe/trunk/test/OpenMP/declare_target_ast_print.cpp
+++ cfe/trunk/test/OpenMP/declare_target_ast_print.cpp
@@ -108,9 +108,7 @@
 // CHECK: #pragma omp end declare target
 
 int c1, c2, c3;
-void f3() {
-}
-#pragma omp declare target link(c1) link(c2), link(c3, f3)
+#pragma omp declare target link(c1) link(c2), link(c3)
 // CHECK: #pragma omp declare target link
 // CHECK: int c1;
 // CHECK: #pragma omp end declare target
@@ -120,9 +118,6 @@
 // CHECK: #pragma omp declare target link
 // CHECK: int c3;
 // CHECK: #pragma omp end declare target
-// CHECK: #pragma omp declare target link
-// CHECK: void f3()
-// CHECK: #pragma omp end declare target
 
 struct SSSt {
 #pragma omp declare target
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm

Re: [PATCH] D41039: Add support for attribute "trivial_abi"

[John McCall via cfe-commits]

On Tue, Dec 12, 2017 at 1:45 PM, David Blaikie  wrote:

> On Mon, Dec 11, 2017 at 5:38 PM John McCall  wrote:
>
>> On Mon, Dec 11, 2017 at 6:19 PM, David Blaikie 
>> wrote:
>>
>>> On Mon, Dec 11, 2017 at 3:16 PM John McCall via Phabricator <
>>> revi...@reviews.llvm.org> wrote:
>>>
 rjmccall added a comment.

 In https://reviews.llvm.org/D41039#951648, @ahatanak wrote:

 > I had a discussion with Duncan today and he pointed out that perhaps
 we shouldn't allow users to annotate a struct with "trivial_abi" if one of
 its subobjects is non-trivial and is not annotated with "trivial_abi" since
 that gives users too much power.
 >
 > Should we error out or drop "trivial_abi" from struct Outer when the
 following code is compiled?
 >
 >   struct Inner1 {
 > ~Inner1(); // non-trivial
 > int x;
 >   };
 >
 >   struct __attribute__((trivial_abi)) Outer {
 > ~Outer();
 > Inner1 x;
 >   };
 >
 >
 > The current patch doesn't error out or drop the attribute, but the
 patch would probably be much simpler if we didn't allow it.


 I think it makes sense to emit an error if there is provably a
 non-trivial-ABI component.  However, for class temploids I think that
 diagnostic should only fire on the definition, not on instantiations; for
 example:

   template  struct __attribute__((trivial_abi)) holder {
  T value;
  ~holder() {}
   };
   holder hs; // this instantiation should be legal despite
 the fact that holder cannot be trivial-ABI.

 But we should still be able to emit the diagnostic in template
 definitions, e.g.:

   template  struct __attribute__((trivial_abi)) named_holder {
  std::string name; // there are no instantiations of this template
 that could ever be trivial-ABI
  T value;
  ~named_holder() {}
   };

 The wording should be something akin to the standard template rule that
 a template is ill-formed if it has no valid instantiations, no diagnostic
 required.

 I would definitely like to open the conversation about the name of the
 attribute.  I don't think we've used "abi" in an existing attribute name;
 usually it's more descriptive.  And "trivial" is a weighty word in the
 standard.  I'm not sure I have a great counter-proposal off the top of my
 head, though.

>>>
>>> Agreed on both counts (would love a better name, don't have any
>>> stand-out candidates off the top of my head).
>>>
>>> I feel like a more descriptive term about the property of the object
>>> would make me happier - something like "address_independent_identity"
>>> (s/identity/value/?) though, yeah, that's not spectacular by any stretch.
>>>
>>
>> Incidentally, your comments are not showing up on Phabricator for some
>> reason.
>>
>
> Yeah, Phab doesn't do a great job looking on the mailing list for
> interesting replies - I forget, maybe it only catches bottom post or top
> post but not infix replies or something...
>

Well, fortunately the mailing list is also archived. :)


> The term "trivially movable" suggests itself, with two caveats:
>>   - What we're talking about is trivial *destructive* movability, i.e.
>> that the combination of moving the value to a new object and not destroying
>> the old object can be done trivially, which is not quite the same as
>> trivial movability in the normal C++ sense, which I guess could be a
>> property that someone theoretically might care about (if the type is
>> trivially destructed, but it isn't copyable for semantic reasons?).
>>   - Trivial destructive movability is a really common property, and it's
>> something that a compiler would really like to optimize based on even in
>> cases where trivial_abi can't be adopted for binary-compatibility reasons.
>> Stealing the term for the stronger property that the type is trivially
>> destructively movable *and its ABI should reflect that in a specific way*
>> would be unfortunate.
>>
>
> Fair point.
>
>
>> "trivially_movable" is a long attribute anyway, and
>> "trivially_destructively_movable" is even worse.
>>
>> Maybe that second point is telling us that this isn't purely descriptive
>> — it's inherently talking about the ABI, not just the semantics of the
>> type.  I might be talking myself into accepting trivial_abi if we don't end
>> up with a better suggestion.
>>
>
> *nod* I think if you want to slice this that way (ensuring there's space
> to support describing a similar property for use only in non-ABI-breaking
> contexts as well) it seems like ABI is the term to use somewhere in the
> name.
>

Yeah.  We just had a little internal conversation about it, and the idea of
"address_invariant_abi" came up, but I'm not sure it has enough to
recommend it over "trivial_abi" to justify the longer name.


> Random thing that occurred to me: is it actually reasonable to enforc

[PATCH] D41050: Fix over-release of return value of lambda implicitly converted to block

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D41050#952720, @danzimm wrote:

> Or do we need to abide by those semantics strictly here? Could you expand on 
> why that is, if that's the case?


The autoreleased-return-value optimization cannot be understood purely in terms 
of its impact on the retain count; it's a hand-off from the callee to the 
caller the requires a specific code-generation pattern in order to work.

Here, calling retainAutoreleasedRV in the block is necessary in order to 
prevent the return value from the lambda from actually being autoreleased.  The 
block then owns a retain on the return value and must call autoreleaseRV in 
order to balance that out and transfer the object to its caller.  The caller 
will hopefully also call retainAutoreleasedRV, thus completing the transfer of 
the return value all the way to the ultimate caller without actually performing 
any retains or releases.

John.


Repository:
  rC Clang

https://reviews.llvm.org/D41050



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

[PATCH] D41031: [clangd] (Attempt to) read clang-format file for document formatting

[Raoul Wols via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320524: [clangd] (Attempt to) read clang-format file for 
document formatting (authored by rwols, committed by ).
Herald added a subscriber: klimek.

Changed prior to commit:
  https://reviews.llvm.org/D41031?vs=126204&id=126603#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D41031

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.h

Index: clang-tools-extra/trunk/clangd/ClangdServer.h
===
--- clang-tools-extra/trunk/clangd/ClangdServer.h
+++ clang-tools-extra/trunk/clangd/ClangdServer.h
@@ -289,12 +289,19 @@
   llvm::Expected>>
   findDocumentHighlights(PathRef File, Position Pos);
 
-  /// Run formatting for \p Rng inside \p File.
-  std::vector formatRange(PathRef File, Range Rng);
-  /// Run formatting for the whole \p File.
-  std::vector formatFile(PathRef File);
-  /// Run formatting after a character was typed at \p Pos in \p File.
-  std::vector formatOnType(PathRef File, Position Pos);
+  /// Run formatting for \p Rng inside \p File with content \p Code.
+  llvm::Expected formatRange(StringRef Code,
+PathRef File, Range Rng);
+
+  /// Run formatting for the whole \p File with content \p Code.
+  llvm::Expected formatFile(StringRef Code,
+   PathRef File);
+
+  /// Run formatting after a character was typed at \p Pos in \p File with
+  /// content \p Code.
+  llvm::Expected
+  formatOnType(StringRef Code, PathRef File, Position Pos);
+
   /// Rename all occurrences of the symbol at the \p Pos in \p File to
   /// \p NewName.
   Expected> rename(PathRef File, Position Pos,
@@ -314,6 +321,13 @@
   void onFileEvent(const DidChangeWatchedFilesParams &Params);
 
 private:
+
+  /// FIXME: This stats several files to find a .clang-format file. I/O can be
+  /// slow. Think of a way to cache this.
+  llvm::Expected
+  formatCode(llvm::StringRef Code, PathRef File,
+ ArrayRef Ranges);
+
   std::future
   scheduleReparseAndDiags(PathRef File, VersionedDraft Contents,
   std::shared_ptr Resources,
Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp
@@ -38,16 +38,6 @@
   std::promise &Promise;
 };
 
-std::vector formatCode(StringRef Code, StringRef Filename,
- ArrayRef Ranges) {
-  // Call clang-format.
-  // FIXME: Don't ignore style.
-  format::FormatStyle Style = format::getLLVMStyle();
-  auto Result = format::reformat(Style, Code, Ranges, Filename);
-
-  return std::vector(Result.begin(), Result.end());
-}
-
 std::string getStandardResourceDir() {
   static int Dummy; // Just an address in this process.
   return CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy);
@@ -331,26 +321,23 @@
   return make_tagged(std::move(Result), TaggedFS.Tag);
 }
 
-std::vector ClangdServer::formatRange(PathRef File,
-Range Rng) {
-  std::string Code = getDocument(File);
-
+llvm::Expected
+ClangdServer::formatRange(StringRef Code, PathRef File, Range Rng) {
   size_t Begin = positionToOffset(Code, Rng.start);
   size_t Len = positionToOffset(Code, Rng.end) - Begin;
   return formatCode(Code, File, {tooling::Range(Begin, Len)});
 }
 
-std::vector ClangdServer::formatFile(PathRef File) {
+llvm::Expected ClangdServer::formatFile(StringRef Code,
+   PathRef File) {
   // Format everything.
-  std::string Code = getDocument(File);
   return formatCode(Code, File, {tooling::Range(0, Code.size())});
 }
 
-std::vector ClangdServer::formatOnType(PathRef File,
- Position Pos) {
+llvm::Expected
+ClangdServer::formatOnType(StringRef Code, PathRef File, Position Pos) {
   // Look for the previous opening brace from the character position and
   // format starting from there.
-  std::string Code = getDocument(File);
   size_t CursorPos = positionToOffset(Code, Pos);
   size_t PreviousLBracePos = StringRef(Code).find_last_of('{', CursorPos);
   if (PreviousLBracePos == StringRef::npos)
@@ -509,6 +496,20 @@
   return llvm::None;
 }
 
+llvm::Expected
+ClangdServer::formatCode(llvm::StringRef Code, PathRef File,
+ ArrayRef Ranges) {
+  // Call clang-format.
+  auto TaggedFS = FSProvider.getTaggedFileSystem(File);
+  auto StyleOrError =
+  format::getStyle("file", File, "LLVM", Code, TaggedFS.Value.get());
+  if (!StyleOrError) {
+return StyleOrError.takeError();
+  } else {
+return format::reformat(StyleO

[PATCH] D36915: [Sema] Diagnose local variables and parameters captured by lambda and block expressions in a default argument

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

This looks good to me.


https://reviews.llvm.org/D36915



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

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Hmm.  The OVE-ing of the RHS of a property assignment is just there to make the 
original source structure clearer.  Maybe the right solution here is to set a 
flag in the OVE that says that it's a unique semantic reference to its source 
expression, and then change IRGen to just recurse through OVEs with that flag 
set (and not pre-bind it).

You should make sure that we don't get this assertion with mandatory 
copy-elision and ?:, which does actually use its LHS multiple times 
semantically (and which cannot safely perform mandatory copy-elision on it).


https://reviews.llvm.org/D39562



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

[PATCH] D36915: [Sema] Diagnose local variables and parameters captured by lambda and block expressions in a default argument

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/Sema.cpp:1677-1684
+Sema::DefaultArgRAII::DefaultArgRAII(Sema &S)
+: Actions(S), PrevParent(S.getParentOfDefaultArg()) {
+  S.setParentOfDefaultArg(S.CurContext);
+}
+
+Sema::DefaultArgRAII::~DefaultArgRAII() {
+  Actions.setParentOfDefaultArg(PrevParent);

Any reason not to inline this in the header file?



Comment at: lib/Sema/SemaExpr.cpp:4523
+  // other parameters in unevaluated contexts.
+  if (FunctionDecl *Pattern = FD->getTemplateInstantiationPattern()) {
+auto I = FD->param_begin();

`const FunctionDecl *`



Comment at: lib/Sema/SemaExpr.cpp:4535
+  unsigned NumExpansions =
+  *getNumArgumentsInExpansion(PVD->getType(), MutiLevelArgList);
+  CurrentInstantiationScope->MakeInstantiatedLocalArgPack(PVD);

Is it possible for the `Optional<>` returned here to not hold a value?



Comment at: lib/Sema/SemaExpr.cpp:13746-13751
+unsigned DiagID;
+if (isa(var))
+  DiagID = diag::err_param_default_argument_references_param;
+else
+  DiagID = diag::err_param_default_argument_references_local;
+S.Diag(loc, DiagID) << var->getDeclName();

Ternary operator in `S.Diag()` would be just as clear, I think.

Also, you should not need to call `getDeclName()` as the diagnostic engine will 
automatically do the right thing. As is stands, this won't properly quote the 
name in the diagnostic.


https://reviews.llvm.org/D36915



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

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added inline comments.



Comment at: tools/scan-build-py/libscanbuild/report.py:257
 def read_bugs(output_dir, html):
+# type: (str, bool) -> Generator[Dict[str, Any], None, None]
 """ Generate a unique sequence of bugs from given output directory.

Minor nitpicking: type comments are semi-standardized with Sphinx-style 
auto-generated documentation, and should be a part of the docstring.


https://reviews.llvm.org/D30691



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

[clang-tools-extra] r320524 - [clangd] (Attempt to) read clang-format file for document formatting

[Raoul Wols via cfe-commits]

Author: rwols
Date: Tue Dec 12 12:25:06 2017
New Revision: 320524

URL: http://llvm.org/viewvc/llvm-project?rev=320524&view=rev
Log:
[clangd] (Attempt to) read clang-format file for document formatting

Summary:
Takes into account the clang-format file of the project, if any.
Reverts to LLVM if nothing is found. Replies with an error if any error occured.
For instance, a parse error in the clang-format YAML file.

Reviewers: ilya-biryukov, sammccall, Nebiroth, malaperle, krasimir

Reviewed By: sammccall

Subscribers: cfe-commits

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


Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=320524&r1=320523&r2=320524&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue Dec 12 12:25:06 2017
@@ -17,18 +17,29 @@ using namespace clang;
 
 namespace {
 
+TextEdit replacementToEdit(StringRef Code, const tooling::Replacement &R) {
+  Range ReplacementRange = {
+  offsetToPosition(Code, R.getOffset()),
+  offsetToPosition(Code, R.getOffset() + R.getLength())};
+  return {ReplacementRange, R.getReplacementText()};
+}
+
 std::vector
 replacementsToEdits(StringRef Code,
 const std::vector &Replacements) {
   // Turn the replacements into the format specified by the Language Server
   // Protocol. Fuse them into one big JSON array.
   std::vector Edits;
-  for (auto &R : Replacements) {
-Range ReplacementRange = {
-offsetToPosition(Code, R.getOffset()),
-offsetToPosition(Code, R.getOffset() + R.getLength())};
-Edits.push_back({ReplacementRange, R.getReplacementText()});
-  }
+  for (const auto &R : Replacements)
+Edits.push_back(replacementToEdit(Code, R));
+  return Edits;
+}
+
+std::vector replacementsToEdits(StringRef Code,
+  const tooling::Replacements &Repls) {
+  std::vector Edits;
+  for (const auto &R : Repls)
+Edits.push_back(replacementToEdit(Code, R));
   return Edits;
 }
 
@@ -153,23 +164,36 @@ void ClangdLSPServer::onDocumentOnTypeFo
 Ctx C, DocumentOnTypeFormattingParams &Params) {
   auto File = Params.textDocument.uri.file;
   std::string Code = Server.getDocument(File);
-  C.reply(json::ary(
-  replacementsToEdits(Code, Server.formatOnType(File, Params.position;
+  auto ReplacementsOrError = Server.formatOnType(Code, File, Params.position);
+  if (ReplacementsOrError)
+C.reply(json::ary(replacementsToEdits(Code, ReplacementsOrError.get(;
+  else
+C.replyError(ErrorCode::UnknownErrorCode,
+ llvm::toString(ReplacementsOrError.takeError()));
 }
 
 void ClangdLSPServer::onDocumentRangeFormatting(
 Ctx C, DocumentRangeFormattingParams &Params) {
   auto File = Params.textDocument.uri.file;
   std::string Code = Server.getDocument(File);
-  C.reply(json::ary(
-  replacementsToEdits(Code, Server.formatRange(File, Params.range;
+  auto ReplacementsOrError = Server.formatRange(Code, File, Params.range);
+  if (ReplacementsOrError)
+C.reply(json::ary(replacementsToEdits(Code, ReplacementsOrError.get(;
+  else
+C.replyError(ErrorCode::UnknownErrorCode,
+ llvm::toString(ReplacementsOrError.takeError()));
 }
 
 void ClangdLSPServer::onDocumentFormatting(Ctx C,
DocumentFormattingParams &Params) {
   auto File = Params.textDocument.uri.file;
   std::string Code = Server.getDocument(File);
-  C.reply(json::ary(replacementsToEdits(Code, Server.formatFile(File;
+  auto ReplacementsOrError = Server.formatFile(Code, File);
+  if (ReplacementsOrError)
+C.reply(json::ary(replacementsToEdits(Code, ReplacementsOrError.get(;
+  else
+C.replyError(ErrorCode::UnknownErrorCode,
+ llvm::toString(ReplacementsOrError.takeError()));
 }
 
 void ClangdLSPServer::onCodeAction(Ctx C, CodeActionParams &Params) {

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=320524&r1=320523&r2=320524&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Dec 12 12:25:06 2017
@@ -38,16 +38,6 @@ private:
   std::promise &Promise;
 };
 
-std::vector formatCode(StringRef Code, StringRef 
Filename,
- ArrayRef Ranges) {
-  // Call clang-format.
-  // FIXME: Don't ignore style.
-  format::FormatStyle Style = format::getLLVMStyle();
-  aut

[PATCH] D41129: Fix `git-clang-format `.

[Mark Lodato via Phabricator via cfe-commits]

lodato created this revision.
lodato added reviewers: djasper, klimek.

This feature had never worked at all because the code incorrectly used 
<`commit2>` for both the "old" and "new", so the diff was always empty! Now we 
correctly use `` where we should and it works fine.


https://reviews.llvm.org/D41129

Files:
  google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format


Index: 
google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
===
--- 
google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
+++ 
google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
@@ -153,7 +153,7 @@
   # those files.
   cd_to_toplevel()
   if len(commits) > 1:
-old_tree = commits[1]
+old_tree = commits[0]
 new_tree = run_clang_format_and_save_to_tree(changed_lines,
  revision=commits[1],
  binary=opts.binary,


Index: google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
===
--- google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
+++ google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
@@ -153,7 +153,7 @@
   # those files.
   cd_to_toplevel()
   if len(commits) > 1:
-old_tree = commits[1]
+old_tree = commits[0]
 new_tree = run_clang_format_and_save_to_tree(changed_lines,
  revision=commits[1],
  binary=opts.binary,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41130: git-clang-format: cleanup: Use run() when possible.

[Mark Lodato via Phabricator via cfe-commits]

lodato created this revision.
lodato added reviewers: djasper, klimek.

This makes the code a bit easier to understand. Also add a docstring to `run()`.

Note: This means that we read the entire index into memory when calling `git 
update-index`, whereas previously we would send the data line-by-line. Git 
already loads the entire index into memory anyway* so this should not cause a 
regression.

- To my knowledge.


https://reviews.llvm.org/D41130

Files:
  google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format


Index: 
google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
===
--- 
google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
+++ 
google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
@@ -356,12 +356,9 @@
   def index_info_generator():
 for filename, line_ranges in iteritems(changed_lines):
   if revision:
-git_metadata_cmd = ['git', 'ls-tree',
-'%s:%s' % (revision, os.path.dirname(filename)),
-os.path.basename(filename)]
-git_metadata = subprocess.Popen(git_metadata_cmd, 
stdin=subprocess.PIPE,
-stdout=subprocess.PIPE)
-stdout = git_metadata.communicate()[0]
+stdout = run('git', 'ls-tree',
+ '%s:%s' % (revision, os.path.dirname(filename)),
+ os.path.basename(filename))
 mode = oct(int(stdout.split()[0], 8))
   else:
 mode = oct(os.stat(filename).st_mode)
@@ -384,14 +381,9 @@
   --index-info", such as "".  Any other mode
   is invalid."""
   assert mode in ('--stdin', '--index-info')
-  cmd = ['git', 'update-index', '--add', '-z', mode]
   with temporary_index_file():
-p = subprocess.Popen(cmd, stdin=subprocess.PIPE)
-for line in input_lines:
-  p.stdin.write(to_bytes('%s\0' % line))
-p.stdin.close()
-if p.wait() != 0:
-  die('`%s` failed' % ' '.join(cmd))
+run('git', 'update-index', '--add', '-z', mode,
+stdin=to_bytes(''.join('%s\0' % line for line in input_lines)))
 tree_id = run('git', 'write-tree')
 return tree_id
 
@@ -522,6 +514,7 @@
 
 
 def run(*args, **kwargs):
+  """Runs the given command and returns stdout; exits on command failure."""
   stdin = kwargs.pop('stdin', '')
   verbose = kwargs.pop('verbose', True)
   strip = kwargs.pop('strip', True)


Index: google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
===
--- google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
+++ google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
@@ -356,12 +356,9 @@
   def index_info_generator():
 for filename, line_ranges in iteritems(changed_lines):
   if revision:
-git_metadata_cmd = ['git', 'ls-tree',
-'%s:%s' % (revision, os.path.dirname(filename)),
-os.path.basename(filename)]
-git_metadata = subprocess.Popen(git_metadata_cmd, stdin=subprocess.PIPE,
-stdout=subprocess.PIPE)
-stdout = git_metadata.communicate()[0]
+stdout = run('git', 'ls-tree',
+ '%s:%s' % (revision, os.path.dirname(filename)),
+ os.path.basename(filename))
 mode = oct(int(stdout.split()[0], 8))
   else:
 mode = oct(os.stat(filename).st_mode)
@@ -384,14 +381,9 @@
   --index-info", such as "".  Any other mode
   is invalid."""
   assert mode in ('--stdin', '--index-info')
-  cmd = ['git', 'update-index', '--add', '-z', mode]
   with temporary_index_file():
-p = subprocess.Popen(cmd, stdin=subprocess.PIPE)
-for line in input_lines:
-  p.stdin.write(to_bytes('%s\0' % line))
-p.stdin.close()
-if p.wait() != 0:
-  die('`%s` failed' % ' '.join(cmd))
+run('git', 'update-index', '--add', '-z', mode,
+stdin=to_bytes(''.join('%s\0' % line for line in input_lines)))
 tree_id = run('git', 'write-tree')
 return tree_id
 
@@ -522,6 +514,7 @@
 
 
 def run(*args, **kwargs):
+  """Runs the given command and returns stdout; exits on command failure."""
   stdin = kwargs.pop('stdin', '')
   verbose = kwargs.pop('verbose', True)
   strip = kwargs.pop('strip', True)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D40854#950640, @JonasToth wrote:

> > The "enforcement" listed on the C++ Core Guidelines is very unhelpful, so 
> > it might be worth bringing it up as an issue on their bug tracker. ES.100 
> > basically says "you know what we mean, wink wink" as enforcement and 
> > doesn't give any guidance as to what is safe or unsafe. It gives no 
> > exceptions, which I think is unlikely to be useful to most developers. For 
> > instance: `void f(unsigned i) { if (i > 0) {} }`, this is a mixed signed 
> > and unsigned comparison (literals are signed by default) -- should this 
> > check diagnose?
>
> I think the guidelines mostly aim for actual calculations here. For ES.102 
> there is an exception to "we want actual modulo arithmetic". They  seem 
> mostly concerned with the mixing of the signedness of integer types that 
> comes from explicitly expressing non-negative values with `unsigned` types. 
> Because this is a common pattern (e.g. STL containers), the mixing from 
> literals and co is quickly overseen.
>  See `ES.106: Don’t try to avoid negative values by using unsigned`
>
> > How about `unsigned int i = 12; i += 1;`? ES.102 is equally as strange with 
> > "Flag unsigned literals (e.g. -2) used as container subscripts." That's a 
> > signed literal (2) with a negation operator -- do they mean "Flag container 
> > subscripts whose value is known to be negative", or something else?
>
> I think here ES.106 is again the rationale but your example shows a hole. 
> `unsigned int i = -1` is explicitly forbidden and the example shows a chain 
> of implicit conversion of integer types as bad code.
>
> My gut feeling is that the guidelines want programers to write `auto i = 12u` 
> or `unsigned int = 12u` but they are not clear about that. Other rules that 
> could relate to this are:
>  `ES.11: Use auto to avoid redundant repetition of type names`, `ES.23: 
> Prefer the {} initializer syntax` (forbidding implicit conversions in 
> initialization), `ES.48: Avoid casts` (maybe, but implicit conversion are not 
> covered there).
>
> I will ask them about this issue and hopefully we have some clarification. 
> But I didn't have much success before :D


Thanks! As it stands, I'm not able to determine whether your implementation 
actually does or does not enforce those rules. That's why I was wondering what 
the extent of the coverage is according to the rule authors, because I am not 
certain whether we're actually implementing the intent of the rules or not.

> @aaron.ballman You are a maintainer for the `cert` rules, are you? How do you 
> handle these common issues with integer types? Maybe we could already propose 
> some guidance based on `cert`. It is mentioned as well written standard in 
> the guidelines :)

Yes, I used to maintain the CERT rules when I worked for the SEI (and I 
authored a considerable number of them). CERT and the C++ Core Guidelines have 
a somewhat fundamental difference in opinion on how we write the rules. CERT 
flags things that are demonstrably going to lead to incorrectness 
(specifically, vulnerabilities). The C++ Core Guidelines flag things that could 
possibly lead to correctness issues but may also be perfectly correct code. 
Because of this difference, CERT rules are a bit harder to implement in 
clang-tidy because they often will have some component of data or flow analysis 
to them, while C++ Core Guidelines are often blanket bans.

That said, CERT's guidance on integers mostly falls back to the C rules: 
https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152052

I would recommend looking at the C++ Core Guideline wording and try to come up 
with example code that would run afoul of the rule to see if *you* think a 
diagnostic would be useful with that code. If you can't convince yourself that 
diagnosing the code is a good idea, it's worth bringing up to the authors to 
see if they agree. Similarly, see if you can find example code that would not 
run afoul of the rules but you think *should* to see if they agree. (You can 
use the CERT rules on integers to help give ideas.) Once you have a corpus of 
examples you want to see diagnosed and not diagnosed, try to devise (possibly 
new) wording for the enforcement section of the rule and see if the rule 
authors agree with the enforcement. From there, writing the check becomes a 
pretty simple exercise in translating the enforcement into a clang-tidy check.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40854



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

[PATCH] D41129: Fix `git-clang-format `.

[Mark Lodato via Phabricator via cfe-commits]

lodato added a comment.

Actually the old code might have been working fine. Please hold off on 
reviewing.


https://reviews.llvm.org/D41129



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

[PATCH] D41129: Fix `git-clang-format `.

[Mark Lodato via Phabricator via cfe-commits]

lodato added a comment.

Yeah, I was incorrect. The feature worked fine. I don't know what I was 
thinking.


https://reviews.llvm.org/D41129



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

[PATCH] D41123: [OpenMP] Add function attribute for triggering data sharing.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320527: [OpenMP] Add function attribute for triggering data 
sharing. (authored by gbercea, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41123?vs=126594&id=126618#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D41123

Files:
  cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  cfe/trunk/test/OpenMP/nvptx_data_sharing.cpp


Index: cfe/trunk/test/OpenMP/nvptx_data_sharing.cpp
===
--- cfe/trunk/test/OpenMP/nvptx_data_sharing.cpp
+++ cfe/trunk/test/OpenMP/nvptx_data_sharing.cpp
@@ -22,15 +22,15 @@
 
 /// = In the worker function = ///
 
-// CK1: define internal void 
@__omp_offloading_{{.*}}test_ds{{.*}}worker(){{.*}}{
+// CK1: define internal void @__omp_offloading_{{.*}}test_ds{{.*}}worker() 
[[ATTR1:#.*]] {
 // CK1: [[SHAREDARGS:%.+]] = alloca i8**
 // CK1: call i1 @__kmpc_kernel_parallel(i8** %work_fn, i8*** [[SHAREDARGS]])
 // CK1: [[SHARGSTMP:%.+]] = load i8**, i8*** [[SHAREDARGS]]
 // CK1: call void @__omp_outlined___wrapper{{.*}}({{.*}}, i8** [[SHARGSTMP]])
 
 /// = In the kernel function = ///
 
-// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}()
+// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}() [[ATTR2:#.*]] 
{
 // CK1: [[SHAREDARGS1:%.+]] = alloca i8**
 // CK1: call void @__kmpc_kernel_prepare_parallel({{.*}}, i8*** 
[[SHAREDARGS1]], i32 1)
 // CK1: [[SHARGSTMP1:%.+]] = load i8**, i8*** [[SHAREDARGS1]]
@@ -40,13 +40,18 @@
 
 /// = In the data sharing wrapper function = ///
 
-// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**){{.*}}{
+// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**) 
[[ATTR1]] {
 // CK1: [[SHAREDARGS2:%.+]] = alloca i8**
 // CK1: store i8** %2, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP3:%.+]] = load i8**, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP4:%.+]] = getelementptr inbounds i8*, i8** [[SHARGSTMP3]]
 // CK1: [[SHARGSTMP5:%.+]] = bitcast i8** [[SHARGSTMP4]] to i32**
 // CK1: [[SHARGSTMP6:%.+]] = load i32*, i32** [[SHARGSTMP5]]
 // CK1: call void @__omp_outlined__({{.*}}, i32* [[SHARGSTMP6]])
 
+/// = Attributes = ///
+
+// CK1-NOT: attributes [[ATTR1]] = { {{.*}}"has-nvptx-shared-depot"{{.*}} }
+// CK1: attributes [[ATTR2]] = { {{.*}}"has-nvptx-shared-depot"{{.*}} }
+
 #endif
Index: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -942,6 +942,8 @@
 llvm::Value *ID = Bld.CreateBitOrPointerCast(WFn, CGM.Int8PtrTy);
 
 if (!CapturedVars.empty()) {
+  // There's somehting to share, add the attribute
+  CGF.CurFn->addFnAttr("has-nvptx-shared-depot");
   // Prepare for parallel region. Indicate the outlined function.
   Address SharedArgs =
   CGF.CreateDefaultAlignTempAlloca(CGF.VoidPtrPtrTy,


Index: cfe/trunk/test/OpenMP/nvptx_data_sharing.cpp
===
--- cfe/trunk/test/OpenMP/nvptx_data_sharing.cpp
+++ cfe/trunk/test/OpenMP/nvptx_data_sharing.cpp
@@ -22,15 +22,15 @@
 
 /// = In the worker function = ///
 
-// CK1: define internal void @__omp_offloading_{{.*}}test_ds{{.*}}worker(){{.*}}{
+// CK1: define internal void @__omp_offloading_{{.*}}test_ds{{.*}}worker() [[ATTR1:#.*]] {
 // CK1: [[SHAREDARGS:%.+]] = alloca i8**
 // CK1: call i1 @__kmpc_kernel_parallel(i8** %work_fn, i8*** [[SHAREDARGS]])
 // CK1: [[SHARGSTMP:%.+]] = load i8**, i8*** [[SHAREDARGS]]
 // CK1: call void @__omp_outlined___wrapper{{.*}}({{.*}}, i8** [[SHARGSTMP]])
 
 /// = In the kernel function = ///
 
-// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}()
+// CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}() [[ATTR2:#.*]] {
 // CK1: [[SHAREDARGS1:%.+]] = alloca i8**
 // CK1: call void @__kmpc_kernel_prepare_parallel({{.*}}, i8*** [[SHAREDARGS1]], i32 1)
 // CK1: [[SHARGSTMP1:%.+]] = load i8**, i8*** [[SHAREDARGS1]]
@@ -40,13 +40,18 @@
 
 /// = In the data sharing wrapper function = ///
 
-// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**){{.*}}{
+// CK1: {{.*}}define internal void @__omp_outlined___wrapper({{.*}}i8**) [[ATTR1]] {
 // CK1: [[SHAREDARGS2:%.+]] = alloca i8**
 // CK1: store i8** %2, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP3:%.+]] = load i8**, i8*** [[SHAREDARGS2]]
 // CK1: [[SHARGSTMP4:%.+]] = getelementptr inbounds i8*, i8** [[SHARGSTMP3]]
 // CK1: [[SHARGSTMP5:%.+]] = bitcast i8** [[SHARGSTMP4]] to i32**
 // CK1: [[SHARGSTMP6:%.+]] = load i32*, i32** [[SHARGSTMP5]]
 // CK1: call void @__omp_outlined__({{.*}}, i32* [[SHARGSTMP6]])
 
+/// = Attributes = ///
+
+// CK1-NOT: attributes [[ATTR1]] = { {{.*}}"has-nvptx

[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320528: [libunwind][MIPS]: Add support for unwinding in O32 
and N64 processes. (authored by jhb, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D38110

Files:
  libunwind/trunk/include/__libunwind_config.h
  libunwind/trunk/include/libunwind.h
  libunwind/trunk/src/Registers.hpp
  libunwind/trunk/src/UnwindCursor.hpp
  libunwind/trunk/src/UnwindRegistersRestore.S
  libunwind/trunk/src/UnwindRegistersSave.S
  libunwind/trunk/src/config.h
  libunwind/trunk/src/libunwind.cpp

Index: libunwind/trunk/src/UnwindRegistersRestore.S
===
--- libunwind/trunk/src/UnwindRegistersRestore.S
+++ libunwind/trunk/src/UnwindRegistersRestore.S
@@ -534,6 +534,118 @@
   l.jr r9
l.nop
 
+#elif defined(__mips__) && defined(_ABIO32) && defined(__mips_soft_float)
+
+//
+// void libunwind::Registers_mips_o32::jumpto()
+//
+// On entry:
+//  thread state pointer is in a0 ($4)
+//
+DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind18Registers_mips_o326jumptoEv)
+  .set push
+  .set noat
+  .set noreorder
+  .set nomacro
+  // restore hi and lo
+  lw$8, (4 * 33)($4)
+  mthi  $8
+  lw$8, (4 * 34)($4)
+  mtlo  $8
+  // r0 is zero
+  lw$1, (4 * 1)($4)
+  lw$2, (4 * 2)($4)
+  lw$3, (4 * 3)($4)
+  // skip a0 for now
+  lw$5, (4 * 5)($4)
+  lw$6, (4 * 6)($4)
+  lw$7, (4 * 7)($4)
+  lw$8, (4 * 8)($4)
+  lw$9, (4 * 9)($4)
+  lw$10, (4 * 10)($4)
+  lw$11, (4 * 11)($4)
+  lw$12, (4 * 12)($4)
+  lw$13, (4 * 13)($4)
+  lw$14, (4 * 14)($4)
+  lw$15, (4 * 15)($4)
+  lw$16, (4 * 16)($4)
+  lw$17, (4 * 17)($4)
+  lw$18, (4 * 18)($4)
+  lw$19, (4 * 19)($4)
+  lw$20, (4 * 20)($4)
+  lw$21, (4 * 21)($4)
+  lw$22, (4 * 22)($4)
+  lw$23, (4 * 23)($4)
+  lw$24, (4 * 24)($4)
+  lw$25, (4 * 25)($4)
+  lw$26, (4 * 26)($4)
+  lw$27, (4 * 27)($4)
+  lw$28, (4 * 28)($4)
+  lw$29, (4 * 29)($4)
+  lw$30, (4 * 30)($4)
+  // load new pc into ra
+  lw$31, (4 * 32)($4)
+  // jump to ra, load a0 in the delay slot
+  jr$31
+  lw$4, (4 * 4)($4)
+  .set pop
+
+#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+
+//
+// void libunwind::Registers_mips_n64::jumpto()
+//
+// On entry:
+//  thread state pointer is in a0 ($4)
+//
+DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind18Registers_mips_n646jumptoEv)
+  .set push
+  .set noat
+  .set noreorder
+  .set nomacro
+  // restore hi and lo
+  ld$8, (8 * 33)($4)
+  mthi  $8
+  ld$8, (8 * 34)($4)
+  mtlo  $8
+  // r0 is zero
+  ld$1, (8 * 1)($4)
+  ld$2, (8 * 2)($4)
+  ld$3, (8 * 3)($4)
+  // skip a0 for now
+  ld$5, (8 * 5)($4)
+  ld$6, (8 * 6)($4)
+  ld$7, (8 * 7)($4)
+  ld$8, (8 * 8)($4)
+  ld$9, (8 * 9)($4)
+  ld$10, (8 * 10)($4)
+  ld$11, (8 * 11)($4)
+  ld$12, (8 * 12)($4)
+  ld$13, (8 * 13)($4)
+  ld$14, (8 * 14)($4)
+  ld$15, (8 * 15)($4)
+  ld$16, (8 * 16)($4)
+  ld$17, (8 * 17)($4)
+  ld$18, (8 * 18)($4)
+  ld$19, (8 * 19)($4)
+  ld$20, (8 * 20)($4)
+  ld$21, (8 * 21)($4)
+  ld$22, (8 * 22)($4)
+  ld$23, (8 * 23)($4)
+  ld$24, (8 * 24)($4)
+  ld$25, (8 * 25)($4)
+  ld$26, (8 * 26)($4)
+  ld$27, (8 * 27)($4)
+  ld$28, (8 * 28)($4)
+  ld$29, (8 * 29)($4)
+  ld$30, (8 * 30)($4)
+  // load new pc into ra
+  ld$31, (8 * 32)($4)
+  // jump to ra, load a0 in the delay slot
+  jr$31
+  ld$4, (8 * 4)($4)
+  .set pop
+
 #endif
 
 #endif /* !defined(__USING_SJLJ_EXCEPTIONS__) */
Index: libunwind/trunk/src/Registers.hpp
===
--- libunwind/trunk/src/Registers.hpp
+++ libunwind/trunk/src/Registers.hpp
@@ -2041,6 +2041,418 @@
 
 }
 #endif // _LIBUNWIND_TARGET_OR1K
+
+#if defined(_LIBUNWIND_TARGET_MIPS_O32)
+/// Registers_mips_o32 holds the register state of a thread in a 32-bit MIPS
+/// process.
+class _LIBUNWIND_HIDDEN Registers_mips_o32 {
+public:
+  Registers_mips_o32();
+  Registers_mips_o32(const void *registers);
+
+  boolvalidRegister(int num) const;
+  uint32_tgetRegister(int num) const;
+  voidsetRegister(int num, uint32_t value);
+  boolvalidFloatRegister(int num) const;
+  double  getFloatRegister(int num) const;
+  voidsetFloatRegister(int num, double value);
+  boolvalidVectorRegister(int num) const;
+  v128getVectorRegister(int num) const;
+  voidsetVectorRegister(int num, v128 value);
+  const char *getRegisterName(int num);
+  voidjumpto();
+  static int  lastDwarfRegNum() { return _LIBUNWIND_HIGHEST_DWARF_REGISTER_MIPS; }
+
+  uint32_t  getSP() const { return _registers.__r[29]; }
+  void  setSP(uint32_t value) { _registers.__r[29] = value; }
+  uint32_t  getIP() const { return _registers.__pc; }
+ 

[libunwind] r320528 - [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

[John Baldwin via cfe-commits]

Author: jhb
Date: Tue Dec 12 13:43:36 2017
New Revision: 320528

URL: http://llvm.org/viewvc/llvm-project?rev=320528&view=rev
Log:
[libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

This supports the soft-float ABI only and has been tested with both clang
and gcc on FreeBSD.

Reviewed By: sdardis, compnerd

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

Modified:
libunwind/trunk/include/__libunwind_config.h
libunwind/trunk/include/libunwind.h
libunwind/trunk/src/Registers.hpp
libunwind/trunk/src/UnwindCursor.hpp
libunwind/trunk/src/UnwindRegistersRestore.S
libunwind/trunk/src/UnwindRegistersSave.S
libunwind/trunk/src/config.h
libunwind/trunk/src/libunwind.cpp

Modified: libunwind/trunk/include/__libunwind_config.h
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/include/__libunwind_config.h?rev=320528&r1=320527&r2=320528&view=diff
==
--- libunwind/trunk/include/__libunwind_config.h (original)
+++ libunwind/trunk/include/__libunwind_config.h Tue Dec 12 13:43:36 2017
@@ -21,6 +21,7 @@
 #define _LIBUNWIND_HIGHEST_DWARF_REGISTER_ARM64 95
 #define _LIBUNWIND_HIGHEST_DWARF_REGISTER_ARM   287
 #define _LIBUNWIND_HIGHEST_DWARF_REGISTER_OR1K  31
+#define _LIBUNWIND_HIGHEST_DWARF_REGISTER_MIPS  65
 
 #if defined(_LIBUNWIND_IS_NATIVE_ONLY)
 # if defined(__i386__)
@@ -63,6 +64,19 @@
 #  define _LIBUNWIND_CONTEXT_SIZE 16
 #  define _LIBUNWIND_CURSOR_SIZE 24
 #  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 
_LIBUNWIND_HIGHEST_DWARF_REGISTER_OR1K
+# elif defined(__mips__)
+#  if defined(_ABIO32) && defined(__mips_soft_float)
+#define _LIBUNWIND_TARGET_MIPS_O32 1
+#define _LIBUNWIND_CONTEXT_SIZE 18
+#define _LIBUNWIND_CURSOR_SIZE 24
+#  elif defined(_ABI64) && defined(__mips_soft_float)
+#define _LIBUNWIND_TARGET_MIPS_N64 1
+#define _LIBUNWIND_CONTEXT_SIZE 35
+#define _LIBUNWIND_CURSOR_SIZE 47
+#  else
+#error "Unsupported MIPS ABI and/or environment"
+#  endif
+#  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 
_LIBUNWIND_HIGHEST_DWARF_REGISTER_MIPS
 # else
 #  error "Unsupported architecture."
 # endif
@@ -73,6 +87,8 @@
 # define _LIBUNWIND_TARGET_AARCH64 1
 # define _LIBUNWIND_TARGET_ARM 1
 # define _LIBUNWIND_TARGET_OR1K 1
+# define _LIBUNWIND_TARGET_MIPS_O32 1
+# define _LIBUNWIND_TARGET_MIPS_N64 1
 # define _LIBUNWIND_CONTEXT_SIZE 128
 # define _LIBUNWIND_CURSOR_SIZE 140
 # define _LIBUNWIND_HIGHEST_DWARF_REGISTER 287

Modified: libunwind/trunk/include/libunwind.h
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/include/libunwind.h?rev=320528&r1=320527&r2=320528&view=diff
==
--- libunwind/trunk/include/libunwind.h (original)
+++ libunwind/trunk/include/libunwind.h Tue Dec 12 13:43:36 2017
@@ -563,4 +563,42 @@ enum {
   UNW_OR1K_R31 = 31,
 };
 
+// MIPS registers
+enum {
+  UNW_MIPS_R0  = 0,
+  UNW_MIPS_R1  = 1,
+  UNW_MIPS_R2  = 2,
+  UNW_MIPS_R3  = 3,
+  UNW_MIPS_R4  = 4,
+  UNW_MIPS_R5  = 5,
+  UNW_MIPS_R6  = 6,
+  UNW_MIPS_R7  = 7,
+  UNW_MIPS_R8  = 8,
+  UNW_MIPS_R9  = 9,
+  UNW_MIPS_R10 = 10,
+  UNW_MIPS_R11 = 11,
+  UNW_MIPS_R12 = 12,
+  UNW_MIPS_R13 = 13,
+  UNW_MIPS_R14 = 14,
+  UNW_MIPS_R15 = 15,
+  UNW_MIPS_R16 = 16,
+  UNW_MIPS_R17 = 17,
+  UNW_MIPS_R18 = 18,
+  UNW_MIPS_R19 = 19,
+  UNW_MIPS_R20 = 20,
+  UNW_MIPS_R21 = 21,
+  UNW_MIPS_R22 = 22,
+  UNW_MIPS_R23 = 23,
+  UNW_MIPS_R24 = 24,
+  UNW_MIPS_R25 = 25,
+  UNW_MIPS_R26 = 26,
+  UNW_MIPS_R27 = 27,
+  UNW_MIPS_R28 = 28,
+  UNW_MIPS_R29 = 29,
+  UNW_MIPS_R30 = 30,
+  UNW_MIPS_R31 = 31,
+  UNW_MIPS_HI = 64,
+  UNW_MIPS_LO = 65,
+};
+
 #endif

Modified: libunwind/trunk/src/Registers.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Registers.hpp?rev=320528&r1=320527&r2=320528&view=diff
==
--- libunwind/trunk/src/Registers.hpp (original)
+++ libunwind/trunk/src/Registers.hpp Tue Dec 12 13:43:36 2017
@@ -2041,6 +2041,418 @@ inline const char *Registers_or1k::getRe
 
 }
 #endif // _LIBUNWIND_TARGET_OR1K
+
+#if defined(_LIBUNWIND_TARGET_MIPS_O32)
+/// Registers_mips_o32 holds the register state of a thread in a 32-bit MIPS
+/// process.
+class _LIBUNWIND_HIDDEN Registers_mips_o32 {
+public:
+  Registers_mips_o32();
+  Registers_mips_o32(const void *registers);
+
+  boolvalidRegister(int num) const;
+  uint32_tgetRegister(int num) const;
+  voidsetRegister(int num, uint32_t value);
+  boolvalidFloatRegister(int num) const;
+  double  getFloatRegister(int num) const;
+  voidsetFloatRegister(int num, double value);
+  boolvalidVectorRegister(int num) const;
+  v128getVectorRegister(int num) const;
+  voidsetVectorRegister(int num, v128 value);
+  const char *getRegisterName(int num);
+  voidjumpto();
+  static int  lastDwarfRegNum() { ret