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

2017-12-13 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman updated this revision to Diff 126856.
arphaman marked an inline comment as done.
arphaman added a comment.

- Use separate functions for checks.
- "ARM" should match "thumb" arch too.


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,51 @@
+// RUN: %clang_cc1 -fsyntax-only -triple thumbv7--windows-msvc19.11.0 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -triple armv7--windows-msvc19.11.0 -DARM -verify %s
+// expected-no-diagnostics
+
+// ARM does match arm and thumb.
+#if !__is_target_arch(arm)
+  #error "mismatching arch"
+#endif
+
+#if __is_target_arch(armeb) || __is_target_arch(armebv7) || __is_target_arch(thumbeb) || __is_target_arch(thumbebv7)
+  #error "mismatching arch"
+#endif
+
+// ARMV7 does match armv7 and thumbv7.
+#if !__is_target_arch(armv7)
+  #error "mismatching arch"
+#endif
+
+// ARMV6 does not match armv7 or thumbv7.
+#if __is_target_arch(armv6)
+  #error "mismatching arch"
+#endif
+
+#if __is_target_arch(arm64)
+  #error "mismatching arch"
+#endif
+
+#ifndef ARM
+
+// 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
+
+#else
+
+#if __is_target_arch(thumbv7) || __is_target_arch(thumb)
+  #error "mismatching arch"
+#endif
+
+#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 w

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

2017-12-13 Thread Steven Wu via Phabricator via cfe-commits
steven_wu accepted this revision.
steven_wu added a comment.

LGTM


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] D40671: [clang-tidy] Support specific checks for NOLINT directive

2017-12-13 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision.
alexfh added a comment.

In https://reviews.llvm.org/D40671#949732, @xgsa wrote:

> In https://reviews.llvm.org/D40671#949687, @alexfh wrote:
>
> > How are unknown check names handled? More specifically: will the `// 
> > NOLINT(runtime/explicit)` comment disable all clang-tidy checks or none?
>
>
> None. If comment is syntactically correct and contains parenthesis, it works 
> only for the known checks inside.
>
> Still, I don't think it worth mentioning all the corner cases in 
> documentation to keep things simple.


Documenting interaction with cpplint-style NOLINT categories would potentially 
save time to the users and clang-tidy maintainers (at least for codebases using 
Google style and cpplint). Fine for a follow-up.

In https://reviews.llvm.org/D40671#953888, @aaron.ballman wrote:

> In https://reviews.llvm.org/D40671#953291, @xgsa wrote:
>
> > @aaron.ballman, sorry for my insistence, but it seems all the comments are 
> > fixed and the patch is ready for commit or am I missing something? Could 
> > you please commit it on my behalf, as I don't have rights to do that?
>
>
> The check now LGTM, but I am going to wait to commit in case @alexfh has 
> concerns regarding unknown check names.
>
> FWIW, I think we should do something about unknown check names in NOLINT 
> comments, but that can be done as a follow-up patch. If we're ignoring the 
> comment, we might want to diagnose that fact so users have an idea what's 
> going on.


IIUC, cpplint can output a diagnostic about unknown categories inside NOLINT 
and about NOLINT directives that happen on lines where no warning is emitted. 
Both would be useful in clang-tidy, IMO.


https://reviews.llvm.org/D40671



___
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/function pointer

2017-12-13 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

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

> Change tests to use non-O2 generated IR. It looks like the combined 
> objc_retainAutoreleasedReturnValue/objc_autoreleaseReturnValue calls 
> annihilate each other and we just get a call/ret.


Is that really happening at -O0?  Maybe you need to add -disable-llvm-optzns to 
the test line if so.


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] D41148: [libcxx] implement declarations based on P0214R7.

2017-12-13 Thread Tim Shen via Phabricator via cfe-commits
timshen updated this revision to Diff 126872.
timshen marked 7 inline comments as done.
timshen added a comment.

Address second round of comments.


https://reviews.llvm.org/D41148

Files:
  libcxx/include/experimental/__config
  libcxx/include/experimental/simd
  libcxx/test/std/experimental/simd/nothing_to_do.pass.cpp
  libcxx/test/std/experimental/simd/simd.casts/simd_cast.pass.cpp
  libcxx/test/std/experimental/simd/simd.casts/static_simd_cast.pass.cpp
  libcxx/test/std/experimental/simd/simd.cons/broadcast.pass.cpp
  libcxx/test/std/experimental/simd/simd.cons/geneartor.pass.cpp
  libcxx/test/std/experimental/simd/simd.traits/abi_for_size.pass.cpp
  libcxx/test/std/experimental/simd/simd.traits/is_abi_tag.pass.cpp
  libcxx/test/std/experimental/simd/simd.traits/is_simd.pass.cpp
  libcxx/test/std/experimental/simd/simd.traits/is_simd_flag_type.pass.cpp
  libcxx/test/std/experimental/simd/simd.traits/is_simd_mask.pass.cpp

Index: libcxx/test/std/experimental/simd/simd.traits/is_simd_mask.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/experimental/simd/simd.traits/is_simd_mask.pass.cpp
@@ -0,0 +1,119 @@
+//===--===//
+//
+// 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
+
+// 
+//
+// [simd.traits]
+// template  struct is_simd_mask;
+// template  inline constexpr bool is_simd_mask_v = is_simd_mask::value;
+
+#include 
+#include 
+
+using namespace std::experimental::parallelism_v2;
+
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+static_assert(is_simd_mask>::value, "");
+
+static_assert(!is_simd_mask::value, "");
+
+#if TEST_STD_VER > 14
+
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_v>, "");
+static_assert(is_simd_mask_

[PATCH] D41148: [libcxx] implement declarations based on P0214R7.

2017-12-13 Thread Tim Shen via Phabricator via cfe-commits
timshen added inline comments.



Comment at: libcxx/include/experimental/simd:594
+
+#warning " is under construction and not for practical use 
for now."
+

EricWF wrote:
> I would just remove this. The `experimental` part of the name should say 
> enough.
> 
> Also we normally wait until an implementation is mostly complete before 
> landing it. 
> Also we normally wait until an implementation is mostly complete before 
> landing it.

What's the definition of "landing"? I prefer to implement this library in 
multiple patches, does that mean I keep working on the next ones and finally 
commit all of them all at once? If that's the case, SGTM.



Comment at: libcxx/test/std/experimental/simd/traits.pass.cpp:15
+// [simd.traits]
+// template  struct is_abi_tag;
+// template  inline constexpr bool is_abi_tag_v = 
is_abi_tag::value;

EricWF wrote:
> Please create a directory called `simd/traits/` and then put each trait in a 
> separate test file under that directory.
> The `foo_v` tests should be in the same file as the `foo` tests.
> 
> 
Thanks! Done for all tests. Used directory names like "simd.cons", 
"simd.traits", "simd.casts".


https://reviews.llvm.org/D41148



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


[PATCH] D41213: [libcxx] [test] Improve MSVC portability.

2017-12-13 Thread Stephan T. Lavavej via Phabricator via cfe-commits
STL_MSFT created this revision.
STL_MSFT added reviewers: EricWF, mclow.lists.
Herald added a subscriber: jfb.

[libcxx] [test] Improve MSVC portability.

test/support/msvc_stdlib_force_include.hpp
When testing MSVC's STL with C1XX, simulate a couple more compiler feature-test 
macros.

When testing MSVC's STL, simulate a few library feature-test macros.

test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp
The vector_size attribute is a non-Standard extension that's supported by Clang 
and GCC,
but not C1XX. Therefore, guard this with `__has_attribute(vector_size)`.

Additionally, while these tests pass when MSVC's STL is compiled with Clang,
I don't consider this to be a supported scenario for our library,
so also guard this with defined(_LIBCPP_VERSION).

test/std/utilities/function.objects/func.not_fn/not_fn.pass.cpp
N4713 23.14.10 [func.not_fn]/1 depicts only `call_wrapper(call_wrapper&&) = 
default;`
and `call_wrapper(const call_wrapper&) = default;`. According to
15.8.2 [class.copy.assign]/2 and /4, this makes call_wrapper non-assignable.
Therefore, guard the assignability tests as libc++ specific.

Add a (void) cast to tolerate not_fn() being marked as nodiscard.


https://reviews.llvm.org/D41213

Files:
  test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp
  test/std/utilities/function.objects/func.not_fn/not_fn.pass.cpp
  test/support/msvc_stdlib_force_include.hpp

Index: test/support/msvc_stdlib_force_include.hpp
===
--- test/support/msvc_stdlib_force_include.hpp
+++ test/support/msvc_stdlib_force_include.hpp
@@ -52,6 +52,13 @@
 #define _MSVC_HAS_FEATURE_memory_sanitizer  0
 #define _MSVC_HAS_FEATURE_thread_sanitizer  0
 
+#define __has_attribute(X) _MSVC_HAS_ATTRIBUTE_ ## X
+#define _MSVC_HAS_ATTRIBUTE_vector_size 0
+
+#ifdef _NOEXCEPT_TYPES_SUPPORTED
+#define __cpp_noexcept_function_type201510
+#endif // _NOEXCEPT_TYPES_SUPPORTED
+
 // Silence compiler warnings.
 #pragma warning(disable: 4180) // qualifier applied to function type has no meaning; ignored
 #pragma warning(disable: 4324) // structure was padded due to alignment specifier
@@ -85,4 +92,12 @@
 #define TEST_STD_VER 14
 #endif // _HAS_CXX17
 
+// Simulate library feature-test macros.
+#define __cpp_lib_invoke 201411
+#define __cpp_lib_void_t 201411
+
+#if _HAS_CXX17
+#define __cpp_lib_atomic_is_always_lock_free 201603
+#endif // _HAS_CXX17
+
 #endif // SUPPORT_MSVC_STDLIB_FORCE_INCLUDE_HPP
Index: test/std/utilities/function.objects/func.not_fn/not_fn.pass.cpp
===
--- test/std/utilities/function.objects/func.not_fn/not_fn.pass.cpp
+++ test/std/utilities/function.objects/func.not_fn/not_fn.pass.cpp
@@ -305,31 +305,35 @@
 using RetT = decltype(std::not_fn(value));
 static_assert(std::is_move_constructible::value, "");
 static_assert(std::is_copy_constructible::value, "");
-static_assert(std::is_move_assignable::value, "");
-static_assert(std::is_copy_assignable::value, "");
+LIBCPP_STATIC_ASSERT(std::is_move_assignable::value, "");
+LIBCPP_STATIC_ASSERT(std::is_copy_assignable::value, "");
 auto ret = std::not_fn(value);
 assert(ret() == false);
 auto ret2 = std::not_fn(value2);
 assert(ret2() == true);
+#if defined(_LIBCPP_VERSION)
 ret = ret2;
 assert(ret() == true);
 assert(ret2() == true);
+#endif // _LIBCPP_VERSION
 }
 {
 using T = MoveAssignableWrapper;
 T value(true);
 T value2(false);
 using RetT = decltype(std::not_fn(std::move(value)));
 static_assert(std::is_move_constructible::value, "");
 static_assert(!std::is_copy_constructible::value, "");
-static_assert(std::is_move_assignable::value, "");
+LIBCPP_STATIC_ASSERT(std::is_move_assignable::value, "");
 static_assert(!std::is_copy_assignable::value, "");
 auto ret = std::not_fn(std::move(value));
 assert(ret() == false);
 auto ret2 = std::not_fn(std::move(value2));
 assert(ret2() == true);
+#if defined(_LIBCPP_VERSION)
 ret = std::move(ret2);
 assert(ret() == true);
+#endif // _LIBCPP_VERSION
 }
 }
 
@@ -426,7 +430,7 @@
 {
 ThrowsOnCopy cp;
 try {
-std::not_fn(cp);
+(void)std::not_fn(cp);
 assert(false);
 } catch (int const& value) {
 assert(value == 42);
Index: test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp
===
--- test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp
+++ test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp
@@ -89,6 +89,7 @@
 CHECK_ALWAYS_LOCK_FREE(float);
 CHECK_ALWAYS_LOCK_FREE(double);
 CHECK_A

Buildbot numbers for the week of 11/26/2017 - 12/02/2017

2017-12-13 Thread Galina Kistanova via cfe-commits
Hello everyone,

Below are some buildbot numbers for the week of 11/26/2017 - 12/02/2017.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:

buildername| was_red
---+-
 clang-cuda-build  | 61:24:16
 libcxx-libcxxabi-x86_64-linux-debian-noexceptions | 41:12:57
 lldb-windows7-android | 32:43:26
 clang-x64-ninja-win7  | 32:06:36
 clang-ppc64be-linux-lnt   | 31:39:03
 clang-ppc64be-linux-multistage| 29:13:55
 clang-ppc64be-linux   | 28:11:58
 clang-with-lto-ubuntu | 28:09:40
 sanitizer-ppc64be-linux   | 27:56:11
 ubuntu-gcc7.1-werror  | 27:07:44
 polly-amd64-linux | 25:26:36
 clang-s390x-linux-multistage  | 23:13:18
 polly-arm-linux   | 21:41:21
 clang-cmake-aarch64-lld   | 20:42:04
 clang-with-thin-lto-ubuntu| 19:25:29
 clang-ppc64le-linux-multistage| 14:20:48
 clang-x86_64-linux-abi-test   | 14:06:45
 clang-s390x-linux | 13:42:18
 sanitizer-x86_64-linux-autoconf   | 13:04:48
 openmp-clang-x86_64-linux-debian  | 12:37:31
 openmp-ompt-clang-x86_64-linux-debian | 12:01:16
 clang-cmake-aarch64-global-isel   | 12:01:07
 openmp-gcc-x86_64-linux-debian| 11:49:14
 sanitizer-x86_64-linux| 11:47:20
 clang-cmake-thumbv7-a15-full-sh   | 10:30:29
 sanitizer-x86_64-linux-android| 10:22:47
 sanitizer-x86_64-linux-bootstrap-msan | 10:20:06
 sanitizer-x86_64-linux-bootstrap-ubsan| 10:18:54
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast  | 10:09:31
 clang-lld-x86_64-2stage   | 10:00:13
 clang-atom-d525-fedora-rel| 09:35:18
 clang-cmake-aarch64-full  | 08:34:44
 sanitizer-x86_64-linux-fast   | 08:26:45
 llvm-mips-linux   | 07:56:36
 llvm-clang-x86_64-expensive-checks-win| 07:40:39
 sanitizer-x86_64-linux-bootstrap  | 07:16:52
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast| 05:29:37
 clang-cmake-x86_64-avx2-linux-perf| 05:20:59
 clang-cmake-x86_64-avx2-linux | 05:19:55
 clang-ppc64le-linux-lnt   | 05:11:45
 clang-cmake-thumbv7-a15   | 05:10:56
 clang-cmake-x86_64-sde-avx512-linux   | 05:04:39
 clang-cmake-aarch64-quick | 04:53:17
 lld-x86_64-win7   | 04:38:20
 clang-native-arm-lnt  | 04:31:43
 clang-s390x-linux-lnt | 04:05:59
 lldb-x86_64-ubuntu-14.04-buildserver  | 04:01:22
 clang-ppc64le-linux   | 04:00:17
 clang-x86_64-linux-selfhost-modules-2 | 03:56:22
 lldb-amd64-ninja-netbsd8  | 03:45:47
 perf-x86_64-penryn-O3-polly-unprofitable  | 03:34:31
 libcxx-libcxxabi-libunwind-arm-linux-noexceptions | 03:26:24
 libcxx-libcxxabi-libunwind-arm-linux  | 03:25:48
 lld-x86_64-darwin13   | 03:24:02
 clang-x86-windows-msvc2015| 02:47:15
 clang-cmake-armv7-a15-full| 02:36:06
 clang-cmake-armv7-a15 | 02:35:01
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx14| 01:57:19
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx11| 01:56:45
 clang-x86_64-linux-selfhost-modules   | 01:47:50
 lldb-x86_64-ubuntu-14.04-android  | 01:38:09
 clang-with-thin-lto-windows   | 01:37:00
 reverse-iteration | 01:30:16
 clang-hexagon-elf | 01:26:50
 clang-x86_64-debian-fast  | 01:26:02
 lldb-x86_64-darwin-13.4   | 01:25:19
 lldb-x86-windows-msvc2015 | 01:16:03
 perf-x86_64-penryn-O3-polly-parallel-fast | 01:08:41
 libcxx-libcxxabi-x86_64-linux-

Buildbot numbers for the last week of 12/3/2017 - 12/9/2017

2017-12-13 Thread Galina Kistanova via cfe-commits
Hello everyone,

Below are some buildbot numbers for the last week of 12/3/2017 - 12/9/2017.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:

  buildername  | was_red
---+-
 clang-s390x-linux-multistage  | 77:40:28
 perf-x86_64-penryn-O3-polly-unprofitable  | 58:02:48
 clang-with-thin-lto-windows   | 57:19:47
 clang-x86_64-linux-selfhost-modules-2 | 56:49:12
 clang-x86_64-linux-selfhost-modules   | 56:06:27
 clang-cmake-armv7-a15-full| 33:41:50
 clang-cmake-aarch64-full  | 33:34:05
 clang-cmake-thumbv7-a15-full-sh   | 32:48:17
 clang-x64-ninja-win7  | 31:58:43
 clang-s390x-linux-lnt | 29:33:04
 clang-s390x-linux | 28:24:40
 sanitizer-x86_64-linux-bootstrap-ubsan| 28:11:01
 clang-ppc64be-linux-multistage| 28:07:20
 sanitizer-x86_64-linux-fast   | 27:37:22
 clang-ppc64le-linux   | 27:21:16
 clang-ppc64be-linux   | 27:14:47
 sanitizer-x86_64-linux-bootstrap-msan | 27:12:49
 clang-ppc64le-linux-multistage| 27:12:01
 sanitizer-x86_64-linux-bootstrap  | 27:07:12
 clang-ppc64le-linux-lnt   | 27:02:06
 clang-ppc64be-linux-lnt   | 26:30:54
 openmp-clang-x86_64-linux-debian  | 26:08:14
 clang-x86_64-debian-fast  | 25:41:36
 lld-x86_64-win7   | 22:48:43
 clang-with-lto-ubuntu | 22:45:24
 clang-cmake-aarch64-lld   | 22:08:52
 clang-with-thin-lto-ubuntu| 21:55:03
 clang-cmake-armv7-a15-selfhost-neon   | 21:54:05
 llvm-clang-x86_64-expensive-checks-win| 21:53:32
 clang-lld-x86_64-2stage   | 21:35:04
 clang-cmake-x86_64-avx2-linux | 21:31:56
 clang-cmake-x86_64-avx2-linux-perf| 21:31:31
 clang-cmake-armv7-a15-selfhost| 21:25:25
 sanitizer-x86_64-linux-fuzzer | 21:22:02
 perf-x86_64-penryn-O3-polly-parallel-fast | 20:53:31
 clang-cmake-x86_64-sde-avx512-linux   | 20:50:15
 sanitizer-windows | 19:57:59
 sanitizer-ppc64le-linux   | 17:03:04
 polly-amd64-linux | 16:23:42
 sanitizer-ppc64be-linux   | 16:20:58
 sanitizer-x86_64-linux| 16:06:23
 libcxx-libcxxabi-libunwind-aarch64-linux-noexceptions | 15:49:16
 polly-arm-linux   | 15:29:28
 clang-cmake-aarch64-quick | 14:25:05
 clang-cmake-thumbv7-a15   | 14:23:17
 clang-cmake-armv7-a15 | 14:21:20
 clang-cmake-aarch64-global-isel   | 14:17:21
 clang-bpf-build   | 12:33:12
 sanitizer-x86_64-linux-android| 10:37:46
 lld-x86_64-darwin13   | 05:54:54
 libcxx-libcxxabi-x86_64-linux-debian-noexceptions | 05:26:25
 reverse-iteration | 05:14:53
 clang-cuda-build  | 04:47:30
 clang-atom-d525-fedora-rel| 04:22:35
 lldb-windows7-android | 04:15:37
 ubuntu-gcc7.1-werror  | 03:57:16
 libcxx-libcxxabi-x86_64-linux-debian  | 03:53:19
 lldb-amd64-ninja-netbsd8  | 03:10:33
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast| 02:53:35
 lldb-x86_64-ubuntu-14.04-buildserver  | 02:41:53
 lldb-x86_64-ubuntu-14.04-cmake| 02:31:01
 lld-x86_64-freebsd| 02:17:59
 lldb-x86-windows-msvc2015 | 02:17:19
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast  | 01:57:35
 openmp-clan

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

2017-12-13 Thread Dan Zimmerman via Phabricator via cfe-commits
danzimm added a comment.

In https://reviews.llvm.org/D41050#954668, @rjmccall wrote:

> In https://reviews.llvm.org/D41050#953917, @danzimm wrote:
>
> > Change tests to use non-O2 generated IR. It looks like the combined 
> > objc_retainAutoreleasedReturnValue/objc_autoreleaseReturnValue calls 
> > annihilate each other and we just get a call/ret.
>
>
> Is that really happening at -O0?  Maybe you need to add -disable-llvm-optzns 
> to the test line if so.


Unfortunately it looks like adding `-disable-llvm-optzns` and/or 
`-disable-llvm-passes` (I also manually added `-O0` since I don't know what 
clang defaults to) doesn't generate the explicit call to 
`objc_retainAutoreleasedReturnValue`. Should we add a flag to disable that 
merging?


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] D41217: [Concepts] Concept Specialization Expressions

2017-12-13 Thread Saar Raz via Phabricator via cfe-commits
saar.raz created this revision.
saar.raz added reviewers: changyu, rsmith, hubert.reinterpretcast, nwilson.
Herald added a subscriber: cfe-commits.

Part of the P0734r0 Concepts implementation effort. Added Concept 
Specialization Expressions and tests thereof. Also added constraint expression 
type verification. This diff depends on https://reviews.llvm.org/D40381.


Repository:
  rC Clang

https://reviews.llvm.org/D41217

Files:
  include/clang/AST/DeclTemplate.h
  include/clang/AST/ExprCXX.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/StmtNodes.td
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/Expr.cpp
  lib/AST/ExprCXX.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseTemplate.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
  test/Parser/cxx-concept-declaration.cpp
  tools/libclang/CXCursor.cpp

Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -231,6 +231,7 @@
   case Stmt::TypeTraitExprClass:
   case Stmt::CoroutineBodyStmtClass:
   case Stmt::CoawaitExprClass:
+  case Stmt::ConceptSpecializationExprClass:
   case Stmt::DependentCoawaitExprClass:
   case Stmt::CoreturnStmtClass:
   case Stmt::CoyieldExprClass:
Index: test/Parser/cxx-concept-declaration.cpp
===
--- test/Parser/cxx-concept-declaration.cpp
+++ test/Parser/cxx-concept-declaration.cpp
@@ -9,8 +9,6 @@
 
 template concept D1 = true; // expected-error {{expected template parameter}}
 
-template concept C2 = 0.f; // expected-error {{constraint expression must be 'bool'}}
-
 struct S1 {
   template concept C1 = true; // expected-error {{concept declarations may only appear in global or namespace scope}}
 };
@@ -29,3 +27,12 @@
 
 // TODO: Add test to prevent explicit specialization, partial specialization
 // and explicit instantiation of concepts.
+
+template concept C7 = 2; // expected-error {{atomic constraint '2' must be of type 'bool' (found 'int')}}
+template concept C8 = 2 && x; // expected-error {{atomic constraint '2' must be of type 'bool' (found 'int')}}
+template concept C9 = x || 2 || x; // expected-error {{atomic constraint '2' must be of type 'bool' (found 'int')}}
+template concept C10 = 8ull && x || x; // expected-error {{atomic constraint '8ULL' must be of type 'bool' (found 'unsigned long long')}}
+template concept C11 = sizeof(T); // expected-error {{atomic constraint 'sizeof(T)' must be of type 'bool' (found 'unsigned long')}}
+template concept C12 = T{};
+template concept C13 = (bool&&)true;
+template concept C14 = (const bool&)true;
Index: test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
===
--- test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
+++ test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
@@ -1,5 +1,52 @@
 // RUN:  %clang_cc1 -std=c++2a -fconcepts-ts -verify %s
-// expected-no-diagnostics
 
-template concept C = true;
-static_assert(C);
+template concept C1 = true;
+static_assert(C1);
+
+template concept C2 = sizeof(T) == 4;
+static_assert(C2);
+static_assert(!C2);
+static_assert(C2);
+static_assert(!C2);
+
+template concept C3 = sizeof(*T{}) == 4;
+static_assert(C3);
+static_assert(!C3);
+
+struct A {
+static constexpr int add(int a, int b) {
+return a + b;
+}
+};
+struct B {
+static int add(int a, int b) {
+return a + b;
+}
+};
+template
+concept C4 = U::add(1, 2) == 3;
+static_assert(C4);
+static_assert(!C4); // expected-error {{concept specialization 'C4' resulted in a non-constant expression 'B::add(1, 2) == 3'}}
+
+template
+constexpr bool is_same_v = false;
+
+template
+constexpr bool is_same_v = true;
+
+template
+concept Same = is_same_v;
+
+static_assert(Same);
+static_assert(Same);
+static_assert(!Same);
+static_assert(!Same);
+static_assert(Same);
+
+static_assert(Same)>);
+static_assert(Same)>);
+static_assert(Same)>);
+static_assert(Same)>);
+
+template concept C5 = T{}; // expected-error {{atomic constraint 'int{}' must be of type 'bool' (found 'int')}}
+constexpr bool x = C5; // expected-note {{in concept specialization 'C5'}}
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1018,6 +1018,7 @@
 case Stmt::CUDAKernelCallExprClass:
 case Stmt:

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

2017-12-13 Thread 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; }
+

