Page MenuHomePhabricator

ldionne (Louis Dionne)
UserAdministrator

Projects

User does not belong to any projects.

User Details

User Since
Feb 11 2015, 3:26 PM (310 w, 1 d)
Roles
Administrator

Recent Activity

Today

ldionne committed rG03b6dc300531: [libc++] Fix broken build when merging libc++abi into libc++ on Apple (authored by ldionne).
[libc++] Fix broken build when merging libc++abi into libc++ on Apple
Fri, Jan 22, 9:40 AM
ldionne added a comment to D94924: [libc++] first steps of a private modulemap.

I would rather expect that we have to split some headers into smaller/modular ones.

Fri, Jan 22, 7:03 AM · Unknown Object (Project)

Yesterday

ldionne added a comment to D95177: [libc++] Bring back mach_absolute_time implementation of steady_clock.

Please try this out and let me know if that solves your problems.

Thu, Jan 21, 2:56 PM · Unknown Object (Project)
ldionne added a comment to D74489: [libc++] Remove workarounds for the lack of clock_gettime on older macOS platforms.

See tentative fix in https://reviews.llvm.org/D95177

Thu, Jan 21, 2:55 PM · Unknown Object (Project)
ldionne requested review of D95177: [libc++] Bring back mach_absolute_time implementation of steady_clock.
Thu, Jan 21, 2:54 PM · Unknown Object (Project)
ldionne added a comment to D94924: [libc++] first steps of a private modulemap.
  1. You don't have to do anything.
  2. If you use -fcxx-modules, it reduces the size of the surface of the std module. The std_private module is hidden from users. There is a " // FIXME: These should be private." in the module.modulemap. These headers should not be visible to users of the std module. The private module is exactly trying to do that.
Thu, Jan 21, 1:55 PM · Unknown Object (Project)
ldionne added a comment to D93190: [libc++abi] Simplify scan_eh_tab.

This really isn't in my area, so I'm deferring to @compnerd . Please feel free to go ahead. :-)

Thu, Jan 21, 1:52 PM · Unknown Object (Project)
ldionne added a comment to D88189: [libc++abi] Add an option to avoid demangling in terminate..

Sorry this got through the cracks.

Thu, Jan 21, 1:40 PM · Unknown Object (Project)
ldionne added inline comments to D94452: [libc++] Support immovable return types in std::function..
Thu, Jan 21, 12:40 PM · Unknown Object (Project)
ldionne requested changes to D94452: [libc++] Support immovable return types in std::function..
Thu, Jan 21, 11:52 AM · Unknown Object (Project)
ldionne added a comment to D94924: [libc++] first steps of a private modulemap.

Generally speaking, I have no objection with doing this if it's an improvement. But I don't understand the benefits / tradeoffs here cause I don't have much experience with Clang modules (so far the libc++ Clang modules are kind of a side thing maintained by people who care about it, but we still use the non-modular build by default). Can you explain what introducing private modules is going to buy us, and what sort of changes are going to be required? Will this have any visible change for people building with -fcxx-modules or -fmodules today?

Thu, Jan 21, 7:55 AM · Unknown Object (Project)
ldionne accepted D94564: [libcxx] Check return value for asprintf().

LGTM, thanks!

Thu, Jan 21, 7:45 AM · Unknown Object (Project)
ldionne accepted D94953: [libc++] Use ioctl when available to get random_device entropy..

LGTM, thanks!

Thu, Jan 21, 7:42 AM · Unknown Object (Project)
ldionne added a comment to D93922: Mangle `__alignof__` differently than `alignof`..

Was the "group" libc++abi accept intended to be an accept for the whole revision? Or should still ping for review from rsmith or someone else?

Thu, Jan 21, 7:11 AM · Restricted Project, Unknown Object (Project), Restricted Project

Wed, Jan 20

ldionne accepted D94452: [libc++] Support immovable return types in std::function..

Per the discussion on the LWG reflector, this LGTM.

Wed, Jan 20, 10:27 AM · Unknown Object (Project)
ldionne added a comment to D95055: [clang] Don't look into <sysroot> for C++ headers if they are found alongside the toolchain.

