ldionne (Louis Dionne)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 11 2015, 3:26 PM (178 w, 6 d)

Recent Activity

Today

ldionne created D49509: [libc++] Allow running ABI list tests with different ABI versions.
Wed, Jul 18, 2:21 PM
ldionne updated the diff for D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY.

Split the definition of _LIBCPP_ALWAYS_INLINE into compiler-specific sections
of __config, per Marshall's comment.

Wed, Jul 18, 12:40 PM
ldionne added inline comments to D49502: [CMake] Support statically linking dependencies only to shared or static library.
Wed, Jul 18, 12:20 PM
ldionne added a reviewer for D49502: [CMake] Support statically linking dependencies only to shared or static library: ldionne.
Wed, Jul 18, 12:11 PM
ldionne added a reviewer for D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY: dexonsmith.
Wed, Jul 18, 10:06 AM
ldionne added a comment to D48912: [libc++] Add deprecated attributes to many deprecated components.

Ping

Wed, Jul 18, 8:46 AM

Mon, Jul 16

ldionne updated subscribers of D45805: [libcxx] Remove redundant specializations in type_traits..
Mon, Jul 16, 9:01 AM

Fri, Jul 13

ldionne updated the diff for D48912: [libc++] Add deprecated attributes to many deprecated components.

Made sure the attributes worked properly on GCC.

Fri, Jul 13, 1:43 PM
ldionne accepted D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets.

LGTM -- great! Just make sure the commit message does not pretend the change includes merge.

Fri, Jul 13, 12:56 PM
ldionne added inline comments to D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets.
Fri, Jul 13, 9:33 AM

Thu, Jul 12

ldionne added inline comments to D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY.
Thu, Jul 12, 8:08 AM
ldionne created D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY.
Thu, Jul 12, 8:08 AM
ldionne planned changes to D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY.
Thu, Jul 12, 8:08 AM

Wed, Jul 11

