ldionne (Louis Dionne)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 11 2015, 3:26 PM (188 w, 5 d)

Recent Activity

Today

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

You did not get a thumbs up from any of the code owners for libc++. Reverted in r342938.

Mon, Sep 24, 9:14 PM
ldionne committed rCXX342938: Revert r342936 "Remove redundant null pointer check in operator delete".
Revert r342936 "Remove redundant null pointer check in operator delete"
Mon, Sep 24, 9:14 PM
ldionne committed rL342938: Revert r342936 "Remove redundant null pointer check in operator delete".
Revert r342936 "Remove redundant null pointer check in operator delete"
Mon, Sep 24, 9:14 PM
ldionne updated subscribers of D46443: Add missing cstdalign header.

I believe this header has been deprecated. According to http://eel.is/c++draft/diff.cpp17.library, the effect of this header is nothing. @mclow.lists can you chime in?

Mon, Sep 24, 9:56 AM
ldionne committed rL342855: [libcxx] Fix the binder deprecation tests on Clang 5..
[libcxx] Fix the binder deprecation tests on Clang 5.
Mon, Sep 24, 7:23 AM
ldionne committed rL342854: [libcxx] Fix buildbots on Debian.
[libcxx] Fix buildbots on Debian
Mon, Sep 24, 7:23 AM
ldionne committed rL342849: [libcxx] Document new symbols __u64toa and __u32toa on Darwin.
[libcxx] Document new symbols __u64toa and __u32toa on Darwin
Mon, Sep 24, 7:23 AM
ldionne committed rL342843: [libc++] Add deprecated attributes to many deprecated components.
[libc++] Add deprecated attributes to many deprecated components
Mon, Sep 24, 7:23 AM
ldionne committed rCXX342855: [libcxx] Fix the binder deprecation tests on Clang 5..
[libcxx] Fix the binder deprecation tests on Clang 5.
Mon, Sep 24, 7:22 AM
ldionne committed rL342840: [NFC][libcxx] Rename helpers with 4 underscores to something more reasonable.
[NFC][libcxx] Rename helpers with 4 underscores to something more reasonable
Mon, Sep 24, 7:22 AM
ldionne committed rCXX342854: [libcxx] Fix buildbots on Debian.
[libcxx] Fix buildbots on Debian
Mon, Sep 24, 7:22 AM
ldionne committed rCXX342849: [libcxx] Document new symbols __u64toa and __u32toa on Darwin.
[libcxx] Document new symbols __u64toa and __u32toa on Darwin
Mon, Sep 24, 7:22 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Mon, Sep 24, 7:22 AM
ldionne committed rCXX342843: [libc++] Add deprecated attributes to many deprecated components.
[libc++] Add deprecated attributes to many deprecated components
Mon, Sep 24, 7:22 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Mon, Sep 24, 7:22 AM
ldionne committed rCXX342840: [NFC][libcxx] Rename helpers with 4 underscores to something more reasonable.
[NFC][libcxx] Rename helpers with 4 underscores to something more reasonable
Mon, Sep 24, 7:22 AM

Yesterday

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

Was this true pre-C11 too? If not, then this needs to be guarded by #if _LIBCPP_STD_VER >= 17, because C++ is only on top of C11 in C++17 and above (Marshall can double-check this).

Sun, Sep 23, 12:09 AM

Sat, Sep 22

ldionne added a comment to D45639: [Driver] Support default libc++ library location on Darwin.

I think on Darwin it would not make sense to have libc++fs.a ship in libc++.dylib especially considering that it ends up in the dyld cache and that has a lot of other implications. It would make sense to ship it as a separate library, perhaps as part of the SDK, at least for now.

Sat, Sep 22, 5:14 PM
ldionne accepted D47395: [libcxx] [test] Remove nonportable locale assumption in basic.ios.members/narrow.pass.cpp.

LGTM and has been committed -- you can close this.

Sat, Sep 22, 5:07 PM
ldionne updated the diff for D52397: [libc++] Remove Fuchsia-specific knowledge to pick the ABI version.

Clarify that version 2 of the ABI is _currently_ unstable, it is not _the_ unstable version.

Sat, Sep 22, 4:55 PM
ldionne added a comment to D45639: [Driver] Support default libc++ library location on Darwin.