I would like to cherry-pick this change onto LLVM 11.1.0, if there is still time to do so. It is breaking Clang when the sysroot contains headers for libc++. Is it too late?

Wed, Jan 20, 9:30 AM · Restricted Project
ldionne requested review of D95055: [clang] Don't look into <sysroot> for C++ headers if they are found alongside the toolchain.
Wed, Jan 20, 9:18 AM · Restricted Project
ldionne committed rG6c1bc0d24cea: [docs] Fix overly specific link to uploading patches on Phabricator (authored by ldionne).
[docs] Fix overly specific link to uploading patches on Phabricator
Wed, Jan 20, 8:14 AM
ldionne closed D94929: [docs] Fix overly specific link to uploading patches on Phabricator.
Wed, Jan 20, 8:14 AM · Restricted Project
ldionne updated the diff for D94929: [docs] Fix overly specific link to uploading patches on Phabricator.

Fix broken link (I generated the documentation to test, this time).

Wed, Jan 20, 8:14 AM · Restricted Project

Tue, Jan 19

ldionne requested changes to D94924: [libc++] first steps of a private modulemap.

Is this ready for review? If not, please ping me when it is. In the meantime, I'll "request changes" so it shows up correctly in the queue.

Tue, Jan 19, 12:00 PM · Unknown Object (Project)
ldionne added a comment to D94571: [libcxx] random_device, for OpenBSD specify optimal entropy properties.

@ldionne I don't understand how a Linux specific diff would help with another operating system. I could update this after D94953 goes in and eliminate the first chunk.

Tue, Jan 19, 11:48 AM · Unknown Object (Project)
ldionne added inline comments to D94953: [libc++] Use ioctl when available to get random_device entropy..
Tue, Jan 19, 11:48 AM · Unknown Object (Project)
ldionne added a comment to D93912: [libc++][P1679] add string contains.

Thanks for your contribution!

Tue, Jan 19, 11:35 AM · Unknown Object (Project)
ldionne committed rG6ac9cb2a7c6c: [libc++][P1679] add string contains (authored by WimLeflere).
[libc++][P1679] add string contains
Tue, Jan 19, 11:35 AM
ldionne closed D93912: [libc++][P1679] add string contains.
Tue, Jan 19, 11:35 AM · Unknown Object (Project)
ldionne accepted D93912: [libc++][P1679] add string contains.

It's unclear to me if I should add noexcept to the const char* overloads.

I think @mclow.lists is right. We shouldn't have noexcept here, and we also need to remove it from some of the __str_find overloads (as a separate patch, which I'm happy to make).

Tue, Jan 19, 11:33 AM · Unknown Object (Project)
ldionne requested changes to D90968: Fix for the Bug 41784.

In addition to the comments I made, can you please add tests using the type traits like static_assert(!std::is_copy_constructible_v<std::atomic<int>>);?

Tue, Jan 19, 11:24 AM · Unknown Object (Project)
ldionne committed rG933518fff82c: [libc++] Make LIBCXX_ENABLE_FILESYSTEM fully consistent (authored by ldionne).
[libc++] Make LIBCXX_ENABLE_FILESYSTEM fully consistent
Tue, Jan 19, 11:16 AM
ldionne closed D94921: [libc++] Make LIBCXX_ENABLE_FILESYSTEM fully consistent.
Tue, Jan 19, 11:16 AM · Unknown Object (Project)
ldionne committed rG68dba7eae1df: [libc++] Unbreak the debug mode (authored by ldionne).
[libc++] Unbreak the debug mode
Tue, Jan 19, 11:15 AM
ldionne closed D94718: [libc++] Unbreak the debug mode.
Tue, Jan 19, 11:15 AM · Unknown Object (Project)
ldionne accepted D94921: [libc++] Make LIBCXX_ENABLE_FILESYSTEM fully consistent.
Tue, Jan 19, 11:12 AM · Unknown Object (Project)
ldionne added inline comments to D92229: [libc++] Update clang-format configuration.
Tue, Jan 19, 10:45 AM · Unknown Object (Project)
ldionne updated the diff for D94983: [libc++] Make feature-test macros consistent with availability macros.

