[PATCH] D41087: [Preprocessor] Implement __is_target_{arch|vendor|os|environment} function-like builtin macros
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
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
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
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.
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.
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
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
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.
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
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.
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
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
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
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
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
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
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
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
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.
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
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.
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
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.
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.
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
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
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
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
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.
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
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)
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.
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
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.
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.
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.
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
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.
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.
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
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
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
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
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.
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
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
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.
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
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
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
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
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
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
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
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!)
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
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
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
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
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
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
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
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.
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
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.
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