@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 dylib from one version to the next, so _of course_ it won't always work.

@ldionne it's the other way round. Today, Clang driver on macOS automatically uses headers that are part of the compiler distribution (i.e. -I../include/c++/v1) but it always links against the system library (/usr/lib/libc++.dylib) because the library path to libraries that are shipped with the compiler (-L../lib) is never added to the link-line. Those two may not necessarily be the same version, and almost certainly won't be if you use Clang build from trunk.

Sat, Sep 22, 4:29 PM
ldionne created D52397: [libc++] Remove Fuchsia-specific knowledge to pick the ABI version.
Sat, Sep 22, 3:24 PM
ldionne committed rCXX342822: [NFC][libc++] Fix typo in the description of LIBCXX_INCLUDE_BENCHMARKS.
[NFC][libc++] Fix typo in the description of LIBCXX_INCLUDE_BENCHMARKS
Sat, Sep 22, 2:33 PM
ldionne committed rL342822: [NFC][libc++] Fix typo in the description of LIBCXX_INCLUDE_BENCHMARKS.
[NFC][libc++] Fix typo in the description of LIBCXX_INCLUDE_BENCHMARKS
Sat, Sep 22, 2:33 PM
ldionne committed rUNW342811: [libunwind][NFC] Suppress unused parameter warnings.
[libunwind][NFC] Suppress unused parameter warnings
Sat, Sep 22, 12:53 PM
ldionne added a comment to D48912: [libc++] Add deprecated attributes to many deprecated components.

Are we waiting for anything to ship this?

Sat, Sep 22, 11:49 AM
ldionne committed rCXX342813: [libcxx] Fix the definition of the check-cxx-abilist target on Darwin.
[libcxx] Fix the definition of the check-cxx-abilist target on Darwin
Sat, Sep 22, 11:41 AM
ldionne committed rL342813: [libcxx] Fix the definition of the check-cxx-abilist target on Darwin.
[libcxx] Fix the definition of the check-cxx-abilist target on Darwin
Sat, Sep 22, 11:41 AM
ldionne closed D52394: [libcxx] Fix the definition of the check-cxx-abilist target on Darwin.
Sat, Sep 22, 11:41 AM
ldionne created D52396: [libcxx] Document new symbols __u64toa and __u32toa on Darwin.
Sat, Sep 22, 11:38 AM
ldionne committed rL342811: [libunwind][NFC] Suppress unused parameter warnings.
[libunwind][NFC] Suppress unused parameter warnings
Sat, Sep 22, 11:22 AM
ldionne closed D52393: [libunwind][NFC] Suppress unused parameter warnings.
Sat, Sep 22, 11:22 AM
ldionne updated the diff for D52394: [libcxx] Fix the definition of the check-cxx-abilist target on Darwin.

Remove the change to the Darwin ABI list file, which was meant for a different commit.

Sat, Sep 22, 11:16 AM
ldionne created D52394: [libcxx] Fix the definition of the check-cxx-abilist target on Darwin.
Sat, Sep 22, 11:16 AM
ldionne created D52393: [libunwind][NFC] Suppress unused parameter warnings.
Sat, Sep 22, 11:03 AM

Fri, Sep 21

ldionne updated the diff for D51789: [clang] Add the exclude_from_explicit_instantiation attribute.

Fix warnings on undefined entities

Fri, Sep 21, 6:35 PM

Wed, Sep 19

ldionne accepted D52240: Partial Fix for PR#38964.

Marshall, can you confirm that the change does not break check-cxx-abilist on OS X. I'm fine with the change if that's not broken.

Wed, Sep 19, 8:39 AM
ldionne added a comment to D52240: Partial Fix for PR#38964.

Marshall, what's the error we get with GCC in C++03 mode that this patch is fixing? Is there a bug.llvm.org associated to that?

PR#38964 (from the title) ;-)

https://llvm.org/PR38964

Wed, Sep 19, 8:39 AM
ldionne added a comment to D52240: Partial Fix for PR#38964.

Marshall, what's the error we get with GCC in C++03 mode that this patch is fixing? Is there a bug.llvm.org associated to that?

Wed, Sep 19, 7:51 AM

Tue, Sep 18