K-ballo wrote:
> Quuxplusone wrote:
> > mclow.lists wrote:
> > > lichray wrote:
> > > > Oh, common, I persuaded the committee to allow you to print a `(null)`  
> > > > and you don't do it...
> > > I think that `(null)` is a better thing to output here than `0x0`.
> > Are you two implying that
> > 
> > *this << (const void *)0;
> > 
> > does *not* print `(null)`? It certainly should, IMO. (I mean, it'll call 
> > `num_put` for `void*` in the current locale, but I would naturally expect 
> > that to print `(null)`.)
> > Anyway, whether the current locale prints null as `(null)` or not, there is 
> > great potential utility in using the same format for all pointers, as 
> > K-ballo is doing here. I'd really like to be able to
> > 
> > std::ostringstream oss;
> > oss << nullptr;  // equivalently: oss << (void*)0;
> > void *p;
> > std::istringstream(oss.str()) >> p;  // should read in a null pointer, 
> > not derp out
> > 
> > FWIW, it looks like libc++ currently *does* print null pointers as `(nil)`, 
> > which is not quite the same thing as `(null)`. libstdc++ prints them as `0`.
> > https://wandbox.org/permlink/yAM6tjMzvEX9HhhE
> It's been a while now, but I seem to recall that the reason we settled on 
> `(*this) << (const void*)0` was precisely so that it would go through 
> `num_put`, as the rest of the pointers do. As I read back on the 
> specification, however, I am no longer sure that such an implementation is 
> conforming. Maybe if we were to call the facet directly...
> 
> I am no longer interested in this issue. If anyone wants to take over control 
> I would be happy to yield it; otherwise, I'll implement whatever I'm 
> instructed to.
Your test code prints 0x0 0x0 on FreeBSD though.
If reliable round-tripping is the goal, maybe we can enforce printing `(nil)`?  
fmtlib does this as well:

https://github.com/fmtlib/fmt/commit/b5fda1c90d94aeaf96081477c9bfe13789c00e03


https://reviews.llvm.org/D33776



___
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/function pointer

2017-12-13 Thread Dan Zimmerman via Phabricator via cfe-commits
danzimm added a comment.

I just dug into how the ARC optimization pass is invoked... it really shouldn't 
be invoked if `-disable-llvm-passes` is passed (or `-disable-llvm-optzns` which 
appears to just alias the first). Can you verify that my command is sane:

  /Users/danzimm/oss/build/bin/clang -cc1 -internal-isystem 
/Users/danzimm/oss/build/lib/clang/6.0.0/include -nostdsysteminc -triple 
x86_64-apple-macosx10.12.0 -disable-llvm-passes -emit-llvm -fblocks -fobjc-arc 
-std=c++11 -O0 -o - 
/Users/danzimm/oss/llvm/tools/clang/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm


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/function pointer

2017-12-13 Thread Dan Zimmerman via Phabricator via cfe-commits
danzimm updated this revision to Diff 126892.
danzimm added a comment.

Pass -disable-llvm-passes with -O3 so that we can test that the IR we generate 
actually is generated


Repository:
  rC Clang

https://reviews.llvm.org/D41050

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenObjCXX/arc-forwarded-lambda-call.mm


Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm 
-disable-llvm-passes -O3 -fblocks -fobjc-arc -fobjc-runtime-has-weak -std=c++11 
-o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: [[T0:%.*]] = call i8* 
@"_ZZ5test0P11objc_objectENK3$_0clEv"({{%.*}}* {{%.*}})
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* 
[[T0]]) [[A0:#[0-9]+]]
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* 
[[T1]]) [[A0]]
+  // CHECK-NEXT: ret i8* [[T2]]
+}
+
+id test1_rv;
+
+void test1() {
+  extern void test1_helper(id (*)(void));
+  test1_helper([](){ return test1_rv; });
+  // CHECK-LABEL: define internal i8* @"_ZZ5test1vEN3$_18__invokeEv"
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test1vENK3$_1clEv"({{%.*}}* undef)
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* 
[[T0]]) [[A0]]
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* 
[[T1]]) [[A0]]
+  // CHECK-NEXT: ret i8* [[T2]]
+}
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2776,9 +2776,12 @@
   RValue RV = EmitCall(calleeFnInfo, callee, returnSlot, callArgs);
 
   // If necessary, copy the returned value into the slot.
-  if (!resultType->isVoidType() && returnSlot.isNull())
+  if (!resultType->isVoidType() && returnSlot.isNull()) {
+if (getLangOpts().ObjCAutoRefCount && resultType->isObjCRetainableType()) {
+  RV = 
RValue::get(EmitARCRetainAutoreleasedReturnValue(RV.getScalarVal()));
+}
 EmitReturnOfRValue(RV, resultType);
-  else
+  } else
 EmitBranchThroughCleanup(ReturnBlock);
 }
 


Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm -disable-llvm-passes -O3 -fblocks -fobjc-arc -fobjc-runtime-has-weak -std=c++11 -o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test0P11objc_objectENK3$_0clEv"({{%.*}}* {{%.*}})
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* [[T0]]) [[A0:#[0-9]+]]
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* [[T1]]) [[A0]]
+  // CHECK-NEXT: ret i8* [[T2]]
+}
+
+id test1_rv;
+
+void test1() {
+  extern void test1_helper(id (*)(void));
+  test1_helper([](){ return test1_rv; });
+  // CHECK-LABEL: define internal i8* @"_ZZ5test1vEN3$_18__invokeEv"
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test1vENK3$_1clEv"({{%.*}}* undef)
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* [[T0]]) [[A0]]
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* [[T1]]) [[A0]]
+  // CHECK-NEXT: ret i8* [[T2]]
+}
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2776,9 +2776,12 @@
   RValue RV = EmitCall(calleeFnInfo, callee, returnSlot, callArgs);
 
   // If necessary, copy the returned value into the slot.
-  if (!resultType->isVoidType() && returnSlot.isNull())
+  if (!resultType->isVoidType() && returnSlot.isNull()) {
+if (getLangOpts().ObjCAutoRefCount && resultType->isObjCRetainableType()) {
+  RV = RValue::get(EmitARCRetainAutoreleasedReturnValue(RV.getScalarVal()));
+}
 EmitReturnOfRValue(RV, resultType);
-  else
+  } else
 EmitBranchThroughCleanup(ReturnBlock);
 }
 
___
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/function pointer

2017-12-13 Thread Dan Zimmerman via Phabricator via cfe-commits
danzimm updated this revision to Diff 126893.
danzimm added a comment.

Remove unnecessary checks from tests (sorry for unbatched updates)


Repository:
  rC Clang

https://reviews.llvm.org/D41050

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenObjCXX/arc-forwarded-lambda-call.mm


Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm 
-disable-llvm-passes -O3 -fblocks -fobjc-arc -fobjc-runtime-has-weak -std=c++11 
-o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test0P11objc_objectENK3$_0clEv"
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* 
[[T0]])
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* 
[[T1]])
+  // CHECK-NEXT: ret i8* [[T2]]
+}
+
+id test1_rv;
+
+void test1() {
+  extern void test1_helper(id (*)(void));
+  test1_helper([](){ return test1_rv; });
+  // CHECK-LABEL: define internal i8* @"_ZZ5test1vEN3$_18__invokeEv"
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test1vENK3$_1clEv"
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* 
[[T0]])
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* 
[[T1]])
+  // CHECK-NEXT: ret i8* [[T2]]
+}
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2776,9 +2776,12 @@
   RValue RV = EmitCall(calleeFnInfo, callee, returnSlot, callArgs);
 
   // If necessary, copy the returned value into the slot.
-  if (!resultType->isVoidType() && returnSlot.isNull())
+  if (!resultType->isVoidType() && returnSlot.isNull()) {
+if (getLangOpts().ObjCAutoRefCount && resultType->isObjCRetainableType()) {
+  RV = 
RValue::get(EmitARCRetainAutoreleasedReturnValue(RV.getScalarVal()));
+}
 EmitReturnOfRValue(RV, resultType);
-  else
+  } else
 EmitBranchThroughCleanup(ReturnBlock);
 }
 


Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm -disable-llvm-passes -O3 -fblocks -fobjc-arc -fobjc-runtime-has-weak -std=c++11 -o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test0P11objc_objectENK3$_0clEv"
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* [[T0]])
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* [[T1]])
+  // CHECK-NEXT: ret i8* [[T2]]
+}
+
+id test1_rv;
+
+void test1() {
+  extern void test1_helper(id (*)(void));
+  test1_helper([](){ return test1_rv; });
+  // CHECK-LABEL: define internal i8* @"_ZZ5test1vEN3$_18__invokeEv"
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test1vENK3$_1clEv"
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* [[T0]])
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* [[T1]])
+  // CHECK-NEXT: ret i8* [[T2]]
+}
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2776,9 +2776,12 @@
   RValue RV = EmitCall(calleeFnInfo, callee, returnSlot, callArgs);
 
   // If necessary, copy the returned value into the slot.
-  if (!resultType->isVoidType() && returnSlot.isNull())
+  if (!resultType->isVoidType() && returnSlot.isNull()) {
+if (getLangOpts().ObjCAutoRefCount && resultType->isObjCRetainableType()) {
+  RV = RValue::get(EmitARCRetainAutoreleasedReturnValue(RV.getScalarVal()));
+}
 EmitReturnOfRValue(RV, resultType);
-  else
+  } else
 EmitBranchThroughCleanup(ReturnBlock);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D41189: [Frontend] Treat function with skipped body as definition

2017-12-13 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff accepted this revision.
sepavloff added a comment.
This revision is now accepted and ready to land.

LGTM.

Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D41189



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


[PATCH] D40809: [WIP] [analyzer] Dump counterexample traces as C programs

2017-12-13 Thread Devin Coughlin via Phabricator via cfe-commits
dcoughlin added a comment.

This is seems like a very useful visualization, *especially* for loops. Can we 
this patch get it into a state where it can be committed and used for debugging 
purposes?

One possibility is to turn this into a debug checker similar to 
debug.ViewExplodedGraph. That checker registers for a checkEndAnalysis() 
callback and traverses the node graph (see DebugCheckers.cpp). Can you do the 
same here? It doesn't look like you really need this to be a BugReporterVisitor 
-- and making it a debug checker would avoid outputting multiple copies for 
each diagnostic consumer.

You could also control file output with an analyzer-config option that takes an 
optional path.


https://reviews.llvm.org/D40809



___
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

2017-12-13 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision.
george.karpenkov added a comment.
This revision now requires changes to proceed.

I've tried using the patch, and I got blocked at the following: CTU options are 
only exposed when one goes through `analyze-build` frontend, which requires 
`compile_commands.json` to be present. I've used `libear` to generate 
`compile_commands.json`, but the generated JSON does not contain the `command` 
field, which causes `@require` before `run` to die (also, due to the passing 
style this error was unnecessarily difficult to debug).
So could you write a short documentation somewhere how all pieces fit together? 
What entry point should be used, what should people do who don't have a build 
system-generated `compile_commands.json`etc. etc.


https://reviews.llvm.org/D30691



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


[PATCH] D40809: [WIP] [analyzer] Dump counterexample traces as C programs

2017-12-13 Thread Alexander Shaposhnikov via Phabricator via cfe-commits
alexshap added inline comments.



Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1976
+unsigned FileEndOffset = SM.getFileOffset(SM.getLocForEndOfFile(FID));
+for (unsigned i=Offset; BufferStart[i] != '\n' && i < FileEndOffset; ++i)
+  Ostream << BufferStart[i];

spaces around '=' (maybe clang-format this diff ?)


https://reviews.llvm.org/D40809



___
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/function pointer

2017-12-13 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Heh, alright, that works.  It's unfortunate that -disable-llvm-passes doesn't 
suppress running the pass under -O0; that seems like an oversight.

Anyway, LGTM.


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] D41217: [Concepts] Concept Specialization Expressions

2017-12-13 Thread changyu via Phabricator via cfe-commits
changyu added inline comments.



Comment at: lib/AST/ExprCXX.cpp:1481
+if (E.isInvalid() || Trap.hasErrorOccurred()) {
+// C++2a [temp.constr.atomic]p1
+//   ...If substitution results in an invalid type or expression, the

You have four spaces of indenting here.



Comment at: lib/Parse/ParseExpr.cpp:225
+  if (Res.isUsable() && !Actions.CheckConstraintExpression(Res.get())) {
+  return ExprError();
+  }

Indenting.


Repository:
  rC Clang

https://reviews.llvm.org/D41217



___
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/function pointer

2017-12-13 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

In https://reviews.llvm.org/D41050#954863, @rjmccall wrote:

> Heh, alright, that works.  It's unfortunate that -disable-llvm-passes doesn't 
> suppress running the pass under -O0; that seems like an oversight.
>
> Anyway, LGTM.


I suspect `-O0` just generates different IR.  You can confirm what passes are 
executed by adding `-mllvm -debug-pass=Executions`.


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] D40841: [analyzer] Fix a crash on C++17 AST for non-trivial construction into a trivial brace initializer.

2017-12-13 Thread Devin Coughlin via Phabricator via cfe-commits
dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

This looks good to me for a quick fix that we plan to address in a more 
principled fashion later.

However, I'd like you to add a comment at each of the places where you use the 
parent map to note something like  "This is a quick dirty fix and we really 
shouldn't be using the parent map to model behavior." I understand why you're 
doing it, but I'd like to make sure that someone reading this code doesn't 
think that it is a good pattern to use the parent map and decide to use it 
elsewhere!

Using the parent map is slow and is a sign that our reasoning isn't 
compositional. The analyzer should be able to figure out the correct behavior 
based on the expression and its state/context alone. The fact that we need the 
parent map here is an indication that we don't have the right representation 
for constructors in the analyzer. (I know you're planning on tackling this in 
the future!)


https://reviews.llvm.org/D40841



___
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/function pointer

2017-12-13 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Oh, yes, that could be.


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] D41102: Setup clang-doc frontend framework

2017-12-13 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

I am happy now. But I don't have any authority to allow this patch to land 
whatsoever. Who will be the code owner for `clang-doc`? I think the tooling 
guys need to accept.




Comment at: tools/clang-doc/ClangDoc.cpp:54
+
+  // TODO: Move set attached to the initial comment parsing, not here
+  if (Comment) {

Full sentence. 
`set attached` == `setAttached`?
Removing the not here and using the method name is probably enough already.



Comment at: tools/clang-doc/tool/ClangDocMain.cpp:42
+
+  doc::OutFormat EmitFormat;
+  EmitLLVM ? EmitFormat = clang::doc::OutFormat::LLVM

The two lines could be merged when initializing `EmitFormat` directly.



Comment at: tools/clang-doc/tool/ClangDocMain.cpp:47
+  // TODO: Update the source path list to only consider changed files for
+  // incremental doc updates
+  doc::ClangDocReporter Reporter(OptionsParser.getSourcePathList());

Missing full stop. Comments are supposed to be full sentences by convention.


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] D41223: [libc++] Fix PR35491 - std::array of zero-size doesn't work with non-default constructible types.

2017-12-13 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF created this revision.
EricWF added a reviewer: mclow.lists.

This patch fixes llvm.org/PR35491

The fix attempts to maintain ABI compatibility by replacing the array with a 
instance of `aligned_storage`.


https://reviews.llvm.org/D41223

Files:
  include/array
  test/std/containers/sequences/array/array.cons/default.pass.cpp


Index: test/std/containers/sequences/array/array.cons/default.pass.cpp
===
--- test/std/containers/sequences/array/array.cons/default.pass.cpp
+++ test/std/containers/sequences/array/array.cons/default.pass.cpp
@@ -14,6 +14,10 @@
 #include 
 #include 
 
+struct NoDefault {
+  NoDefault(int) {}
+};
+
 int main()
 {
 {
@@ -28,4 +32,9 @@
 C c;
 assert(c.size() == 0);
 }
+  {
+typedef std::array C;
+C c;
+assert(c.size() == 0);
+  }
 }
Index: include/array
===
--- include/array
+++ include/array
@@ -134,7 +134,12 @@
 typedef std::reverse_iterator   reverse_iterator;
 typedef std::reverse_iterator const_reverse_iterator;
 
-value_type __elems_[_Size > 0 ? _Size : 1];
+typedef typename conditional<_Size == 0,
+typename aligned_storage::value>::type,
+value_type[_Size == 0 ? 1 : _Size]
+  >::type _StorageType;
+
+_StorageType __elems_;
 
 // No explicit construct/copy/destroy for aggregate type
 _LIBCPP_INLINE_VISIBILITY void fill(const value_type& __u)


Index: test/std/containers/sequences/array/array.cons/default.pass.cpp
===
--- test/std/containers/sequences/array/array.cons/default.pass.cpp
+++ test/std/containers/sequences/array/array.cons/default.pass.cpp
@@ -14,6 +14,10 @@
 #include 
 #include 
 
+struct NoDefault {
+  NoDefault(int) {}
+};
+
 int main()
 {
 {
@@ -28,4 +32,9 @@
 C c;
 assert(c.size() == 0);
 }
+  {
+typedef std::array C;
+C c;
+assert(c.size() == 0);
+  }
 }
Index: include/array
===
--- include/array
+++ include/array
@@ -134,7 +134,12 @@
 typedef std::reverse_iterator   reverse_iterator;
 typedef std::reverse_iterator const_reverse_iterator;
 
-value_type __elems_[_Size > 0 ? _Size : 1];
+typedef typename conditional<_Size == 0,
+typename aligned_storage::value>::type,
+value_type[_Size == 0 ? 1 : _Size]
+  >::type _StorageType;
+
+_StorageType __elems_;
 
 // No explicit construct/copy/destroy for aggregate type
 _LIBCPP_INLINE_VISIBILITY void fill(const value_type& __u)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D40809: [WIP] [analyzer] Dump counterexample traces as C programs

2017-12-13 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

In https://reviews.llvm.org/D40809#954858, @dcoughlin wrote:

> One possibility is to turn this into a debug checker similar to 
> debug.ViewExplodedGraph. That checker registers for a checkEndAnalysis() 
> callback and traverses the node graph (see DebugCheckers.cpp). Can you do the 
> same here? It doesn't look like you really need this to be a 
> BugReporterVisitor -- and making it a debug checker would avoid outputting 
> multiple copies for each diagnostic consumer.


These prints are only for actual bugs, not for the whole graph


https://reviews.llvm.org/D40809



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


[PATCH] D41223: [libc++] Fix PR35491 - std::array of zero-size doesn't work with non-default constructible types.

2017-12-13 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF updated this revision to Diff 126898.
EricWF added a comment.

Fix begin/end/data.
===


https://reviews.llvm.org/D41223

Files:
  include/array
  test/std/containers/sequences/array/array.cons/default.pass.cpp
  test/std/containers/sequences/array/array.data/data.pass.cpp
  test/std/containers/sequences/array/array.data/data_const.pass.cpp
  test/std/containers/sequences/array/begin.pass.cpp

Index: test/std/containers/sequences/array/begin.pass.cpp
===
--- test/std/containers/sequences/array/begin.pass.cpp
+++ test/std/containers/sequences/array/begin.pass.cpp
@@ -31,4 +31,13 @@
 *i = 5.5;
 assert(c[0] == 5.5);
 }
+{
+  struct NoDefault {
+NoDefault(int) {}
+  };
+  typedef NoDefault T;
+  typedef std::array C;
+  C c = {};
+  assert(c.begin() == c.end());
+}
 }
Index: test/std/containers/sequences/array/array.data/data_const.pass.cpp
===
--- test/std/containers/sequences/array/array.data/data_const.pass.cpp
+++ test/std/containers/sequences/array/array.data/data_const.pass.cpp
@@ -38,6 +38,16 @@
 const T* p = c.data();
 (void)p; // to placate scan-build
 }
+{
+  struct NoDefault {
+NoDefault(int) {}
+  };
+  typedef NoDefault T;
+  typedef std::array C;
+  const C c = {};
+  const T* p = c.data();
+  assert(p != nullptr);
+}
 #if TEST_STD_VER > 14
 {
 typedef std::array C;
Index: test/std/containers/sequences/array/array.data/data.pass.cpp
===
--- test/std/containers/sequences/array/array.data/data.pass.cpp
+++ test/std/containers/sequences/array/array.data/data.pass.cpp
@@ -36,4 +36,14 @@
 T* p = c.data();
 (void)p; // to placate scan-build
 }
+{
+  struct NoDefault {
+NoDefault(int) {}
+  };
+  typedef NoDefault T;
+  typedef std::array C;
+  C c = {};
+  T* p = c.data();
+  assert(p != nullptr);
+}
 }
Index: test/std/containers/sequences/array/array.cons/default.pass.cpp
===
--- test/std/containers/sequences/array/array.cons/default.pass.cpp
+++ test/std/containers/sequences/array/array.cons/default.pass.cpp
@@ -14,6 +14,10 @@
 #include 
 #include 
 
+struct NoDefault {
+  NoDefault(int) {}
+};
+
 int main()
 {
 {
@@ -28,4 +32,9 @@
 C c;
 assert(c.size() == 0);
 }
+{
+  typedef std::array C;
+  C c;
+  assert(c.size() == 0);
+}
 }