Re-generate feature-test macros after fixing the typo.

Tue, Jan 19, 10:26 AM · Unknown Object (Project)
ldionne added a comment to D94983: [libc++] Make feature-test macros consistent with availability macros.

Note that I expect to have to make changes to the tests. I'm submitting a patch mainly to run the CI.

Tue, Jan 19, 10:25 AM · Unknown Object (Project)
ldionne updated the diff for D94983: [libc++] Make feature-test macros consistent with availability macros.

Fix typo.

Tue, Jan 19, 10:25 AM · Unknown Object (Project)
ldionne requested review of D94983: [libc++] Make feature-test macros consistent with availability macros.
Tue, Jan 19, 10:24 AM · Unknown Object (Project)
ldionne added a comment to D91630: [Parse] Add parsing support for C++ attributes on using-declarations.

Gentle ping! Can we merge this? I'd love to move forward with https://reviews.llvm.org/D90257.

Tue, Jan 19, 8:59 AM
ldionne accepted D91004: [libc++] Implements concept destructible.

LGTM. Feel free to commit after fixing the minor comments.

Tue, Jan 19, 8:31 AM · Unknown Object (Project)
ldionne requested changes to D92214: [libc++] Implement format_error..
Tue, Jan 19, 8:09 AM · Unknown Object (Project)
ldionne updated the diff for D94921: [libc++] Make LIBCXX_ENABLE_FILESYSTEM fully consistent.

Fix modules tests.

Tue, Jan 19, 7:33 AM · Unknown Object (Project)
ldionne added a comment to D94571: [libcxx] random_device, for OpenBSD specify optimal entropy properties.

@brad I suggest we abandon this in favour of D94953. Does it solve your problem?

Tue, Jan 19, 7:28 AM · Unknown Object (Project)
ldionne requested changes to D94953: [libc++] Use ioctl when available to get random_device entropy..

@tcanens You may want to consider updating cppreference in light of this patch when it gets in.

Tue, Jan 19, 7:22 AM · Unknown Object (Project)
ldionne accepted D94969: [libc++] Split re.alg tests into locale-dependent and independent tests.

LGTM once CI passes!

Tue, Jan 19, 7:17 AM · Unknown Object (Project)

Mon, Jan 18

ldionne requested changes to D94452: [libc++] Support immovable return types in std::function..

I think allowing a non-copyable type to be returned makes more sense, however I think this needs a LWG issue so we can clear any doubts on the standardese. Are you willing to file it?

Mon, Jan 18, 2:50 PM · Unknown Object (Project)
ldionne added inline comments to D93912: [libc++][P1679] add string contains.
Mon, Jan 18, 2:49 PM · Unknown Object (Project)
ldionne requested changes to D93912: [libc++][P1679] add string contains.

Thanks Arthur. Indeed, this doesn't apply cleanly on top of main. Please rebase and address Arthur's comments. Requesting changes just so it shows up in the review queue, but this LGTM except for what I just said.

Mon, Jan 18, 2:41 PM · Unknown Object (Project)
ldionne committed rG2cb4a96a99e8: [libc++] NFCI: Refactor allocator_traits (authored by ldionne).
[libc++] NFCI: Refactor allocator_traits
Mon, Jan 18, 2:38 PM
ldionne closed D94544: [libc++] NFCI: Refactor allocator_traits.
Mon, Jan 18, 2:37 PM · Unknown Object (Project)
ldionne accepted D94544: [libc++] NFCI: Refactor allocator_traits.

Thanks for the review Zoe!

Mon, Jan 18, 2:36 PM · Unknown Object (Project)
ldionne added a comment to D92044: Implement P0655R1 visit<R>: Explicit Return Type for visit.

Do you need help committing this?

I think so.

I would prefer to go through this path myself, to make commits directly (if it's possible and if I have enough permissions to do that). It's just may save the time of other people and your time in particular:) Currently, I just don't know what to do. So if you (or somebody else) can teach me it would be great.

But I don't mind if you merge it yourself. Please choose the way that works better for you.

