[PATCH] D48616: Implement LWG 2946, 3075 and 3076

2018-06-29 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D48616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D48616: Implement LWG 2946, 3075 and 3076

2018-06-27 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. LGTM. All comments/questions are just for my education. Other things I did: double-check that you changed all the functions changed by https://cplusplus.github.io/LWG/lwg-defects.html#2946. Comment at: include/memory:5647 + typename

[PATCH] D48616: Implement LWG 2946, 3075 and 3076

2018-06-27 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. More missed `noexcept`s. Comment at: include/string:279 +template +size_type find_first_of(const T& t, size_type pos = 0) const noexcept; // C++17 size_type find_first_of(const value_type* s, size_type pos, size_type n) const

[PATCH] D48616: Implement LWG 2946, 3075 and 3076

2018-06-27 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments. Comment at: include/string:842 +explicit basic_string(const _Tp& __t, + typename enable_if<__can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value, void>::type* = 0); + mclow.lists wrote: >

[PATCH] D48694: [libc++abi] Limit libc++ header search to specified paths

2018-07-03 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D48694#1148718, @EricWF wrote: > As an aside, libc++abi should really live in the libc++ repository. That way > it would always have the correct headers available. But every time I pitch > that idea I get a ton of push back. FWIW, I think

[PATCH] D50205: [libc++] Add availability markup for aligned new/delete

2018-08-02 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. ldionne added reviewers: EricWF, vsapsai. Herald added subscribers: cfe-commits, dexonsmith, christof. The aligned allocation and deallocation functions were added in the dylib in Mac OSX 10.13. This commit does two things: first, it adds availability markup to

[PATCH] D50205: [libc++] Add availability markup for aligned new/delete

2018-08-02 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D50205#1186419, @ahatanak wrote: > I just wanted to make sure that this doesn't have the same problem as > https://reviews.llvm.org/D34556. Is that correct? > > The patch was reverted in r306859. https://reviews.llvm.org/D34574#791158 >

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

2018-07-30 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D49317#1180200, @mclow.lists wrote: > I am not in favor of this patch. > > I'm in favor of fixing the problem that Arthur has described, but not like > this, for the following reasons: > > - Conceptually, these are (similar to)

[PATCH] D50008: [libc++] Remove _LIBCPP_BUILDING_XXX macros, which are redundant since _LIBCPP_BUILDING_LIBRARY

2018-07-30 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. ldionne added a reviewer: mclow.lists. Herald added a reviewer: EricWF. Herald added subscribers: cfe-commits, dexonsmith, christof. As suggested by Marshall in https://reviews.llvm.org/D49914 Repository: rCXX libc++ https://reviews.llvm.org/D50008 Files:

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

2018-07-30 Thread Louis Dionne via Phabricator via cfe-commits
ldionne requested changes to this revision. ldionne added a comment. This revision now requires changes to proceed. After thinking about this for some more, I'm not sure this patch is worth doing in its current form. The minimal patch for allowing specializations of `allocator_traits` would be:

[PATCH] D49863: [istream] Fix error flags and exceptions propagated from input stream operations

2018-07-26 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. This is a very tricky change since it touches all the input stream operations, both formatted and unformatted. However, I think it's an important change as it fixes extremely broken behavior. The paper I'm working on to fix this in the Standard is