Index: include/array
===
--- include/array
+++ include/array
@@ -117,6 +117,55 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
+template 
+struct __array_traits {
+  typedef _Tp _StorageT[_Size];
+
+  _LIBCPP_INLINE_VISIBILITY
+  static _LIBCPP_CONSTEXPR_AFTER_CXX14 _Tp* __data(_StorageT& __store) {
+return __store;
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  static _LIBCPP_CONSTEXPR_AFTER_CXX14 _Tp const* __data(const _StorageT& __store) {
+return __store;
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  static void __swap(_StorageT& __lhs, _StorageT& __rhs) {
+std::swap_ranges(__lhs, __lhs + _Size, __rhs);
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  static void __fill(_StorageT& __arr, _Tp const& __val) {
+_VSTD::fill_n(__arr, _Size, __val);
+  }
+};
+
+template 
+struct __array_traits<_Tp, 0> {
+  typedef typename aligned_storage::value>::type _StorageT;
+
+  _LIBCPP_INLINE_VISIBILITY
+  static _Tp* __data(_StorageT& __store) {
+_StorageT *__ptr = std::addressof(__store);
+return reinterpret_cast<_Tp*>(__ptr);
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  static const _Tp* __data(const _StorageT& __store) {
+const _StorageT *__ptr = std::addressof(__store);
+return reinterpret_cast(__ptr);
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  static void __swap(_StorageT&, _StorageT&) {}
+
+  _LIBCPP_INLINE_VISIBILITY
+  static void __fill(_StorageT&, _Tp const&) {
+  }
+};
+
 template 
 struct _LIBCPP_TEMPLATE_VIS array
 {
@@ -134,31 +183,26 @@
 typedef std::reverse_iterator   reverse_iterator;
 typedef std::reverse_iterator const_reverse_iterator;
 
-value_type __elems_[_Size > 0 ? _Size : 1];
+typedef __array_traits<_Tp, _Size> _Traits;
+typename _Traits::_StorageT __elems_;
 
 // No explicit construct/copy/destroy for aggregate type
 _LIBCPP_INLINE_VISIBILITY void fill(const value_type& __u)
-{_VSTD::fill_n(__elems_, _Size, __u);}
-_LIBCPP_INLINE_VISIBILITY
-void swap(array& __a) _NOEXCEPT_(_Size == 0 || __is_nothrow_swappable<_Tp>::value)
-{ __swap_dispatch((std::integral_constant()), __a); }
+{_Traits::__fill(__elems_, __u);}
 
 _LIBCPP_INLINE_VISIBILITY
-void __swap_dispatch(std::true_type, array&) {}
-
-_LIBCPP_INLINE_VISIBILITY
-

[PATCH] D41195: [ClangFormat] IndentWrappedFunctionNames should be true in the google ObjC style

2017-12-13 Thread Daniel Jasper via Phabricator via cfe-commits
djasper accepted this revision.
djasper added inline comments.
This revision is now accepted and ready to land.



Comment at: unittests/Format/FormatTestObjC.cpp:388
+  // Wrapped method parameters should be indented.
+  verifyFormat("- (VeryLongReturnTypeName)\n"
+   "veryLongMethodParameter:(VeryLongParameterName)"

Set Style.ColumnLimit to something lower so that you don't have to wrap single 
lines (for better test readability).


Repository:
  rC Clang

https://reviews.llvm.org/D41195



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


[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

2017-12-13 Thread Anton via Phabricator via cfe-commits
xgsa added a comment.

In https://reviews.llvm.org/D40671#954661, @alexfh wrote:

> In https://reviews.llvm.org/D40671#953888, @aaron.ballman wrote:
>
> > FWIW, I think we should do something about unknown check names in NOLINT 
> > comments, but that can be done as a follow-up patch. If we're ignoring the 
> > comment, we might want to diagnose that fact so users have an idea what's 
> > going on.
>
>
> IIUC, cpplint can output a diagnostic about unknown categories inside NOLINT 
> and about NOLINT directives that happen on lines where no warning is emitted. 
> Both would be useful in clang-tidy, IMO.


I agree with your statements and I think there should be the following 
diagnostics about NOLINT usage:

- as you described, using of NOLINT with unknown check names;
- using of NOLINT for the line, on which there is no diagnostics (at all with 
NOLINT and for the swpecified diagnostics); this should help to detect dangling 
NOLINT comments, that have no meaning anymore.

Moreover, there should be a way to turn on/off these diagnostics, so possibily 
they should be a separate checks. What do you think? Is there a way for a check 
to collect the emitted diagnostics?


https://reviews.llvm.org/D40671



___
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

2017-12-13 Thread Raoul Wols via Phabricator via cfe-commits
rwols closed this revision.
rwols added a comment.

https://reviews.llvm.org/D41118 fixes both the start positions as well as the 
ranges, let's merge that.


https://reviews.llvm.org/D40860



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


[clang-tools-extra] r320554 - [clangd] Overload hash_value for SymbolID, fix struct/class warning

2017-12-13 Thread Sam McCall via cfe-commits
Author: sammccall
Date: Wed Dec 13 00:34:48 2017
New Revision: 320554

URL: http://llvm.org/viewvc/llvm-project?rev=320554&view=rev
Log:
[clangd] Overload hash_value for SymbolID, fix struct/class warning

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

Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=320554&r1=320553&r2=320554&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Wed Dec 13 00:34:48 2017
@@ -12,6 +12,7 @@
 
 #include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringExtras.h"
 
 #include 
@@ -49,7 +50,9 @@ public:
   }
 
 private:
-  friend class llvm::DenseMapInfo;
+  friend llvm::hash_code hash_value(const SymbolID &ID) {
+return hash_value(ArrayRef(ID.HashValue));
+  }
 
   std::array HashValue;
 };
@@ -122,8 +125,7 @@ template <> struct DenseMapInfo(Sym.HashValue.data(), Sym.HashValue.size()));
+return hash_value(Sym);
   }
   static bool isEqual(const clang::clangd::SymbolID &LHS,
   const clang::clangd::SymbolID &RHS) {


___
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

2017-12-13 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

You can erase some namespace prefix in code, e.g. llvm or clang, because you 
are working in them and/or had `using namespace clang`. Imo this is not 
required only if you find it ok to shorten the code.




Comment at: tools/clang-doc/ClangDoc.cpp:30
+bool ClangDocVisitor::VisitNamedDecl(const NamedDecl *D) {
+  SourceManager &Manager = Context->getSourceManager();
+  if (!IsNewComment(D->getLocation(), Manager))

Here manager can be const& if it is on the other places too



Comment at: tools/clang-doc/ClangDoc.cpp:50
+void ClangDocVisitor::ParseUnattachedComments() {
+  SourceManager &Manager = Context->getSourceManager();
+  for (RawComment *Comment : Context->getRawCommentList().getComments()) {

I think Manager can be const&. Looks like only read methods were called.



Comment at: tools/clang-doc/ClangDoc.cpp:61
+bool ClangDocVisitor::IsNewComment(SourceLocation Loc,
+   SourceManager &Manager) const {
+  if (!Loc.isValid())

Manager could be const&.



Comment at: tools/clang-doc/ClangDoc.h:38
+
+  virtual bool VisitNamedDecl(const NamedDecl *D);
+

Not sure if this method should be virtual. `RecursiveASTVisitor` uses the crtp 
to not need virtual methods but still behaving the same.



Comment at: tools/clang-doc/ClangDocReporter.cpp:155
+  CurrentCI->SelfClosing = C->isSelfClosing();
+  if (C->getNumAttrs() != 0) {
+for (unsigned i = 0, e = C->getNumAttrs(); i != e; ++i) {

The condition for this if might trigger unexpected behaviour if `getNumAttrs` 
returns negative values. To be safe you could use > 0 instead != 0



Comment at: tools/clang-doc/ClangDocReporter.cpp:170
+  CurrentCI->Name = getCommandName(C->getCommandID());
+  for (unsigned i = 0, e = C->getNumArgs(); i != e; ++i)
+CurrentCI->Args.push_back(C->getArgText(i));

Similar here. If e is negative you will execute the loop until I wraps around. 
Not sure if this is a real issue, depending on the postcondition of getNumArgs.



Comment at: tools/clang-doc/ClangDocReporter.cpp:188
+  if (C->isPositionValid()) {
+for (unsigned i = 0, e = C->getDepth(); i != e; ++i)
+  CurrentCI->Position.push_back(C->getIndex(i));

Similar thing.



Comment at: tools/clang-doc/ClangDocReporter.cpp:228
+  for (comments::Comment *Child :
+   llvm::make_range(C->child_begin(), C->child_end())) {
+CommentInfo ChildCI;

Extract range into utility method of `Comment`



Comment at: tools/clang-doc/ClangDocReporter.cpp:249
+const char *ClangDocReporter::getCommandName(unsigned CommandID) {
+  ;
+  const CommandInfo *Info = CommandTraits::getBuiltinCommandInfo(CommandID);

Empty Statement



Comment at: tools/clang-doc/ClangDocReporter.h:51
+  llvm::SmallVector Position;
+  std::vector Children;
+};

Here a short `children()` method return llvm::make_range shortens the code in a 
later loop and might benefit in the future for iterations over children.


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] D41118: [clangd] Emit ranges for clangd diagnostics, and fix off-by-one positions

2017-12-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.

LGTM


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


[clang-tools-extra] r320555 - [clangd] Emit ranges for clangd diagnostics, and fix off-by-one positions

2017-12-13 Thread Sam McCall via cfe-commits
Author: sammccall
Date: Wed Dec 13 00:48:42 2017
New Revision: 320555

URL: http://llvm.org/viewvc/llvm-project?rev=320555&view=rev
Log:
[clangd] Emit ranges for clangd diagnostics, and fix off-by-one positions

Summary:
 - 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 D40860, and borrows heavily from it

Reviewers: rwols, hokein

Subscribers: klimek, ilya-biryukov, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/test/clangd/diagnostics.test
clang-tools-extra/trunk/test/clangd/execute-command.test
clang-tools-extra/trunk/test/clangd/extra-flags.test
clang-tools-extra/trunk/test/clangd/fixits.test

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=320555&r1=320554&r2=320555&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Dec 13 00:48:42 2017
@@ -202,9 +202,7 @@ void ClangdLSPServer::onCodeAction(Ctx C
   std::string Code = Server.getDocument(Params.textDocument.uri.file);
   json::ary Commands;
   for (Diagnostic &D : Params.context.diagnostics) {
-std::vector Fixes =
-getFixIts(Params.textDocument.uri.file, D);
-auto Edits = replacementsToEdits(Code, Fixes);
+auto Edits = getFixIts(Params.textDocument.uri.file, D);
 if (!Edits.empty()) {
   WorkspaceEdit WE;
   WE.changes = {{Params.textDocument.uri.uri, std::move(Edits)}};
@@ -306,8 +304,8 @@ bool ClangdLSPServer::run(std::istream &
   return ShutdownRequestReceived;
 }
 
-std::vector
-ClangdLSPServer::getFixIts(StringRef File, const clangd::Diagnostic &D) {
+std::vector ClangdLSPServer::getFixIts(StringRef File,
+ const clangd::Diagnostic &D) {
   std::lock_guard Lock(FixItsMutex);
   auto DiagToFixItsIter = FixItsMap.find(File);
   if (DiagToFixItsIter == FixItsMap.end())

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=320555&r1=320554&r2=320555&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Wed Dec 13 00:48:42 2017
@@ -74,8 +74,7 @@ private:
   void onCommand(Ctx C, ExecuteCommandParams &Params) override;
   void onRename(Ctx C, RenameParams &Parames) override;
 
-  std::vector
-  getFixIts(StringRef File, const clangd::Diagnostic &D);
+  std::vector getFixIts(StringRef File, const clangd::Diagnostic &D);
 
   JSONOutput &Out;
   /// Used to indicate that the 'shutdown' request was received from the
@@ -88,7 +87,7 @@ private:
   bool IsDone = false;
 
   std::mutex FixItsMutex;
-  typedef std::map>
+  typedef std::map>
   DiagnosticToReplacementMap;
   /// Caches FixIts per file and diagnostics
   llvm::StringMap FixItsMap;

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=320555&r1=320554&r2=320555&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Wed Dec 13 00:48:42 2017
@@ -120,39 +120,100 @@ static int getSeverity(DiagnosticsEngine
   llvm_unreachable("Unknown diagnostic level!");
 }
 
-llvm::Optional toClangdDiag(const StoredDiagnostic &D) {
-  auto Location = D.getLocation();
-  if (!Location.isValid() || !Location.getManager().isInMainFile(Location))
-return llvm::None;
+// Checks whether a location is within a half-open range.
+// Note that clang also uses closed source ranges, which this can't handle!
+bool locationInRange(SourceLocation L, CharSourceRange R,
+ const SourceManager &M) {
+  assert(R.isCharRange());
+  if (!R.isValid() || M.getFileID(R.getBegin()) != M.getFileID(R.getEnd()) ||
+  M.getFileID(R.getBegin()) != M.getFileID(L))
+return false;
+  return L != R.getEnd() && M.isPointWithin(L, R.getBegin(), R.getEnd());
+}
+
+// Converts a half-open clang source range to a

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

2017-12-13 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE320555: [clangd] Emit ranges for clangd diagnostics, and 
fix off-by-one positions (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41118?vs=126563&id=126679#toc

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: clangd/ClangdLSPServer.h
===
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -74,8 +74,7 @@
   void onCommand(Ctx C, ExecuteCommandParams &Params) override;
   void onRename(Ctx C, RenameParams &Parames) override;
 
-  std::vector
-  getFixIts(StringRef File, const clangd::Diagnostic &D);
+  std::vector getFixIts(StringRef File, const clangd::Diagnostic &D);
 
   JSONOutput &Out;
   /// Used to indicate that the 'shutdown' request was received from the
@@ -88,7 +87,7 @@
   bool IsDone = false;
 
   std::mutex FixItsMutex;
-  typedef std::map>
+  typedef std::map>
   DiagnosticToReplacementMap;
   /// Caches FixIts per file and diagnostics
   llvm::StringMap FixItsMap;
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -120,39 +120,100 @@
   llvm_unreachable("Unknown diagnostic level!");
 }
 
-llvm::Optional toClangdDiag(const StoredDiagnostic &D) {
-  auto Location = D.getLocation();
-  if (!Location.isValid() || !Location.getManager().isInMainFile(Location))
+// Checks whether a location is within a half-open range.
+// Note that clang also uses closed source ranges, which this can't handle!
+bool locationInRange(SourceLocation L, CharSourceRange R,
+ const SourceManager &M) {
+  assert(R.isCharRange());
+  if (!R.isValid() || M.getFileID(R.getBegin()) != M.getFileID(R.getEnd()) ||
+  M.getFileID(R.getBegin()) != M.getFileID(L))
+return false;
+  return L != R.getEnd() && M.isPointWithin(L, R.getBegin(), R.getEnd());
+}
+
+// Converts a half-open clang source range to an LSP range.
+// Note that clang also uses closed source ranges, which this can't handle!
+Range toRange(CharSourceRange R, const SourceManager &M) {
+  // Clang is 1-based, LSP uses 0-based indexes.
+  return {{static_cast(M.getSpellingLineNumber(R.getBegin())) - 1,
+   static_cast(M.getSpellingColumnNumber(R.getBegin())) - 1},
+  {static_cast(M.getSpellingLineNumber(R.getEnd())) - 1,
+   static_cast(M.getSpellingColumnNumber(R.getEnd())) - 1}};
+}
+
+// Clang diags have a location (shown as ^) and 0 or more ranges ().
+// LSP needs a single range.
+Range diagnosticRange(const clang::Diagnostic &D, const LangOptions &L) {
+  auto &M = D.getSourceManager();
+  auto Loc = M.getFileLoc(D.getLocation());
+  // Accept the first range that contains the location.
+  for (const auto &CR : D.getRanges()) {
+auto R = Lexer::makeFileCharRange(CR, M, L);
+if (locationInRange(Loc, R, M))
+  return toRange(R, M);
+  }
+  // The range may be given as a fixit hint instead.
+  for (const auto &F : D.getFixItHints()) {
+auto R = Lexer::makeFileCharRange(F.RemoveRange, M, L);
+if (locationInRange(Loc, R, M))
+  return toRange(R, M);
+  }
+  // If no suitable range is found, just use the token at the location.
+  auto R = Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Loc), M, L);
+  if (!R.isValid()) // Fall back to location only, let the editor deal with it.
+R = CharSourceRange::getCharRange(Loc);
+  return toRange(R, M);
+}
+
+TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
+const LangOptions &L) {
+  TextEdit Result;
+  Result.range = toRange(Lexer::makeFileCharRange(FixIt.RemoveRange, M, L), M);
+  Result.newText = FixIt.CodeToInsert;
+  return Result;
+}
+
+llvm::Optional toClangdDiag(const clang::Diagnostic &D,
+DiagnosticsEngine::Level Level,
+const LangOptions &LangOpts) {
+  if (!D.hasSourceManager() || !D.getLocation().isValid() ||
+  !D.getSourceManager().isInMainFile(D.getLocation()))
 return llvm::None;
 
-  Position P;
-  P.line = Location.getSpellingLineNumber() - 1;
-  P.character = Location.getSpellingColumnNumber();
-  Range R = {P, P};
-  clangd::Diagnostic Diag = {R, getSeverity(D.getLevel()), D.getMessage()};
-
-  llvm::SmallVector FixItsForDiagnostic;
-  for (const FixItHint &Fix : D.getFixIts()) {
-FixItsForDiagnostic.push_back(clang::tooling::Replacement(
-Location.getManager(), Fix.RemoveRange, Fix.CodeToInsert));
-  }
-  return DiagWithFixIts{Diag, std::move(FixItsForDiagnostic)};
+  DiagWithFixIts Result;
+  Resul

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

2017-12-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall 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.

malaperle wrote:
> 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 

[PATCH] D41145: git-clang-format: refactor to support upcoming --staged flag

2017-12-13 Thread Manuel Klimek via Phabricator via cfe-commits
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

lg




Comment at: 
google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format:252
+  assert len(commits) != 0
+  if len(commits) >= 2:
+return Revision(commits[0]), Revision(commits[1]), files

Don't we want to die if len(commits) > 2?


https://reviews.llvm.org/D41145



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


[PATCH] D41147: git-clang-format: Add new --staged option.

2017-12-13 Thread Manuel Klimek via Phabricator via cfe-commits
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

lg


https://reviews.llvm.org/D41147



___
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

2017-12-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 126680.
ilya-biryukov added a comment.

- Make Context the first parameter in rename and a few helper methods.


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()
+  .second.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;
+  .second.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 = FooC

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

2017-12-13 Thread Manuel Klimek via Phabricator via cfe-commits
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

lg


https://reviews.llvm.org/D41130



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

2017-12-13 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



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

sammccall wrote:
> ioeric wrote:
> > warning: class 'DenseMapInfo' was previously declared as a struct 
> > [-Wmismatched-tags]
> Fixed in rCTE320554
Thanks for the fix!


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] D36892: [clang-tidy] check_clang_tidy.py: support CHECK-NOTES prefix

2017-12-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 126681.
lebedev.ri changed the repository for this revision from rL LLVM to rCTE Clang 
Tools Extra.
lebedev.ri added a comment.

Rebased.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D36892

Files:
  test/clang-tidy/check_clang_tidy.py


Index: test/clang-tidy/check_clang_tidy.py
===
--- test/clang-tidy/check_clang_tidy.py
+++ test/clang-tidy/check_clang_tidy.py
@@ -80,9 +80,11 @@
 
   has_check_fixes = input_text.find('CHECK-FIXES') >= 0
   has_check_messages = input_text.find('CHECK-MESSAGES') >= 0
+  has_check_notes = input_text.find('CHECK-NOTES') >= 0
 
-  if not has_check_fixes and not has_check_messages:
-sys.exit('Neither CHECK-FIXES nor CHECK-MESSAGES found in the input')
+  if not has_check_fixes and not has_check_messages and not has_check_notes:
+sys.exit('CHECK-FIXES, CHECK-MESSAGES, or CHECK-NOTES not found in the '
+ 'input')
 
   # Remove the contents of the CHECK lines to avoid CHECKs matching on
   # themselves.  We need to keep the comments to preserve line numbers while
@@ -143,5 +145,18 @@
   print('FileCheck failed:\n' + e.output.decode())
   raise
 
+  if has_check_notes:
+messages_file = temp_file_name + '.msg'
+write_file(messages_file, clang_tidy_output)
+try:
+  subprocess.check_output(
+  ['FileCheck', '-input-file=' + messages_file, input_file_name,
+   '-check-prefix=CHECK-NOTES',
+   '-implicit-check-not={{note|warning|error}}:'],
+  stderr=subprocess.STDOUT)
+except subprocess.CalledProcessError as e:
+  print('FileCheck failed:\n' + e.output.decode())
+  raise
+
 if __name__ == '__main__':
   main()


Index: test/clang-tidy/check_clang_tidy.py
===
--- test/clang-tidy/check_clang_tidy.py
+++ test/clang-tidy/check_clang_tidy.py
@@ -80,9 +80,11 @@
 
   has_check_fixes = input_text.find('CHECK-FIXES') >= 0
   has_check_messages = input_text.find('CHECK-MESSAGES') >= 0
+  has_check_notes = input_text.find('CHECK-NOTES') >= 0
 
-  if not has_check_fixes and not has_check_messages:
-sys.exit('Neither CHECK-FIXES nor CHECK-MESSAGES found in the input')
+  if not has_check_fixes and not has_check_messages and not has_check_notes:
+sys.exit('CHECK-FIXES, CHECK-MESSAGES, or CHECK-NOTES not found in the '
+ 'input')
 
   # Remove the contents of the CHECK lines to avoid CHECKs matching on
   # themselves.  We need to keep the comments to preserve line numbers while
@@ -143,5 +145,18 @@
   print('FileCheck failed:\n' + e.output.decode())
   raise
 
+  if has_check_notes:
+messages_file = temp_file_name + '.msg'
+write_file(messages_file, clang_tidy_output)
+try:
+  subprocess.check_output(
+  ['FileCheck', '-input-file=' + messages_file, input_file_name,
+   '-check-prefix=CHECK-NOTES',
+   '-implicit-check-not={{note|warning|error}}:'],
+  stderr=subprocess.STDOUT)
+except subprocess.CalledProcessError as e:
+  print('FileCheck failed:\n' + e.output.decode())
+  raise
+
 if __name__ == '__main__':
   main()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D36836: [clang-tidy] Implement sonarsource-function-cognitive-complexity check

2017-12-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 126682.
lebedev.ri added a comment.

Rebased.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D36836

Files:
  LICENSE.TXT
  clang-tidy/CMakeLists.txt
  clang-tidy/plugin/CMakeLists.txt
  clang-tidy/sonarsource/CMakeLists.txt
  clang-tidy/sonarsource/FunctionCognitiveComplexityCheck.cpp
  clang-tidy/sonarsource/FunctionCognitiveComplexityCheck.h
  clang-tidy/sonarsource/LICENSE.TXT
  clang-tidy/sonarsource/SONARSOURCETidyModule.cpp
  clang-tidy/tool/CMakeLists.txt
  clang-tidy/tool/ClangTidyMain.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/sonarsource-function-cognitive-complexity.rst
  test/clang-tidy/sonarsource-function-cognitive-complexity.cpp

Index: test/clang-tidy/sonarsource-function-cognitive-complexity.cpp
===
--- /dev/null
+++ test/clang-tidy/sonarsource-function-cognitive-complexity.cpp
@@ -0,0 +1,954 @@
+// RUN: %check_clang_tidy %s sonarsource-function-cognitive-complexity %t -- -config='{CheckOptions: [{key: sonarsource-function-cognitive-complexity.Threshold, value: 0}]}' -- -std=c++11 -w
+
+// any function should be checked.
+
+extern int ext_func(int x = 0);
+
+int some_func(int x = 0);
+
+static int some_other_func(int x = 0) {}
+
+template void some_templ_func(T x = 0) {}
+
+class SomeClass {
+public:
+  int *begin(int x = 0);
+  int *end(int x = 0);
+  static int func(int x = 0);
+  template void some_templ_func(T x = 0) {}
+  SomeClass() = default;
+  SomeClass(SomeClass&) = delete;
+};
+
+// nothing ever decreases cognitive complexity, so we can check all the things
+// in one go. none of the following should increase cognitive complexity:
+void unittest_false() {
+  {};
+  ext_func();
+  some_func();
+  some_other_func();
+  some_templ_func();
+  some_templ_func();
+  SomeClass::func();
+  SomeClass C;
+  C.some_templ_func();
+  C.some_templ_func();
+  C.func();
+  C.end();
+  int i = some_func();
+  i = i;
+  i++;
+  --i;
+  i < 0;
+  int j = 0 ?: 1;
+  auto k = new int;
+  delete k;
+  throw i;
+  {
+throw i;
+  }
+end:
+  return;
+}
+
+#if 1
+#define CC100
+#else
+// this macro has cognitive complexity of 100.
+// it is needed to be able to compare the testcases with the
+// reference Sonar implementation. please place it right after the first
+// CHECK-NOTES in each function
+#define CC100 if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){}if(1){}
+#endif
+
+////
+//-- B1. Increments --//
+////
+// Check that every thing listed in B1 of the specification does indeed   //
+// recieve the base increment, and that not-body does not increase nesting//
+////
+
+// break does not increase cognitive complexity.
+// only  break LABEL  does, but it is unavaliable in C or C++
+
+// continue does not increase cognitive complexity.
+// only  continue LABEL  does, but it is unavaliable in C or C++
+
+void unittest_b1_00() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_00' has cognitive complexity of 33 (threshold 0) [sonarsource-function-cognitive-complexity]
+  CC100;
+
+  if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:9: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+
+if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:11: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+} else if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:12: note: +1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:18: note: +3, including nesting penalty of 2, nesting level increased to 3{{$}}
+} else {
+// CHECK-NOTES: :[[@LINE-1]]:7: note: +1, nesting level increased to 2{{$}}
+}
+  } else if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:10: note: +1, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:16: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+
+if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:11: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+} else if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:12: note: +1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:18: note: +3, including nesting penalty of 2, nesting level increased to 3{{$}}
+} else {
+// CHECK-NOTES: :[[@LINE-1]]:7: note: +1, nesting level

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

2017-12-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 126683.
ilya-biryukov added a comment.

These should be the last usages where Context is not a first paramter.

- Make Context the first parameter in findDefinitions and CppFile::rebuild.
- Make Context the first parameter in invokeCodeCompletion.


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()
+  .second.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;
+  .second.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());
+  /*

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

2017-12-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks! Just style and comment nits left really.




Comment at: clangd/ClangdServer.cpp:216
 
-std::future>
-ClangdServer::codeComplete(PathRef File, Position Pos,
+std::future, Context>>
+ClangdServer::codeComplete(Context Ctx, PathRef File, Position Pos,

this is a pretty gross return type.
Even after (if?) we replace tagged with context, we'll still have 
completionlist and context.
Should we define a struct here?

(If not, we should consider having the context first, for consistency)



Comment at: clangd/ClangdServer.cpp:558
   [this, FileStr, Version,
-   Tag](UniqueFunction>()>
+   Tag](UniqueFunction>(
+const Context &)>

This wrapping has gone beyond the point of readability for me.
Pull out using `RebuildAction = llvm::Optional<...>(const Context &)`?



Comment at: clangd/ClangdServer.h:252
+  ///
+  /// The callers are responsible for making sure \p Ctx stays alive until
+  /// std::future<> is ready (i.e. wait() or get() returns)

Is this comment obsolete? (and others below)
Ctx is passed by value so I don't understand what it could mean.



Comment at: clangd/ClangdServer.h:262
   /// when codeComplete results become available.
-  void codeComplete(UniqueFunction)> Callback,
-PathRef File, Position Pos,
-const clangd::CodeCompleteOptions &Opts,
-llvm::Optional OverridenContents = llvm::None,
-IntrusiveRefCntPtr *UsedFS = nullptr);
+  /// The callers are responsible for making sure \p Ctx stays alive until \p
+  /// Callback is executed.

(and this comment)



Comment at: clangd/ClangdServer.h:305
+ llvm::StringRef NewName,
+ const Context &Ctx);
 

ctx to first param



Comment at: clangd/ClangdServer.h:321
 private:
-  std::future
-  scheduleReparseAndDiags(PathRef File, VersionedDraft Contents,
-  std::shared_ptr Resources,
-  Tagged> 
TaggedFS);
+  std::future scheduleReparseAndDiags(
+  PathRef File, VersionedDraft Contents, std::shared_ptr 
Resources,

ctx to first param



Comment at: clangd/ClangdServer.h:325
 
-  std::future scheduleCancelRebuild(std::shared_ptr Resources);
+  std::future scheduleCancelRebuild(std::shared_ptr 
Resources,
+ Context Ctx);

ctx to first param



Comment at: clangd/ClangdUnit.cpp:555
+std::shared_ptr PCHs) {
+  return std::shared_ptr(new CppFile(
+  FileName, std::move(Command), StorePreamblesInMemory, std::move(PCHs)));

(while here, this should really be `make_shared`)



Comment at: clangd/ClangdUnit.h:175
   llvm::Optional>
-  rebuild(StringRef NewContents, IntrusiveRefCntPtr VFS);
+  rebuild(StringRef NewContents, IntrusiveRefCntPtr VFS,
+  const Context &Ctx);

ctx to first param



Comment at: clangd/ClangdUnit.h:262
 /// Get definition of symbol at a specified \p Pos.
 std::vector findDefinitions(ParsedAST &AST, Position Pos,
+  const Context &Ctx);

ctx to first param



Comment at: clangd/CodeComplete.cpp:594
 std::shared_ptr PCHs,
-Logger &Logger) {
+const Context &Ctx) {
   std::vector ArgStrs;

ctx to first param



Comment at: clangd/Function.h:143
+template  struct ScopeExitGuard {
+  static_assert(std::is_same::type, Func>::value,
+"Func must be decayed");

FWIW I think this assert adds more noise than value - this can clearly only be 
triggered by someone instantiating the template directly, which is probably 
better avoided using `namespace detail` or so.



Comment at: clangd/Function.h:157
+
+  ScopeExitGuard(ScopeExitGuard &&Other)
+  : F(std::move(Other.F)), Moved(Other.Moved) {

I'm struggling to think of cases where moving these guards is the right 
solution.
Can we delete the move ops until we need them? It seems to make this class 
almost trivial.



Comment at: clangd/GlobalCompilationDatabase.cpp:107
 if (ReturnValue == nullptr)
-  Logger.log("Failed to find compilation database for " + Twine(File) +
- "in overriden directory " + CompileCommandsDir.getValue());
+  log(Context::empty(), "Failed to find compilation database for " +
+Twine(File) + "in overriden directory " +

[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.

2017-12-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 126686.
ioeric added a comment.

Merged with origin/master and moved index-related files to index/ subdirectory.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40548

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  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/index/FileIndex.cpp
  clangd/index/FileIndex.h
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  clangd/tool/ClangdMain.cpp

Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -90,6 +90,15 @@
 "Trace internal events and timestamps in chrome://tracing JSON format"),
 llvm::cl::init(""), llvm::cl::Hidden);
 
+static llvm::cl::opt EnableIndexBasedCompletion(
+"enable-index-based-completion",
+llvm::cl::desc(
+"Enable index-based global code completion (experimental). Clangd will "
+"use symbols built from ASTs of opened files and additional indexes "
+"(e.g. offline built codebase-wide symbol table) to complete partial "
+"symbols."),
+llvm::cl::init(false));
+
 int main(int argc, char *argv[]) {
   llvm::cl::ParseCommandLineOptions(argc, argv, "clangd");
 
@@ -170,9 +179,15 @@
   clangd::CodeCompleteOptions CCOpts;
   CCOpts.EnableSnippets = EnableSnippets;
   CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
+  if (EnableIndexBasedCompletion) {
+// Disable sema code completion for qualified code completion and use global
+// symbol index instead.
+CCOpts.IncludeNamespaceLevelDecls = false;
+  }
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(Out, WorkerThreadsCount, StorePreamblesInMemory,
-CCOpts, ResourceDirRef, CompileCommandsDirPath);
+CCOpts, ResourceDirRef, CompileCommandsDirPath,
+EnableIndexBasedCompletion);
   constexpr int NoShutdownRequestErrorCode = 1;
   llvm::set_thread_name("clangd.main");
   return LSPServer.run(std::cin) ? 0 : NoShutdownRequestErrorCode;
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -24,6 +24,12 @@
 public:
   SymbolCollector() = default;
 
+  void initialize(ASTContext &Ctx) override { ASTCtx = &Ctx; }
+
+  void setPreprocessor(std::shared_ptr PP) override {
+this->PP = std::move(PP);
+  }
+
   bool
   handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
   ArrayRef Relations, FileID FID,
@@ -37,6 +43,9 @@
 private:
   // All Symbols collected from the AST.
   SymbolSlab Symbols;
+
+  ASTContext *ASTCtx;
+  std::shared_ptr PP;
 };
 
 } // namespace clangd
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -8,8 +8,8 @@
 //===--===//
 
 #include "SymbolCollector.h"
-
 #include "clang/AST/ASTContext.h"
+#include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/Basic/SourceManager.h"
@@ -57,6 +57,107 @@
   }
   return AbsolutePath.str();
 }
+
+std::string getDocumentation(const CodeCompletionString &CCS) {
+  // Things like __attribute__((nonnull(1,3))) and [[noreturn]]. Present this
+  // information in the documentation field.
+  std::string Result;
+  const unsigned AnnotationCount = CCS.getAnnotationCount();
+  if (AnnotationCount > 0) {
+Result += "Annotation";
+if (AnnotationCount == 1) {
+  Result += ": ";
+} else /* AnnotationCount > 1 */ {
+  Result += "s: ";
+}
+for (unsigned I = 0; I < AnnotationCount; ++I) {
+  Result += CCS.getAnnotation(I);
+  Result.push_back(I == AnnotationCount - 1 ? '\n' : ' ');
+}
+  }
+  // Add brief documentation (if there is any).
+  if (CCS.getBriefComment() != nullptr) {
+if (!Result.empty()) {
+  // This means we previously added annotations. Add an extra newline
+  // character to make the annotations stand out.
+  Result.push_back('\n');
+}
+Result += CCS.getBriefComment();
+  }
+  return Result;
+}
+
+void ProcessChunks(const CodeCompletionString &CCS, Symbol *Sym) {
+  for (const auto &Chunk : CCS) {
+// Informative qualifier chunks only clutter completion results, skip
+// them.
+if (Chunk.Kind == CodeCompletionString::CK_Informative &&
+StringRef(Chunk.Text).endswith("::"))
+  continue;
+
+switch (Chunk.Kind) {
+case CodeCompletionString

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

2017-12-13 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: test/clangd/diagnostics.test:10
 
 
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.c","languageId":"c","version":1,"text":"void
 main() {}"}}}
 #  CHECK:  "method": "textDocument/publishDiagnostics",

Maybe the current tests are enough. Do we want to add a test where the 
diagnostic location is at the end of the line?

```
f()
// ^ missing ";"
f();
```


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] D40903: [Sanitizers] Basic Solaris sanitizer support (PR 33274)

2017-12-13 Thread Rainer Orth via Phabricator via cfe-commits
ro added inline comments.



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:684
   if (AddExportDynamic)
-CmdArgs.push_back("-export-dynamic");
+CmdArgs.push_back("--export-dynamic");
 

alekseyshl wrote:
> If it does not exist on Solaris, why change it then? For the consistency sake?
Consistency for one.  Besides, I'm talking to the Solaris linker
engineers whether to just accept and ignore that option
for GNU ld compatibility.


Repository:
  rC Clang

https://reviews.llvm.org/D40903



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


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

2017-12-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.

I'll LGTM this to unblock @ioeric . We'll also get a test using the flag in 
clangd soon. @arphaman, let us know if you feel there's something wrong with 
the patch.


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] D40712: [Driver] Add flag enabling the function stack size section that was added in r319430

2017-12-13 Thread Sean Eveson via Phabricator via cfe-commits
seaneveson marked an inline comment as done.
seaneveson added a comment.

In https://reviews.llvm.org/D40712#949979, @MatzeB wrote:

> I also wonder whether it would be possible to move the fact that it defaults 
> to on for PS4 into the PS4CPU.cpp file somehow (not that things are 
> distributed that cleanly right now anyway).


I see what you're saying, but I can't see an easy way to do that right now, and 
there are already a number of target dependent defaults done in the same way.


https://reviews.llvm.org/D40712



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


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

2017-12-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 126694.
ioeric added a comment.

- Merged with origin/master


Repository:
  rC Clang

https://reviews.llvm.org/D40562

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Sema/CodeCompleteConsumer.h
  include/clang/Sema/CodeCompleteOptions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/ignore-ns-level-decls.cpp

Index: test/CodeCompletion/ignore-ns-level-decls.cpp
===
--- /dev/null
+++ test/CodeCompletion/ignore-ns-level-decls.cpp
@@ -0,0 +1,21 @@
+namespace ns {
+  struct bar {
+  };
+
+  struct baz {
+  };
+
+  int func(int a, bar b, baz c);
+}
+
+void test() {
+  ns::
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:7 %s -o - | FileCheck %s --check-prefix=CHECK-1
+// CHECK-1-DAG: COMPLETION: bar : bar
+// CHECK-1-DAG: COMPLETION: baz : baz
+// CHECK-1-DAG: COMPLETION: func : [#int#]func(<#int a#>, <#bar b#>, <#baz c#>)
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:7 -no-code-completion-ns-level-decls %s -o - | FileCheck %s --check-prefix=CHECK-EMPTY
+// CHECK-EMPTY-NOT: COMPLETION: bar : bar
+// CHECK-EMPTY: {{^}}{{$}}
+}
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4647,10 +4647,13 @@
 MaybeAddOverrideCalls(*this, Ctx, Results);
   Results.ExitScope();
 
-  CodeCompletionDeclConsumer Consumer(Results, CurContext);
-  LookupVisibleDecls(Ctx, LookupOrdinaryName, Consumer,
- /*IncludeGlobalScope=*/true,
- /*IncludeDependentBases=*/true);
+  if (CodeCompleter->includeNamespaceLevelDecls() ||
+  (!Ctx->isNamespace() && !Ctx->isTranslationUnit())) {
+CodeCompletionDeclConsumer Consumer(Results, CurContext);
+LookupVisibleDecls(Ctx, LookupOrdinaryName, Consumer,
+   /*IncludeGlobalScope=*/true,
+   /*IncludeDependentBases=*/true);
+  }
 
   auto CC = Results.getCompletionContext();
   CC.setCXXScopeSpecifier(SS);
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1380,6 +1380,8 @@
 = Args.hasArg(OPT_code_completion_patterns);
   Opts.CodeCompleteOpts.IncludeGlobals
 = !Args.hasArg(OPT_no_code_completion_globals);
+  Opts.CodeCompleteOpts.IncludeNamespaceLevelDecls
+= !Args.hasArg(OPT_no_code_completion_ns_level_decls);
   Opts.CodeCompleteOpts.IncludeBriefComments
 = Args.hasArg(OPT_code_completion_brief_comments);
 
Index: include/clang/Sema/CodeCompleteOptions.h
===
--- include/clang/Sema/CodeCompleteOptions.h
+++ include/clang/Sema/CodeCompleteOptions.h
@@ -24,15 +24,20 @@
   /// Show top-level decls in code completion results.
   unsigned IncludeGlobals : 1;
 
+  /// Show decls in namespace (including the global namespace) in code
+  /// completion results. If this is 0, `IncludeGlobals` will be ignored.
+  ///
+  /// Currently, this only works when completing qualified IDs (i.e.
+  /// `Sema::CodeCompleteQualifiedId`).
+  /// FIXME: consider supporting more completion cases with this option.
+  unsigned IncludeNamespaceLevelDecls : 1;
+
   /// Show brief documentation comments in code completion results.
   unsigned IncludeBriefComments : 1;
 
-  CodeCompleteOptions() :
-  IncludeMacros(0),
-  IncludeCodePatterns(0),
-  IncludeGlobals(1),
-  IncludeBriefComments(0)
-  { }
+  CodeCompleteOptions()
+  : IncludeMacros(0), IncludeCodePatterns(0), IncludeGlobals(1),
+IncludeNamespaceLevelDecls(1), IncludeBriefComments(0) {}
 };
 
 } // namespace clang
Index: include/clang/Sema/CodeCompleteConsumer.h
===
--- include/clang/Sema/CodeCompleteConsumer.h
+++ include/clang/Sema/CodeCompleteConsumer.h
@@ -919,8 +919,13 @@
   }
 
   /// \brief Whether to include global (top-level) declaration results.
-  bool includeGlobals() const {
-return CodeCompleteOpts.IncludeGlobals;
+  bool includeGlobals() const { return CodeCompleteOpts.IncludeGlobals; }
+
+  /// \brief Whether to include declarations in namespace contexts (including
+  /// the global namespace). If this is false, `includeGlobals()` will be
+  /// ignored.
+  bool includeNamespaceLevelDecls() const {
+return CodeCompleteOpts.IncludeNamespaceLevelDecls;
   }
 
   /// \brief Whether to include brief documentation comments within the set of
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -437,6 +437,8 @@
   HelpText<"Include code patterns in code-completion results">;
 def no_code_completion_globals

r320563 - [Sema] Ignore decls in namespaces when global decls are not wanted.

2017-12-13 Thread Eric Liu via cfe-commits
Author: ioeric
Date: Wed Dec 13 02:26:49 2017
New Revision: 320563

URL: http://llvm.org/viewvc/llvm-project?rev=320563&view=rev
Log:
[Sema] Ignore decls in namespaces when global decls are not wanted.

Summary: ... in qualified code completion and decl lookup.

Reviewers: ilya-biryukov, arphaman

Reviewed By: ilya-biryukov

Subscribers: cfe-commits

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

Added:
cfe/trunk/test/CodeCompletion/ignore-ns-level-decls.cpp
Modified:
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
cfe/trunk/include/clang/Sema/CodeCompleteOptions.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Sema/SemaCodeComplete.cpp

Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=320563&r1=320562&r2=320563&view=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Wed Dec 13 02:26:49 2017
@@ -437,6 +437,8 @@ def code_completion_patterns : Flag<["-"
   HelpText<"Include code patterns in code-completion results">;
 def no_code_completion_globals : Flag<["-"], "no-code-completion-globals">,
   HelpText<"Do not include global declarations in code-completion results.">;
+def no_code_completion_ns_level_decls : Flag<["-"], 
"no-code-completion-ns-level-decls">,
+  HelpText<"Do not include declarations inside namespaces (incl. global 
namespace) in the code-completion results.">;
 def code_completion_brief_comments : Flag<["-"], 
"code-completion-brief-comments">,
   HelpText<"Include brief documentation comments in code-completion results.">;
 def disable_free : Flag<["-"], "disable-free">,

Modified: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h?rev=320563&r1=320562&r2=320563&view=diff
==
--- cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h (original)
+++ cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h Wed Dec 13 02:26:49 2017
@@ -919,8 +919,13 @@ public:
   }
 
   /// \brief Whether to include global (top-level) declaration results.
-  bool includeGlobals() const {
-return CodeCompleteOpts.IncludeGlobals;
+  bool includeGlobals() const { return CodeCompleteOpts.IncludeGlobals; }
+
+  /// \brief Whether to include declarations in namespace contexts (including
+  /// the global namespace). If this is false, `includeGlobals()` will be
+  /// ignored.
+  bool includeNamespaceLevelDecls() const {
+return CodeCompleteOpts.IncludeNamespaceLevelDecls;
   }
 
   /// \brief Whether to include brief documentation comments within the set of

Modified: cfe/trunk/include/clang/Sema/CodeCompleteOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/CodeCompleteOptions.h?rev=320563&r1=320562&r2=320563&view=diff
==
--- cfe/trunk/include/clang/Sema/CodeCompleteOptions.h (original)
+++ cfe/trunk/include/clang/Sema/CodeCompleteOptions.h Wed Dec 13 02:26:49 2017
@@ -24,15 +24,20 @@ public:
   /// Show top-level decls in code completion results.
   unsigned IncludeGlobals : 1;
 
+  /// Show decls in namespace (including the global namespace) in code
+  /// completion results. If this is 0, `IncludeGlobals` will be ignored.
+  ///
+  /// Currently, this only works when completing qualified IDs (i.e.
+  /// `Sema::CodeCompleteQualifiedId`).
+  /// FIXME: consider supporting more completion cases with this option.
+  unsigned IncludeNamespaceLevelDecls : 1;
+
   /// Show brief documentation comments in code completion results.
   unsigned IncludeBriefComments : 1;
 
-  CodeCompleteOptions() :
-  IncludeMacros(0),
-  IncludeCodePatterns(0),
-  IncludeGlobals(1),
-  IncludeBriefComments(0)
-  { }
+  CodeCompleteOptions()
+  : IncludeMacros(0), IncludeCodePatterns(0), IncludeGlobals(1),
+IncludeNamespaceLevelDecls(1), IncludeBriefComments(0) {}
 };
 
 } // namespace clang

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=320563&r1=320562&r2=320563&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Wed Dec 13 02:26:49 2017
@@ -1380,6 +1380,8 @@ static InputKind ParseFrontendArgs(Front
 = Args.hasArg(OPT_code_completion_patterns);
   Opts.CodeCompleteOpts.IncludeGlobals
 = !Args.hasArg(OPT_no_code_completion_globals);
+  Opts.CodeCompleteOpts.IncludeNamespaceLevelDecls
+= !Args.hasArg(OPT_no_code_completion_ns_level_decls);
   Opts.CodeCom

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

2017-12-13 Thread Eric Liu 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 rL320563: [Sema] Ignore decls in namespaces when global decls 
are not wanted. (authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D40562?vs=126694&id=126701#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40562

Files:
  cfe/trunk/include/clang/Driver/CC1Options.td
  cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
  cfe/trunk/include/clang/Sema/CodeCompleteOptions.h
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/lib/Sema/SemaCodeComplete.cpp
  cfe/trunk/test/CodeCompletion/ignore-ns-level-decls.cpp

Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -1380,6 +1380,8 @@
 = Args.hasArg(OPT_code_completion_patterns);
   Opts.CodeCompleteOpts.IncludeGlobals
 = !Args.hasArg(OPT_no_code_completion_globals);
+  Opts.CodeCompleteOpts.IncludeNamespaceLevelDecls
+= !Args.hasArg(OPT_no_code_completion_ns_level_decls);
   Opts.CodeCompleteOpts.IncludeBriefComments
 = Args.hasArg(OPT_code_completion_brief_comments);
 
Index: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
===
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp
@@ -4647,10 +4647,13 @@
 MaybeAddOverrideCalls(*this, Ctx, Results);
   Results.ExitScope();
 
-  CodeCompletionDeclConsumer Consumer(Results, CurContext);
-  LookupVisibleDecls(Ctx, LookupOrdinaryName, Consumer,
- /*IncludeGlobalScope=*/true,
- /*IncludeDependentBases=*/true);
+  if (CodeCompleter->includeNamespaceLevelDecls() ||
+  (!Ctx->isNamespace() && !Ctx->isTranslationUnit())) {
+CodeCompletionDeclConsumer Consumer(Results, CurContext);
+LookupVisibleDecls(Ctx, LookupOrdinaryName, Consumer,
+   /*IncludeGlobalScope=*/true,
+   /*IncludeDependentBases=*/true);
+  }
 
   auto CC = Results.getCompletionContext();
   CC.setCXXScopeSpecifier(SS);
Index: cfe/trunk/include/clang/Driver/CC1Options.td
===
--- cfe/trunk/include/clang/Driver/CC1Options.td
+++ cfe/trunk/include/clang/Driver/CC1Options.td
@@ -437,6 +437,8 @@
   HelpText<"Include code patterns in code-completion results">;
 def no_code_completion_globals : Flag<["-"], "no-code-completion-globals">,
   HelpText<"Do not include global declarations in code-completion results.">;
+def no_code_completion_ns_level_decls : Flag<["-"], "no-code-completion-ns-level-decls">,
+  HelpText<"Do not include declarations inside namespaces (incl. global namespace) in the code-completion results.">;
 def code_completion_brief_comments : Flag<["-"], "code-completion-brief-comments">,
   HelpText<"Include brief documentation comments in code-completion results.">;
 def disable_free : Flag<["-"], "disable-free">,
Index: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
===
--- cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
+++ cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
@@ -919,8 +919,13 @@
   }
 
   /// \brief Whether to include global (top-level) declaration results.
-  bool includeGlobals() const {
-return CodeCompleteOpts.IncludeGlobals;
+  bool includeGlobals() const { return CodeCompleteOpts.IncludeGlobals; }
+
+  /// \brief Whether to include declarations in namespace contexts (including
+  /// the global namespace). If this is false, `includeGlobals()` will be
+  /// ignored.
+  bool includeNamespaceLevelDecls() const {
+return CodeCompleteOpts.IncludeNamespaceLevelDecls;
   }
 
   /// \brief Whether to include brief documentation comments within the set of
Index: cfe/trunk/include/clang/Sema/CodeCompleteOptions.h
===
--- cfe/trunk/include/clang/Sema/CodeCompleteOptions.h
+++ cfe/trunk/include/clang/Sema/CodeCompleteOptions.h
@@ -24,15 +24,20 @@
   /// Show top-level decls in code completion results.
   unsigned IncludeGlobals : 1;
 
+  /// Show decls in namespace (including the global namespace) in code
+  /// completion results. If this is 0, `IncludeGlobals` will be ignored.
+  ///
+  /// Currently, this only works when completing qualified IDs (i.e.
+  /// `Sema::CodeCompleteQualifiedId`).
+  /// FIXME: consider supporting more completion cases with this option.
+  unsigned IncludeNamespaceLevelDecls : 1;
+
   /// Show brief documentation comments in code completion results.
   unsigned IncludeBriefComments : 1;
 
-  CodeCompleteOptions() :
-  IncludeMacros(0),
-  IncludeCodePatterns(0),
-  IncludeGlobals(1),
-  Inclu

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

2017-12-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: test/clangd/diagnostics.test:10
 
 
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.c","languageId":"c","version":1,"text":"void
 main() {}"}}}
 #  CHECK:  "method": "textDocument/publishDiagnostics",

hokein wrote:
> Maybe the current tests are enough. Do we want to add a test where the 
> diagnostic location is at the end of the line?
> 
> ```
> f()
> // ^ missing ";"
> f();
> ```
Good point - the current tests only incidentally cover much of the diagnostics 
functionality.
The lit tests for this are brittle and hard to read, I'll write some gtest 
cases similar to CodeComplete.


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] D40486: [clangd] Implemented logging using Context

2017-12-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 126712.
ilya-biryukov marked 13 inline comments as done.
ilya-biryukov added a comment.

- Removed obsolete comments.
- Moved ScopeExitGuard to namespace detail.
- Added FIXMEs to pass context to compilation database.
- Addded a FIXME to JSONOutput.
- Removed unused #include directive.
- Revert accidental whitespace change.


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/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()
+  .second.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;
+  .second.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"
@@ -123,8 +123,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 +134,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 +187,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"
@@ -2

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

2017-12-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov marked 13 inline comments as done.
ilya-biryukov added inline comments.



Comment at: clangd/ClangdServer.cpp:216
 
-std::future>
-ClangdServer::codeComplete(PathRef File, Position Pos,
+std::future, Context>>
+ClangdServer::codeComplete(Context Ctx, PathRef File, Position Pos,

sammccall wrote:
> this is a pretty gross return type.
> Even after (if?) we replace tagged with context, we'll still have 
> completionlist and context.
> Should we define a struct here?
> 
> (If not, we should consider having the context first, for consistency)
I agree, callback-based APIs look way better though.
Since we'll eventually switch to callback-based versions anyway, I'm gonna 
leave it as `pair` for now.



Comment at: clangd/ClangdServer.cpp:558
   [this, FileStr, Version,
-   Tag](UniqueFunction>()>
+   Tag](UniqueFunction>(
+const Context &)>

sammccall wrote:
> This wrapping has gone beyond the point of readability for me.
> Pull out using `RebuildAction = llvm::Optional<...>(const Context &)`?
I'd refrain from doing that as this code will go away in the threading patch 
anyway. Hope that's ok.



Comment at: clangd/ClangdUnit.cpp:555
+std::shared_ptr PCHs) {
+  return std::shared_ptr(new CppFile(
+  FileName, std::move(Command), StorePreamblesInMemory, std::move(PCHs)));

sammccall wrote:
> (while here, this should really be `make_shared`)
It can't, because `CppFile`'s constructor is private. This code is also going 
away in the threading patch.



Comment at: clangd/Function.h:143
+template  struct ScopeExitGuard {
+  static_assert(std::is_same::type, Func>::value,
+"Func must be decayed");

sammccall wrote:
> FWIW I think this assert adds more noise than value - this can clearly only 
> be triggered by someone instantiating the template directly, which is 
> probably better avoided using `namespace detail` or so.
Moved it to `namespace detail`.



Comment at: clangd/Function.h:157
+
+  ScopeExitGuard(ScopeExitGuard &&Other)
+  : F(std::move(Other.F)), Moved(Other.Moved) {

sammccall wrote:
> I'm struggling to think of cases where moving these guards is the right 
> solution.
> Can we delete the move ops until we need them? It seems to make this class 
> almost trivial.
We need moves as we return the class by value from the `onScopeGuard` function. 
Neither of these will compile without move constructors:
```
auto Guard = onScopeGuard([]() { /*...*/ });
auto Guard2(onScopeGuard([]() { /*...*/ }));
auto Guard3{onScopeGuard([]() { /*...*/ })};
```




Comment at: clangd/JSONRPCDispatcher.cpp:145
+
+  auto Ctx = Context::empty()
+ .derive(RequestOut, &Out)

sammccall wrote:
> There are *lots* of `Context::empty()` calls in this file.
> Maybe conceptually clearer for JSONRPCDispatcher to own a "background" 
> context that it derives from? It'd be empty for now in either case, so not a 
> big deal.
> 
> The idea being that explicit Context::empty() calls are a bit suspect, but 
> these ones aren't really "different" from each other - we may want to provide 
> an extension point to customize it, but there'd only be one.
> 
> Up to you.
Since these usages are easy to spot (almost all uses of `Context::empty` in 
JSONRPCDispatcher.cpp), I'd leave them as is for now. It's easy to update them 
later.



Comment at: clangd/JSONRPCDispatcher.cpp:148
+ .derive(RequestSpan, std::move(Tracer));
+  if (ID)
+Ctx = std::move(Ctx).derive(RequestID, *ID);

sammccall wrote:
> I'm not sure this is better than unconditionally attaching the optional `ID`.
> Being able to distinguish between request-with-no-ID and no-request seems 
> like a good thing.
I like that we can avoid having a two-level unwrap for `RequestID`. If we're 
going down that route, maybe we should have a dedicated struct instead:
```
// name may be bad, just for the sake of example.
struct RequestData { 
  llvm::Optional RequestID;
  Span RequestSpan;
  JSONOutput Out;
};

Key ReqData;
```

That would also make it clear that we either attach all of those fields or none 
of them.
Given that `RequestID` is not yet public, I'd keep it as is and change it in a 
separate patch.


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

2017-12-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added inline comments.



Comment at: clangd/Function.h:157
+
+  ScopeExitGuard(ScopeExitGuard &&Other)
+  : F(std::move(Other.F)), Moved(Other.Moved) {

ilya-biryukov wrote:
> sammccall wrote:
> > I'm struggling to think of cases where moving these guards is the right 
> > solution.
> > Can we delete the move ops until we need them? It seems to make this class 
> > almost trivial.
> We need moves as we return the class by value from the `onScopeGuard` 
> function. Neither of these will compile without move constructors:
> ```
> auto Guard = onScopeGuard([]() { /*...*/ });
> auto Guard2(onScopeGuard([]() { /*...*/ }));
> auto Guard3{onScopeGuard([]() { /*...*/ })};
> ```
> 
Of course :-)
As you pointed out offline, making the member Optional would let you use 
the default move operators at least.


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] D40295: -fsanitize=vptr warnings on bad static types in dynamic_cast and typeid

2017-12-13 Thread Stephan Bergmann via Phabricator via cfe-commits
sberg added a comment.

friendly ping; any input on my two questions maybe?


https://reviews.llvm.org/D40295



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


[PATCH] D40720: No -fsanitize=function warning when calling noexcept function through non-noexcept pointer in C++17

2017-12-13 Thread Stephan Bergmann via Phabricator via cfe-commits
sberg added a comment.

friendly ping


https://reviews.llvm.org/D40720



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


[PATCH] D41168: [X86][avx512] Lowering X86 avx512 sqrt intrinsics to IR

2017-12-13 Thread Uriel Korach via Phabricator via cfe-commits
uriel.k created this revision.
uriel.k added reviewers: craig.topper, igorb.

seperates the non-round version from the round version of sqrt builtins
and catching them in CGBuiltin.cpp to replace builtin with IR.


https://reviews.llvm.org/D41168

Files:
  include/clang/Basic/BuiltinsX86.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/avx512fintrin.h
  test/CodeGen/avx512f-builtins.c

Index: test/CodeGen/avx512f-builtins.c
===
--- test/CodeGen/avx512f-builtins.c
+++ test/CodeGen/avx512f-builtins.c
@@ -5,21 +5,25 @@
 __m512d test_mm512_sqrt_pd(__m512d a)
 {
   // CHECK-LABEL: @test_mm512_sqrt_pd
-  // CHECK: @llvm.x86.avx512.mask.sqrt.pd.512
+  // CHECK: @llvm.sqrt.v8f64
   return _mm512_sqrt_pd(a);
 }
 
 __m512d test_mm512_mask_sqrt_pd (__m512d __W, __mmask8 __U, __m512d __A)
 {
   // CHECK-LABEL: @test_mm512_mask_sqrt_pd 
-  // CHECK: @llvm.x86.avx512.mask.sqrt.pd.512
+  // CHECK: @llvm.sqrt.v8f64
+  // CHECK: bitcast
+  // CHECK: select
   return _mm512_mask_sqrt_pd (__W,__U,__A);
 }
 
 __m512d test_mm512_maskz_sqrt_pd (__mmask8 __U, __m512d __A)
 {
   // CHECK-LABEL: @test_mm512_maskz_sqrt_pd 
-  // CHECK: @llvm.x86.avx512.mask.sqrt.pd.512
+  // CHECK: @llvm.sqrt.v8f64
+  // CHECK: bitcast
+  // CHECK: select
   return _mm512_maskz_sqrt_pd (__U,__A);
 }
 
@@ -47,21 +51,25 @@
 __m512 test_mm512_sqrt_ps(__m512 a)
 {
   // CHECK-LABEL: @test_mm512_sqrt_ps
-  // CHECK: @llvm.x86.avx512.mask.sqrt.ps.512
+  // CHECK: @llvm.sqrt.v16f32
   return _mm512_sqrt_ps(a);
 }
 
 __m512 test_mm512_mask_sqrt_ps(__m512 __W, __mmask16 __U, __m512 __A)
 {
   // CHECK-LABEL: @test_mm512_mask_sqrt_ps
-  // CHECK: @llvm.x86.avx512.mask.sqrt.ps.512
+  // CHECK: @llvm.sqrt.v16f32
+  // CHECK: bitcast
+  // CHECK: select
   return _mm512_mask_sqrt_ps( __W, __U, __A);
 }
 
 __m512 test_mm512_maskz_sqrt_ps( __mmask16 __U, __m512 __A)
 {
   // CHECK-LABEL: @test_mm512_maskz_sqrt_ps
-  // CHECK: @llvm.x86.avx512.mask.sqrt.ps.512
+  // CHECK: @llvm.sqrt.v16f32
+  // CHECK: bitcast
+  // CHECK: select
   return _mm512_maskz_sqrt_ps(__U ,__A);
 }
 
@@ -4618,7 +4626,13 @@
 }
 
 __m128d test_mm_mask_sqrt_sd(__m128d __W, __mmask8 __U, __m128d __A, __m128d __B){
-  // CHECK: @llvm.x86.avx512.mask.sqrt.sd
+  // CHECK: extractelement
+  // CHECK: extractelement
+  // CHECK: bitcast
+  // CHECK: extractelement
+  // CHECK: llvm.sqrt.f64
+  // CHECK: select
+  // CHECK: insertelement
 return _mm_mask_sqrt_sd(__W,__U,__A,__B);
 }
 
@@ -4628,7 +4642,12 @@
 }
 
 __m128d test_mm_maskz_sqrt_sd(__mmask8 __U, __m128d __A, __m128d __B){
-  // CHECK: @llvm.x86.avx512.mask.sqrt.sd
+  // CHECK: extractelement
+  // CHECK: bitcast
+  // CHECK: extractelement
+  // CHECK: llvm.sqrt.f64
+  // CHECK: select
+  // CHECK: insertelement
 return _mm_maskz_sqrt_sd(__U,__A,__B);
 }
 
@@ -4644,7 +4663,13 @@
 }
 
 __m128 test_mm_mask_sqrt_ss(__m128 __W, __mmask8 __U, __m128 __A, __m128 __B){
-  // CHECK: @llvm.x86.avx512.mask.sqrt.ss
+  // CHECK: extractelement
+  // CHECK: extractelement
+  // CHECK: bitcast
+  // CHECK: extractelement
+  // CHECK: llvm.sqrt.f32
+  // CHECK: select
+  // CHECK: insertelement
 return _mm_mask_sqrt_ss(__W,__U,__A,__B);
 }
 
@@ -4654,7 +4679,12 @@
 }
 
 __m128 test_mm_maskz_sqrt_ss(__mmask8 __U, __m128 __A, __m128 __B){
-  // CHECK: @llvm.x86.avx512.mask.sqrt.ss
+  // CHECK: extractelement
+  // CHECK: bitcast
+  // CHECK: extractelement
+  // CHECK: llvm.sqrt.f32
+  // CHECK: select
+  // CHECK: insertelement
 return _mm_maskz_sqrt_ss(__U,__A,__B);
 }
 
Index: lib/Headers/avx512fintrin.h
===
--- lib/Headers/avx512fintrin.h
+++ lib/Headers/avx512fintrin.h
@@ -1601,29 +1601,26 @@
 static  __inline__ __m512d __DEFAULT_FN_ATTRS
 _mm512_sqrt_pd(__m512d __a)
 {
-  return (__m512d)__builtin_ia32_sqrtpd512_mask((__v8df)__a,
+  return (__m512d)__builtin_ia32_sqrtpd512_mask_nr ((__v8df)__a,
 (__v8df) _mm512_setzero_pd (),
-(__mmask8) -1,
-_MM_FROUND_CUR_DIRECTION);
+(__mmask8) -1);
 }
 
 static __inline__ __m512d __DEFAULT_FN_ATTRS
 _mm512_mask_sqrt_pd (__m512d __W, __mmask8 __U, __m512d __A)
 {
-  return (__m512d) __builtin_ia32_sqrtpd512_mask ((__v8df) __A,
+  return (__m512d) __builtin_ia32_sqrtpd512_mask_nr ((__v8df) __A,
(__v8df) __W,
-   (__mmask8) __U,
-   _MM_FROUND_CUR_DIRECTION);
+   (__mmask8) __U);
 }
 
 static __inline__ __m512d __DEFAULT_FN_ATTRS
 _mm512_maskz_sqrt_pd (__mmask8 __U, __m512d __A)
 {
-  return (__m512d) __builtin_ia32_sqrtpd512_mask ((__v8df) __A,
+  return (__m512d) __builtin_ia32_sqrtpd512_mask_nr ((__v8df) __A,
(__v8df)
_mm512_setzero_pd (),
-   (__mmask8) __U,
- 

[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.

2017-12-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

I think this patch is too big, and there are several separate pieces here:

- the index interface, and the implementation of the dynamic symbol index 
(these probably belong together). We're missing unit tests for the 
implementation.
- building and updating the dynamic symbol index in clangd
- code completion based on a symbol index

As a consequence I'm finding it pretty intimidating to review, I've tried to 
leave the main high-level comments.




Comment at: clangd/ClangdIndex.h:24
+
+/// \brief This manages symbols from source files that can be updated or
+/// removed. Symbols are used by a symbol index which builds indexes on symbols

nit: maybe call this out as a container? that tells a lot about what it 
does/doesn't do.

e.g. `\brief A container of Symbols from several source files. It can be 
updated at source-file granularity, replacing all symbols from one file with a 
new set.` ...



Comment at: clangd/ClangdUnit.cpp:854
  NewAST->getDiagnostics().end());
+  if (That->FileSyms) {
+auto Symbols =

Having clangdunit know how the index is maintained feels like a layering 
violation.

Can we instead have clangdunit take a callback 
(`UniqueFunction` or so) that gets invoked after each 
successful build? That can be the "hook" where we do indexing.

The common higher layer (ClangdServer) then just has to have the glue code 
tying the ClangdUnit API to the Index functionality.



Comment at: clangd/ClangdUnitStore.h:29
 public:
+  explicit CppFileCollection(FileSymbols *FileSyms) : FileSyms(FileSyms) {}
+

Similarly here, this can take a post-build callback of void(Filename, 
ParsedAST&) that is translated into per-cppfile callbacks. Then it doesn't need 
to know about FileSymbols.

(Or ParsedAST*, with nullptr meaning this file is gone)



Comment at: clangd/CodeComplete.h:50
 
+  /// Add symbols in namespace context (including global namespace) to code
+  /// completion.

I find the config changes pretty confusing.
CodeComplete options is for options we want to offer users. This change doesn't 
seem motivated by a user requirement, but rather by implementation details.
If we're going to do that, we should be clear about it, rather than just expose 
every option of clang::CodeCompletionOptions we might want to set ourselves

If we do want to ensure that we can do clangd::CodeCompleteOptions -> 
clang::CodeCompleteOptions without extra args:

CodeCompleteOptions {
  ...
  const Index* Index = nullptr; // Populated internally by clangd, do not 
set
}

then we can use the presence of index to determine whether to restrict what we 
ask Sema for.
(We could also let users override the index used for code completion this way 
if we want, but I don't see a use case)



Comment at: clangd/index/FileIndex.h:62
+/// whose symbols can be easily managed in memory.
+class FileSymbolsMemIndex : public SymbolIndex {
+public:

I think it'd be possible and nice to decouple `MemIndex` from FileSymbols, such 
that we can use it for the on-disk index (which is one big slab).

At the moment it seems like an unhappy mix - we don't actually get any 
incrementality, but we're coupled to the incremental storage.

I think it's worth addressing now as MemIndex should be named differently and 
move to another file if this is the case.

One way this could work:
  class MemIndex {
shared_ptr> Symbols;

// Symbols must remain accessible as long as S is kept alive.
build(shared_ptr> S) {
  // TODO: build smarter index structures later
  lock_guard
  Symbols = move(S); // releases old symbols
  // TODO: move smart index structures
}
  }

  class FileSymbols {
// The shared_ptr keeps the symbols alive
shared_ptr> allSymbols() {
  struct Snapshot {
vector Pointers;
vector> KeepAlive;
  };
  auto Snap = make_shared();
  for (Slab in FileToSymbols) {
Snap->KeepAlive.push_back(Slab);
for (Sym in *Slab)
  Snap.Pointers.push_back(&Sym);
  }
  return shared_ptr>(move(Snap), &Snap->Pointers);
}
  }

  // In ClangdServer, when getting new symbols for a file
  FileSymbols.update(Path, Collector.takeSymbols());
  DynamicIndex.build(FileSymbols.allSymbols());

This seems pretty nice, but vector> would work in 
practice for the interface too, if you don't like aliasing shared_ptr tricks.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40548



___
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

2017-12-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 126725.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.

- Simplify ScopeExitGuard by using llvm::Optional<>.


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/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()
+  .second.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;
+  .second.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"
@@ -123,8 +123,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 +134,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 +187,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 +202,21 @@
   FS.ExpectedFile = FooCpp;
 
   // To sync reparses before checking for errors.
-  std::future ParseFuture;
+  std::future ParseFuture;
 
-  ParseFuture = Server.addD

[clang-tools-extra] r320574 - [clangd] Remove the const specifier of the takeSymbol method

2017-12-13 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Wed Dec 13 04:39:06 2017
New Revision: 320574

URL: http://llvm.org/viewvc/llvm-project?rev=320574&view=rev
Log:
[clangd] Remove the const specifier of the takeSymbol method

otherwise we will copy an object.

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

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.h?rev=320574&r1=320573&r2=320574&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.h (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.h Wed Dec 13 04:39:06 
2017
@@ -32,7 +32,7 @@ public:
 
   void finish() override;
 
-  SymbolSlab takeSymbols() const { return std::move(Symbols); }
+  SymbolSlab takeSymbols() { return std::move(Symbols); }
 
 private:
   // All Symbols collected from the AST.


___
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

2017-12-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 126728.
ilya-biryukov added a comment.

Merged with head


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/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()
+  .second.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;
+  .second.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"
@@ -123,8 +123,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 +134,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 +187,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 +202,21 @@
   FS.ExpectedFile = FooCpp;
 
   // To sync reparses before checking for errors.
-  std::future ParseFuture;
+  std::future ParseFuture;
 
-  ParseFuture = Server.addDocument(FooCpp, SourceContents);
+  ParseFuture = Server.addDocument(Context::empty(

[clang-tools-extra] r320576 - [clangd] Implemented logging using Context

2017-12-13 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov
Date: Wed Dec 13 04:51:22 2017
New Revision: 320576

URL: http://llvm.org/viewvc/llvm-project?rev=320576&view=rev
Log:
[clangd] Implemented logging using Context

Reviewers: sammccall, ioeric, hokein

Reviewed By: sammccall

Subscribers: klimek, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
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/ClangdUnitStore.cpp
clang-tools-extra/trunk/clangd/ClangdUnitStore.h
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeComplete.h
clang-tools-extra/trunk/clangd/Function.h
clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h
clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp
clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h
clang-tools-extra/trunk/clangd/Logger.cpp
clang-tools-extra/trunk/clangd/Logger.h
clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
clang-tools-extra/trunk/clangd/ProtocolHandlers.h
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=320576&r1=320575&r2=320576&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Dec 13 04:51:22 2017
@@ -46,7 +46,7 @@ std::vector replacementsToEdit
 } // namespace
 
 void ClangdLSPServer::onInitialize(Ctx C, InitializeParams &Params) {
-  C.reply(json::obj{
+  reply(C, json::obj{
   {{"capabilities",
 json::obj{
 {"textDocumentSync", 1},
@@ -84,7 +84,7 @@ void ClangdLSPServer::onInitialize(Ctx C
 void ClangdLSPServer::onShutdown(Ctx C, ShutdownParams &Params) {
   // Do essentially nothing, just say we're ready to exit.
   ShutdownRequestReceived = true;
-  C.reply(nullptr);
+  reply(C, nullptr);
 }
 
 void ClangdLSPServer::onExit(Ctx C, ExitParams &Params) { IsDone = true; }
@@ -94,16 +94,17 @@ void ClangdLSPServer::onDocumentDidOpen(
   if (Params.metadata && !Params.metadata->extraFlags.empty())
 CDB.setExtraFlagsForFile(Params.textDocument.uri.file,
  std::move(Params.metadata->extraFlags));
-  Server.addDocument(Params.textDocument.uri.file, Params.textDocument.text);
+  Server.addDocument(std::move(C), Params.textDocument.uri.file,
+ Params.textDocument.text);
 }
 
 void ClangdLSPServer::onDocumentDidChange(Ctx C,
   DidChangeTextDocumentParams &Params) 
{
   if (Params.contentChanges.size() != 1)
-return C.replyError(ErrorCode::InvalidParams,
-"can only apply one change at a time");
+return replyError(C, ErrorCode::InvalidParams,
+  "can only apply one change at a time");
   // We only support full syncing right now.
-  Server.addDocument(Params.textDocument.uri.file,
+  Server.addDocument(std::move(C), Params.textDocument.uri.file,
  Params.contentChanges[0].text);
 }
 
@@ -125,39 +126,39 @@ void ClangdLSPServer::onCommand(Ctx C, E
 
 ApplyWorkspaceEditParams ApplyEdit;
 ApplyEdit.edit = *Params.workspaceEdit;
-C.reply("Fix applied.");
+reply(C, "Fix applied.");
 // We don't need the response so id == 1 is OK.
 // Ideally, we would wait for the response and if there is no error, we
 // would reply success/failure to the original RPC.
-C.call("workspace/applyEdit", ApplyEdit);
+call(C, "workspace/applyEdit", ApplyEdit);
   } else {
 // We should not get here because ExecuteCommandParams would not have
 // parsed in the first place and this handler should not be called. But if
 // more commands are added, this will be here has a safe guard.
-C.replyError(
-ErrorCode::InvalidParams,
+replyError(
+C, ErrorCode::InvalidParams,
 llvm::formatv("Unsupported command \"{0}\".", Params.command).str());
   }
 }
 
 void ClangdLSPServer::onRename(Ctx C, RenameParams &Params) {
   auto File = Params.textDocument.uri.file;
-  auto Replacements = Server.rename(File, Params.position, Params.newName);
+  auto Replacements = Server.rename(C, File, Params.position, Params.newName);
   if (!Replacements) {
-C.replyError(ErrorCode::InternalError,
- llvm::toString(Replacements.takeError()));
+replyError(C, ErrorCode::InternalError,
+   llvm::toString(Replacem

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

2017-12-13 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE320576: [clangd] Implemented logging using Context 
(authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D40486?vs=126728&id=126729#toc

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/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: clangd/ClangdUnitStore.h
===
--- clangd/ClangdUnitStore.h
+++ clangd/ClangdUnitStore.h
@@ -14,6 +14,7 @@
 
 #include "ClangdUnit.h"
 #include "GlobalCompilationDatabase.h"
+#include "Logger.h"
 #include "Path.h"
 #include "clang/Tooling/CompilationDatabase.h"
 
@@ -28,8 +29,7 @@
   std::shared_ptr
   getOrCreateFile(PathRef File, PathRef ResourceDir,
   GlobalCompilationDatabase &CDB, bool StorePreamblesInMemory,
-  std::shared_ptr PCHs,
-  clangd::Logger &Logger) {
+  std::shared_ptr PCHs) {
 std::lock_guard Lock(Mutex);
 
 auto It = OpenedFiles.find(File);
@@ -39,7 +39,7 @@
   It = OpenedFiles
.try_emplace(File, CppFile::Create(File, std::move(Command),
   StorePreamblesInMemory,
-  std::move(PCHs), Logger))
+  std::move(PCHs)))
.first;
 }
 return It->second;
@@ -61,8 +61,8 @@
   /// will be returned in RecreateResult.RemovedFile.
   RecreateResult recreateFileIfCompileCommandChanged(
   PathRef File, PathRef ResourceDir, GlobalCompilationDatabase &CDB,
-  bool StorePreamblesInMemory, std::shared_ptr PCHs,
-  clangd::Logger &Logger);
+  bool StorePreamblesInMemory,
+  std::shared_ptr PCHs);
 
   std::shared_ptr getFile(PathRef File) {
 std::lock_guard Lock(Mutex);
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -584,14 +584,14 @@
 
 }; // SignatureHelpCollector
 
-bool invokeCodeComplete(std::unique_ptr Consumer,
+bool invokeCodeComplete(const Context &Ctx,
+std::unique_ptr Consumer,
 const clang::CodeCompleteOptions &Options,
 PathRef FileName,
 const tooling::CompileCommand &Command,
 PrecompiledPreamble const *Preamble, StringRef Contents,
 Position Pos, IntrusiveRefCntPtr VFS,
-std::shared_ptr PCHs,
-Logger &Logger) {
+std::shared_ptr PCHs) {
   std::vector ArgStrs;
   for (const auto &S : Command.CommandLine)
 ArgStrs.push_back(S.c_str());
@@ -634,12 +634,12 @@
 
   SyntaxOnlyAction Action;
   if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])) {
-Logger.log("BeginSourceFile() failed when running codeComplete for " +
-   FileName);
+log(Ctx,
+"BeginSourceFile() failed when running codeComplete for " + FileName);
 return false;
   }
   if (!Action.Execute()) {
-Logger.log("Execute() failed when running codeComplete for " + FileName);
+log(Ctx, "Execute() failed when running codeComplete for " + FileName);
 return false;
   }
 
@@ -660,13 +660,13 @@
   return Result;
 }
 
-CompletionList codeComplete(PathRef FileName,
+CompletionList codeComplete(const Context &Ctx, PathRef FileName,
 const tooling::CompileCommand &Command,
 PrecompiledPreamble const *Preamble,
 StringRef Contents, Position Pos,
 IntrusiveRefCntPtr VFS,
 std::shared_ptr PCHs,
-CodeCompleteOptions Opts, Logger &Logger) {
+CodeCompleteOptions Opts) {
   CompletionList Results;
   std::unique_ptr Consumer;
   if (Opts.EnableSnippets) {
@@ -676,26 +676,28 @@
 Consumer =
 llvm::make_unique(Opts, Results);
   }
-  invokeCodeComplete(std::move(Consumer), Opts.getClangCompleteOpts(), FileName,
- Command, Preamble, Contents, Pos, std::move(VFS),
- std::move(PCHs), Logger);
+  invokeCodeComplete(Ctx, std::move(Consumer), Opts.getClangCompleteOpt

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

2017-12-13 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov
Date: Wed Dec 13 04:53:16 2017
New Revision: 320577

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

Modified:
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/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=320577&r1=320576&r2=320577&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Wed Dec 13 04:53:16 2017
@@ -23,25 +23,18 @@ ArrayRef toArrayRef(StringRef S
 SymbolID::SymbolID(llvm::StringRef USR)
 : HashValue(llvm::SHA1::hash(toArrayRef(USR))) {}
 
-SymbolSlab::const_iterator SymbolSlab::begin() const {
-  return Symbols.begin();
-}
+SymbolSlab::const_iterator SymbolSlab::begin() const { return Symbols.begin(); 
}
 
-SymbolSlab::const_iterator SymbolSlab::end() const {
-  return Symbols.end();
-}
+SymbolSlab::const_iterator SymbolSlab::end() const { return Symbols.end(); }
 
-SymbolSlab::const_iterator SymbolSlab::find(const SymbolID& SymID) const {
+SymbolSlab::const_iterator SymbolSlab::find(const SymbolID &SymID) const {
   return Symbols.find(SymID);
 }
 
-void SymbolSlab::freeze() {
-  Frozen = true;
-}
+void SymbolSlab::freeze() { Frozen = true; }
 
 void SymbolSlab::insert(Symbol S) {
-  assert(!Frozen &&
- "Can't insert a symbol after the slab has been frozen!");
+  assert(!Frozen && "Can't insert a symbol after the slab has been frozen!");
   Symbols[S.ID] = std::move(S);
 }
 

Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=320577&r1=320576&r2=320577&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Wed Dec 13 04:53:16 2017
@@ -45,7 +45,7 @@ public:
   SymbolID() = default;
   SymbolID(llvm::StringRef USR);
 
-  bool operator==(const SymbolID& Sym) const {
+  bool operator==(const SymbolID &Sym) const {
 return HashValue == Sym.HashValue;
   }
 
@@ -89,14 +89,14 @@ struct Symbol {
 // FIXME: Use a space-efficient implementation, a lot of Symbol fields could
 // share the same storage.
 class SymbolSlab {
- public:
+public:
   using const_iterator = llvm::DenseMap::const_iterator;
 
   SymbolSlab() = default;
 
   const_iterator begin() const;
   const_iterator end() const;
-  const_iterator find(const SymbolID& SymID) const;
+  const_iterator find(const SymbolID &SymID) const;
 
   // Once called, no more symbols would be added to the SymbolSlab. This
   // operation is irreversible.
@@ -104,7 +104,7 @@ class SymbolSlab {
 
   void insert(Symbol S);
 
- private:
+private:
   bool Frozen = false;
 
   llvm::DenseMap Symbols;

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=320577&r1=320576&r2=320577&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Wed Dec 13 
04:53:16 2017
@@ -94,9 +94,7 @@ bool SymbolCollector::handleDeclOccurenc
   return true;
 }
 
-void SymbolCollector::finish() {
-  Symbols.freeze();
-}
+void SymbolCollector::finish() { Symbols.freeze(); }
 
 } // namespace clangd
 } // namespace clang

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=320577&r1=320576&r2=320577&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Wed Dec 13 
04:53:16 2017
@@ -8,8 +8,8 @@
 
//===--===//
 #include "ClangdServer.h"
 #include "Compiler.h"
-#include "Matchers.h"
 #include "Context.h"
+#include "Matchers.h"
 #include "Protocol.h"
 #include "TestFS.h"
 #include "gmock/gmock.h"

Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=320577&r1=320576&r2=320577&view=diff
==
--- clang-tools-extra/trunk

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2017-12-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: include/clang/Frontend/ASTUnit.h:196
+  /// \brief Counter indicating how often the preamble was build in total.
+  unsigned PreambleCounter;
+

nik wrote:
> Any better name for this one? Otherwise I would suggest renaming 
> PreambleRebuildCounter to PreambleRebuildCountdownCounter to make them more 
> distinct.
+1, names PreambleRebuildCounter and PreambleCounter are too similar.




Comment at: lib/Frontend/PrecompiledPreamble.cpp:437
 
+  vfs::OverlayFileSystem OFS(VFS);
+  IntrusiveRefCntPtr IMFS(

nik wrote:
> ilya-biryukov wrote:
> > Can we do the same thing without creating an additional `OverlayFileSystem`?
> > 
> > If we add a separate map to check for those:
> > ```
> > llvm::StringMap OverriddenFilesWithoutFS.
> > // Populate the map with paths not existing in VFS.
> > 
> > for (const auto &F : FilesInPreamble) {
> > vfs::Status Status;
> > if (!moveOnNoError(OFS.status(F.first()), Status)) {
> >   // If we can't stat the file, check whether it was overriden
> >   auto It = OverriddenFilesWithoutFS[F.first()];
> >   if (It == OverriddenFilesWithoutFS.end())
> > return false;  
> >   //.
> > }
> > //..
> > 
> > }
> > ```
> > Can we do the same thing without creating an additional OverlayFileSystem?
> 
> Hmm, I thought I had a good reason for this one. I don't remember it and the 
> test still passes, so I did it without it now.
> 
> Is there any real advantage in first trying the stat, then checking 
> OverriddenFilesWithoutFS? Since I don't see any, I've changed order because 
> the stat can then be avoided; also, it removes some nesting.
I don't see any advantage.
I used to think it had something to do with overriden symlinks, but after 
thinking more about it I'm not even sure what should the semantics of those be. 
And I don't think the current code handles that (we have neither tests nor 
comments suggesting that this use-case was considered in the first place).

This possibly handles case-insensitive filesystems, like NTFS on Windows. But 
I'm not sure if that was the original intention of this code.



Comment at: lib/Frontend/PrecompiledPreamble.cpp:461
 
 std::map::iterator Overridden =
 OverriddenFiles.find(Status.getUniqueID());

Will anything fail if we remove the map from `UniqueID` to hashes of overriden 
files and the corresponding checks?

We should either document why having `UniqueID`-based checks is important or 
remove the code doing those checks.


Repository:
  rC Clang

https://reviews.llvm.org/D41005



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


[PATCH] D40707: [libcxx] Fix basic_stringbuf constructor

2017-12-13 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki updated this revision to Diff 126738.
miyuki added a comment.

Modified the test to use NULL instead of literal zero.


https://reviews.llvm.org/D40707

Files:
  include/sstream
  test/std/input.output/string.streams/stringbuf/stringbuf.cons/default.pass.cpp


Index: 
test/std/input.output/string.streams/stringbuf/stringbuf.cons/default.pass.cpp
===
--- 
test/std/input.output/string.streams/stringbuf/stringbuf.cons/default.pass.cpp
+++ 
test/std/input.output/string.streams/stringbuf/stringbuf.cons/default.pass.cpp
@@ -17,6 +17,21 @@
 #include 
 #include 
 
+template
+struct testbuf
+: std::basic_stringbuf
+{
+void check()
+{
+assert(this->eback() == NULL);
+assert(this->gptr() == NULL);
+assert(this->egptr() == NULL);
+assert(this->pbase() == NULL);
+assert(this->pptr() == NULL);
+assert(this->epptr() == NULL);
+}
+};
+
 int main()
 {
 {
@@ -27,4 +42,12 @@
 std::wstringbuf buf;
 assert(buf.str() == L"");
 }
+{
+testbuf buf;
+buf.check();
+}
+{
+testbuf buf;
+buf.check();
+}
 }
Index: include/sstream
===
--- include/sstream
+++ include/sstream
@@ -243,7 +243,6 @@
 : __hm_(0),
   __mode_(__wch)
 {
-str(string_type());
 }
 
 template 


Index: test/std/input.output/string.streams/stringbuf/stringbuf.cons/default.pass.cpp
===
--- test/std/input.output/string.streams/stringbuf/stringbuf.cons/default.pass.cpp
+++ test/std/input.output/string.streams/stringbuf/stringbuf.cons/default.pass.cpp
@@ -17,6 +17,21 @@
 #include 
 #include 
 
+template
+struct testbuf
+: std::basic_stringbuf
+{
+void check()
+{
+assert(this->eback() == NULL);
+assert(this->gptr() == NULL);
+assert(this->egptr() == NULL);
+assert(this->pbase() == NULL);
+assert(this->pptr() == NULL);
+assert(this->epptr() == NULL);
+}
+};
+
 int main()
 {
 {
@@ -27,4 +42,12 @@
 std::wstringbuf buf;
 assert(buf.str() == L"");
 }
+{
+testbuf buf;
+buf.check();
+}
+{
+testbuf buf;
+buf.check();
+}
 }
Index: include/sstream
===
--- include/sstream
+++ include/sstream
@@ -243,7 +243,6 @@
 : __hm_(0),
   __mode_(__wch)
 {
-str(string_type());
 }
 
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D40707: [libcxx] Fix basic_stringbuf constructor

2017-12-13 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki updated this revision to Diff 126739.
miyuki added a comment.

Use diff with context.


https://reviews.llvm.org/D40707

Files:
  include/sstream
  test/std/input.output/string.streams/stringbuf/stringbuf.cons/default.pass.cpp


Index: 
test/std/input.output/string.streams/stringbuf/stringbuf.cons/default.pass.cpp
===
--- 
test/std/input.output/string.streams/stringbuf/stringbuf.cons/default.pass.cpp
+++ 
test/std/input.output/string.streams/stringbuf/stringbuf.cons/default.pass.cpp
@@ -17,6 +17,21 @@
 #include 
 #include 
 
+template
+struct testbuf
+: std::basic_stringbuf
+{
+void check()
+{
+assert(this->eback() == NULL);
+assert(this->gptr() == NULL);
+assert(this->egptr() == NULL);
+assert(this->pbase() == NULL);
+assert(this->pptr() == NULL);
+assert(this->epptr() == NULL);
+}
+};
+
 int main()
 {
 {
@@ -27,4 +42,12 @@
 std::wstringbuf buf;
 assert(buf.str() == L"");
 }
+{
+testbuf buf;
+buf.check();
+}
+{
+testbuf buf;
+buf.check();
+}
 }
Index: include/sstream
===
--- include/sstream
+++ include/sstream
@@ -243,7 +243,6 @@
 : __hm_(0),
   __mode_(__wch)
 {
-str(string_type());
 }
 
 template 


Index: test/std/input.output/string.streams/stringbuf/stringbuf.cons/default.pass.cpp
===
--- test/std/input.output/string.streams/stringbuf/stringbuf.cons/default.pass.cpp
+++ test/std/input.output/string.streams/stringbuf/stringbuf.cons/default.pass.cpp
@@ -17,6 +17,21 @@
 #include 
 #include 
 
+template
+struct testbuf
+: std::basic_stringbuf
+{
+void check()
+{
+assert(this->eback() == NULL);
+assert(this->gptr() == NULL);
+assert(this->egptr() == NULL);
+assert(this->pbase() == NULL);
+assert(this->pptr() == NULL);
+assert(this->epptr() == NULL);
+}
+};
+
 int main()
 {
 {
@@ -27,4 +42,12 @@
 std::wstringbuf buf;
 assert(buf.str() == L"");
 }
+{
+testbuf buf;
+buf.check();
+}
+{
+testbuf buf;
+buf.check();
+}
 }
Index: include/sstream
===
--- include/sstream
+++ include/sstream
@@ -243,7 +243,6 @@
 : __hm_(0),
   __mode_(__wch)
 {
-str(string_type());
 }
 
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39571: [clangd] DidChangeConfiguration Notification

2017-12-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov requested changes to this revision.
ilya-biryukov added a comment.
This revision now requires changes to proceed.

It seems this patch is out of date, we need to merge it with the latests head.




Comment at: clangd/DraftStore.cpp:45
   std::lock_guard Lock(Mutex);
+  assert(!File.empty());
 

Why do we need this assert? I don't see how empty file is worse than any other 
invalid value.



Comment at: clangd/DraftStore.cpp:55
   std::lock_guard Lock(Mutex);
+  assert(!File.empty());
 

Why do we need this assert? I don't see how empty file is worse than any other 
invalid value.



Comment at: clangd/GlobalCompilationDatabase.h:65
+  void setCompileCommandsDir(Path P) {
+CompileCommandsDir = P;
+CompilationDatabases.clear();

We need to lock the Mutex here!



Comment at: clangd/Protocol.cpp:368
+json::Expr toJSON(const DidChangeConfigurationParams &CCP) {
+
+  return json::obj{

NIT: maybe remove this empty line?



Comment at: test/clangd/did-change-configuration.test:33
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///compile_commands.json","languageId":"json","version":1,"text":"[\n{\n"directory":"/",\n"command":"/usr/bin/c++-DGTEST_HAS_RTTI=0-D_GNU_SOURCE-D__STDC_CONSTANT_MACROS-D__STDC_FORMAT_MACROS-D__STDC_LIMIT_MACROS-Ilib/Demangle-I../lib/Demangle-I/usr/include/libxml2-Iinclude-I../include-fPIC-fvisibility-inlines-hidden-Werror=date-time-std=c++11-Wall-W-Wno-unused-parameter-Wwrite-strings-Wcast-qual-Wno-missing-field-initializers-pedantic-Wno-long-long-Wno-maybe-uninitialized-Wdelete-non-virtual-dtor-Wno-comment-O0-g-fno-exceptions-fno-rtti-o/foo.c.o-c/foo.c",\n"file":"/foo.c"\n},"}}}
+

Nebiroth wrote:
> ilya-biryukov wrote:
> > clangd won't see this file. `didOpen` only sets contents for diagnostics, 
> > not any other features.
> > You would rather want to add more `# RUN:` directives at the top of the 
> > file to create `compile_commands.json`, etc.
> > 
> > Writing it under root ('/') is obviously not an option. Lit tests allow you 
> > to use temporary paths, this is probably an approach you could take. See [[ 
> > https://llvm.org/docs/TestingGuide.html#substitutions | lit docs ]] for 
> > more details.
> Are there examples available on how to use this? I have to use a # RUN: to 
> create a file and then use it's path in a workspace/didChangeConfiguration 
> notification?
After thinking about it, I really don't see an easy way to test it currently. 
We just don't have the infrastructure in place for that.
I think it's fine to add a FIXME to the body of 
`ClangdLSPServer::onChangeConfiguration` that we need to test it and remove 
this test.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D39571



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


[PATCH] D41178: [clangd] Construct SymbolSlab from YAML format.

2017-12-13 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: ilya-biryukov, mgorny, klimek.

This will be used together with https://reviews.llvm.org/D40548 for the global 
index source (experimental).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41178

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

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -8,6 +8,7 @@
 //===--===//
 
 #include "index/SymbolCollector.h"
+#include "index/SymbolFromYAML.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
@@ -103,6 +104,12 @@
   runSymbolCollector(Header, Main);
   EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Foo::f"),
 QName("f1"), QName("f2")));
+
+  std::string YAMLContent = SymbolToYAML(Symbols);
+  auto SymbolsReadFromYAML = SymbolFromYAML(YAMLContent);
+  EXPECT_THAT(SymbolsReadFromYAML,
+  UnorderedElementsAre(QName("Foo"), QName("Foo::f"), QName("f1"),
+   QName("f2")));
 }
 
 } // namespace
Index: clangd/index/SymbolFromYAML.h
===
--- /dev/null
+++ clangd/index/SymbolFromYAML.h
@@ -0,0 +1,28 @@
+//===--- SymbolFromYAML.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_SYMBOL_FROM_YAML_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_FROM_YAML_H
+
+#include "Index.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+
+// Read symbols from a YAML-format string.
+SymbolSlab SymbolFromYAML(llvm::StringRef YAMLContent);
+
+// Convert symbols to a YAML-format string.
+std::string SymbolToYAML(const SymbolSlab& Symbols);
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_FROM_YAML_H
Index: clangd/index/SymbolFromYAML.cpp
===
--- /dev/null
+++ clangd/index/SymbolFromYAML.cpp
@@ -0,0 +1,154 @@
+//===--- SymbolFromYAML.cpp --*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "SymbolFromYAML.h"
+
+#include "Index.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/YAMLTraits.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/Errc.h"
+
+LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(clang::clangd::Symbol)
+
+namespace llvm {
+namespace yaml {
+
+using clang::clangd::Symbol;
+using clang::clangd::SymbolID;
+using clang::clangd::SymbolLocation;
+using clang::index::SymbolInfo;
+using clang::index::SymbolLanguage;
+using clang::index::SymbolKind;
+
+// Helpers to (de)serialize the SymbolID underlying data as std::array is not
+// supported in YAML I/O.
+struct NPArray {
+  NPArray(IO &) {}
+  NPArray(IO &, std::array HashValue) {
+Value = {HashValue.begin(), HashValue.end()};
+  }
+  std::array denormalize(IO &) {
+assert(Value.size() == 20);
+std::array Result;
+std::copy(Value.begin(), Value.end(), Result.begin());
+return Result;
+  }
+
+  std::vector Value;
+};
+
+template <> struct MappingTraits {
+  static void mapping(IO &IO, SymbolID &ID) {
+MappingNormalization> NPArry(
+IO, ID.HashValue);
+IO.mapOptional("Value", NPArry->Value);
+  }
+};
+
+template <> struct MappingTraits {
+  static void mapping(IO &IO, SymbolLocation &Value) {
+IO.mapRequired("StartOffset", Value.StartOffset);
+IO.mapRequired("EndOffset", Value.EndOffset);
+IO.mapRequired("FilePath", Value.FilePath);
+  }
+};
+
+template <> struct MappingTraits {
+  static void mapping(IO &io, SymbolInfo &SymInfo) {
+// FIXME: expose other fields?
+io.mapRequired("Kind", SymInfo.Kind);
+io.mapRequired("Lang", SymInfo.Lang);
+  }
+};
+
+template<> struct MappingTraits {
+  static void mapping(IO &IO, Symbol &Sym) {
+IO.mapRequired("ID", Sym.ID);
+IO.mapRequired("QualifiedName", Sym.QualifiedName);
+IO.mapRequired("SymInfo", Sym.SymInfo);
+IO.mapRequired("CanonicalDeclaration", Sym.CanonicalDeclaration);
+  

[clang-tools-extra] r320578 - [clangd] Try to workaround MSVC compilation failure.

2017-12-13 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov
Date: Wed Dec 13 05:43:47 2017
New Revision: 320578

URL: http://llvm.org/viewvc/llvm-project?rev=320578&view=rev
Log:
[clangd] Try to workaround MSVC compilation failure.

Modified:
clang-tools-extra/trunk/clangd/Context.h

Modified: clang-tools-extra/trunk/clangd/Context.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Context.h?rev=320578&r1=320577&r2=320578&view=diff
==
--- clang-tools-extra/trunk/clangd/Context.h (original)
+++ clang-tools-extra/trunk/clangd/Context.h Wed Dec 13 05:43:47 2017
@@ -95,6 +95,11 @@ private:
   Context(std::shared_ptr DataPtr);
 
 public:
+  /// Same as Context::empty(), please use Context::empty() instead.
+  /// Constructor is defined to workaround a bug in MSVC's version of STL.
+  /// (arguments of std::future<> must be default-construcitble in MSVC).
+  Context() = default;
+
   /// Move-only.
   Context(Context const &) = delete;
   Context &operator=(const Context &) = delete;


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


[PATCH] D7375: [clang-tidy] Assert related checkers

2017-12-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.
Herald added a subscriber: mgorny.



Comment at: test/clang-tidy/misc-static-assert.cpp:71
+
+  assert(ZERO_MACRO && "Report me!");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be

Hmm. Are you sure about this?
I'm having false-positives (as far i'm concerned at least) due to this.

In particular with ``. If the build is with asan,
then i can include the header and use e.g. `__asan_region_is_poisoned()`
But if it is not, i can't do that. So i need to wrap it into a macro, which 
expands
to `__asan_region_is_poisoned()` if the build is with asan, and to some constant
(e.g. `0`) otherwise. And the check, naturally, does not like this.

As a minimal reproducer:
```
#ifndef MYDEFINE
#define mycheck(x) 0
#define notmycheck(x) 1
#else
int __mycheck(int x);
int __notmycheck(int x);
#define mycheck(x) __mycheck(x)
#define notmycheck(x) __notmycheck(x)
#endif

void f(int x) {
  assert(!mycheck(x));
  assert(!notmycheck(x));
  assert(mycheck(x));
  assert(notmycheck(x));
}
```
I'd expect the check to not warn regardless of `MYDEFINE` presence.


https://reviews.llvm.org/D7375



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


r320579 - [Hexagon] Add front-end support for Hexagon V65

2017-12-13 Thread Krzysztof Parzyszek via cfe-commits
Author: kparzysz
Date: Wed Dec 13 05:48:07 2017
New Revision: 320579

URL: http://llvm.org/viewvc/llvm-project?rev=320579&view=rev
Log:
[Hexagon] Add front-end support for Hexagon V65

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Basic/Targets/Hexagon.cpp
cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
cfe/trunk/test/Driver/hexagon-hvx.c
cfe/trunk/test/Driver/hexagon-toolchain-elf.c
cfe/trunk/test/Preprocessor/hexagon-predefines.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=320579&r1=320578&r2=320579&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Wed Dec 13 05:48:07 2017
@@ -2407,6 +2407,8 @@ def mv60 : Flag<["-"], "mv60">, Group, AliasArgs<["hexagonv60"]>;
 def mv62 : Flag<["-"], "mv62">, Group,
Alias, AliasArgs<["hexagonv62"]>;
+def mv65 : Flag<["-"], "mv65">, Group,
+   Alias, AliasArgs<["hexagonv65"]>;
 def mhexagon_hvx : Flag<[ "-" ], "mhvx">,
Group,
HelpText<"Enable Hexagon Vector eXtensions">;

Modified: cfe/trunk/lib/Basic/Targets/Hexagon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/Hexagon.cpp?rev=320579&r1=320578&r2=320579&view=diff
==
--- cfe/trunk/lib/Basic/Targets/Hexagon.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/Hexagon.cpp Wed Dec 13 05:48:07 2017
@@ -52,6 +52,9 @@ void HexagonTargetInfo::getTargetDefines
   } else if (CPU == "hexagonv62") {
 Builder.defineMacro("__HEXAGON_V62__");
 Builder.defineMacro("__HEXAGON_ARCH__", "62");
+  } else if (CPU == "hexagonv65") {
+Builder.defineMacro("__HEXAGON_V65__");
+Builder.defineMacro("__HEXAGON_ARCH__", "65");
   }
 
   if (hasFeature("hvx-length64b")) {
@@ -145,6 +148,7 @@ const char *HexagonTargetInfo::getHexago
   .Case("hexagonv55", "55")
   .Case("hexagonv60", "60")
   .Case("hexagonv62", "62")
+  .Case("hexagonv65", "65")
   .Default(nullptr);
 }
 

Modified: cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp?rev=320579&r1=320578&r2=320579&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp Wed Dec 13 05:48:07 2017
@@ -32,6 +32,7 @@ static StringRef getDefaultHvxLength(Str
   return llvm::StringSwitch(Cpu)
   .Case("v60", "64b")
   .Case("v62", "64b")
+  .Case("v65", "64b")
   .Default("128b");
 }
 

Modified: cfe/trunk/test/Driver/hexagon-hvx.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hexagon-hvx.c?rev=320579&r1=320578&r2=320579&view=diff
==
--- cfe/trunk/test/Driver/hexagon-hvx.c (original)
+++ cfe/trunk/test/Driver/hexagon-hvx.c Wed Dec 13 05:48:07 2017
@@ -2,18 +2,31 @@
 // Tests for the hvx features and warnings.
 // 
-
 
+// RUN: %clang -c %s -### -target hexagon-unknown-elf -mv65 -mhvx \
+// RUN:  2>&1 | FileCheck -check-prefix=CHECKHVX165 %s
+// CHECKHVX165: "-target-feature" "+hvxv65"
+
 // RUN: %clang -c %s -### -target hexagon-unknown-elf -mv62 -mhvx \
 // RUN:  2>&1 | FileCheck -check-prefix=CHECKHVX162 %s
 // CHECKHVX162: "-target-feature" "+hvxv62"
 
+// RUN: %clang -c %s -### -target hexagon-unknown-elf -mv65 -mhvx \
+// RUN:  -mhvx-double 2>&1 | FileCheck -check-prefix=CHECKHVX2 %s
+
 // RUN: %clang -c %s -### -target hexagon-unknown-elf -mv62 -mhvx \
 // RUN:  -mhvx-double 2>&1 | FileCheck -check-prefix=CHECKHVX2 %s
 
+// RUN: %clang -c %s -### -target hexagon-unknown-elf -mv65 -mhvx \
+// RUN:  -mhvx-length=128B 2>&1 | FileCheck -check-prefix=CHECKHVX2 %s
+
 // RUN: %clang -c %s -### -target hexagon-unknown-elf -mv62 -mhvx \
 // RUN:  -mhvx-length=128B 2>&1 | FileCheck -check-prefix=CHECKHVX2 %s
 // CHECKHVX2-NOT: "-target-feature" "+hvx-length64b"
 // CHECKHVX2: "-target-feature" "+hvx-length128b"
 
+// RUN: %clang -c %s -### -target hexagon-unknown-elf -mv65 2>&1 \
+// RUN:  | FileCheck -check-prefix=CHECKHVX3 %s
+
 // RUN: %clang -c %s -### -target hexagon-unknown-elf -mv62 2>&1 \
 // RUN:  | FileCheck -check-prefix=CHECKHVX3 %s
 // CHECKHVX3-NOT: "-target-feature" "+hvx

Modified: cfe/trunk/test/Driver/hexagon-toolchain-elf.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hexagon-toolchain-elf.c?rev=320579&r1=320578&r2=320579&view=diff
==
--- cfe/trunk/test/Driver/hexagon-toolchain-elf.c (original)
+++ cfe/trunk/test/Driver/hexagon-toolchain-elf

[PATCH] D40488: [clangd] Implemented tracing using Context

2017-12-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 126748.
ilya-biryukov added a comment.

Merged with head


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40488

Files:
  clangd/ClangdUnit.cpp
  clangd/JSONRPCDispatcher.cpp
  clangd/Trace.cpp
  clangd/Trace.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/TraceTests.cpp

Index: unittests/clangd/TraceTests.cpp
===
--- unittests/clangd/TraceTests.cpp
+++ unittests/clangd/TraceTests.cpp
@@ -7,6 +7,7 @@
 //
 //===--===//
 
+#include "Context.h"
 #include "Trace.h"
 
 #include "llvm/ADT/DenseMap.h"
@@ -74,10 +75,11 @@
   std::string JSON;
   {
 raw_string_ostream OS(JSON);
-auto Session = trace::Session::create(OS);
+auto JSONTracer = trace::createJSONTracer(OS);
+trace::TracingSession Session(*JSONTracer);
 {
-  trace::Span S("A");
-  trace::log("B");
+  trace::Span S(Context::empty(), "A");
+  trace::log(Context::empty(), "B");
 }
   }
 
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -115,19 +115,25 @@
<< EC.message();
 }
   }
+
+  // Setup tracing facilities.
   llvm::Optional TraceStream;
-  std::unique_ptr TraceSession;
+  std::unique_ptr Tracer;
   if (!TraceFile.empty()) {
 std::error_code EC;
 TraceStream.emplace(TraceFile, /*ref*/ EC, llvm::sys::fs::F_RW);
 if (EC) {
   TraceFile.reset();
   llvm::errs() << "Error while opening trace file: " << EC.message();
 } else {
-  TraceSession = trace::Session::create(*TraceStream, PrettyPrint);
+  Tracer = trace::createJSONTracer(*TraceStream, PrettyPrint);
 }
   }
 
+  llvm::Optional TracingSession;
+  if (Tracer)
+TracingSession.emplace(*Tracer);
+
   llvm::raw_ostream &Outs = llvm::outs();
   llvm::raw_ostream &Logs = llvm::errs();
   JSONOutput Out(Outs, Logs,
Index: clangd/Trace.h
===
--- clangd/Trace.h
+++ clangd/Trace.h
@@ -8,60 +8,74 @@
 //===--===//
 //
 // Supports writing performance traces describing clangd's behavior.
-// Traces are written in the Trace Event format supported by chrome's trace
-// viewer (chrome://tracing).
+// Traces are consumed by implementations of the EventTracer interface.
 //
-// The format is documented here:
-// https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview
 //
 // All APIs are no-ops unless a Session is active (created by ClangdMain).
 //
 //===--===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_
 
+#include "Context.h"
 #include "JSONExpr.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
 namespace clangd {
 namespace trace {
 
-// A session directs the output of trace events. Only one Session can exist.
-// It should be created before clangd threads are spawned, and destroyed after
-// they exit.
-// TODO: we may want to add pluggable support for other tracing backends.
-class Session {
+/// A consumer of trace events. The events are produced by Spans and trace::log.
+class EventTracer {
 public:
-  // Starts a sessions capturing trace events and writing Trace Event JSON.
-  static std::unique_ptr create(llvm::raw_ostream &OS,
- bool Pretty = false);
-  ~Session();
+  virtual ~EventTracer() = default;
+  /// Consume a trace event.
+  virtual void event(const Context &Ctx, llvm::StringRef Phase,
+ json::obj &&Contents) = 0;
+};
 
-private:
-  Session() = default;
+/// Sets up a global EventTracer that consumes events produced by Span and
+/// trace::log. Only one TracingSession can be active at a time and it should be
+/// set up before calling any clangd-specific functions.
+class TracingSession {
+public:
+  TracingSession(EventTracer &Tracer);
+  ~TracingSession();
 };
 
-// Records a single instant event, associated with the current thread.
-void log(const llvm::Twine &Name);
+/// Create an instance of EventTracer that produces an output in the Trace Event
+/// format supported by Chrome's trace viewer (chrome://tracing).
+///
+/// The format is documented here:
+/// https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview
+///
+/// The implementation supports concurrent calls and can be used as a global
+/// tracer (i.e., can be put into a global Context).
+std::unique_ptr createJSONTracer(llvm::raw_ostream &OS,
+  bool Pretty = false);
 
-// Records an event whose duration is the lifetime of the Span object.
-//
-// Arbitrary JSON metadata 

[PATCH] D41179: [Sema] Diagnose template specializations with C linkage

2017-12-13 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki created this revision.
miyuki added reviewers: faisalv, rsmith, rogfer01.

[Sema] Diagnose template specializations with C linkage

According to the C++ standard, templates as well as their
specializations cannot have C language linkage. Clang currently does
not diagnose function template specializations and class template
specializations with C linkage.

This patch implements such a diagnostic and adds tests for it.


https://reviews.llvm.org/D41179

Files:
  lib/Sema/SemaTemplate.cpp
  test/SemaTemplate/class-template-spec.cpp
  test/SemaTemplate/function-template-specialization.cpp


Index: test/SemaTemplate/function-template-specialization.cpp
===
--- test/SemaTemplate/function-template-specialization.cpp
+++ test/SemaTemplate/function-template-specialization.cpp
@@ -56,3 +56,8 @@
   template<>
   static void Bar(const long& input) {}  // expected-error{{explicit 
specialization of 'Bar' in class scope}}
 };
+
+template void f3(T) {}
+extern "C" { // expected-note {{extern "C" language linkage specification 
begins here}}
+  template<> void f3(int) {} // expected-error{{templates must have C++ 
linkage}}
+}
Index: test/SemaTemplate/class-template-spec.cpp
===
--- test/SemaTemplate/class-template-spec.cpp
+++ test/SemaTemplate/class-template-spec.cpp
@@ -235,3 +235,8 @@
   > struct S;
   template struct S {}; // expected-error {{non-type template 
argument specializes a template parameter with dependent type 'T'}}
 }
+
+template struct C {};
+extern "C" { // expected-note {{extern "C" language linkage specification 
begins here}}
+  template<> struct C {}; // expected-error{{templates must have C++ 
linkage}}
+}
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -7017,6 +7017,16 @@
 return true;
   }
 
+  // C++ [temp]p6:
+  //   A template, a template explicit specialization, and a class template
+  //   partial specialization shall not have C linkage.
+  if (S.CurContext->isExternCContext()) {
+S.Diag(Loc, diag::err_template_linkage);
+if (const LinkageSpecDecl *LSD = S.CurContext->getExternCContext())
+  S.Diag(LSD->getExternLoc(), diag::note_extern_c_begins_here);
+return true;
+  }
+
   // C++ [temp.expl.spec]p2:
   //   An explicit specialization shall be declared in the namespace
   //   of which the template is a member, or, for member templates, in


Index: test/SemaTemplate/function-template-specialization.cpp
===
--- test/SemaTemplate/function-template-specialization.cpp
+++ test/SemaTemplate/function-template-specialization.cpp
@@ -56,3 +56,8 @@
   template<>
   static void Bar(const long& input) {}  // expected-error{{explicit specialization of 'Bar' in class scope}}
 };
+
+template void f3(T) {}
+extern "C" { // expected-note {{extern "C" language linkage specification begins here}}
+  template<> void f3(int) {} // expected-error{{templates must have C++ linkage}}
+}
Index: test/SemaTemplate/class-template-spec.cpp
===
--- test/SemaTemplate/class-template-spec.cpp
+++ test/SemaTemplate/class-template-spec.cpp
@@ -235,3 +235,8 @@
   > struct S;
   template struct S {}; // expected-error {{non-type template argument specializes a template parameter with dependent type 'T'}}
 }
+
+template struct C {};
+extern "C" { // expected-note {{extern "C" language linkage specification begins here}}
+  template<> struct C {}; // expected-error{{templates must have C++ linkage}}
+}
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -7017,6 +7017,16 @@
 return true;
   }
 
+  // C++ [temp]p6:
+  //   A template, a template explicit specialization, and a class template
+  //   partial specialization shall not have C linkage.
+  if (S.CurContext->isExternCContext()) {
+S.Diag(Loc, diag::err_template_linkage);
+if (const LinkageSpecDecl *LSD = S.CurContext->getExternCContext())
+  S.Diag(LSD->getExternLoc(), diag::note_extern_c_begins_here);
+return true;
+  }
+
   // C++ [temp.expl.spec]p2:
   //   An explicit specialization shall be declared in the namespace
   //   of which the template is a member, or, for member templates, in
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D41178: [clangd] Construct SymbolSlab from YAML format.

2017-12-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Nice! just nits




Comment at: clangd/index/SymbolFromYAML.cpp:32
+// supported in YAML I/O.
+struct NPArray {
+  NPArray(IO &) {}

what does NP stand for?



Comment at: clangd/index/SymbolFromYAML.cpp:44
+
+  std::vector Value;
+};

Nit: for readability, I suggest you encode as a hex string instead.

We don't yet have operator<< for SymbolID, but I think we should, and it should 
be consistent.
So preferably define `operator<<` for SymbolID, and use it here, rather than 
just defining everything here.



Comment at: clangd/index/SymbolFromYAML.h:1
+//===--- SymbolFromYAML.h *- 
C++-*-===//
+//

nit: `SymbolFromYAML.h` seems slightly off for the file, because you support 
both conversions (and also multiple symbols)

Consider just `YAML.h`?



Comment at: clangd/index/SymbolFromYAML.h:8
+//
+//===--===//
+

File comment, describing what's going on here, use cases.

In particular, I'd note that the format is designed for simplicity but isn't a 
suitable/efficient store and isn't intended for real use by end-users.



Comment at: clangd/index/SymbolFromYAML.h:23
+// Convert symbols to a YAML-format string.
+std::string SymbolToYAML(const SymbolSlab& Symbols);
+

We may have multiple slabs, e.g. if we want to dump the dynamic index.
If we dump each slab and concatenate the resulting strings, is the result valid 
YAML (containing all symbols)?
If so, then this interface is good, but it may be worth a comment `// The YAML 
is safe to concatenate if you have multiple slabs`.



Comment at: unittests/clangd/SymbolCollectorTests.cpp:108
+
+  std::string YAMLContent = SymbolToYAML(Symbols);
+  auto SymbolsReadFromYAML = SymbolFromYAML(YAMLContent);

round-trip is good to test the serialization, but can you add a separate test 
case loading a simple symbol from an actual YAML string?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41178



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


[PATCH] D39937: [Sema] Improve diagnostics for const- and ref-qualified member functions

2017-12-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

You're set to commit (if there are concerns, @rsmith can raise them 
post-commit). Do you need me to commit on your behalf?


https://reviews.llvm.org/D39937



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


[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

2017-12-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In https://reviews.llvm.org/D40671#953291, @xgsa wrote:

> @aaron.ballman, sorry for my insistence, but it seems all the comments are 
> fixed and the patch is ready for commit or am I missing something? Could you 
> please commit it on my behalf, as I don't have rights to do that?


The check now LGTM, but I am going to wait to commit in case @alexfh has 
concerns regarding unknown check names.

FWIW, I think we should do something about unknown check names in NOLINT 
comments, but that can be done as a follow-up patch. If we're ignoring the 
comment, we might want to diagnose that fact so users have an idea what's going 
on.


https://reviews.llvm.org/D40671



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


r320590 - [OPENMP] Fix handling of clauses in clause parsing mode.

2017-12-13 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Wed Dec 13 07:28:44 2017
New Revision: 320590

URL: http://llvm.org/viewvc/llvm-project?rev=320590&view=rev
Log:
[OPENMP] Fix handling of clauses in clause parsing mode.

The compiler may generate incorrect code if we try to capture the
variable in clause parsing mode.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/distribute_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/distribute_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/for_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/sections_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/single_firstprivate_codegen.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=320590&r1=320589&r2=320590&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Wed Dec 13 07:28:44 2017
@@ -178,6 +178,10 @@ public:
   explicit DSAStackTy(Sema &S) : SemaRef(S) {}
 
   bool isClauseParsingMode() const { return ClauseKindMode != OMPC_unknown; }
+  OpenMPClauseKind getClauseParsingMode() const {
+assert(isClauseParsingMode() && "Must be in clause parsing mode.");
+return ClauseKindMode;
+  }
   void setClauseParsingMode(OpenMPClauseKind K) { ClauseKindMode = K; }
 
   bool isForceVarCapturing() const { return ForceCapturing; }
@@ -1102,8 +1106,6 @@ DSAStackTy::DSAVarData DSAStackTy::hasIn
 bool DSAStackTy::hasExplicitDSA(
 ValueDecl *D, const llvm::function_ref &CPred,
 unsigned Level, bool NotLastprivate) {
-  if (CPred(ClauseKindMode))
-return true;
   if (isStackEmpty())
 return false;
   D = getCanonicalDecl(D);
@@ -1367,6 +1369,8 @@ bool Sema::isOpenMPPrivateDecl(ValueDecl
   return DSAStack->hasExplicitDSA(
  D, [](OpenMPClauseKind K) -> bool { return K == OMPC_private; },
  Level) ||
+ (DSAStack->isClauseParsingMode() &&
+  DSAStack->getClauseParsingMode() == OMPC_private) ||
  // Consider taskgroup reduction descriptor variable a private to avoid
  // possible capture in the region.
  (DSAStack->hasExplicitDirective(

Modified: cfe/trunk/test/OpenMP/distribute_firstprivate_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/distribute_firstprivate_codegen.cpp?rev=320590&r1=320589&r2=320590&view=diff
==
--- cfe/trunk/test/OpenMP/distribute_firstprivate_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/distribute_firstprivate_codegen.cpp Wed Dec 13 
07:28:44 2017
@@ -63,16 +63,13 @@ int main() {
 #pragma omp teams
 #pragma omp distribute firstprivate(g, g1, svar, sfvar)
 for (int i = 0; i < 2; ++i) {
-  // LAMBDA-64: define{{.*}} internal{{.*}} void [[OMP_OUTLINED]](i32* 
noalias %{{.+}}, i32* noalias %{{.+}}, i{{[0-9]+}} [[G_IN:%.+]], i{{[0-9]+}} 
[[G1_IN:%.+]], i{{[0-9]+}} [[SVAR_IN:%.+]], i{{[0-9]+}} [[SFVAR_IN:%.+]])
-  // LAMBDA-32: define internal{{.*}} void [[OMP_OUTLINED]](i32* noalias 
%{{.+}}, i32* noalias %{{.+}}, double*{{.*}} [[G_IN:%.+]], i{{[0-9]+}} 
[[G1_IN:%.+]], i{{[0-9]+}} [[SVAR_IN:%.+]], i{{[0-9]+}} [[SFVAR_IN:%.+]])
+  // LAMBDA: define internal{{.*}} void [[OMP_OUTLINED]](i32* noalias 
%{{.+}}, i32* noalias %{{.+}}, double*{{.*}} [[G_IN:%.+]], double*{{.*}} 
[[G1_IN:%.+]], i32*{{.*}} [[SVAR_IN:%.+]], float*{{.*}} [[SFVAR_IN:%.+]])
   // Private alloca's for conversion
-  // LAMBDA-64: [[G_ADDR:%.+]] = alloca i{{[0-9]+}},
-  // LAMBDA-32: [[G_ADDR:%.+]] = alloca double*,
-  // LAMBDA: [[G1_ADDR:%.+]] = alloca i{{[0-9]+}},
-  // LAMBDA: [[SVAR_ADDR:%.+]] = alloca i{{[0-9]+}},
-  // LAMBDA: [[SFVAR_ADDR:%.+]] = alloca i{{[0-9]+}},
+  // LAMBDA: [[G_ADDR:%.+]] = alloca double*,
+  // LAMBDA: [[G1_ADDR:%.+]] = alloca double*,
+  // LAMBDA: [[SVAR_ADDR:%.+]] = alloca i{{[0-9]+}}*,
+  // LAMBDA: [[SFVAR_ADDR:%.+]] = alloca float*,
   // LAMBDA: [[G1_REF:%.+]] = alloca double*,
-  // LAMBDA: [[TMP:%.+]] = alloca double*,
 
   // Actual private variables to be used in the body (tmp is used for the 
reference type)
   // LAMBDA: [[G_PRIVATE:%.+]] = alloca double,
@@ -82,31 +79,25 @@ int main() {
   // LAMBDA: [[SFVAR_PRIVATE:%.+]] = alloca float,
 
   // Store input parameter addresses into private alloca's for conversion
-  // LAMBDA-64: store i{{[0-9]+}} [[G_IN]], i{{[0-9]+}}* [[G_ADDR]],
-  // LAMBDA-32: store double* [[G_IN]], double** [[G_ADDR]],
-  // LAMBDA: store i{{[0-9]+}} [[G1_IN]], i{{[0-9]+}}* [[G1_ADDR]],
-  // LAMBDA: store i{{[0-9]+}} [[SVAR_IN]], i{{[0-9]+}}* [[SVAR_ADDR]],
-  // LAMBDA: store i{{[0-9]+}} [[SFVAR_IN]], i{{[0-9]+}}* [[SFVAR_ADD

[PATCH] D36892: [clang-tidy] check_clang_tidy.py: support CHECK-NOTES prefix

2017-12-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

I think you're set to commit this -- if @alexfh has concerns, they can be 
addressed post-commit.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D36892



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


[PATCH] D40707: [libcxx] Fix basic_stringbuf constructor

2017-12-13 Thread 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.

LGTM.  Do you need someone to commit it?


https://reviews.llvm.org/D40707



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


[PATCH] D40707: [libcxx] Fix basic_stringbuf constructor

2017-12-13 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki added a comment.

Yes, I don't have write access.


https://reviews.llvm.org/D40707



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


[clang-tools-extra] r320591 - [clangd] Fix bool conversion operator of UniqueFunction

2017-12-13 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov
Date: Wed Dec 13 07:42:59 2017
New Revision: 320591

URL: http://llvm.org/viewvc/llvm-project?rev=320591&view=rev
Log:
[clangd] Fix bool conversion operator of UniqueFunction

Usages of it were giving compiler errors because of the missing
explicit conversion.

Modified:
clang-tools-extra/trunk/clangd/Function.h

Modified: clang-tools-extra/trunk/clangd/Function.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Function.h?rev=320591&r1=320590&r2=320591&view=diff
==
--- clang-tools-extra/trunk/clangd/Function.h (original)
+++ clang-tools-extra/trunk/clangd/Function.h Wed Dec 13 07:42:59 2017
@@ -49,7 +49,7 @@ public:
 FunctionCallImpl::type>>(
 std::forward(Func))) {}
 
-  operator bool() { return CallablePtr; }
+  operator bool() { return bool(CallablePtr); }
 
   Ret operator()(Args... As) {
 assert(CallablePtr);


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


[PATCH] D40489: [clangd] Changed tracing interfaces

2017-12-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 126762.
ilya-biryukov marked 3 inline comments as done.
ilya-biryukov added a comment.

- Changed the EventTracer interface to return a callback from beginEvent 
instead of using begin_event/end_event method pairs.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489

Files:
  clangd/Trace.cpp
  clangd/Trace.h

Index: clangd/Trace.h
===
--- clangd/Trace.h
+++ clangd/Trace.h
@@ -19,6 +19,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_
 
 #include "Context.h"
+#include "Function.h"
 #include "JSONExpr.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/raw_ostream.h"
@@ -28,12 +29,24 @@
 namespace trace {
 
 /// A consumer of trace events. The events are produced by Spans and trace::log.
+/// Implmentations of this interface must be thread-safe.
 class EventTracer {
 public:
+  /// A callback executed when an event with duration ends. Args represent data
+  /// that was attached to the event via SPAN_ATTACH.
+  using EndEventCallback = UniqueFunction;
+
   virtual ~EventTracer() = default;
-  /// Consume a trace event.
-  virtual void event(const Context &Ctx, llvm::StringRef Phase,
- json::obj &&Contents) = 0;
+
+  /// Called when event that has a duration starts. The returned callback will
+  /// be executed when the event ends. \p Name is a descriptive name
+  /// of the event that was passed to Span constructor.
+  virtual EndEventCallback beginSpan(const Context &Ctx,
+ llvm::StringRef Name) = 0;
+
+  /// Called for instant events.
+  virtual void instant(const Context &Ctx, llvm::StringRef Name,
+   json::obj &&Args) = 0;
 };
 
 /// Sets up a global EventTracer that consumes events produced by Span and
@@ -50,9 +63,6 @@
 ///
 /// The format is documented here:
 /// https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview
-///
-/// The implementation supports concurrent calls and can be used as a global
-/// tracer (i.e., can be put into a global Context).
 std::unique_ptr createJSONTracer(llvm::raw_ostream &OS,
   bool Pretty = false);
 
@@ -67,16 +77,16 @@
 /// SomeJSONExpr is evaluated and copied only if actually needed.
 class Span {
 public:
-  Span(const Context &Ctx, std::string Name);
+  Span(const Context &Ctx, llvm::StringRef Name);
   ~Span();
 
   /// Returns mutable span metadata if this span is interested.
   /// Prefer to use SPAN_ATTACH rather than accessing this directly.
   json::obj *args() { return Args.get(); }
 
 private:
-  llvm::Optional Ctx;
   std::unique_ptr Args;
+  EventTracer::EndEventCallback Callback;
 };
 
 #define SPAN_ATTACH(S, Name, Expr) \
Index: clangd/Trace.cpp
===
--- clangd/Trace.cpp
+++ clangd/Trace.cpp
@@ -44,10 +44,24 @@
 Out.flush();
   }
 
+  EndEventCallback beginSpan(const Context &Ctx,
+ llvm::StringRef Name) override {
+jsonEvent("B", json::obj{{"name", Name}});
+
+// The callback that will run when event ends.
+return [this](json::Expr &&Args) {
+  jsonEvent("E", json::obj{{"args", std::move(Args)}});
+};
+  }
+
+  void instant(const Context &Ctx, llvm::StringRef Name,
+   json::obj &&Args) override {
+jsonEvent("i", json::obj{{"name", Name}, {"args", std::move(Args)}});
+  }
+
   // Record an event on the current thread. ph, pid, tid, ts are set.
   // Contents must be a list of the other JSON key/values.
-  void event(const Context &Ctx, StringRef Phase,
- json::obj &&Contents) override {
+  void jsonEvent(StringRef Phase, json::obj &&Contents) {
 uint64_t TID = get_threadid();
 std::lock_guard Lock(Mu);
 // If we haven't already, emit metadata describing this thread.
@@ -109,30 +123,26 @@
 void log(const Context &Ctx, const Twine &Message) {
   if (!T)
 return;
-  T->event(Ctx, "i",
-   json::obj{
-   {"name", "Log"},
-   {"args", json::obj{{"Message", Message.str()}}},
-   });
+  T->instant(Ctx, "Log", json::obj{{"Message", Message.str()}});
 }
 
-Span::Span(const Context &Ctx, std::string Name) {
+Span::Span(const Context &Ctx, llvm::StringRef Name) {
   if (!T)
 return;
-  // Clone the context, so that the original Context can be moved.
-  this->Ctx.emplace(Ctx.clone());
 
-  T->event(*this->Ctx, "B", json::obj{{"name", std::move(Name)}});
+  Callback = T->beginSpan(Ctx, Name);
+  if (!Callback)
+return;
+
   Args = llvm::make_unique();
 }
 
 Span::~Span() {
-  if (!T)
+  if (!Callback)
 return;
-  if (!Args)
-Args = llvm::make_unique();
-  T->event(*Ctx, "E",
-   Args ? json::obj{{"args", std::move(*Args)}} : json::obj{});
+
+  assert(Args && "Args must be non-null if Callback is defined");
+  Callback

[PATCH] D40489: [clangd] Changed tracing interfaces

2017-12-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clangd/Trace.h:39
+  /// Called when event with \p Name starts.
+  virtual void begin_event(const ContextData &Ctx, llvm::StringRef Name) = 0;
+  /// Called when event with \p Name ends.

sammccall wrote:
> just call this begin?
> Otherwise style is `beginEvent` I think
The new name is `beginSpan`. Hope that sounds good.



Comment at: clangd/Trace.h:41
+  /// Called when event with \p Name ends.
+  virtual void end_event(const ContextData &Ctx, llvm::StringRef Name,
+ json::obj &&Args) = 0;

sammccall wrote:
> How is identity represented between begin/end event? via Name doesn't seem 
> robust, so why the need to pass it twice? Does providing different contexts 
> for start/end make sense?
> 
> It might be cleaner/more flexible to have
> 
> std::function begin()
> and call the return value to signal end.
> 
> This seems likely to be pretty easy for different providers to implement, and 
> is easy to use from Span.
Done. As discussed offline, the interface you propose seems much nicer.
Used names `beginSpan` and `instant`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489



___
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/function pointer

2017-12-13 Thread Dan Zimmerman via Phabricator via cfe-commits
danzimm updated this revision to Diff 126763.
danzimm added a comment.

Change tests to use non-O2 generated IR. It looks like the combined 
objc_retainAutoreleasedReturnValue/objc_autoreleaseReturnValue calls annihilate 
each other and we just get a call/ret.


Repository:
  rC Clang

https://reviews.llvm.org/D41050

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenObjCXX/arc-forwarded-lambda-call.mm


Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm -fblocks 
-fobjc-arc -fobjc-runtime-has-weak -std=c++11 -o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: [[T0:%.*]] = call i8* 
@"_ZZ5test0P11objc_objectENK3$_0clEv"({{%.*}}* {{%.*}})
+  // CHECK-NEXT: ret i8* [[T0]]
+}
+
+id test1_rv;
+
+void test1() {
+  extern void test1_helper(id (*)(void));
+  test1_helper([](){ return test1_rv; });
+  // CHECK-LABEL: define internal i8* @"_ZZ5test1vEN3$_18__invokeEv"
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test1vENK3$_1clEv"({{%.*}}* undef)
+  // CHECK-NEXT: ret i8* [[T0]]
+}
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2776,9 +2776,12 @@
   RValue RV = EmitCall(calleeFnInfo, callee, returnSlot, callArgs);
 
   // If necessary, copy the returned value into the slot.
-  if (!resultType->isVoidType() && returnSlot.isNull())
+  if (!resultType->isVoidType() && returnSlot.isNull()) {
+if (getLangOpts().ObjCAutoRefCount && resultType->isObjCRetainableType()) {
+  RV = 
RValue::get(EmitARCRetainAutoreleasedReturnValue(RV.getScalarVal()));
+}
 EmitReturnOfRValue(RV, resultType);
-  else
+  } else
 EmitBranchThroughCleanup(ReturnBlock);
 }
 


Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm -fblocks -fobjc-arc -fobjc-runtime-has-weak -std=c++11 -o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test0P11objc_objectENK3$_0clEv"({{%.*}}* {{%.*}})
+  // CHECK-NEXT: ret i8* [[T0]]
+}
+
+id test1_rv;
+
+void test1() {
+  extern void test1_helper(id (*)(void));
+  test1_helper([](){ return test1_rv; });
+  // CHECK-LABEL: define internal i8* @"_ZZ5test1vEN3$_18__invokeEv"
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test1vENK3$_1clEv"({{%.*}}* undef)
+  // CHECK-NEXT: ret i8* [[T0]]
+}
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2776,9 +2776,12 @@
   RValue RV = EmitCall(calleeFnInfo, callee, returnSlot, callArgs);
 
   // If necessary, copy the returned value into the slot.
-  if (!resultType->isVoidType() && returnSlot.isNull())
+  if (!resultType->isVoidType() && returnSlot.isNull()) {
+if (getLangOpts().ObjCAutoRefCount && resultType->isObjCRetainableType()) {
+  RV = RValue::get(EmitARCRetainAutoreleasedReturnValue(RV.getScalarVal()));
+}
 EmitReturnOfRValue(RV, resultType);
-  else
+  } else
 EmitBranchThroughCleanup(ReturnBlock);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

2017-12-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:1809
   bool DuplicateArchitecture = false;
+  bool operator==(const ParsedTargetAttr &Other) {
+return DuplicateArchitecture == Other.DuplicateArchitecture &&

This function should be marked `const`.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9333-9335
+  "multiversion function would have identical mangling to a previous "
+  "definition.  Duplicate declarations must have identical target attribute "
+  "values">;

Diagnostics are not complete sentences, so this should be reworded to be even 
less grammatically correct. ;-)



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9339-9340
+def err_multiversion_no_other_attrs : Error<
+  "attribute 'target' multiversioning cannot be combined with other "
+  "attributes">;
+def err_multiversion_diff : Error<

This worries me slightly. Is there a benefit to prohibiting this attribute with 
any other attribute? For instance, I'm thinking about a multiversioned noreturn 
function or a multiversioned function with a calling convention as plausible 
use cases.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9349
+def err_multiversion_not_allowed_on_main : Error<
+  "function multiversion not permitted on 'main'">;
+

How about `'main' cannot be a multiversion function`?



Comment at: lib/CodeGen/CodeGenFunction.cpp:2307-2308
+SmallVector FeatureList;
+std::for_each(std::begin(RO.ParsedAttribute.Features),
+  std::end(RO.ParsedAttribute.Features),
+  [&FeatureList](const std::string &Feature) {

You can use `llvm::for_each` instead.



Comment at: lib/CodeGen/CodeGenFunction.h:3941
+Priority = std::max(Priority, TargInfo.multiVersionSortPriority(
+  StringRef{Feat}.substr(1)));
+

`Feat` is already a `StringRef`?



Comment at: lib/CodeGen/CodeGenModule.cpp:741
+  const auto &Target = CGM.getTarget();
+  auto CmpFunc = [&Target](StringRef LHS, StringRef RHS) {
+return Target.multiVersionSortPriority(LHS) >

Might as well sink this into the call to `std::sort()`.



Comment at: lib/Sema/SemaDecl.cpp:9298
+  TargetAttr::ParsedTargetAttr ParseInfo = TA->parse();
+  const auto &TargetInfo = S.Context.getTargetInfo();
+  enum ErrType { Feature = 0, Architecture = 1 };

Don't use `auto` here as the type is not explicitly spelled out in the 
initialization.



Comment at: lib/Sema/SemaDecl.cpp:9326
+
+static bool CheckMultiVersionAdditionalRules(Sema &S, const FunctionDecl 
*OldFD,
+ const FunctionDecl *NewFD,

This function uses quite a few magic numbers that might be better expressed 
with named values instead.



Comment at: lib/Sema/SemaDecl.cpp:9362-9364
+QualType NewQType = S.getASTContext().getCanonicalType(NewFD->getType());
+const FunctionType *NewType = cast(NewQType);
+QualType NewReturnType = NewType->getReturnType();

Formatting is off here.



Comment at: lib/Sema/SemaDecl.cpp:9363
+QualType NewQType = S.getASTContext().getCanonicalType(NewFD->getType());
+const FunctionType *NewType = cast(NewQType);
+QualType NewReturnType = NewType->getReturnType();

Can use `const auto *` here.



Comment at: lib/Sema/SemaDecl.cpp:9381
+QualType OldQType = S.getASTContext().getCanonicalType(OldFD->getType());
+const FunctionType *OldType = cast(OldQType);
+FunctionType::ExtInfo OldTypeInfo = OldType->getExtInfo();

Can use `const auto *` here.



Comment at: lib/Sema/SemaDecl.cpp:9448
+  if (NewFD->isMain()) {
+if (NewTA && NewTA->getFeaturesStr() == "default") {
+  S.Diag(NewFD->getLocation(), diag::err_multiversion_not_allowed_on_main);

So any other form of target attribute and feature string is fine to place on 
`main()`?



Comment at: lib/Sema/SemaDecl.cpp:9478
+
+  auto *OldFD = OldDecl->getAsFunction();
+  // Unresolved 'using' statements (the other way OldDecl can be not a 
function)

Do not use `auto` here.



Comment at: lib/Sema/SemaDecl.cpp:9494
+  TargetAttr::ParsedTargetAttr NewParsed = NewTA->parse();
+  // sort order doesn't matter, it just needs to be consistent.
+  std::sort(NewParsed.Features.begin(), NewParsed.Features.end());

Capitalization.



Comment at: lib/Sema/SemaDecl.cpp:9562
+  for (NamedDecl *ND : Previous) {
+auto *CurFD = ND->getAsFunction();
+if (!CurFD)

Do not use `auto` here.



Comment at: lib/Sema/SemaDecl

[PATCH] D36892: [clang-tidy] check_clang_tidy.py: support CHECK-NOTES prefix

2017-12-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In https://reviews.llvm.org/D36892#953891, @aaron.ballman wrote:

> I think you're set to commit this


Should i commit it though?
Until licensing concerns with https://reviews.llvm.org/D36836 are finally 
magically resolved and it is committed, there won't be any users, and since 
there is no tests for this, that would be a dead code with all the consequences.

> - if @alexfh has concerns, they can be addressed post-commit.




Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D36892



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


[PATCH] D40489: [clangd] Changed tracing interfaces

2017-12-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clangd/Trace.cpp:134
+  Callback = T->beginSpan(Ctx, Name);
+  if (!Callback)
+return;

I'm not sure this is useful. Tracers that aren't interested in args can't opt 
out by providing a null callback, unless they also don't care about the end 
time.
This seems unlikely (a null tracer, I guess)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489



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


[PATCH] D36892: [clang-tidy] check_clang_tidy.py: support CHECK-NOTES prefix

2017-12-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In https://reviews.llvm.org/D36892#953953, @lebedev.ri wrote:

> In https://reviews.llvm.org/D36892#953891, @aaron.ballman wrote:
>
> > I think you're set to commit this
>
>
> Should i commit it though?
>  Until licensing concerns with https://reviews.llvm.org/D36836 are finally 
> magically resolved and it is committed, there won't be any users, and since 
> there is no tests for this, that would be a dead code with all the 
> consequences.


Hmm, I thought we had other tests that could make use of this functionality? 
However, I might be misremembering. Hold off on committing until there's test 
coverage, but it'd be good to look at existing tests to see if any of them can 
be modified.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D36892



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


[PATCH] D41179: [Sema] Diagnose template specializations with C linkage

2017-12-13 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits
rogfer01 added inline comments.



Comment at: lib/Sema/SemaTemplate.cpp:7021
+  // C++ [temp]p6:
+  //   A template, a template explicit specialization, and a class template
+  //   partial specialization shall not have C linkage.

Can you add a test for a partial specialization? Your test for the class case 
only includes an explicit specialization.

```lang=cpp
template 
struct A { };

extern "C" { 

template// I'd expect a diagnostic around here
struct A
{
};

}
```


https://reviews.llvm.org/D41179



___
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/function pointer

2017-12-13 Thread Dan Zimmerman via Phabricator via cfe-commits
danzimm added a comment.

Ah, just found test/Transforms/ObjCARC/rv.ll test3:

  ; Delete a redundant retainRV,autoreleaseRV when forwaring a call result
  ; directly to a return value.
  
  ; CHECK-LABEL: define i8* @test3(
  ; CHECK: call i8* @returner()
  ; CHECK-NEXT: ret i8* %call
  define i8* @test3() {
  entry:
%call = call i8* @returner()
%0 = call i8* @objc_retainAutoreleasedReturnValue(i8* %call) nounwind
%1 = call i8* @objc_autoreleaseReturnValue(i8* %0) nounwind
ret i8* %1
  }


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] D39963: [RISCV] Add initial RISC-V target and driver support

2017-12-13 Thread Alex Bradbury via Phabricator via cfe-commits
asb updated this revision to Diff 126769.
asb edited the summary of this revision.
asb added a comment.

Update to add test cases based on the multilib Linux SDK produced by 
https://github.com/riscv/riscv-gnu-toolchain/.


https://reviews.llvm.org/D39963

Files:
  lib/Basic/CMakeLists.txt
  lib/Basic/Targets.cpp
  lib/Basic/Targets/RISCV.cpp
  lib/Basic/Targets/RISCV.h
  lib/Driver/CMakeLists.txt
  lib/Driver/ToolChains/Arch/RISCV.cpp
  lib/Driver/ToolChains/Arch/RISCV.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/Clang.h
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/ToolChains/Linux.cpp
  test/Driver/Inputs/multilib_riscv_linux_sdk/bin/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/include/.keep
  
test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/crtbegin.o
  
test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32/crtbegin.o
  
test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32d/crtbegin.o
  
test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib64/lp64/crtbegin.o
  
test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib64/lp64d/crtbegin.o
  test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/ld
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib32/ilp32/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib32/ilp32d/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib64/lp64/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib64/lp64d/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ilp32/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ilp32d/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/lp64/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/lp64d/.keep
  test/Driver/frame-pointer.c
  test/Driver/riscv-abi.c
  test/Driver/riscv-features.c
  test/Driver/riscv32-toolchain.c
  test/Driver/riscv64-toolchain.c
  test/Preprocessor/init.c

Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -10003,3 +10003,398 @@
 // ARM-DARWIN-BAREMETAL-64: #define __PTRDIFF_TYPE__ long int
 // ARM-DARWIN-BAREMETAL-64: #define __SIZE_TYPE__ long unsigned int
 
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=riscv32 < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefix=RISCV32 %s
+// RISCV32: #define _ILP32 1
+// RISCV32: #define __ATOMIC_ACQUIRE 2
+// RISCV32: #define __ATOMIC_ACQ_REL 4
+// RISCV32: #define __ATOMIC_CONSUME 1
+// RISCV32: #define __ATOMIC_RELAXED 0
+// RISCV32: #define __ATOMIC_RELEASE 3
+// RISCV32: #define __ATOMIC_SEQ_CST 5
+// RISCV32: #define __BIGGEST_ALIGNMENT__ 16
+// RISCV32: #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
+// RISCV32: #define __CHAR16_TYPE__ unsigned short
+// RISCV32: #define __CHAR32_TYPE__ unsigned int
+// RISCV32: #define __CHAR_BIT__ 8
+// RISCV32: #define __DBL_DECIMAL_DIG__ 17
+// RISCV32: #define __DBL_DENORM_MIN__ 4.9406564584124654e-324
+// RISCV32: #define __DBL_DIG__ 15
+// RISCV32: #define __DBL_EPSILON__ 2.2204460492503131e-16
+// RISCV32: #define __DBL_HAS_DENORM__ 1
+// RISCV32: #define __DBL_HAS_INFINITY__ 1
+// RISCV32: #define __DBL_HAS_QUIET_NAN__ 1
+// RISCV32: #define __DBL_MANT_DIG__ 53
+// RISCV32: #define __DBL_MAX_10_EXP__ 308
+// RISCV32: #define __DBL_MAX_EXP__ 1024
+// RISCV32: #define __DBL_MAX__ 1.7976931348623157e+308
+// RISCV32: #define __DBL_MIN_10_EXP__ (-307)
+// RISCV32: #define __DBL_MIN_EXP__ (-1021)
+// RISCV32: #define __DBL_MIN__ 2.2250738585072014e-308
+// RISCV32: #define __DECIMAL_DIG__ __LDBL_DECIMAL_DIG__
+// RISCV32: #define __ELF__ 1
+// RISCV32: #define __FINITE_MATH_ONLY__ 0
+// RISCV32: #define __FLT_DECIMAL_DIG__ 9
+// RISCV32: #define __FLT_DENORM_MIN__ 1.40129846e-45F
+// RISCV32: #define __FLT_DIG__ 6
+// RISCV32: #define __FLT_EPSILON__ 1.19209290e-7F
+// RISCV32: #define __FLT_EVAL_METHOD__ 0
+// RISCV32: #define __FLT_HAS_DENORM__ 1
+// RISCV32: #define __FLT_HAS_INFINITY__ 1
+// RISCV32: #define __FLT_HAS_QUIET_NAN__ 1
+// RISCV32: #define __FLT_MANT_DIG__ 24
+// RISCV32: #define __FLT_MAX_10_EXP__ 38
+// RISCV32: #define __FLT_MAX_EXP__ 128
+// RISCV32: #define __FLT_MAX__ 3.40282347e+38F
+// RISCV32: #define __FLT_MIN_10_EXP__ (-37)
+// RISCV32: #define __FLT_MIN_EXP__ (-125)
+// RISCV32: #define __FLT_MIN__ 1.17549435e-38F
+// RISCV32: #define __FLT_RADIX__ 2
+// RISCV32: #define __GCC_ATOMIC_BOOL_LOCK_FREE 1
+// RISCV32: #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 1
+// RISCV32: #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 1
+// RISCV32: #define __GCC_ATOMIC_CHAR_LOCK_FREE 1
+// RISCV32: #define __GCC_ATOMIC_INT_LOCK_FREE 1
+// RISCV32: #define __GCC_ATOMIC_LLONG_LOCK_FREE 1
+// RISCV32: #define __

[PATCH] D39963: [RISCV] Add initial RISC-V target and driver support

2017-12-13 Thread Alex Bradbury via Phabricator via cfe-commits
asb updated this revision to Diff 126771.
asb added a comment.

Apologies, the last version had a few lines of debug code left in.

I should say that this is to the best of my knowledge ready to merge (i.e. 
there are no outstanding flagged issues). Particularly now that the majority of 
the RV32I codegen patches are merged in LLVM, it would be really helpful to get 
this merged in to clang asap in order to make larger scale testing of the LLVM 
backend possible without out-of-tree patches.


https://reviews.llvm.org/D39963

Files:
  lib/Basic/CMakeLists.txt
  lib/Basic/Targets.cpp
  lib/Basic/Targets/RISCV.cpp
  lib/Basic/Targets/RISCV.h
  lib/Driver/CMakeLists.txt
  lib/Driver/ToolChains/Arch/RISCV.cpp
  lib/Driver/ToolChains/Arch/RISCV.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/Clang.h
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/ToolChains/Linux.cpp
  test/Driver/Inputs/multilib_riscv_linux_sdk/bin/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/include/.keep
  
test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/crtbegin.o
  
test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32/crtbegin.o
  
test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32d/crtbegin.o
  
test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib64/lp64/crtbegin.o
  
test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib64/lp64d/crtbegin.o
  test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/ld
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib32/ilp32/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib32/ilp32d/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib64/lp64/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib64/lp64d/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ilp32/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ilp32d/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/lp64/.keep
  test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/lp64d/.keep
  test/Driver/frame-pointer.c
  test/Driver/riscv-abi.c
  test/Driver/riscv-features.c
  test/Driver/riscv32-toolchain.c
  test/Driver/riscv64-toolchain.c
  test/Preprocessor/init.c

Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -10003,3 +10003,398 @@
 // ARM-DARWIN-BAREMETAL-64: #define __PTRDIFF_TYPE__ long int
 // ARM-DARWIN-BAREMETAL-64: #define __SIZE_TYPE__ long unsigned int
 
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=riscv32 < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefix=RISCV32 %s
+// RISCV32: #define _ILP32 1
+// RISCV32: #define __ATOMIC_ACQUIRE 2
+// RISCV32: #define __ATOMIC_ACQ_REL 4
+// RISCV32: #define __ATOMIC_CONSUME 1
+// RISCV32: #define __ATOMIC_RELAXED 0
+// RISCV32: #define __ATOMIC_RELEASE 3
+// RISCV32: #define __ATOMIC_SEQ_CST 5
+// RISCV32: #define __BIGGEST_ALIGNMENT__ 16
+// RISCV32: #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
+// RISCV32: #define __CHAR16_TYPE__ unsigned short
+// RISCV32: #define __CHAR32_TYPE__ unsigned int
+// RISCV32: #define __CHAR_BIT__ 8
+// RISCV32: #define __DBL_DECIMAL_DIG__ 17
+// RISCV32: #define __DBL_DENORM_MIN__ 4.9406564584124654e-324
+// RISCV32: #define __DBL_DIG__ 15
+// RISCV32: #define __DBL_EPSILON__ 2.2204460492503131e-16
+// RISCV32: #define __DBL_HAS_DENORM__ 1
+// RISCV32: #define __DBL_HAS_INFINITY__ 1
+// RISCV32: #define __DBL_HAS_QUIET_NAN__ 1
+// RISCV32: #define __DBL_MANT_DIG__ 53
+// RISCV32: #define __DBL_MAX_10_EXP__ 308
+// RISCV32: #define __DBL_MAX_EXP__ 1024
+// RISCV32: #define __DBL_MAX__ 1.7976931348623157e+308
+// RISCV32: #define __DBL_MIN_10_EXP__ (-307)
+// RISCV32: #define __DBL_MIN_EXP__ (-1021)
+// RISCV32: #define __DBL_MIN__ 2.2250738585072014e-308
+// RISCV32: #define __DECIMAL_DIG__ __LDBL_DECIMAL_DIG__
+// RISCV32: #define __ELF__ 1
+// RISCV32: #define __FINITE_MATH_ONLY__ 0
+// RISCV32: #define __FLT_DECIMAL_DIG__ 9
+// RISCV32: #define __FLT_DENORM_MIN__ 1.40129846e-45F
+// RISCV32: #define __FLT_DIG__ 6
+// RISCV32: #define __FLT_EPSILON__ 1.19209290e-7F
+// RISCV32: #define __FLT_EVAL_METHOD__ 0
+// RISCV32: #define __FLT_HAS_DENORM__ 1
+// RISCV32: #define __FLT_HAS_INFINITY__ 1
+// RISCV32: #define __FLT_HAS_QUIET_NAN__ 1
+// RISCV32: #define __FLT_MANT_DIG__ 24
+// RISCV32: #define __FLT_MAX_10_EXP__ 38
+// RISCV32: #define __FLT_MAX_EXP__ 128
+// RISCV32: #define __FLT_MAX__ 3.40282347e+38F
+// RISCV32: #define __FLT_MIN_10_EXP__ (-37)
+// RISCV32: #define __FLT_MIN_EXP__ (-125)
+// RISCV32: #define __FLT_MIN__ 1.17549435e-38F
+// RISCV32: #define __FLT_RADIX__ 2
+// RISCV32: #define __GCC_ATOMIC_BOOL_LOCK_FREE 1
+// RISCV32:

r320596 - [OPENMP] Support `reduction` clause on target-based directives.

2017-12-13 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Wed Dec 13 09:31:39 2017
New Revision: 320596

URL: http://llvm.org/viewvc/llvm-project?rev=320596&view=rev
Log:
[OPENMP] Support `reduction` clause on target-based directives.

OpenMP 5.0 added support for `reduction` clause in target-based
directives. Patch adds this support to clang.

Added:
cfe/trunk/test/OpenMP/target_reduction_codegen.cpp
cfe/trunk/test/OpenMP/target_reduction_messages.cpp
Modified:
cfe/trunk/include/clang/Basic/OpenMPKinds.def
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_reduction_codegen.cpp

Modified: cfe/trunk/include/clang/Basic/OpenMPKinds.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/OpenMPKinds.def?rev=320596&r1=320595&r2=320596&view=diff
==
--- cfe/trunk/include/clang/Basic/OpenMPKinds.def (original)
+++ cfe/trunk/include/clang/Basic/OpenMPKinds.def Wed Dec 13 09:31:39 2017
@@ -454,6 +454,7 @@ OPENMP_TARGET_CLAUSE(depend)
 OPENMP_TARGET_CLAUSE(defaultmap)
 OPENMP_TARGET_CLAUSE(firstprivate)
 OPENMP_TARGET_CLAUSE(is_device_ptr)
+OPENMP_TARGET_CLAUSE(reduction)
 
 // Clauses allowed for OpenMP directive 'target data'.
 OPENMP_TARGET_DATA_CLAUSE(if)

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=320596&r1=320595&r2=320596&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Wed Dec 13 09:31:39 2017
@@ -6037,6 +6037,8 @@ private:
 
   /// \brief Set of all first private variables in the current directive.
   llvm::SmallPtrSet FirstPrivateDecls;
+  /// Set of all reduction variables in the current directive.
+  llvm::SmallPtrSet ReductionDecls;
 
   /// Map between device pointer declarations and their expression components.
   /// The key value for declarations in 'this' is null.
@@ -6429,6 +6431,12 @@ private:
 if (FirstPrivateDecls.count(Cap.getCapturedVar()))
   return MappableExprsHandler::OMP_MAP_PRIVATE |
  MappableExprsHandler::OMP_MAP_TO;
+// Reduction variable  will use only the 'private ptr' and 'map to_from'
+// flag.
+if (ReductionDecls.count(Cap.getCapturedVar())) {
+  return MappableExprsHandler::OMP_MAP_TO |
+ MappableExprsHandler::OMP_MAP_FROM;
+}
 
 // We didn't modify anything.
 return CurrentModifiers;
@@ -6442,6 +6450,12 @@ public:
   for (const auto *D : C->varlists())
 FirstPrivateDecls.insert(
 
cast(cast(D)->getDecl())->getCanonicalDecl());
+for (const auto *C : Dir.getClausesOfKind()) {
+  for (const auto *D : C->varlists()) {
+ReductionDecls.insert(
+
cast(cast(D)->getDecl())->getCanonicalDecl());
+  }
+}
 // Extract device pointer clause information.
 for (const auto *C : Dir.getClausesOfKind())
   for (auto L : C->component_lists())
@@ -6721,15 +6735,9 @@ public:
   // The default map type for a scalar/complex type is 'to' because by
   // default the value doesn't have to be retrieved. For an aggregate
   // type, the default is 'tofrom'.
-  CurMapTypes.push_back(ElementType->isAggregateType()
-? (OMP_MAP_TO | OMP_MAP_FROM)
-: OMP_MAP_TO);
-
-  // If we have a capture by reference we may need to add the private
-  // pointer flag if the base declaration shows in some first-private
-  // clause.
-  CurMapTypes.back() =
-  adjustMapModifiersForPrivateClauses(CI, CurMapTypes.back());
+  CurMapTypes.emplace_back(adjustMapModifiersForPrivateClauses(
+  CI, ElementType->isAggregateType() ? (OMP_MAP_TO | OMP_MAP_FROM)
+ : OMP_MAP_TO));
 }
 // Every default map produces a single argument which is a target 
parameter.
 CurMapTypes.back() |= OMP_MAP_TARGET_PARAM;

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=320596&r1=320595&r2=320596&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Wed Dec 13 09:31:39 2017
@@ -1279,9 +1279,13 @@ bool Sema::IsOpenMPCapturedByRef(ValueDe
   // reference except if it is a pointer that is dereferenced somehow.
   IsByRef = !(Ty->isPointerType() && IsVariableAssociatedWithSection);
 } else {
-  // By default, all the data that has a scalar type is mapped by copy.
-  IsByRef = !Ty->isScalarType() ||
-DSAStack->getDefaultDMAAtLevel(Level) == DMA_tofrom_scalar;
+  // By default, all the data that has a scalar type is mapped by copy
+  //

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

2017-12-13 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment.

Other than the actual text being output, this LGTM.
I'ld like to see the changes I suggested in the test go in, but they're really 
minor.




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

lichray wrote:
> Oh, common, I persuaded the committee to allow you to print a `(null)`  and 
> you don't do it...
I think that `(null)` is a better thing to output here than `0x0`.



Comment at: 
test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.inserters.arithmetic/nullptr_t.pass.cpp:72
+// at least ensure that it does not generate an empty string.
+assert(!s.empty());
+}

You could just say `assert(!sb.str().empty()) here; no need to save the string 
in a variable.



Comment at: 
test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.inserters.arithmetic/nullptr_t.pass.cpp:79
+os << n;
+assert(os.good());
+}

Might as well check for a non-empty string here, too.


https://reviews.llvm.org/D33776



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


[PATCH] D39457: [OPENMP] Current status of OpenMP support.

2017-12-13 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev updated this revision to Diff 126773.
ABataev added a comment.

Status update.


Repository:
  rC Clang

https://reviews.llvm.org/D39457

Files:
  docs/OpenMPSupport.rst
  docs/index.rst


Index: docs/index.rst
===
--- docs/index.rst
+++ docs/index.rst
@@ -39,6 +39,7 @@
SourceBasedCodeCoverage
Modules
MSVCCompatibility
+   OpenMPSupport
ThinLTO
CommandGuide/index
FAQ
Index: docs/OpenMPSupport.rst
===
--- /dev/null
+++ docs/OpenMPSupport.rst
@@ -0,0 +1,69 @@
+.. raw:: html
+
+  
+.none { background-color: #FF }
+.partial { background-color: #99 }
+.good { background-color: #CCFF99 }
+  
+
+.. role:: none
+.. role:: partial
+.. role:: good
+
+==
+OpenMP Support
+==
+
+Clang fully supports OpenMP 3.1 + some elements of OpenMP 4.5. Clang supports 
offloading to X86_64, AArch64, PPC64[LE] and Cuda devices.
+The status of major OpenMP 4.5 features support in Clang.
+
+Standalone directives
+=
+
+* #pragma omp [for] simd: :good:`Complete`.
+
+* #pragma omp declare simd: :partial:`Partial`.  We support parsing/semantic
+  analysis + generation of special attributes for X86 target, but still
+  missing the LLVM pass for vectorization.
+
+* #pragma omp taskloop [simd]: :good:`Complete`.
+
+* #pragma omp target [enter|exit] data: :good:`Mostly complete`.  Some rework 
is
+  required for better stability.
+
+* #pragma omp target update: :good:`Mostly complete`.  Some rework is
+  required for better stability.
+
+* #pragma omp target: :partial:`Partial`.  No support for the `nowait` and 
`depend` clauses.
+
+* #pragma omp declare target: :partial:`Partial`.  No full codegen support.
+
+* #pragma omp teams: :good:`Complete`.
+
+* #pragma omp distribute [simd]: :good:`Complete`.
+
+* #pragma omp distribute parallel for [simd]: :good:`Complete`.
+
+Combined directives
+===
+
+* #pragma omp parallel for simd: :good:`Complete`.
+
+* #pragma omp target parallel: :partial:`Partial`.  No support for the 
`nowait` and `depend` clauses.
+
+* #pragma omp target parallel for [simd]: :partial:`Partial`.  No support for 
the `nowait` and `depend` clauses.
+
+* #pragma omp target simd: :partial:`Partial`.  No support for the `nowait` 
and `depend` clauses.
+
+* #pragma omp target teams: :partial:`Partial`.  No support for the `nowait` 
and `depend` clauses.
+
+* #pragma omp teams distribute [simd]: :good:`Complete`.
+
+* #pragma omp target teams distribute [simd]: :partial:`Partial`.  No full 
codegen support.
+
+* #pragma omp teams distribute parallel for [simd]: :good:`Complete`.
+
+* #pragma omp target teams distribute parallel for [simd]: :partial:`Partial`. 
 No full codegen support.
+
+Clang does not support any constructs/updates from upcoming OpenMP 5.0 except 
for `reduction`-based clauses in the `task` and `target`-based directives.
+


Index: docs/index.rst
===
--- docs/index.rst
+++ docs/index.rst
@@ -39,6 +39,7 @@
SourceBasedCodeCoverage
Modules
MSVCCompatibility
+   OpenMPSupport
ThinLTO
CommandGuide/index
FAQ
Index: docs/OpenMPSupport.rst
===
--- /dev/null
+++ docs/OpenMPSupport.rst
@@ -0,0 +1,69 @@
+.. raw:: html
+
+  
+.none { background-color: #FF }
+.partial { background-color: #99 }
+.good { background-color: #CCFF99 }
+  
+
+.. role:: none
+.. role:: partial
+.. role:: good
+
+==
+OpenMP Support
+==
+
+Clang fully supports OpenMP 3.1 + some elements of OpenMP 4.5. Clang supports offloading to X86_64, AArch64, PPC64[LE] and Cuda devices.
+The status of major OpenMP 4.5 features support in Clang.
+
+Standalone directives
+=
+
+* #pragma omp [for] simd: :good:`Complete`.
+
+* #pragma omp declare simd: :partial:`Partial`.  We support parsing/semantic
+  analysis + generation of special attributes for X86 target, but still
+  missing the LLVM pass for vectorization.
+
+* #pragma omp taskloop [simd]: :good:`Complete`.
+
+* #pragma omp target [enter|exit] data: :good:`Mostly complete`.  Some rework is
+  required for better stability.
+
+* #pragma omp target update: :good:`Mostly complete`.  Some rework is
+  required for better stability.
+
+* #pragma omp target: :partial:`Partial`.  No support for the `nowait` and `depend` clauses.
+
+* #pragma omp declare target: :partial:`Partial`.  No full codegen support.
+
+* #pragma omp teams: :good:`Complete`.
+
+* #pragma omp distribute [simd]: :good:`Complete`.
+
+* #pragma omp distribute parallel for [simd]: :good:`Complete`.
+
+Combined directives
+===
+
+* #pragma omp parallel for simd: :good:`Complete`.
+
+* #pragma omp target parallel: :partial:`Partial`.  No support for the `nowait` and `de

  1   2   >