ldionne added a comment to D51789: [clang] Add the exclude_from_explicit_instantiation attribute.

That may work for libc++'s purposes, but it's clearly inappropriate as a compiler rule. There are good reasons why something with hidden visibility would need to be explicitly instantiated.

Tue, Sep 18, 8:21 AM
ldionne added a comment to D51789: [clang] Add the exclude_from_explicit_instantiation attribute.

I'm thinking about this some more, and I'm wondering whether it would be a viable solution to just exclude members marked with hidden visibility from explicit template instantiations (and declarations thereof). I thought I had been convinced by Hubert that visibility and this attribute should be left separate, but now I'm not sure anymore. I don't think there's a single case of a member function with hidden visibility that we don't want to exclude from explicit instantiations. @rsmith Thoughts?

Tue, Sep 18, 6:36 AM

Mon, Sep 17

ldionne updated the diff for D51789: [clang] Add the exclude_from_explicit_instantiation attribute.

Add example of how to use the attribute, per Aaron's comment.

Mon, Sep 17, 7:49 AM
ldionne updated the diff for D51789: [clang] Add the exclude_from_explicit_instantiation attribute.

Reformat warning message and run clang-format on changed lines.

Mon, Sep 17, 7:19 AM
ldionne added inline comments to D51789: [clang] Add the exclude_from_explicit_instantiation attribute.
Mon, Sep 17, 7:18 AM

Fri, Sep 14

ldionne retitled D51789: [clang] Add the exclude_from_explicit_instantiation attribute from [clang] Add the no_extern_template attribute to [clang] Add the exclude_from_explicit_instantiation attribute.
Fri, Sep 14, 12:38 PM
ldionne updated the diff for D51789: [clang] Add the exclude_from_explicit_instantiation attribute.

Change no_extern_template to exclude_from_explicit_instantiation

Fri, Sep 14, 12:37 PM
ldionne added a comment to D51789: [clang] Add the exclude_from_explicit_instantiation attribute.

OK, so the semantics of this attribute are "explicit instantiation declarations or definitions applied to the enclosing class do not apply to this member", right? So it opts the member out of both extern template and (non-extern) template, but only when applied to an enclosing class.

Yes, but it also (obviously) opts the member out when applied to the member itself, not only to an enclosing class.

Assuming I'm understanding you correctly, I don't think the current implementation does that (nor does the documentation change say that), and I think that's a feature not a bug. For example, given:

template<typename T> struct A {
  [[clang::no_extern_template]] void f() {}
  void g() {}
};
template struct A<int>; // instantiates A<int> and definition of A<int>::g(), does not instantiate definition of A<int>::f()
template void A<int>::f(); // instantiates definition of A<int>::f() despite attribute