Mon, Jan 18, 2:35 PM · Unknown Object (Project)
ldionne requested changes to D93819: [libc++] Implement [P0769] "Add shift to algorithm" (shift_left, shift_right).

I'm not done with the review, but I figured I might as well give you the feedback I have right away.

Mon, Jan 18, 2:29 PM · Unknown Object (Project)
ldionne requested changes to D94569: [libcxx] Wipe some more macros that do not belong in C++ forwarding headers.

These symbols are usually macros + libc calls.

Mon, Jan 18, 1:01 PM · Unknown Object (Project)
ldionne added a comment to D93912: [libc++][P1679] add string contains.

@Quuxplusone Arthur, did you still have objections? If not, please accept and I'll commit this.

Mon, Jan 18, 12:40 PM · Unknown Object (Project)
ldionne accepted D93912: [libc++][P1679] add string contains.
Mon, Jan 18, 12:39 PM · Unknown Object (Project)
ldionne committed rG2776be43f0c2: [libc++] improve feature test macro script (authored by WimLeflere).
[libc++] improve feature test macro script
Mon, Jan 18, 12:19 PM
ldionne closed D94530: [libc++] improve feature test macro script.
Mon, Jan 18, 12:19 PM · Unknown Object (Project)
ldionne added a comment to D94530: [libc++] improve feature test macro script.

Do you need help committing this? If so, please provide Author Name <email@domain>. Thanks!

Is it an action I can take in the web interface?
Do I need certain access rights?

Info: Wim Leflere wim.leflere@gmail.com

Mon, Jan 18, 12:16 PM · Unknown Object (Project)
ldionne requested review of D94929: [docs] Fix overly specific link to uploading patches on Phabricator.
Mon, Jan 18, 12:15 PM · Restricted Project
ldionne accepted D91292: [libc++] [P0935] [C++20] Eradicating unnecessarily explicit default constructors from the standard library..

I didn't look again in detail, but I remember I was OK with the patch except for the question of whether to implement it as a DR. This has been settled now, so this LGTM. Thanks!

Mon, Jan 18, 12:01 PM · Unknown Object (Project)
ldionne accepted D94530: [libc++] improve feature test macro script.

Do you need help committing this? If so, please provide Author Name <email@domain>. Thanks!

Mon, Jan 18, 11:58 AM · Unknown Object (Project)
ldionne accepted D92044: Implement P0655R1 visit<R>: Explicit Return Type for visit.

Do you need help committing this?

Mon, Jan 18, 11:48 AM · Unknown Object (Project)
ldionne set the repository for D92044: Implement P0655R1 visit<R>: Explicit Return Type for visit to rG LLVM Github Monorepo.
Mon, Jan 18, 11:46 AM · Unknown Object (Project)
ldionne committed rG01a13f127a8b: [libc++] Rename check-cxx-deps to cxx-test-depends for consistency (authored by ldionne).
[libc++] Rename check-cxx-deps to cxx-test-depends for consistency
Mon, Jan 18, 11:42 AM
ldionne closed D94499: [libc++] Rename check-cxx-deps to cxx-test-depends for consistency.
Mon, Jan 18, 11:42 AM · Unknown Object (Project)
ldionne accepted D94499: [libc++] Rename check-cxx-deps to cxx-test-depends for consistency.
Mon, Jan 18, 11:41 AM · Unknown Object (Project)
ldionne added a comment to D94499: [libc++] Rename check-cxx-deps to cxx-test-depends for consistency.

LGTM!