[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY

2018-07-27 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. Note: I resolved Eric's comments before pushing. Repository: rCXX libc++ https://reviews.llvm.org/D49240 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY

2018-07-27 Thread Louis Dionne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCXX338122: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY (authored by ldionne, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D49914: [libc++] Add the _LIBCPP_HIDE_FROM_ABI_AFTER_V1 macro

2018-07-27 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. ldionne added reviewers: EricWF, mclow.lists. Herald added subscribers: cfe-commits, dexonsmith, christof. ldionne added a comment. This is the second step of the proposal aiming to remove uses of `__always_inline__` in the visibility macros. This proposal is

[PATCH] D49914: [libc++] Add the _LIBCPP_HIDE_FROM_ABI_AFTER_V1 macro

2018-07-27 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. This is the second step of the proposal aiming to remove uses of `__always_inline__` in the visibility macros. This proposal is documented on the list here: http://lists.llvm.org/pipermail/cfe-dev/2018-July/058633.html Repository: rCXX libc++

[PATCH] D49914: [libc++] Add the _LIBCPP_HIDE_FROM_ABI_AFTER_V1 macro

2018-07-27 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 157696. ldionne added a comment. Document the macro in the official documentation instead of the header, per Eric's comment on a previous patch of mine. Repository: rCXX libc++ https://reviews.llvm.org/D49914 Files:

[PATCH] D50106: [libc++] Fix tuple assignment from type derived from a tuple-like

2018-07-31 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. ldionne added reviewers: EricWF, mclow.lists. Herald added subscribers: cfe-commits, dexonsmith, christof. The implementation of operator= for tuple currently diverges from the way the Standard specifies them, which leads to subtle cases where the behavior is not as

[PATCH] D50008: [libc++] Remove _LIBCPP_BUILDING_XXX macros, which are redundant since _LIBCPP_BUILDING_LIBRARY

2018-07-31 Thread Louis Dionne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL338475: [libc++] Remove _LIBCPP_BUILDING_XXX macros, which are redundant since… (authored by ldionne, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D37302: [Headers] Define *_HAS_SUBNORM for FLT, DBL, LDBL

2018-08-03 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. What are we waiting for to move forward with this change? Repository: rC Clang https://reviews.llvm.org/D37302 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D50205: [libc++] Add availability markup for aligned new/delete

2018-08-03 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 159110. ldionne added a comment. Only include availability markup in this patch, do not change whether and when aligned allocation functions are used by the library. This will be handled in a separate patch. Repository: rCXX libc++

[PATCH] D50276: [clang] Fix broken include_next in float.h

2018-08-03 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. ldionne added a reviewer: jfb. Herald added subscribers: cfe-commits, dexonsmith. The code defines __FLOAT_H and then includes the next , which is guarded on __FLOAT_H so it gets skipped entirely. This commit uses the header guard __CLANG_FLOAT_H, like other headers

[PATCH] D49997: [libcxx] Fix _LIBCPP_NO_EXCEPTIONS redefined warning

2018-08-01 Thread Louis Dionne via Phabricator via cfe-commits
ldionne closed this revision. ldionne added a comment. Committed as r32f108a0736 Repository: rCXX libc++ https://reviews.llvm.org/D49997 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D49914: [libc++] Add the _LIBCPP_HIDE_FROM_ABI_AFTER_V1 macro

2018-07-27 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments. Comment at: libcxx/include/__config:798 -// Just so we can migrate to _LIBCPP_HIDE_FROM_ABI gradually. -#define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI - -#ifndef _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY -# if

[PATCH] D49927: [libc++] Exclude posix_l/strtonum fallback inclusion for newlib > 2.4

2018-07-27 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. Just to make sure I understand properly: this means we will use newlib's implementation of `iswcntrl_l` & friends instead of our own emulation (which is an ODR violation currently going unnoticed)? And this is OK because newlib provides `iswcntrl_l` & friends starting

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

2018-07-27 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. It would be nice if all the TMP required to determine whether to call `__move_construct_forward(..., true_type)` or `__move_construct_forward(..., false_type)` was done in `__move_construct_forward` itself (or a helper). This way, callers wouldn't have to do it

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

2018-07-27 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D49317#1178767, @Quuxplusone wrote: > @ldionne: I don't know if your "LGTM" is necessarily sufficient to commit > this or not; but either way, I don't have commit privs, so could I ask you > (or someone else) to commit this on my behalf?

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

2018-07-27 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM Comment at: include/vector:296 +_LIBCPP_INLINE_VISIBILITY +inline void +__copy_construct_forward(_Alloc& __a, _Iter __begin1, _Iter __end1,

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

2018-08-08 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. @phosek I don't understand how you can expect code compiled with new headers to link against an old dylib, unless you're setting the target platform, in which case anything that would fail to link will instead be a compiler error. I mean, we're adding stuff to the

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

2018-08-08 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. I must say I therefore don't understand the purpose of this patch. Repository: rC Clang https://reviews.llvm.org/D45639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY

2018-08-10 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D49240#1195723, @rnk wrote: > In https://reviews.llvm.org/D49240#1195237, @ldionne wrote: > > > In https://reviews.llvm.org/D49240#1195125, @thakis wrote: > > > > > When we updated out clang bundle in chromium (which includes libc++ > > >

[PATCH] D50534: [libc++] Fix handling of negated character classes in regex

2018-08-10 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D50534#1194664, @timshen wrote: > I'm not fully equipped with the context right now, but something doesn't add > up. if `__neg_chars_.empty()` check is removed, the `(__neg_mask_ == 0)` > above should be removed too. They have to be

[PATCH] D50341: [libcxx] Mark aligned allocation tests as XFAIL on old OSX versions

2018-08-10 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. I still think we should go forward with this change since the tests _are_ expected to fail on the provided OS X versions, which do not contain the required operators. Repository: rCXX libc++ https://reviews.llvm.org/D50341

[PATCH] D50344: [libc++] Enable aligned allocation based on feature test macro, irrespective of standard

2018-08-09 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 159903. ldionne marked an inline comment as done. ldionne added a comment. Address vsapsai's comments Repository: rCXX libc++ https://reviews.llvm.org/D50344 Files: libcxx/include/__config libcxx/include/new

[PATCH] D50344: [libc++] Enable aligned allocation based on feature test macro, irrespective of standard

2018-08-09 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments. Comment at: libcxx/include/new:111-116 #if !__has_builtin(__builtin_operator_new) || \ __has_builtin(__builtin_operator_new) < 201802L || \ defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) || \ !defined(__cpp_aligned_new) ||

[PATCH] D47400: [libcxx] [test] Allow a standard library that implements LWG 1203 in istream.rvalue/rvalue.pass.cpp

2018-08-09 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D47400 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D50344: [libc++] Enable aligned allocation based on feature test macro, irrespective of standard

2018-08-09 Thread Louis Dionne via Phabricator via cfe-commits
ldionne marked an inline comment as done. ldionne added inline comments. Comment at: libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp:11 +// UNSUPPORTED: c++98, c++03, c++11, c++14 +// XFAIL: with_system_cxx_lib=macosx10.12 +// XFAIL: with_system_cxx_lib=macosx10.11

[PATCH] D50534: [libc++] Fix handling of negated character classes in regex

2018-08-10 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 160169. ldionne added a comment. Rewrite the patch in light of timshen's comment, and add tests. Repository: rCXX libc++ https://reviews.llvm.org/D50534 Files: libcxx/include/regex

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

2018-08-13 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 160414. ldionne added a comment. Update documentation for _LIBCPP_HIDE_FROM_ABI Repository: rCXX libc++ https://reviews.llvm.org/D50652 Files: libcxx/docs/DesignDocs/VisibilityMacros.rst libcxx/include/__config Index: libcxx/include/__config

[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY

2018-08-13 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. I opened a straw man proposal to fix this at https://reviews.llvm.org/D50652. Repository: rCXX libc++ https://reviews.llvm.org/D49240 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D50341: [libcxx] Mark aligned allocation tests as XFAIL on old OSX versions

2018-08-13 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 160440. ldionne added a comment. Rewrite all XFAILs in light of issues brought up by Marshall. Repository: rCXX libc++ https://reviews.llvm.org/D50341 Files: libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

2018-08-13 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. The intent is for this patch to be cherry-picked onto the LLVM 7 release. This is a straw man proposal to fix issues raised in https://reviews.llvm.org/D49240. The idea is that in the future, we would probably want the non-`internal_linkage` case to be the default. By

[PATCH] D50341: [libcxx] Fix XFAILs for aligned allocation tests on older OSX versions

2018-08-13 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. This works with all the dylibs I have locally, with various combinations of availability enabled/disabled and all standards. I've asked Marshall to check on his system, where he initially reported some failures. Repository: rCXX libc++

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

2018-08-13 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. ldionne added reviewers: EricWF, mclow.lists, dexonsmith, hans, rnk. Herald added subscribers: cfe-commits, christof. This led to symbol size problems in Chromium, and we expect this may be the case in other projects built in debug mode too. Instead, unless users

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

2018-08-13 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D50652#1197830, @rnk wrote: > In https://reviews.llvm.org/D50652#1197780, @ldionne wrote: > > > One thing to keep in mind is that we do not have a solution that allows > > removing both `internal_linkage` and `always_inline`. It's either > >

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

2018-08-13 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D50652#1197775, @rnk wrote: > I'd prefer not to do this, since internal_linkage generates smaller, more > debuggable code by default. I think the symbol table size increase may be > specific to MachO, and it may be possible to work around

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

2018-08-15 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D50652#1201236, @thakis wrote: > I haven't read all the messages in these threads, forgive me if someone asked > this already. It's a bit weird to me that we have to override this behavior > in Chromium while the default is different. Why

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

2018-08-15 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. If that's possible at all, I would like for the Chromium people to build with this patch applied. The expectation is that we'll cherry-pick this patch onto LLVM 7, and it would suck if that did not solve Chromium's problem for some stupid reason. Repository: rCXX

[PATCH] D50799: Fix for PR 38495: no longer compiles on FreeBSD, due to lack of timespec_get()

2018-08-15 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D50799 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

2018-08-15 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 160873. ldionne added a comment. Allow picking a custom default behavior for vendors, per Duncan's comment. Also, revert to a better name for the macro. Repository: rCXX libc++ https://reviews.llvm.org/D50652 Files: libcxx/CMakeLists.txt

[PATCH] D50815: Establish the header

2018-08-15 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D50815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D50341: [libcxx] Fix XFAILs for aligned allocation tests on older OSX versions

2018-08-13 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 160457. ldionne added a comment. Switch from XFAIL to UNSUPPORTED, per Marshall's offline comment. Repository: rCXX libc++ https://reviews.llvm.org/D50341 Files: libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp

[PATCH] D50341: [libcxx] Fix XFAILs for aligned allocation tests on older OSX versions

2018-08-13 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 160465. ldionne marked an inline comment as done. ldionne added a comment. Replace more XFAILs by UNSUPPORTEDs, per Marshall's comment. Repository: rCXX libc++ https://reviews.llvm.org/D50341 Files:

[PATCH] D50341: [libcxx] Fix XFAILs for aligned allocation tests on older OSX versions

2018-08-13 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments. Comment at: libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp:18 +// XFAIL: macosx10.8 +// XFAIL: macosx10.7 mclow.lists wrote: > These should probably be `UNSUPPORTED` as well. I think the correct thing for those is

[PATCH] D50674: [libc++] Add missing #include in C11 features tests

2018-08-13 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. ldionne added a reviewer: mclow.lists. Herald added a reviewer: EricWF. Herald added subscribers: cfe-commits, dexonsmith, christof. These #includes are quite important, since otherwise any #if TEST_STD_VER > 14 && defined(TEST_HAS_C11_FEATURES) checks are

[PATCH] D50719: [libc++] Fix incorrect definition of TEST_HAS_C11_FEATURES

2018-08-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne marked an inline comment as done. ldionne added a comment. Ok, I'm pushing with JF's suggested change (use `TEST_STD_VER >= 11` instead of `__cplusplus >= 201103L`). Let's cross fingers that this is going to unbreak the testers -- like I said it fixed my Docker container. Repository:

[PATCH] D50719: [libc++] Fix incorrect definition of TEST_HAS_C11_FEATURES

2018-08-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. The goal of this commit is to fix build bot failures here: http://lab.llvm.org:8011/builders/libcxx-libcxxabi-libunwind-armv7-linux-noexceptions/builds/286/steps/test.libcxx/logs/stdio. This failure was introduced when I fixed some unit tests that were no-ops in

[PATCH] D50719: [libc++] Fix incorrect definition of TEST_HAS_C11_FEATURES

2018-08-14 Thread Louis Dionne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCXX339702: [libc++] Fix incorrect definition of TEST_HAS_C11_FEATURES (authored by ldionne, committed by ). Changed prior to commit: https://reviews.llvm.org/D50719?vs=160616=160648#toc Repository:

[PATCH] D50719: [libc++] Fix incorrect definition of TEST_HAS_C11_FEATURES

2018-08-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. ldionne added a reviewer: mclow.lists. Herald added a reviewer: EricWF. Herald added subscribers: cfe-commits, dexonsmith, christof. The macro was not defined in C++11 mode when it should have been, at least according to how _LIBCPP_HAS_C11_FEATURES is defined.

[PATCH] D50719: [libc++] Fix incorrect definition of TEST_HAS_C11_FEATURES

2018-08-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D50719#1199427, @mclow.lists wrote: > I have pissed in this area, too - See > https://bugs.llvm.org/show_bug.cgi?id=38495 and the proposed resolution here: > https://bugs.llvm.org/attachment.cgi?id=20692 > How about I just make this change

[PATCH] D50341: [libcxx] Fix XFAILs for aligned allocation tests on older OSX versions

2018-08-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D50341#1198863, @ldionne wrote: > In https://reviews.llvm.org/D50341#1198339, @vsapsai wrote: > > > What about defining a feature for unsupported configurations? I've tried > > > > if '__cpp_aligned_new' not in macros or \ > >

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

2018-08-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 160680. ldionne added a comment. Herald added a subscriber: mgorny. Add a way to change the default behavior of _LIBCPP_HIDE_FROM_ABI at build time. Also, rename the macro to _LIBCPP_ABI_HIDDEN_USE_INTERNAL_LINKAGE to align with other ABI-related macros.

[PATCH] D50739: Clean up macros to detect underling C library functionality

2018-08-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments. Comment at: include/__config:433 -#if __ISO_C_VISIBLE >= 2011 || __cplusplus >= 201103L +#if __ISO_C_VISIBLE >= 2011 # if defined(__FreeBSD__) mclow.lists wrote: > Should we be using `__ISO_C_VISIBLE` here, or

[PATCH] D50739: Clean up macros to detect underling C library functionality

2018-08-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments. Comment at: include/__config:433 -#if __ISO_C_VISIBLE >= 2011 || __cplusplus >= 201103L +#if __ISO_C_VISIBLE >= 2011 # if defined(__FreeBSD__) ldionne wrote: > mclow.lists wrote: > > Should we be using `__ISO_C_VISIBLE` here,

[PATCH] D50748: [libc++] Detect C11 features on non-Clang compilers

2018-08-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. ldionne added a reviewer: mclow.lists. Herald added a reviewer: EricWF. Herald added subscribers: cfe-commits, dexonsmith, christof, krytarowski. The macros were inside `#if defined(_LIBCPP_COMPILER_CLANG)`, which means we would never detect C11 features on

[PATCH] D50341: [libcxx] Fix XFAILs for aligned allocation tests on older OSX versions

2018-08-14 Thread Louis Dionne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL339743: [libcxx] Fix XFAILs for aligned allocation tests on older OSX versions (authored by ldionne, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D50748: [libc++] Detect C11 features on non-Clang compilers

2018-08-14 Thread Louis Dionne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCXX339741: [libc++] Detect C11 features on non-Clang compilers (authored by ldionne, committed by ). Changed prior to commit: https://reviews.llvm.org/D50748?vs=160725=160727#toc Repository: rCXX

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

2018-08-16 Thread Louis Dionne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL339874: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI (authored by ldionne, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D49914: [libc++] Add the _LIBCPP_HIDE_FROM_ABI_AFTER_V1 macro

2018-08-06 Thread Louis Dionne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL339012: [libc++] Add the _LIBCPP_HIDE_FROM_ABI_AFTER_V1 macro (authored by ldionne, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D50205: [libc++] Add availability markup for aligned new/delete

2018-08-06 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D50205#1188454, @EricWF wrote: > How does this work when a user provides their own definitions? Does the > attribute from the declaration still produce a warning? If so, then I think > an in-compiler approach is better. Yes. I do agree

[PATCH] D50276: [clang] Fix broken include_next in float.h

2018-08-06 Thread Louis Dionne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL339016: [clang] Fix broken include_next in float.h (authored by ldionne, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D50205: [libc++] Add availability markup for aligned new/delete

2018-08-06 Thread Louis Dionne via Phabricator via cfe-commits
ldionne abandoned this revision. ldionne added a comment. Nevermind, it looks like this patch is not necessary anymore since https://reviews.llvm.org/D45015 landed. Repository: rCXX libc++ https://reviews.llvm.org/D50205 ___ cfe-commits mailing

[PATCH] D37302: [Headers] Define *_HAS_SUBNORM for FLT, DBL, LDBL

2018-08-06 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D37302#1189576, @pirama wrote: > Sorry this fell of my radar. I've rebased the patch. > > Since this has been inactive for a while, lets wait for a couple of days to > see if there are any other comments. If there are no objections, I'll

[PATCH] D50341: [libcxx] Mark aligned allocation tests as XFAIL on old OSX versions

2018-08-06 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. ldionne added a reviewer: vsapsai. Herald added a reviewer: EricWF. Herald added subscribers: cfe-commits, dexonsmith, christof. Since r338934, Clang emits an error when aligned allocation functions are used in conjunction with a system libc++ dylib that does not

[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY

2018-08-13 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D49240#1196878, @hans wrote: > The reason we noticed this was that it caused a *50 GB* size increase of the > build output on our buildbots, which was enough to cause infrastructure > problems. > > This change was also committed shortly

[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY

2018-08-13 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D49240#1197149, @hans wrote: > In https://reviews.llvm.org/D49240#1197052, @ldionne wrote: > > > In https://reviews.llvm.org/D49240#1196878, @hans wrote: > > > > > The reason we noticed this was that it caused a *50 GB* size increase of > > >

[PATCH] D50341: [libcxx] Mark aligned allocation tests as XFAIL on old OSX versions

2018-08-09 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments. Comment at: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align.fail.cpp:15 // UNSUPPORTED: c++98, c++03, c++11, c++14, c++17 // UNSUPPORTED: clang-3.3, clang-3.4, clang-3.5, clang-3.6, clang-3.7,

[PATCH] D50543: [libcxx] Mark charconv tests as failing for previous libcxx versions.

2018-08-10 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. Ship it! https://reviews.llvm.org/D50543 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D50344: [libc++] Enable aligned allocation based on feature test macro, irrespective of standard

2018-08-10 Thread Louis Dionne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. ldionne marked an inline comment as done. Closed by commit rL339431: [libc++] Enable aligned allocation based on feature test macro, irrespective of… (authored by ldionne, committed by ). Herald added a subscriber:

[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY

2018-08-10 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D49240#1195125, @thakis wrote: > When we updated out clang bundle in chromium (which includes libc++ headers), > our ios simulator bots regressed debug info size by ~50% due to this commit >

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

2018-08-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D50652#1198885, @hans wrote: > Oh, or could we do > > -D_LIBCPP_HIDE_FROM_ABI= > > > and just get regular odr linkage for these functions? No, you do need to use `_LIBCPP_HIDDEN _LIBCPP_ALWAYS_INLINE` because of the issue described in

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

2018-08-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D50652#1198893, @ldionne wrote: > In https://reviews.llvm.org/D50652#1198885, @hans wrote: > > > Oh, or could we do > > > > -D_LIBCPP_HIDE_FROM_ABI= > > > > > > and just get regular odr linkage for these functions? > > > No, you do need to

[PATCH] D50341: [libcxx] Fix XFAILs for aligned allocation tests on older OSX versions

2018-08-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D50341#1198339, @vsapsai wrote: > What about defining a feature for unsupported configurations? I've tried > > if '__cpp_aligned_new' not in macros or \ > intMacroValue(macros['__cpp_aligned_new']) < 201606: >

[PATCH] D50674: [libc++] Add missing #include in C11 features tests

2018-08-14 Thread Louis Dionne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL339675: [libc++] Add missing #include in C11 features tests (authored by ldionne, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D50344: [libc++] Enable aligned allocation based on feature test macro, irrespective of standard

2018-08-06 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. ldionne added a reviewer: vsapsai. Herald added a reviewer: EricWF. Herald added subscribers: cfe-commits, dexonsmith, christof. The current code enables aligned allocation functions when compiling in C++17 and later. This is a problem because aligned allocation

[PATCH] D50534: [libc++] Fix handling of negated character classes in regex

2018-08-09 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. ldionne added reviewers: mclow.lists, timshen. Herald added a reviewer: EricWF. Herald added subscribers: cfe-commits, dexonsmith, christof. This commit fixes a regression introduced in r316095, where we don't match inverted character classes when there's no negated

[PATCH] D50534: [libc++] Fix handling of negated character classes in regex

2018-08-09 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. The changeset that introduced this regression was reviewed as https://reviews.llvm.org/D37955. @timshen, I'm curious to understand why you or'ed with `__neg_chars_.empty()` when initializing `__in_neg_chars`. The logic's not clear to me, and my tests seem to show that

[PATCH] D50876: Clean up newly created header

2018-08-17 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM with the `__clz` you missed. Comment at: include/bit:61 +inline _LIBCPP_INLINE_VISIBILITY +int __popcount(unsigned __x) { return __builtin_popcount (__x);

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

2018-08-17 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In https://reviews.llvm.org/D50652#1202524, @hans wrote: > Thanks! Merged to 7.0 in r339882. Now that this has been done, I guess we need to document somewhere in the release notes that the default contract given by libc++ is changing in LLVM 7, right? Repository:

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

2018-08-21 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments. Comment at: include/stddef.h:55 // Re-use the compiler's max_align_t where possible. #if !defined(__CLANG_MAX_ALIGN_T_DEFINED) && !defined(_GCC_MAX_ALIGN_T) && \ chandlerc wrote: > Unrelated to your patch, but this comment is

[PATCH] D51170: [libc++] Remove race condition in std::async

2018-08-23 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. ldionne added a reviewer: mclow.lists. Herald added a reviewer: EricWF. Herald added subscribers: cfe-commits, dexonsmith, christof. The state associated to the future was set in one thread (with synchronization) but read in another thread without synchronization,

[PATCH] D51172: [libcxx] Comment out #define __cpp_lib_node_extract, we only support half of that functionality

2018-08-23 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM. Don't forget to update https://reviews.llvm.org/D48896 so it uncomments this. Also, this should be merged into LLVM 7. Repository: rCXX libc++ https://reviews.llvm.org/D51172

[PATCH] D51170: [libc++] Remove race condition in std::async

2018-08-24 Thread Louis Dionne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL340608: [libc++] Remove race condition in std::async (authored by ldionne, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D51170: [libc++] Remove race condition in std::async

2018-08-24 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments. Comment at: libcxx/include/future:556 bool __has_value() const {return (__state_ & __constructed) || (__exception_ != nullptr);} jfb wrote: > I'm not auditing everything, but it seems like code above can still

[PATCH] D50534: [libc++] Fix handling of negated character classes in regex

2018-08-24 Thread Louis Dionne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCXX340609: [libc++] Fix handling of negated character classes in regex (authored by ldionne, committed by ). Changed prior to commit: https://reviews.llvm.org/D50534?vs=160169=162377#toc Repository:

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

2018-08-21 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. ldionne added a reviewer: ahatanak. Herald added subscribers: cfe-commits, dexonsmith. r306722 introduced a new note called note_silence_unligned_allocation_unavailable where I believe what was meant is note_silence_aligned_allocation_unavailable. Repository:

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

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

[PATCH] D35388: [libc++] Give extern templates default visibility on gcc

2018-07-18 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. ABI-wise, I think this change is OK. Indeed, if `__type_visibility__` is not supported, we're already marking the base template as `__visibility__("default")`, so marking the extern template declaration with `__visibility__("default")` is not a problem. If

[PATCH] D49584: [CMake] Install C++ ABI headers into the right location

2018-07-20 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. This change LGTM. However, I do have a question about the overall change (including r335809 and r337118). Why is it that we're not simply relying on the `CMAKE_INSTALL_PREFIX` being set to the path where we should install instead of using all these custom variables

[PATCH] D49502: [CMake] Support statically linking dependencies only to shared or static library

2018-07-20 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. LGTM Repository: rL LLVM https://reviews.llvm.org/D49502 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49573: [CMake] Option to control whether shared/static library is installed

2018-07-20 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. I don't like the fact that we're adding options like crazy, thus making the build more complicated. If you don't want to have some libraries that were built, why not just remove them afterwards? Repository: rL LLVM https://reviews.llvm.org/D49573

  1   2   3   >