ldionne committed rCXX336866: [libc++] Take 2: Replace uses of _LIBCPP_ALWAYS_INLINE by….
[libc++] Take 2: Replace uses of _LIBCPP_ALWAYS_INLINE by…
Wed, Jul 11, 4:19 PM
ldionne committed rL336866: [libc++] Take 2: Replace uses of _LIBCPP_ALWAYS_INLINE by….
[libc++] Take 2: Replace uses of _LIBCPP_ALWAYS_INLINE by…
Wed, Jul 11, 4:19 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Wed, Jul 11, 4:19 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Wed, Jul 11, 4:19 PM
ldionne added a comment to D48892: [libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by _LIBCPP_INLINE_VISIBILITY.

Pushing since I got an offline O.K. from Eric.

Wed, Jul 11, 4:18 PM
ldionne added a comment to D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets.

I forgot to mention it in my initial review, but it also seems like you forgot to update all the synopsis in the comments at the beginning of headers.

Wed, Jul 11, 2:44 PM
ldionne requested changes to D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets.

Generally looks pretty good, but I have a couple questions/suggestions.

Wed, Jul 11, 2:08 PM

Tue, Jul 10

ldionne committed rCXX336709: [libc++] Declare noop_coroutine() with _LIBCPP_INLINE_VISIBILITY.
[libc++] Declare noop_coroutine() with _LIBCPP_INLINE_VISIBILITY
Tue, Jul 10, 10:43 AM
ldionne committed rL336709: [libc++] Declare noop_coroutine() with _LIBCPP_INLINE_VISIBILITY.
[libc++] Declare noop_coroutine() with _LIBCPP_INLINE_VISIBILITY
Tue, Jul 10, 10:43 AM
ldionne closed D49145: [libc++] Declare noop_coroutine() with _LIBCPP_INLINE_VISIBILITY.
Tue, Jul 10, 10:43 AM
ldionne created D49145: [libc++] Declare noop_coroutine() with _LIBCPP_INLINE_VISIBILITY.
Tue, Jul 10, 9:32 AM
ldionne committed rCXX336665: [libc++] Declare <compare> operators with the proper visibility attribute.
[libc++] Declare <compare> operators with the proper visibility attribute
Tue, Jul 10, 6:26 AM
ldionne committed rL336665: [libc++] Declare <compare> operators with the proper visibility attribute.
[libc++] Declare <compare> operators with the proper visibility attribute
Tue, Jul 10, 6:26 AM
ldionne closed D49104: [libc++] Declare <compare> operators with the proper visibility attribute.
Tue, Jul 10, 6:26 AM

Mon, Jul 9

ldionne added a comment to D49104: [libc++] Declare <compare> operators with the proper visibility attribute.

I have a series of libc++ clang-tidy checks in a clang-tool-extra fork. One of them being visibility-attr-not-on-first-decl, perhaps I should try to upstream it.

Mon, Jul 9, 4:18 PM
ldionne created D49104: [libc++] Declare <compare> operators with the proper visibility attribute.
Mon, Jul 9, 3:27 PM
ldionne added a comment to D48892: [libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by _LIBCPP_INLINE_VISIBILITY.

I've now managed to run the check-cxx-abilist test on my machine and it passes. I'd like to commit this again, @EricWF am I good to go?

Mon, Jul 9, 2:27 PM
ldionne added a comment to D48912: [libc++] Add deprecated attributes to many deprecated components.

Regarding whether we want to enable or disable those warnings by default: it seems like we've reached consensus (on the LLVM IRC) for enabling the deprecation warnings by default. However, I suggest we keep them disabled by default in this commit, and then change the default in a subsequent change. This will make things easier from an integration perspective, and also in case we need to roll back the defaults because of some unforeseen circumstances.

Mon, Jul 9, 11:20 AM

Fri, Jul 6

ldionne added inline comments to D48892: [libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by _LIBCPP_INLINE_VISIBILITY.
Fri, Jul 6, 8:36 AM
ldionne updated the diff for D48892: [libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by _LIBCPP_INLINE_VISIBILITY.

This revision to the patch fixes a problem where __pbump had been applied
_LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY instead of _LIBCPP_INLINE_VISIBILITY,
which caused the symbols exported in the ABI to change and broke the CI.

Fri, Jul 6, 8:33 AM

Thu, Jul 5

ldionne abandoned D48983: [libc++] Replace incorrect visibility on a streambuf method.

I'm dropping this because I reverted https://reviews.llvm.org/D48892 until I figure out why it's breaking LLDB. The next time I submit https://reviews.llvm.org/D48892, I'll make sure to do it without this problem.

Thu, Jul 5, 11:47 AM
ldionne committed rCXX336382: Revert "[libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by….
Revert "[libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by…
Thu, Jul 5, 11:47 AM
ldionne committed rL336382: Revert "[libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by….
Revert "[libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by…
Thu, Jul 5, 11:46 AM
ldionne added a comment to D48892: [libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by _LIBCPP_INLINE_VISIBILITY.

I reverted this commit. Sorry for the blunder. I'll take a look at why LLDB's tests are doing this.

Thu, Jul 5, 11:46 AM
ldionne added a comment to D48983: [libc++] Replace incorrect visibility on a streambuf method.

It's pretty obvious that this is the problem when looking at https://reviews.llvm.org/D48892 (grep for __pbump), but TBH I haven't been able to run the check-cxx-abilist test locally. Mine fails with like 500 differences, so there's got to be something I'm doing wrong.

Thu, Jul 5, 11:28 AM
ldionne created D48983: [libc++] Replace incorrect visibility on a streambuf method.
Thu, Jul 5, 11:18 AM
ldionne committed rCXX336369: [libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by _LIBCPP_INLINE_VISIBILITY.
[libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by _LIBCPP_INLINE_VISIBILITY
Thu, Jul 5, 9:54 AM
ldionne committed rL336369: [libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by _LIBCPP_INLINE_VISIBILITY.
[libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by _LIBCPP_INLINE_VISIBILITY
Thu, Jul 5, 9:54 AM
ldionne closed D48892: [libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by _LIBCPP_INLINE_VISIBILITY.
Thu, Jul 5, 9:54 AM
ldionne closed D48892: [libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by _LIBCPP_INLINE_VISIBILITY.
Thu, Jul 5, 9:54 AM
ldionne committed rCXX336368: [NFC] Add <initializer_list> to the synopsis of <utility>.
[NFC] Add <initializer_list> to the synopsis of <utility>
Thu, Jul 5, 9:21 AM
ldionne committed rL336368: [NFC] Add <initializer_list> to the synopsis of <utility>.
[NFC] Add <initializer_list> to the synopsis of <utility>
Thu, Jul 5, 9:21 AM
ldionne closed D48611: [NFC] Add <initializer_list> to the synopsis of <utility>.
Thu, Jul 5, 9:21 AM

Wed, Jul 4

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

Before the patch, the backtrace refers to the instantiation of the set's destructor instead of referring to the set's instantiation itself:

Wed, Jul 4, 3:01 PM
ldionne created D48955: [libc++] Improve diagnostics for non-const comparators and hashers in associative containers.
Wed, Jul 4, 2:55 PM
ldionne added a comment to D48912: [libc++] Add deprecated attributes to many deprecated components.

FWIW, I would prefer to enable deprecation warnings by default and to have a way of turning them off -- so as to be as close to the standard as possible by default. But I'm fine either way, and this is always a choice we can revisit later by just switching the default from opt-in to opt-out.

Wed, Jul 4, 12:53 PM
ldionne updated the diff for D48912: [libc++] Add deprecated attributes to many deprecated components.

Addressed reviewer feedback.

Wed, Jul 4, 8:50 AM
ldionne added inline comments to D48912: [libc++] Add deprecated attributes to many deprecated components.
Wed, Jul 4, 8:44 AM

Tue, Jul 3

ldionne added a comment to D48892: [libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by _LIBCPP_INLINE_VISIBILITY.

Note that this revision also removes _LIBCPP_ALWAYS_INLINE because it is not used anymore and I want to avoid any potential confusion. If we do come around a need to mark something as always inline in the future, we should reintroduce the macro for that use case. I'm not sure such a use case will come any time soon, though.

Tue, Jul 3, 7:12 PM
ldionne planned changes to D48912: [libc++] Add deprecated attributes to many deprecated components.
Tue, Jul 3, 6:51 PM
ldionne created D48912: [libc++] Add deprecated attributes to many deprecated components.
Tue, Jul 3, 6:51 PM
ldionne added inline comments to D48892: [libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by _LIBCPP_INLINE_VISIBILITY.
Tue, Jul 3, 12:52 PM
ldionne added a comment to D48892: [libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by _LIBCPP_INLINE_VISIBILITY.

Note that even in case we end up renaming _LIBCPP_INLINE_VISIBILITY to something else, this patch is useful because it ensures that nothing was _actually_ meant to be always inline. So any potential rename can then be just that -- a mechanical rename with no potential for semantic change.

Tue, Jul 3, 12:35 PM
ldionne created D48892: [libc++] Replace uses of _LIBCPP_ALWAYS_INLINE by _LIBCPP_INLINE_VISIBILITY.
Tue, Jul 3, 12:33 PM
ldionne added a comment to D48694: [libc++abi] Limit libc++ header search to specified paths.

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.

Tue, Jul 3, 7:37 AM

Fri, Jun 29

ldionne accepted D48616: Implement LWG 2946, 3075 and 3076.

LGTM

Fri, Jun 29, 1:36 PM

Thu, Jun 28

ldionne added a comment to D48694: [libc++abi] Limit libc++ header search to specified paths.

I guess I'm echoing some of Eric's comments, but does it make sense to build libc++abi against a system-installed libc++? If not, this change makes sense.

Thu, Jun 28, 3:38 PM
ldionne added inline comments to D48616: Implement LWG 2946, 3075 and 3076.
Thu, Jun 28, 3:20 PM
ldionne added inline comments to D48616: Implement LWG 2946, 3075 and 3076.
Thu, Jun 28, 11:18 AM

Wed, Jun 27

ldionne created D48669: [pair] Mark constructors as conditionally noexcept.
Wed, Jun 27, 1:18 PM
ldionne added inline comments to D48616: Implement LWG 2946, 3075 and 3076.
Wed, Jun 27, 12:13 PM
ldionne added a comment to D48616: Implement LWG 2946, 3075 and 3076.

More missed noexcepts.

Wed, Jun 27, 10:51 AM
ldionne added a comment to D48616: Implement LWG 2946, 3075 and 3076.

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.

Wed, Jun 27, 10:46 AM
ldionne added a comment to D48611: [NFC] Add <initializer_list> to the synopsis of <utility>.

I can't commit this myself because I don't have commit rights yet.

Wed, Jun 27, 9:03 AM

Tue, Jun 26

ldionne created D48611: [NFC] Add <initializer_list> to the synopsis of <utility>.
Tue, Jun 26, 2:58 PM

May 26 2016

ldionne updated the diff for D15421: [Feature] Add a builtin for indexing into parameter packs.

Rebase the patch on top of the current master. The patch passes all of Clang's tests (make check-clang). I also removed the TODO comment about diagnostics, since that was answered by Richard Smith.

May 26 2016, 8:45 AM

May 25 2016

ldionne added a comment to D15421: [Feature] Add a builtin for indexing into parameter packs.

No problem for the delay; we're all busy :-). Unfortunately, the patch does not apply cleanly on master anymore, so I'll rebase it and someone can then commit it for me.

May 25 2016, 7:29 PM

May 10 2016

ldionne added a comment to D15421: [Feature] Add a builtin for indexing into parameter packs.

ping

May 10 2016, 7:26 PM

Feb 15 2016

ldionne updated the diff for D15421: [Feature] Add a builtin for indexing into parameter packs.

Address some remaining issues:

  • Rename nth_element to type_pack_element, as suggested by Richard Smith
  • Fix template parameter position issue in createTypePackElementParameterList
Feb 15 2016, 2:10 PM

Jan 14 2016

ldionne updated the diff for D15421: [Feature] Add a builtin for indexing into parameter packs.

Address Richard Smith's review comments:

  • Remove IndexType parameter, and make Index a std::size_t
  • Remove assertion about APSInt::GetExtValue()
  • Other style changes
Jan 14 2016, 11:10 AM

Jan 13 2016

ldionne added a comment to D15421: [Feature] Add a builtin for indexing into parameter packs.

Bikeshedding on the name a bit... how about __type_pack_element?

Hmm, I kind of felt like having nth in there implied we're indexing into something... What about __nth_pack_element?

Jan 13 2016, 2:36 PM
ldionne updated the diff for D15421: [Feature] Add a builtin for indexing into parameter packs.

Rebase on top of the master branch, and add Richard Smith as a reviewer, since he also reviewed David Majnemer's original patch (suggested by Nathan Wilson).

Jan 13 2016, 12:29 PM

Dec 21 2015

ldionne added a comment to D12502: [libcxx] Better constain tuples constructors -- Fix PR23256 and PR22806.

This LGTM. I also just confirmed that it fixes the original issue in Hana that caused me to report PR22806.

Dec 21 2015, 6:32 PM

Dec 10 2015

ldionne added a comment to D15421: [Feature] Add a builtin for indexing into parameter packs.

We should probably consider changing the name from __nth_element to something else, perhaps something that contains the word "pack". This is especially true if we are to implement other intrinsics for manipulating parameter packs, like slicing (as proposed by Eric Fiselier). Unless someone with a clearer view of the project suggests something, I would probably go with __pack_element (and then we could add __pack_slice, etc...). I have no strong preference.

Dec 10 2015, 10:04 AM
ldionne updated the diff for D15421: [Feature] Add a builtin for indexing into parameter packs.

Added full lines of context. Sorry for not doing it the first time around.

Dec 10 2015, 10:00 AM
ldionne retitled D15421: [Feature] Add a builtin for indexing into parameter packs from to [Feature] Add a builtin for indexing into parameter packs.
Dec 10 2015, 9:32 AM
ldionne updated subscribers of D14411: Use __attribute__((internal_linkage)) when available..
Dec 10 2015, 9:18 AM

Nov 26 2015

ldionne updated subscribers of D14814: [libcxx] Use __make_integer_seq builtin when available.
Nov 26 2015, 4:07 AM

Mar 22 2015

ldionne added a comment to D6171: Fix PR20619 - failure to define implicit copy ctor of a by val capture in a generic lambda.

As a motivation: if we get through this review and we ever meet at a conference, I'm buying both of you guys a beer. Let's finish this!

Mar 22 2015, 7:49 AM

Mar 20 2015

ldionne updated subscribers of D6171: Fix PR20619 - failure to define implicit copy ctor of a by val capture in a generic lambda.
Mar 20 2015, 2:03 PM

Feb 12 2015

ldionne added a comment to D7569: [libc++] Try and prevent evaluation of `is_default_constructible` on tuples default constructor if it is not needed..

Could you explain why the dependent type trick actually works? I'm
unsure as to why clang thinks it can evaluate one set of template
arguments but not the other even though it has essentially the same
information when evaluating tuple().

Feb 12 2015, 7:34 AM

Feb 11 2015

ldionne added a comment to D7569: [libc++] Try and prevent evaluation of `is_default_constructible` on tuples default constructor if it is not needed..

The faulty code was mine. With the pair example, it is clear that all those is_default_constructible<T> are instantiated even when the default constructor is not. I think my intention when writing Dummy && is_default_constructible<_Tp>::value was to delay the instantiation of is_default_constructible<_Tp> as a compile-time optimization (which it fails to do), but not for correctness purposes.

Feb 11 2015, 3:45 PM