... the attribute affects the first explicit instantiation but not the second (I think you're saying it should affect both, and make the second explicit instantiation have no effect). I think the current behavior in this patch is appropriate: making the attribute also affect the second case makes it strictly less useful. [...]

Fri, Sep 14, 10:32 AM
ldionne added a comment to D51789: [clang] Add the exclude_from_explicit_instantiation attribute.

I think now's a good time to bikeshed the name of the attribute if you have other suggestions.

OK, so the semantics of this attribute are "explicit instantiation declarations or definitions applied to the enclosing class do not apply to this member", right? So it opts the member out of both extern template and (non-extern) template, but only when applied to an enclosing class.

Fri, Sep 14, 10:13 AM
ldionne added a comment to D51789: [clang] Add the exclude_from_explicit_instantiation attribute.

I think now's a good time to bikeshed the name of the attribute if you have other suggestions.

Fri, Sep 14, 7:48 AM
ldionne retitled D51789: [clang] Add the exclude_from_explicit_instantiation attribute from [WIP][clang] Add the no_extern_template attribute to [clang] Add the no_extern_template attribute.
Fri, Sep 14, 7:37 AM
ldionne updated the diff for D51789: [clang] Add the exclude_from_explicit_instantiation attribute.

Fix the tests and remove some warnings that I wasn't able to generate properly
(to avoid false positives).

Fri, Sep 14, 7:36 AM
ldionne committed rL342238: [clang] Make sure attributes on member classes are applied properly.
[clang] Make sure attributes on member classes are applied properly
Fri, Sep 14, 7:10 AM
ldionne committed rC342238: [clang] Make sure attributes on member classes are applied properly.
[clang] Make sure attributes on member classes are applied properly
Fri, Sep 14, 7:10 AM
ldionne closed D51997: [clang] Make sure attributes on member classes are applied properly.
Fri, Sep 14, 7:10 AM

Thu, Sep 13

ldionne added a comment to D51997: [clang] Make sure attributes on member classes are applied properly.

I created https://bugs.llvm.org/show_bug.cgi?id=38942 to keep track of the problem for partial specializations and explicit specializations. Moving forward with this patch will allow me to land the no_extern_template attribute.

Thu, Sep 13, 2:18 PM
ldionne updated the diff for D51997: [clang] Make sure attributes on member classes are applied properly.

Drop support for partial specializations and explicit specializations.
Address comments by Erik.

Thu, Sep 13, 2:09 PM
ldionne added inline comments to D51997: [clang] Make sure attributes on member classes are applied properly.
Thu, Sep 13, 2:09 PM
ldionne added inline comments to D51997: [clang] Make sure attributes on member classes are applied properly.
Thu, Sep 13, 10:53 AM

Wed, Sep 12

ldionne added a comment to D51997: [clang] Make sure attributes on member classes are applied properly.

This patch is missing support for partial specializations and explicit specializations. The part I don't understand is how to get the CXXRecordDecls to have the right attribute below. Here's the AST dump for the test file with the current patch:

Wed, Sep 12, 11:14 AM
ldionne created D51997: [clang] Make sure attributes on member classes are applied properly.
Wed, Sep 12, 10:56 AM
ldionne accepted D51955: Create infrastructure for defining and testing feature test macros.

Thanks! I don't like feature test macros either, but we should still strive for conformance.

Wed, Sep 12, 10:04 AM

Mon, Sep 10

ldionne added a comment to D51888: [ADT] bit_cast: check for is_trivially_copyable more portably.

I think that works because memcpy this way is valid IFF To and From are trivially copyable. LGTM.

Mon, Sep 10, 1:50 PM
ldionne added inline comments to D51872: isPodLike: handle tuple and array.
Mon, Sep 10, 1:02 PM
ldionne added inline comments to D51872: isPodLike: handle tuple and array.
Mon, Sep 10, 1:01 PM

Fri, Sep 7

ldionne added a comment to D51789: [clang] Add the exclude_from_explicit_instantiation attribute.

This patch is an early WIP -- I'm just uploading it to serve as a support for a message I'm about to post on cfe-dev. No need to review seriously for now.

Fri, Sep 7, 8:04 AM
ldionne created D51789: [clang] Add the exclude_from_explicit_instantiation attribute.
Fri, Sep 7, 8:03 AM

Thu, Sep 6

ldionne committed rCXX341551: [libc++] Add a link to the Release notes from the main libc++ documentation.
[libc++] Add a link to the Release notes from the main libc++ documentation
Thu, Sep 6, 8:26 AM
ldionne committed rCXX341550: [libcxx] Add ReleaseNotes.rst file for release notes.
[libcxx] Add ReleaseNotes.rst file for release notes
Thu, Sep 6, 8:26 AM
ldionne committed rL341551: [libc++] Add a link to the Release notes from the main libc++ documentation.
[libc++] Add a link to the Release notes from the main libc++ documentation
Thu, Sep 6, 8:08 AM
ldionne committed rL341550: [libcxx] Add ReleaseNotes.rst file for release notes.
[libcxx] Add ReleaseNotes.rst file for release notes
Thu, Sep 6, 7:49 AM

Aug 24 2018

ldionne committed rL340609: [libc++] Fix handling of negated character classes in regex.
[libc++] Fix handling of negated character classes in regex
Aug 24 2018, 7:11 AM
ldionne committed rCXX340609: [libc++] Fix handling of negated character classes in regex.
[libc++] Fix handling of negated character classes in regex
Aug 24 2018, 7:11 AM
ldionne closed D50534: [libc++] Fix handling of negated character classes in regex.
Aug 24 2018, 7:11 AM
ldionne committed rCXX340608: [libc++] Remove race condition in std::async.
[libc++] Remove race condition in std::async
Aug 24 2018, 7:02 AM
ldionne committed rL340608: [libc++] Remove race condition in std::async.
[libc++] Remove race condition in std::async
Aug 24 2018, 7:02 AM
ldionne closed D51170: [libc++] Remove race condition in std::async.
Aug 24 2018, 7:01 AM
ldionne added inline comments to D51170: [libc++] Remove race condition in std::async.
Aug 24 2018, 6:59 AM

Aug 23 2018

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

LGTM. Don't forget to update https://reviews.llvm.org/D48896 so it uncomments this. Also, this should be merged into LLVM 7.

Aug 23 2018, 10:02 AM
ldionne created D51170: [libc++] Remove race condition in std::async.
Aug 23 2018, 9:21 AM

Aug 21 2018

ldionne committed rC340288: [clang][NFC] Fix typo in the name of a note.
[clang][NFC] Fix typo in the name of a note
Aug 21 2018, 8:55 AM
ldionne committed rL340288: [clang][NFC] Fix typo in the name of a note.
[clang][NFC] Fix typo in the name of a note
Aug 21 2018, 8:55 AM
ldionne closed D51043: [clang][NFC] Fix typo in the name of a note.
Aug 21 2018, 8:55 AM
ldionne created D51043: [clang][NFC] Fix typo in the name of a note.
Aug 21 2018, 8:46 AM
ldionne added inline comments to D47814: Teach libc++ to use native NetBSD's max_align_t.
Aug 21 2018, 7:20 AM

Aug 17 2018

ldionne added a comment to D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI.

Thanks! Merged to 7.0 in r339882.

Aug 17 2018, 10:03 AM
ldionne accepted D50876: Clean up newly created <bit> header.

LGTM with the __clz you missed.

Aug 17 2018, 6:46 AM

Aug 16 2018

ldionne committed rCXX339874: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI.
[libcxx] By default, do not use internal_linkage to hide symbols from the ABI
Aug 16 2018, 5:45 AM
ldionne committed rL339874: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI.
[libcxx] By default, do not use internal_linkage to hide symbols from the ABI
Aug 16 2018, 5:45 AM
ldionne closed D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI.
Aug 16 2018, 5:45 AM

Aug 15 2018

ldionne accepted D50815: Establish the <bit> header.

LGTM

Aug 15 2018, 7:16 PM
ldionne added a comment to D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI.

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.

Aug 15 2018, 3:34 PM
ldionne accepted D50799: Fix for PR 38495: <ctime> no longer compiles on FreeBSD, due to lack of timespec_get().

LGTM.

Aug 15 2018, 2:00 PM
ldionne added a comment to D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI.

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 isn't the executable size blowup we see in chromium a problem for everyone else too? Is the plan to fix ld64's string pooling at the same time as rolling this change out, and this is just a workaround for people who have head libc++ but not head ld64?

Aug 15 2018, 12:45 PM
ldionne updated the diff for D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI.

Allow picking a custom default behavior for vendors, per Duncan's comment.

Aug 15 2018, 11:40 AM

Aug 14 2018

ldionne committed rCXX339743: [libcxx] Fix XFAILs for aligned allocation tests on older OSX versions.
[libcxx] Fix XFAILs for aligned allocation tests on older OSX versions
Aug 14 2018, 5:30 PM
ldionne committed rL339743: [libcxx] Fix XFAILs for aligned allocation tests on older OSX versions.
[libcxx] Fix XFAILs for aligned allocation tests on older OSX versions
Aug 14 2018, 5:30 PM
ldionne closed D50341: [libcxx] Fix XFAILs for aligned allocation tests on older OSX versions.
Aug 14 2018, 5:30 PM
ldionne committed rCXX339742: [libc++] Disable failing C11 feature tests for <cfloat> and <float.h>.
[libc++] Disable failing C11 feature tests for <cfloat> and <float.h>
Aug 14 2018, 5:18 PM
ldionne committed rL339742: [libc++] Disable failing C11 feature tests for <cfloat> and <float.h>.
[libc++] Disable failing C11 feature tests for <cfloat> and <float.h>
Aug 14 2018, 5:18 PM
ldionne committed rL339741: [libc++] Detect C11 features on non-Clang compilers.
[libc++] Detect C11 features on non-Clang compilers
Aug 14 2018, 5:17 PM