Page MenuHomePhabricator

mclow.lists (Marshall Clow)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 16 2012, 3:06 PM (334 w, 2 d)

Recent Activity

Today

mclow.lists added a comment to D55427: [libcxx] Call __count_bool_true for bitset count.

This patch aims to help clang with better information so it can inline
__bit_reference count function usage for both std::biset. Current clang
inliner can not infer that the passed typed will be used only to select
the optimized variant, it evaluates the type argument and type check as
a load plus compare (although later optimization phases correctly
optimized this out).

Thu, Dec 13, 7:38 AM

Yesterday

mclow.lists accepted D55610: [libunwind] Avoid code duplication in the SEH version of UnwindCursor::getRegisterName. NFC..

LGTM.

Wed, Dec 12, 1:53 PM

Mon, Dec 10

mclow.lists closed D55308: Implement the second part of P0482 (char8_t).

Committed as revision 348828

Mon, Dec 10, 8:41 PM
mclow.lists accepted D48669: [pair] Mark constructors as conditionally noexcept.

This LGTM now.

Mon, Dec 10, 5:42 PM
mclow.lists added inline comments to D55308: Implement the second part of P0482 (char8_t).
Mon, Dec 10, 5:23 PM
mclow.lists added a comment to D55466: Change `tuple_size` from a `class` to a `struct` (see PR#39871).

I'm waiting for the (editorial) change to go into the standard; once that happens, I will commit this.

Mon, Dec 10, 4:51 PM
mclow.lists added inline comments to D55532: Implement P1209 - Adopt Consistent Container Erasure from Library Fundamentals 2 for C++20.
Mon, Dec 10, 4:51 PM
mclow.lists updated the diff for D55532: Implement P1209 - Adopt Consistent Container Erasure from Library Fundamentals 2 for C++20.

Removed the tuple_size changes - those are in D55466.

Mon, Dec 10, 4:46 PM
mclow.lists added a comment to D55532: Implement P1209 - Adopt Consistent Container Erasure from Library Fundamentals 2 for C++20.

Could you please split this change and the changes from class to struct into different patches? That would make it easier to review.

Mon, Dec 10, 4:43 PM
mclow.lists created D55532: Implement P1209 - Adopt Consistent Container Erasure from Library Fundamentals 2 for C++20.
Mon, Dec 10, 4:07 PM
mclow.lists added inline comments to D55308: Implement the second part of P0482 (char8_t).
Mon, Dec 10, 3:49 PM
mclow.lists added inline comments to D55308: Implement the second part of P0482 (char8_t).
Mon, Dec 10, 3:34 PM
mclow.lists added a comment to D55517: Remove `_VSTD`.

We have two different namespaces in libc++.

Mon, Dec 10, 11:40 AM
mclow.lists added a comment to D55517: Remove `_VSTD`.

I personally like this change but I'd wait a bit to see whether other people oppose (and why) before submitting.

Mon, Dec 10, 10:51 AM

Sun, Dec 9

mclow.lists added inline comments to D55308: Implement the second part of P0482 (char8_t).
Sun, Dec 9, 11:45 AM

Fri, Dec 7

mclow.lists added inline comments to D55308: Implement the second part of P0482 (char8_t).
Fri, Dec 7, 5:22 PM
mclow.lists created D55466: Change `tuple_size` from a `class` to a `struct` (see PR#39871).
Fri, Dec 7, 5:11 PM
mclow.lists added inline comments to D55308: Implement the second part of P0482 (char8_t).
Fri, Dec 7, 1:36 PM
mclow.lists added a comment to D55427: [libcxx] Call __count_bool_true for bitset count.

This looks like a behavior change to me.
The old code calls __count_bool_true if the _Tp can be static_cast to bool, and __count_bool_false otherwise.
The new code calls __count_bool_true if the _Tp is exactly bool, and __count_bool_false otherwise.

Fri, Dec 7, 12:28 PM
mclow.lists added a comment to D55427: [libcxx] Call __count_bool_true for bitset count.

This looks like a behavior change to me.
The old code calls __count_bool_true if the _Tp can be static_cast to bool, and __count_bool_false otherwise.
The new code calls __count_bool_true if the _Tp is exactly bool, and __count_bool_false otherwise.

Fri, Dec 7, 12:03 PM
mclow.lists added a comment to D55404: [libcxx] Support building static library with hidden visibility.

The current libc++ initialization mechanism depends on it being in a shared library.
You'll want to make sure stuff like this still works (will probably require code changes):

Fri, Dec 7, 11:57 AM
mclow.lists added a comment to D55308: Implement the second part of P0482 (char8_t).

But it would be nice to extract this check into a separate utility header to avoid duplication.

I don't think that's a good idea; that header would quickly grow into all the tests for all the feature flags.
But I'm willing to discuss that.

Fri, Dec 7, 9:48 AM
mclow.lists added a comment to D55308: Implement the second part of P0482 (char8_t).

Agreed.
Though I like the formulation that I posted to livcxx-dev:

#if TEST_STD_VER > 17
# if !defined(__cpp_lib_char8_t)  
  LIBCPP_STATIC_ASSERT(false, "__cpp_lib_char8_t is not defined");
# else
#  if __cpp_lib_char8_t < 201811L
#   error "__cpp_lib_char8_t has an invalid value"
#  endif
# endif
#endif

This fails silently if the macro is not defined - for libraries other than libc++.
If other library maintainers want to add (say) a libstdc++-specific assertion, then there's a place for it.

I'd err on the side of what Richard said and always assert, but otherwise I'm fine with something like this.

Fri, Dec 7, 9:46 AM

Thu, Dec 6

mclow.lists added a comment to D55366: [libcxx] Add checks for unique value of array<T, 0>.begin() and array<T, 0>.end() ..

In libc++ these array iterators declared as follows:

typedef value_type*                        iterator;
typedef const value_type*                  const_iterator;
Thu, Dec 6, 10:33 AM

Wed, Dec 5

mclow.lists added a comment to D48669: [pair] Mark constructors as conditionally noexcept.

I checked both N4788 and N4788, and couldn't find any conditional noexcept on the constructors of pair.
Just explicit(see below) constexpr pair(const T1& x, const T2& y);

Wed, Dec 5, 5:33 PM
mclow.lists added inline comments to D55308: Implement the second part of P0482 (char8_t).
Wed, Dec 5, 2:55 PM
mclow.lists updated the diff for D55308: Implement the second part of P0482 (char8_t).

Addressed Louis' comments.

Wed, Dec 5, 2:54 PM
mclow.lists added inline comments to D55308: Implement the second part of P0482 (char8_t).
Wed, Dec 5, 2:52 PM
mclow.lists added a comment to D55308: Implement the second part of P0482 (char8_t).

Two overarching comments.

  1. Maybe we could/should encapsulate the logic for checking the feature test macro in a utility header for the tests. This way, we wouldn't have to copy/paste ` #if TEST_STD_VER > 17 LIBCPP_ASSERT(IS_DEFINED(__cpp_lib_char8_t)); // libc++ implements this
  2. if defined(__cpp_lib_char8_t)
  3. if __cpp_lib_char8_t < 201811L
  4. error "__cpp_lib_char8_t has an invalid value"
  5. endif
  6. endif #endif ` so many times. Also, it would then be reasonable to have this instead: ` #if TEST_STD_VER > 17
  7. if defined(_LIBCPP_VERSION) && !defined(__cpp_lib_char8_t)
  8. error "libc++ implements this"
  9. endif
  10. if defined(__cpp_lib_char8_t)
  11. if __cpp_lib_char8_t < 201811L
  12. error "__cpp_lib_char8_t has an invalid value"
  13. endif
  14. endif #endif `

    This would remove the need for the IS_DEFINED macro, which is a bit tacky.
Wed, Dec 5, 2:13 PM
mclow.lists added a comment to D55308: Implement the second part of P0482 (char8_t).
  1. Maybe we should consider having a LIT feature to represent char8_t support, that way we could mark tests as unsupported?
Wed, Dec 5, 2:01 PM
mclow.lists added inline comments to D55308: Implement the second part of P0482 (char8_t).
Wed, Dec 5, 9:22 AM

Tue, Dec 4

mclow.lists created D55308: Implement the second part of P0482 (char8_t).
Tue, Dec 4, 6:01 PM

Wed, Nov 28

mclow.lists closed D54992: Implement P0966 - string::reserve Should Not Shrink.

Committed as revision 347789

Wed, Nov 28, 10:22 AM
mclow.lists accepted D54801: [libcxx] Remove dynarray.

Sorry to see you go, dynarray.

Wed, Nov 28, 9:59 AM
mclow.lists added inline comments to D54966: Implement P1007R3 `std::assume_aligned`.
Wed, Nov 28, 7:35 AM
mclow.lists updated the diff for D54966: Implement P1007R3 `std::assume_aligned`.

Updated the tests; added constexpr tests (which are currently disabled).
Added _LIBCPP_INLINE_VISIBILITY per Louis' (correct) request.
Added LIBCPP_ASSERT_NOEXCEPT

Wed, Nov 28, 7:34 AM
mclow.lists added a comment to D54966: Implement P1007R3 `std::assume_aligned`.

Chandler has said to me (via IRC) that when called at constexpr time, this should just be a return __p. I'll add that after we have std:: is_constant_evaluated

Wed, Nov 28, 7:10 AM

Tue, Nov 27

mclow.lists created D54992: Implement P0966 - string::reserve Should Not Shrink.
Tue, Nov 27, 10:30 PM
mclow.lists added a comment to D54485: [libcxx] Implement P0318R1: unwrap_ref_decay and unwrap_reference.

I agree with your concerns about this living in <utility>, but as long as people can get it by including <type_traits>, then we're good.

Tue, Nov 27, 1:01 PM
mclow.lists added inline comments to D54966: Implement P1007R3 `std::assume_aligned`.
Tue, Nov 27, 12:51 PM
mclow.lists created D54966: Implement P1007R3 `std::assume_aligned`.
Tue, Nov 27, 12:26 PM

Mon, Nov 26

mclow.lists added a comment to D54838: [libcxx] Fix order checking in unordered_multimap tests..

Hmm, I'm confused by this change request. All the tests I patched are intended to test unordered_multimap container, which was introduced in C++11. So, they won't compile in C++03 mode in any case. Please, clarify the reason for the patch change.

Mon, Nov 26, 1:05 PM
mclow.lists accepted D54767: [libcxx] Use a type that is always an aggregate in variant's tests.

LGTM

Mon, Nov 26, 6:03 AM

Tue, Nov 20

mclow.lists added inline comments to D54772: [libcxx] Reorganize tests since the application of P0602R4.
Tue, Nov 20, 2:19 PM

Wed, Nov 14

mclow.lists accepted D54508: [libcxx] [test] Fix Clang -Wunused-local-typedef, MSVC C4800, missing cassert..
Wed, Nov 14, 7:08 AM

Nov 13 2018

mclow.lists closed D53828: Implement P0972R0: <chrono> zero(), min(), and max() should be noexcept.

Committed revision 346766

Nov 13 2018, 9:25 AM

Nov 1 2018

mclow.lists added a comment to D53978: Add benchmarks for sorting and heap functions..

Is there any value in breaking these out into one benchmark per file? I can see this file ever-growing as we add new benchmarks.

Nov 1 2018, 9:01 AM
mclow.lists added a comment to D53956: Fix test assumption that Linux implies glibc..

I hate this chunk of code. :-(
TEST_HAS_C11_FEATURES basically means that we can use C11 library features like aligned_alloc, and timespec etc.

Nov 1 2018, 8:58 AM

Oct 30 2018

mclow.lists accepted D53867: [libcxx] Implement http://wg21.link/p1006, constexpr in pointer_traits.
Oct 30 2018, 3:51 PM
mclow.lists added a comment to D53867: [libcxx] Implement http://wg21.link/p1006, constexpr in pointer_traits.

Add link in upcoming_meeting.html

Oct 30 2018, 2:28 PM
mclow.lists added inline comments to D53868: Build with -fvisibility=hidden.
Oct 30 2018, 11:03 AM
mclow.lists added inline comments to D53868: Build with -fvisibility=hidden.
Oct 30 2018, 8:27 AM
mclow.lists accepted D53867: [libcxx] Implement http://wg21.link/p1006, constexpr in pointer_traits.

Modulo nits, this looks fine to me.
Can you add a reference to www/upcoming_meeting.html, please?

Oct 30 2018, 7:33 AM

Oct 29 2018

mclow.lists updated the diff for D53828: Implement P0972R0: <chrono> zero(), min(), and max() should be noexcept.

Added libc++-specific tests. Fixed link in summary.

Oct 29 2018, 2:04 PM
mclow.lists created D53828: Implement P0972R0: <chrono> zero(), min(), and max() should be noexcept.
Oct 29 2018, 12:32 PM

Oct 27 2018

mclow.lists added a comment to D52697: Bug 39129: Speeding up partition_point/lower_bound/upper_bound/ by using unsigned division by 2 when possible..

I'm much happier with this patch now. It's a lot simpler, and still accomplishes what you want.

Oct 27 2018, 9:20 AM

Oct 25 2018

mclow.lists added a comment to D53726: [ADT] Work around a bug in older compiler versions..

Works for me! Thanks. Dean?

Oct 25 2018, 6:03 PM
mclow.lists added a comment to D53726: [ADT] Work around a bug in older compiler versions..

Looks promising. I will apply locally this afternoon/evening, and report back.

Oct 25 2018, 3:26 PM
mclow.lists added a comment to D53670: Revert "Teach __libcpp_is_floating_point that __fp16 and _Float16 are".

For whatever it's worth, in libstdc++ std::is_floating_point<__float128>::value yields true by default (which in this case means in -std=gnu++XY but not in -std=c++XY) on targets that have a __float128 type. They also supply an abs overload for __float128 in that case, but nothing else (surprisingly not even numeric_limits).

Oct 25 2018, 11:54 AM
mclow.lists added a comment to D53670: Revert "Teach __libcpp_is_floating_point that __fp16 and _Float16 are".

Sorry I'm late to the party, but we should just be falling back to __is_floating_point if we have it. Since it will always give us the answer we want. (Including for __float128 which we don't handle ATM).

Oct 25 2018, 7:30 AM

Oct 24 2018

mclow.lists added a comment to D50251: [compiler-rt][ubsan] Implicit Conversion Sanitizer - integer sign change - compiler-rt part.

The non-test bits look really straightforward to me.

Oct 24 2018, 12:40 PM · Restricted Project
mclow.lists added a comment to D53486: [libcxx] Only define __libcpp_is_floating_point<_Float16> for Clang.

I have *just* (as in, in the last 10 minutes) become aware of other people interested in "extended floating-point types", and adding such support to the standard, but not for C++20.

Oct 24 2018, 10:55 AM
mclow.lists added inline comments to D53631: Fix libcxx build with MinGW winpthreads.
Oct 24 2018, 9:14 AM
mclow.lists added a comment to D53486: [libcxx] Only define __libcpp_is_floating_point<_Float16> for Clang.

Question: @mclow.lists if __fp16 supported all the proper operations required to be "a floating point type", would there be pushback against defining is_floating_point for __fp16 as a library extension?

Oct 24 2018, 8:30 AM
mclow.lists added a comment to D53486: [libcxx] Only define __libcpp_is_floating_point<_Float16> for Clang.

The point I'm trying to make here is that it's a lot more complicated than saying "Hey - this type here is a floating point type".

Oct 24 2018, 7:33 AM

Oct 23 2018

mclow.lists added a comment to D53486: [libcxx] Only define __libcpp_is_floating_point<_Float16> for Clang.

A possible reason to go with the current option is __libcpp_is_floating_point<_Float16> that has been in libc++ for a few months and has been shipped with 7.0, so there might already be users depending on this trait. Is that a concern? If yes, then current version is probably the right way to go.

Oct 23 2018, 4:33 PM
mclow.lists added a comment to D53486: [libcxx] Only define __libcpp_is_floating_point<_Float16> for Clang.

Searching through the current draft standard, I see the following bits that are not covered by this patch:

Oct 23 2018, 4:16 PM
mclow.lists added a comment to D53486: [libcxx] Only define __libcpp_is_floating_point<_Float16> for Clang.

I'm having trouble getting past the following wording in the standard: "There are three floating-point types: float, double, and long double."

Oct 23 2018, 3:46 PM

Oct 16 2018

mclow.lists closed D52240: Partial Fix for PR#38964.

Committed as revision 344616

Oct 16 2018, 9:05 AM

Oct 15 2018

mclow.lists added a comment to D51762: First part of the <chrono> calendar stuff.
In D51762#1266086, @NoQ wrote:

Had to revert. Sorry! rL344580.

This failure was masked by another error, so i guess it was missed.

Oct 15 2018, 8:26 PM
mclow.lists added a comment to rL344580: Revert r344529 "Implement the first part of the calendar support for C++20".

What happened here?
No one contacted me - I fixed all the buildbot failures that I saw.
Why was this reverted?

Oct 15 2018, 8:22 PM
mclow.lists accepted D51762: First part of the <chrono> calendar stuff.

After discussions with @EricWF, I landed a modified version as revision 344529.

Oct 15 2018, 9:09 AM

Oct 12 2018

mclow.lists added a comment to D51762: First part of the <chrono> calendar stuff.

This is the first of part of this functionality. Some of these bits are not here yet.

Oct 12 2018, 10:04 PM

Oct 11 2018

mclow.lists added inline comments to D53087: Add benchmarks for std::function..
Oct 11 2018, 9:58 AM
mclow.lists added inline comments to D53127: Fix missing defines in libc++abi when compiled against libgcc's arm unwind (follow-up of D42242).
Oct 11 2018, 9:55 AM
mclow.lists added a comment to D53127: Fix missing defines in libc++abi when compiled against libgcc's arm unwind (follow-up of D42242).

BTW, I'm pretty sure that the title here is wrong. It's not "compiling with GCC" that's the problem, it's "compiling with a libsupc++" or something like that.
(and that should drive the tests _URC_FATAL_PHASE2_ERROR etc)

Oct 11 2018, 8:53 AM
mclow.lists added a comment to D53127: Fix missing defines in libc++abi when compiled against libgcc's arm unwind (follow-up of D42242).

Well, if __defined(___GNUC___) does target both gcc and clang, it doesn't make much sense in the first place. How can I state the defined marcro only for gcc?

Oct 11 2018, 8:52 AM

Oct 10 2018

mclow.lists added a comment to D53028: [libcxxabi] Allow building with sanitizers enabled.

Other than above, this looks fine to me - but I haven't done much to the CMake stuff. Waiting for @EricWF to take a look.

Oct 10 2018, 9:29 AM
mclow.lists closed D42242: Make libc++abi work with gcc's ARM unwind library.

Committed as revision 344152

Oct 10 2018, 9:20 AM

Oct 8 2018

mclow.lists added a comment to D48955: [libc++] Improve diagnostics for non-const comparators and hashers in associative containers.

If you can build this w/GCC in C++03 mode, then I'm fine with it.

Oct 8 2018, 1:39 PM
mclow.lists added a comment to D42242: Make libc++abi work with gcc's ARM unwind library.

@mclow.lists , ping. Any chance to get a proper version upstream? I'd rather not pull patches from Fedora when we can have something official.

Oct 8 2018, 1:31 PM
mclow.lists closed D46975: Implement deduction guides for `std::deque`.
Oct 8 2018, 8:11 AM
mclow.lists accepted D46975: Implement deduction guides for `std::deque`.

Committed as r332785

Oct 8 2018, 8:11 AM

Oct 1 2018

mclow.lists added inline comments to D52697: Bug 39129: Speeding up partition_point/lower_bound/upper_bound/ by using unsigned division by 2 when possible..
Oct 1 2018, 11:07 AM
mclow.lists added inline comments to D52697: Bug 39129: Speeding up partition_point/lower_bound/upper_bound/ by using unsigned division by 2 when possible..
Oct 1 2018, 11:01 AM
mclow.lists added a comment to D52697: Bug 39129: Speeding up partition_point/lower_bound/upper_bound/ by using unsigned division by 2 when possible..

These are highly used and important functions and if not to push for every bit of performance in the standard library, where to?

Oct 1 2018, 9:48 AM
mclow.lists accepted D52401: Remove redundant null pointer check in operator delete.

Ok. I'm fine with this. Thanks for your patience.

Oct 1 2018, 9:13 AM
mclow.lists added inline comments to D52697: Bug 39129: Speeding up partition_point/lower_bound/upper_bound/ by using unsigned division by 2 when possible..
Oct 1 2018, 9:09 AM
mclow.lists added a comment to D52697: Bug 39129: Speeding up partition_point/lower_bound/upper_bound/ by using unsigned division by 2 when possible..

You don't think that it's right to merge this patch? The performance wins are pretty nice. There are plenty of other things for optimizer guys to work on, this is a quite specific case.

Oct 1 2018, 7:58 AM

Sep 30 2018

mclow.lists added a comment to D52401: Remove redundant null pointer check in operator delete.

I seem to recall a problem with that

I would like to know if your impression came from the common PWN technique when the attacker found a heap buffer overflow :)

Sep 30 2018, 11:31 PM
mclow.lists added a comment to D52401: Remove redundant null pointer check in operator delete.

I suspect it's fine, but I need to check some stuff on old versions of glibc (I seem to recall a problem with that).

Sep 30 2018, 3:19 PM
mclow.lists added a comment to D52697: Bug 39129: Speeding up partition_point/lower_bound/upper_bound/ by using unsigned division by 2 when possible..
  • In general, this is the kind of optimization that I would rather see the compiler do (everywhere, automatically) than libc++ (here and there, manually).

    I don't think it can, can it? The optimization comes from knowing the last - first will never be < 0. How would the compiler know that?
Sep 30 2018, 3:14 PM

Sep 29 2018

mclow.lists added a comment to D52697: Bug 39129: Speeding up partition_point/lower_bound/upper_bound/ by using unsigned division by 2 when possible..

In general, this is the kind of optimization that I would rather see the compiler do (everywhere, automatically) than libc++ (here and there, manually).

Sep 29 2018, 4:17 PM

Sep 21 2018

mclow.lists accepted D52368: [libc++abi] is_strcmp parameter to is_equal is unused for WIN32.

LGTM.

Sep 21 2018, 12:15 PM
mclow.lists added a comment to D52368: [libc++abi] is_strcmp parameter to is_equal is unused for WIN32.

This seems like overkill to me; why not add (void) use_strcmp right before line 67 instead?

Sep 21 2018, 11:10 AM

Sep 19 2018

mclow.lists added a comment to D44263: Implement LWG 2221 - No formatted output operator for nullptr.

Symbol added: _ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEElsEDn

{'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEElsEDn'}
Sep 19 2018, 2:09 PM
mclow.lists reopened D44263: Implement LWG 2221 - No formatted output operator for nullptr.

Reverted in r342590 while I investigate additions to the dylib.

Sep 19 2018, 2:08 PM
mclow.lists closed D44263: Implement LWG 2221 - No formatted output operator for nullptr.

Landed as revision 342566

Sep 19 2018, 11:32 AM
mclow.lists accepted D50551: [libcxx] [test] Add missing <stdexcept> to several tests..

LFTM

Sep 19 2018, 10:41 AM
mclow.lists closed D50799: Fix for PR 38495: <ctime> no longer compiles on FreeBSD, due to lack of timespec_get().

Landed as r339816

Sep 19 2018, 10:11 AM