Can / should this be added to the catch-all test-depends as well? (I thought llvm-test-depends and clang-test-depends were both added there somewhere, although I admit I just the code and I don't see it...) Regardless, this could be done separately / later if it makes sense to do.

Mon, Jan 18, 11:41 AM · Unknown Object (Project)
ldionne added inline comments to D94544: [libc++] NFCI: Refactor allocator_traits.
Mon, Jan 18, 11:38 AM · Unknown Object (Project)
ldionne updated the diff for D94544: [libc++] NFCI: Refactor allocator_traits.

Address Zoe's comments. Note that this is meant to be a NFC commit, so I don't
think we should incorporate additional changes beyond the refactorign to those
changes.

Mon, Jan 18, 11:38 AM · Unknown Object (Project)
ldionne updated the diff for D94718: [libc++] Unbreak the debug mode.

Fix test on GCC and without localization support.

Mon, Jan 18, 11:25 AM · Unknown Object (Project)
ldionne requested changes to D94564: [libcxx] Check return value for asprintf().

Please fix the CI issues and ping again if you'd like to get this reviewed.

Mon, Jan 18, 11:05 AM · Unknown Object (Project)
ldionne requested changes to D94571: [libcxx] random_device, for OpenBSD specify optimal entropy properties.
Mon, Jan 18, 11:03 AM · Unknown Object (Project)
ldionne added a comment to D94569: [libcxx] Wipe some more macros that do not belong in C++ forwarding headers.

I still don't understand how things work if we #undef those macros. If getc was a macro, then arguably ::getc (after undefining the macro) isn't pointing to a valid declaration, unless they decided to provide both a function ::getc *and then* to define a macro with the same name. How does that work? @brad

Mon, Jan 18, 11:01 AM · Unknown Object (Project)
ldionne requested changes to D93542: [SystemZ][ZOS] Provide CLOCK_MONOTONIC alternative.

If you say it's monotonic..

Mon, Jan 18, 10:56 AM · Unknown Object (Project)
ldionne added inline comments to D91141: [9/N] [libcxx] Implement the stat function family on top of native windows APIs.
Mon, Jan 18, 10:46 AM · Unknown Object (Project)
ldionne added a comment to D74489: [libc++] Remove workarounds for the lack of clock_gettime on older macOS platforms.

We're looking into bumping libc++ in chromium, and this is a problem for us. We statically link libc++, and we still support 10.11. (We _just_ dropped support for 10.10 in our last release iirc.) What do you recommend as path forward?

Mon, Jan 18, 10:18 AM · Unknown Object (Project)
ldionne added a comment to D94824: [MSVC] Disable <fstream> usage of <filesystem>.

@rnk Can you please take a look at https://reviews.llvm.org/D94921 and tell me if that solves your issues? That would be my preferred way forward.

Mon, Jan 18, 10:13 AM · Unknown Object (Project)
ldionne requested review of D94921: [libc++] Make LIBCXX_ENABLE_FILESYSTEM fully consistent.
Mon, Jan 18, 10:12 AM · Unknown Object (Project)
ldionne accepted D92110: [SystemZ][ZOS] Provide PATH_MAX macro for libcxx.

This is why i think that's okay (from the documentation you linked on Dec 9):

Mon, Jan 18, 9:37 AM · Unknown Object (Project)
ldionne accepted D94374: [CMake] Remove dead code setting policies to NEW.

Thanks!

Mon, Jan 18, 9:06 AM · Restricted Project, Restricted Project, Unknown Object (Project), Unknown Object (Project), Restricted Project, Restricted Project, Restricted Project, Unknown Object (Project)
ldionne added a comment to D94908: [libc++] Introduce __bits.

This LGTM, however I'll wait until the follow-up patches have been submitted to approve this. Can you please upload whatever patches depend on this and mark them as children of this one? If the follow-up patches make sense, then I have no objection with this.

Mon, Jan 18, 8:58 AM · Unknown Object (Project)
ldionne requested changes to D94909: [VE] Define FUTEX values.

Thanks for your contribution. However, it would be useful to have more information before going forward. The goal is to avoid adding random bits of complexity to the library for a system that isn't officially supported and maintained.

Mon, Jan 18, 8:56 AM · Restricted Project, Unknown Object (Project)
ldionne updated the diff for D94718: [libc++] Unbreak the debug mode.

Add -fPIC when compiling the tests.

Mon, Jan 18, 8:48 AM · Unknown Object (Project)

Fri, Jan 15

ldionne updated the diff for D94718: [libc++] Unbreak the debug mode.

Add comment to explain the purpose of the unit test.

Fri, Jan 15, 12:14 PM · Unknown Object (Project)
ldionne updated the diff for D94718: [libc++] Unbreak the debug mode.

Add test, fix potential for duplicate macro definition.

Fri, Jan 15, 12:13 PM · Unknown Object (Project)
ldionne accepted D94788: Fix libc++ clang-cl build, swap attribute order.

LGTM, but we should really fix the clang bug. Please wait for CI to finish before committing.

Fri, Jan 15, 9:57 AM · Unknown Object (Project)

Thu, Jan 14

ldionne added a comment to D94718: [libc++] Unbreak the debug mode.

See the discussion in https://reviews.llvm.org/rG1a92de00 for more context.

Thu, Jan 14, 1:43 PM · Unknown Object (Project)
ldionne requested review of D94718: [libc++] Unbreak the debug mode.
Thu, Jan 14, 1:43 PM · Unknown Object (Project)
ldionne added a comment to rG1a92de0064bc: [libc++] NFCI: Remove _LIBCPP_EXTERN_TEMPLATE2.

By the way, I think it is actually the following commit that broke you:

Thu, Jan 14, 1:35 PM
ldionne updated subscribers of rG1a92de0064bc: [libc++] NFCI: Remove _LIBCPP_EXTERN_TEMPLATE2.

This actually caused https://crbug.com/1166386. The explanation is that defining _LIBCPP_DEBUG causes _LIBCPP_EXTERN_TEMPLATE to be defined to nothing, which disables the extern declarations. Chrome uses _LIBCPP_DEBUG in debug builds. IIUC, locale needs these extern template declarations, because locale uses static data members with hidden visibility (need to confirm). If you remove the extern template declarations, then duplicate static data members may be emitted into the user's DSO, and when they use locale facets, they will be registered with a new id. I need to dig more to confirm, but this is what I'm going on.

Thu, Jan 14, 1:05 PM
ldionne accepted D94656: [libcxx testing] Fix UB in tests for std::lock_guard.

LGTM if CI passes. I think you need to re-upload the diff for it to trigger, since the LLVM Monorepo tag was somehow removed.

Thu, Jan 14, 12:41 PM · Unknown Object (Project)
ldionne set the repository for D94656: [libcxx testing] Fix UB in tests for std::lock_guard to rG LLVM Github Monorepo.
Thu, Jan 14, 12:40 PM · Unknown Object (Project)
ldionne accepted D93922: Mangle `__alignof__` differently than `alignof`..
Thu, Jan 14, 12:12 PM · Restricted Project, Unknown Object (Project), Restricted Project
ldionne added inline comments to D92776: [libc++] ADL-proof <algorithm> by adding _VSTD:: qualification on calls..
Thu, Jan 14, 10:01 AM · Unknown Object (Project)
ldionne requested changes to D94656: [libcxx testing] Fix UB in tests for std::lock_guard.

That seems correct to me: http://eel.is/c++draft/thread.mutex#requirements.mutex.general-14

Thu, Jan 14, 7:34 AM · Unknown Object (Project)

Wed, Jan 13

ldionne requested changes to D92110: [SystemZ][ZOS] Provide PATH_MAX macro for libcxx.

@ldionne which suggestion do you prefer?

Wed, Jan 13, 10:38 AM · Unknown Object (Project)
ldionne added a comment to D94569: [libcxx] Wipe some more macros that do not belong in C++ forwarding headers.

The C11 standard states (in section 7.21.7.5 The getc function):

2 The getc function is equivalent to fgetc, except that if it is implemented as a macro, it may evaluate stream more than once, so the argument should never be an expression with side effects.

Wed, Jan 13, 8:37 AM · Restricted Project
ldionne added inline comments to D94571: [libcxx] random_device, for OpenBSD specify optimal entropy properties.
Wed, Jan 13, 8:34 AM · Restricted Project
ldionne added a comment to D93542: [SystemZ][ZOS] Provide CLOCK_MONOTONIC alternative.

Unless I'm heavily mistaken, gettimeofday() isn't monotonic. It's not a good idea to implement a feature in a way that we know to be broken from the start.

I think it would be best for z/OS to provide a monotonic clock, or for libc++ on z/OS to acknowledge the lack of such support and use _LIBCPP_HAS_NO_THREADS.

Wed, Jan 13, 8:31 AM · Restricted Project