Page MenuHomePhabricator

ldionne (Louis Dionne)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 11 2015, 3:26 PM (243 w, 4 d)

Recent Activity

Fri, Oct 11

ldionne added inline comments to D68879: P1152R4: Fix deprecation warnings in libc++ testsuite and in uses of is_invocable that would internally conjure up a deprecated function type..
Fri, Oct 11, 2:50 PM · Restricted Project
ldionne added inline comments to D68879: P1152R4: Fix deprecation warnings in libc++ testsuite and in uses of is_invocable that would internally conjure up a deprecated function type..
Fri, Oct 11, 12:23 PM · Restricted Project
ldionne created D68880: [libc++] Use generator expression in Linker script generation.
Fri, Oct 11, 11:46 AM · Restricted Project
Herald added a reviewer for D68880: [libc++] Use generator expression in Linker script generation: mclow.lists.

@phosek Can you please confirm this also fixes your issue in the Runtimes build?

Fri, Oct 11, 11:46 AM · Restricted Project
ldionne added a comment to D68833: [CMake] Re-order runtimes in the order of dependencies.

CMake tracks dependencies between targets, but not between directories. If the CMakeLists.txt in some directory (e.g. <root>/libcxx/) needs a target defined in another directory (e.g. <root>/libcxxabi/), one has to make sure that libcxxabi's CMakeLists.txt is included before libcxx's CMakeLists.txt. This isn't new or vexing, IMO. If that is what this patch ensures (I don't know the runtimes build very well), I think this is good.

[...]
I'm not sure what you mean by this. What I *think* you are referring to is specifying runtime libraries in LLVM_ENABLE_PROJECTS which uses the llvm/projects subdirectory for building them.

Fri, Oct 11, 11:46 AM · Restricted Project
ldionne accepted D68855: [libunwind] Refactor CMake flag checks to match libc++ and libc++abi.
Fri, Oct 11, 11:01 AM · Restricted Project
ldionne requested changes to D68840: [libc++][P0980] Marked member functions move/copy/assign of char_traits constexpr..
Fri, Oct 11, 9:47 AM · Restricted Project
ldionne accepted D68833: [CMake] Re-order runtimes in the order of dependencies.

CMake tracks dependencies between targets, but not between directories. If the CMakeLists.txt in some directory (e.g. <root>/libcxx/) needs a target defined in another directory (e.g. <root>/libcxxabi/), one has to make sure that libcxxabi's CMakeLists.txt is included before libcxx's CMakeLists.txt. This isn't new or vexing, IMO. If that is what this patch ensures (I don't know the runtimes build very well), I think this is good.

Fri, Oct 11, 9:37 AM · Restricted Project
ldionne requested changes to D68837: [libc++][P0202] Marked algorithms copy/copy_n/copy_if/copy_backward constexpr..
Fri, Oct 11, 9:19 AM · Restricted Project

Thu, Oct 10

ldionne added inline comments to D68791: [libc++] Fix linker script generation.
Thu, Oct 10, 8:42 AM · Restricted Project
ldionne added inline comments to D68791: [libc++] Fix linker script generation.
Thu, Oct 10, 8:40 AM · Restricted Project
ldionne added a comment to D68343: [libc++] Move the linker script generation step to CMake.

This seems to have broke our build, I checked the generated linker script and it has the following format:

INPUT(libc++.so.2 -lunwind_shared -lcxxabi_shared)

libunwind and libc++abi dependencies are incorrect, it should be:

INPUT(libc++.so.2 -lunwind -lc++abi)

How do you configure your build? The paths were certainly correct when I tested this on Linux. I think this may have something to do with the fact that you use the (kind of hacky) HAVE_LIBCXXABI switch. If you let me know how you configure your build, I'll try to repro and fix it.

Thu, Oct 10, 8:40 AM · Restricted Project
ldionne added a comment to D68343: [libc++] Move the linker script generation step to CMake.

This seems to have broke our build, I checked the generated linker script and it has the following format:

INPUT(libc++.so.2 -lunwind_shared -lcxxabi_shared)

libunwind and libc++abi dependencies are incorrect, it should be:

INPUT(libc++.so.2 -lunwind -lc++abi)
Thu, Oct 10, 8:40 AM · Restricted Project

Wed, Oct 9

ldionne accepted D68681: [libc++][test] Miscellaneous MSVC cleanups.

You have commit access, right? If so, go ahead. Otherwise, LMK and I can commit this for you.

Wed, Oct 9, 1:32 PM · Restricted Project

Tue, Oct 8

ldionne committed rGfe53d2dc6b2b: [libc++] Workaround old versions of CMake that don't understand list(JOIN) (authored by ldionne).
[libc++] Workaround old versions of CMake that don't understand list(JOIN)
Tue, Oct 8, 2:34 PM
ldionne committed rL374120: [libc++] Workaround old versions of CMake that don't understand list(JOIN).
[libc++] Workaround old versions of CMake that don't understand list(JOIN)
Tue, Oct 8, 2:33 PM
ldionne committed rG1ea8bb39b9c4: [libc++] Move the linker script generation step to CMake (authored by ldionne).
[libc++] Move the linker script generation step to CMake
Tue, Oct 8, 2:15 PM
ldionne committed rL374116: [libc++] Move the linker script generation step to CMake.
[libc++] Move the linker script generation step to CMake
Tue, Oct 8, 2:15 PM
ldionne closed D68343: [libc++] Move the linker script generation step to CMake.
Tue, Oct 8, 2:15 PM · Restricted Project
ldionne accepted D68343: [libc++] Move the linker script generation step to CMake.

I'll go ahead with this since there doesn't seem to be a clear agreement that we need this to work on Windows.

Tue, Oct 8, 2:05 PM · Restricted Project
ldionne requested changes to D67900: [libc++] Use builtin type traits whenever possible.

Like Richard said before, we can't switch type traits to aliases. However, we can always do:

Tue, Oct 8, 11:07 AM · Restricted Project
ldionne committed rG32300877f9f4: [libc++] Make sure we link all system libraries into the benchmarks (authored by ldionne).
[libc++] Make sure we link all system libraries into the benchmarks
Tue, Oct 8, 9:28 AM
ldionne committed rL374079: [libc++] Make sure we link all system libraries into the benchmarks.
[libc++] Make sure we link all system libraries into the benchmarks
Tue, Oct 8, 9:24 AM
ldionne created D68648: [CMake] Only detect the linker once in AddLLVM.cmake.
Tue, Oct 8, 8:07 AM · Restricted Project
ldionne added a comment to D68343: [libc++] Move the linker script generation step to CMake.

In case of shared libraries, the libpthread and libc dependency is already encoded in the shared library itself (e.g. in case of ELF it'd be as DT_NEEDED entries), specifying them in the linker script is not necessary and in many cases undesirable (e.g. if your application doesn't use any threading API, you probably don't want to get implicit DT_NEEDED dependency on libpthread just because you're linking libc++). Where it'd make sense is for static libraries, but we don't currently generate linker script for those.

Tue, Oct 8, 8:07 AM · Restricted Project
ldionne committed rGc864f73e45ce: [libc++] TAKE 2: Make system libraries PRIVATE dependencies of libc++ (authored by ldionne).
[libc++] TAKE 2: Make system libraries PRIVATE dependencies of libc++
Tue, Oct 8, 7:58 AM
ldionne committed rL374056: [libc++] TAKE 2: Make system libraries PRIVATE dependencies of libc++.
[libc++] TAKE 2: Make system libraries PRIVATE dependencies of libc++
Tue, Oct 8, 7:57 AM
ldionne committed rGcf3ab6d96c3e: [libc++] Add missing link-time dependencies to the benchmarks (authored by ldionne).
[libc++] Add missing link-time dependencies to the benchmarks
Tue, Oct 8, 7:29 AM
ldionne committed rL374053: [libc++] Add missing link-time dependencies to the benchmarks.
[libc++] Add missing link-time dependencies to the benchmarks
Tue, Oct 8, 7:29 AM
ldionne committed rG534c86d17252: [libc++] Use PRIVATE to link benchmark dependencies (authored by ldionne).
[libc++] Use PRIVATE to link benchmark dependencies
Tue, Oct 8, 7:10 AM
ldionne committed rL374050: [libc++] Use PRIVATE to link benchmark dependencies.
[libc++] Use PRIVATE to link benchmark dependencies
Tue, Oct 8, 7:10 AM

Mon, Oct 7

ldionne committed rGbe52ff95063a: [libc++abi] Introduce a LIBCXXABI_LIBRARY_VERSION option (authored by ldionne).
[libc++abi] Introduce a LIBCXXABI_LIBRARY_VERSION option
Mon, Oct 7, 10:18 PM
ldionne committed rL373949: [libc++abi] Introduce a LIBCXXABI_LIBRARY_VERSION option.
[libc++abi] Introduce a LIBCXXABI_LIBRARY_VERSION option
Mon, Oct 7, 10:18 PM
ldionne committed rGd03068c3e1fb: [libc++abi] Do not define -Dcxxabi_shared_EXPORTS when building libc++abi (authored by ldionne).
[libc++abi] Do not define -Dcxxabi_shared_EXPORTS when building libc++abi
Mon, Oct 7, 10:17 PM
ldionne committed rL373940: [libc++abi] Do not define -Dcxxabi_shared_EXPORTS when building libc++abi.
[libc++abi] Do not define -Dcxxabi_shared_EXPORTS when building libc++abi
Mon, Oct 7, 10:17 PM
ldionne committed rGf385a3814047: [libc++abi] Remove redundant link flags on Apple platforms (authored by ldionne).
[libc++abi] Remove redundant link flags on Apple platforms
Mon, Oct 7, 10:16 PM
ldionne committed rL373934: [libc++abi] Remove redundant link flags on Apple platforms.
[libc++abi] Remove redundant link flags on Apple platforms
Mon, Oct 7, 10:16 PM
ldionne accepted D68000: [libc++] Remove C++03 variadics in shared_ptr (v2).
Mon, Oct 7, 8:54 AM · Restricted Project
Herald added a reviewer for D68343: [libc++] Move the linker script generation step to CMake: mclow.lists.

It turns out that @EricWF was the one to originally write the script, so maybe he knows why the output is

Mon, Oct 7, 7:43 AM · Restricted Project
ldionne added a comment to D68343: [libc++] Move the linker script generation step to CMake.

Ping @phosek @thakis

Mon, Oct 7, 6:20 AM · Restricted Project

Fri, Oct 4

ldionne committed rGfadc84ae9a95: [libc++] Localize common build flags into a single CMake function (authored by ldionne).
[libc++] Localize common build flags into a single CMake function
Fri, Oct 4, 3:52 PM
ldionne committed rL373808: [libc++] Localize common build flags into a single CMake function.
[libc++] Localize common build flags into a single CMake function
Fri, Oct 4, 3:51 PM
ldionne added a comment to D68269: [libc++abi] Do not define new/delete by default.

So, the problem with this patch as it stands is that it introduces a link-time dependency from libc++abi back to libc++, because libc++abi needs the definition of new and delete (which would be in libc++ only with this patch). This is what I'm working around in the tests by re-adding -lc++ after -lc++abi on the compiler command line. This isn't a problem on Apple platforms IIUC since the order of command-line -l<xxx> arguments doesn't matter, but it does for linkers on most other platforms.

We (Chromium) currently statically link libc++ on all platforms, but use the platform default abi library. If I read the thread here right, this means we currently get operator new from libc++ but going forward we'd get it from the platform abi libraries -- but at the moment none (except darwin) have it yet. So that'd likely be an issue for us.

Fri, Oct 4, 12:54 PM · Restricted Project
ldionne added a comment to D68269: [libc++abi] Do not define new/delete by default.

So, the problem with this patch as it stands is that it introduces a link-time dependency from libc++abi back to libc++, because libc++abi needs the definition of new and delete (which would be in libc++ only with this patch). This is what I'm working around in the tests by re-adding -lc++ after -lc++abi on the compiler command line. This isn't a problem on Apple platforms IIUC since the order of command-line -l<xxx> arguments doesn't matter, but it does for linkers on most other platforms.

Fri, Oct 4, 12:45 PM · Restricted Project
ldionne added a comment to D68075: Do not #error if no OS is #defined.

I kinda like the suggestion. I'm pretty confident we don't support compilers that don't implement __has_include anyway (maybe we technically do, but I'm sure nothing good happens in that case).

Fri, Oct 4, 12:26 PM · Restricted Project, Restricted Project
ldionne added a comment to D68075: Do not #error if no OS is #defined.

I kinda like the suggestion. I'm pretty confident we don't support compilers that don't implement __has_include anyway (maybe we technically do, but I'm sure nothing good happens in that case).

Fri, Oct 4, 12:23 PM · Restricted Project, Restricted Project
ldionne committed rG13c4254714f0: [libc++] Make the modules-related flags PUBLIC instead of PRIVATE (authored by ldionne).
[libc++] Make the modules-related flags PUBLIC instead of PRIVATE
Fri, Oct 4, 12:13 PM
ldionne committed rL373773: [libc++] Make the modules-related flags PUBLIC instead of PRIVATE.
[libc++] Make the modules-related flags PUBLIC instead of PRIVATE
Fri, Oct 4, 12:12 PM
ldionne committed rGce452b1ca9fd: [libc++abi] Link against libSystem on Apple platforms (authored by ldionne).
[libc++abi] Link against libSystem on Apple platforms
Fri, Oct 4, 11:37 AM
ldionne committed rL373770: [libc++abi] Link against libSystem on Apple platforms.
[libc++abi] Link against libSystem on Apple platforms
Fri, Oct 4, 11:31 AM
ldionne committed rG432ae75f8bb2: [libc++] Move more CMake flags to per-target definitions (authored by ldionne).
[libc++] Move more CMake flags to per-target definitions
Fri, Oct 4, 11:04 AM
ldionne committed rL373767: [libc++] Move more CMake flags to per-target definitions.
[libc++] Move more CMake flags to per-target definitions
Fri, Oct 4, 11:01 AM

Thu, Oct 3

ldionne committed rGc5b74bf6e548: [libc++] Add a per-target flag to include the generated config_site (authored by ldionne).
[libc++] Add a per-target flag to include the generated config_site
Thu, Oct 3, 10:21 AM
ldionne committed rL373631: [libc++] Add a per-target flag to include the generated config_site.
[libc++] Add a per-target flag to include the generated config_site
Thu, Oct 3, 10:20 AM
ldionne committed rG0961a152d855: [libc++] Add missing revision number in ABI changelog (authored by ldionne).
[libc++] Add missing revision number in ABI changelog
Thu, Oct 3, 9:51 AM
ldionne committed rL373625: [libc++] Add missing revision number in ABI changelog.
[libc++] Add missing revision number in ABI changelog
Thu, Oct 3, 9:50 AM
ldionne added a comment to D68269: [libc++abi] Do not define new/delete by default.
In D68269#1692076, @dim wrote:

Hm, we don't use libc++abi in FreeBSD (at least not by default), but libcxxrt, which has its own new/delete definitions. I never noticed a conflict with libc++ though, since when is it providing new/delete by itself?

Thu, Oct 3, 7:58 AM · Restricted Project
ldionne commandeered D68364: Prototype implementation of P0784R7: mark all members of std::allocator and std::allocator_traits as constexpr..
Thu, Oct 3, 7:45 AM · Restricted Project, Restricted Project
ldionne commandeered D68365: Prototype implementation of P1004R2 (making std::vector constexpr)..
Thu, Oct 3, 7:45 AM · Restricted Project
ldionne committed rG6f9459f7fe7d: [libc++abi] Do not export some implementation-detail functions (authored by ldionne).
[libc++abi] Do not export some implementation-detail functions
Thu, Oct 3, 7:27 AM
ldionne added a comment to D68365: Prototype implementation of P1004R2 (making std::vector constexpr)..

I'll pick this up. I wasn't aware you had started working on this, but thanks!

Thu, Oct 3, 7:26 AM · Restricted Project
ldionne committed rL373602: [libc++abi] Do not export some implementation-detail functions.
[libc++abi] Do not export some implementation-detail functions
Thu, Oct 3, 7:23 AM
ldionne closed D68357: [libc++abi] Do not export some implementation-detail functions.
Thu, Oct 3, 7:23 AM · Restricted Project, Restricted Project
ldionne added a comment to D68357: [libc++abi] Do not export some implementation-detail functions.

Do we not build libc++abi with -fvisibility=hidden? That would also have prevented this, right?

Thu, Oct 3, 6:05 AM · Restricted Project, Restricted Project

Wed, Oct 2

ldionne created D68357: [libc++abi] Do not export some implementation-detail functions.
Wed, Oct 2, 2:24 PM · Restricted Project, Restricted Project
ldionne committed rGc5d2746fbea7: [NFC][libc++abi] Convert stray tabs to spaces (authored by ldionne).
[NFC][libc++abi] Convert stray tabs to spaces
Wed, Oct 2, 1:46 PM
ldionne committed rL373524: [NFC][libc++abi] Convert stray tabs to spaces.
[NFC][libc++abi] Convert stray tabs to spaces
Wed, Oct 2, 1:45 PM
ldionne committed rG925d9d2e1443: [libc++] Use functions instead of global variables to set libc++ build flags (authored by ldionne).
[libc++] Use functions instead of global variables to set libc++ build flags
Wed, Oct 2, 1:08 PM
ldionne committed rL373517: [libc++] Use functions instead of global variables to set libc++ build flags.
[libc++] Use functions instead of global variables to set libc++ build flags
Wed, Oct 2, 1:08 PM
ldionne updated subscribers of D68269: [libc++abi] Do not define new/delete by default.

Pinging vendors for awareness of the change: @phosek @danalbert @dim

Wed, Oct 2, 12:44 PM · Restricted Project
ldionne committed rGc78c0e08be21: [libc++] Use a function to set warning flags per target (authored by ldionne).
[libc++] Use a function to set warning flags per target
Wed, Oct 2, 12:32 PM
ldionne committed rL373511: [libc++] Use a function to set warning flags per target.
[libc++] Use a function to set warning flags per target
Wed, Oct 2, 12:32 PM
ldionne added a comment to D68275: [libcxx] [test] Query the target platform, not the host one.

This looks roughly okay to me, but I don't maintain the Windows side of things (I don't really run tests on Windows except when required to). I would wait for @EricWF or other folks that work on Windows more often to give you a thumbs up before committing this.

Wed, Oct 2, 12:32 PM · Restricted Project
ldionne committed rG9cc90ec3499d: [libc++] Revert to using PUBLIC instead of PRIVATE when linking system libs (authored by ldionne).
[libc++] Revert to using PUBLIC instead of PRIVATE when linking system libs
Wed, Oct 2, 12:13 PM
ldionne committed rL373506: [libc++] Revert to using PUBLIC instead of PRIVATE when linking system libs.
[libc++] Revert to using PUBLIC instead of PRIVATE when linking system libs
Wed, Oct 2, 12:13 PM
ldionne added a comment to D68343: [libc++] Move the linker script generation step to CMake.

Sorry, I sent the previous comment while I was still drafting.

Wed, Oct 2, 11:58 AM · Restricted Project
ldionne added a comment to D68343: [libc++] Move the linker script generation step to CMake.
Wed, Oct 2, 11:44 AM · Restricted Project
ldionne added a comment to D68343: [libc++] Move the linker script generation step to CMake.

My goal with this patch is to eliminate some global variables from the CMake sources, which make my life harder while refactoring other parts of the build (I'm working on other, upcoming patches).

Wed, Oct 2, 10:06 AM · Restricted Project
ldionne created D68343: [libc++] Move the linker script generation step to CMake.
Wed, Oct 2, 10:00 AM · Restricted Project
ldionne committed rGef315b5361e4: [libc++] Use PRIVATE instead of PUBLIC when linking against system libs (authored by ldionne).
[libc++] Use PRIVATE instead of PUBLIC when linking against system libs
Wed, Oct 2, 9:52 AM
ldionne committed rL373487: [libc++] Use PRIVATE instead of PUBLIC when linking against system libs.
[libc++] Use PRIVATE instead of PUBLIC when linking against system libs
Wed, Oct 2, 9:52 AM

Tue, Oct 1

ldionne committed rG85ee0c2ec344: [NFC] Fix typos in libc++ documentation (authored by ldionne).
[NFC] Fix typos in libc++ documentation
Tue, Oct 1, 1:35 PM
ldionne committed rL373390: [NFC] Fix typos in libc++ documentation.
[NFC] Fix typos in libc++ documentation
Tue, Oct 1, 1:35 PM
ldionne committed rGbcab95182b37: [libc++] Re-apply workaround for D63883 (authored by ldionne).
[libc++] Re-apply workaround for D63883
Tue, Oct 1, 12:28 PM
ldionne committed rL373385: [libc++] Re-apply workaround for D63883.
[libc++] Re-apply workaround for D63883
Tue, Oct 1, 12:28 PM
ldionne committed rG32f869e0eecc: [libc++] Remove workaround for D63883 (authored by ldionne).
[libc++] Remove workaround for D63883
Tue, Oct 1, 12:14 PM
ldionne committed rL373384: [libc++] Remove workaround for D63883.
[libc++] Remove workaround for D63883
Tue, Oct 1, 12:12 PM
ldionne committed rG04501a22a073: [libc++abi] Remove uses of C++ headers when possible (authored by ldionne).
[libc++abi] Remove uses of C++ headers when possible
Tue, Oct 1, 11:43 AM
ldionne committed rL373381: [libc++abi] Remove uses of C++ headers when possible.
[libc++abi] Remove uses of C++ headers when possible
Tue, Oct 1, 11:43 AM
ldionne committed rG2cee0e2d97c4: [NFC][libc++abi] Remove trailing whitespace from sources (authored by ldionne).
[NFC][libc++abi] Remove trailing whitespace from sources
Tue, Oct 1, 11:28 AM
ldionne committed rL373379: [NFC][libc++abi] Remove trailing whitespace from sources.
[NFC][libc++abi] Remove trailing whitespace from sources
Tue, Oct 1, 11:27 AM
ldionne committed rGa28869596842: [libc++] Update link to Itanium C++ ABI documentation (authored by ldionne).
[libc++] Update link to Itanium C++ ABI documentation
Tue, Oct 1, 11:17 AM
ldionne committed rL373372: [libc++] Update link to Itanium C++ ABI documentation.
[libc++] Update link to Itanium C++ ABI documentation
Tue, Oct 1, 11:13 AM
ldionne committed rG4ff35a8f0b90: [libc++abi] Remove redundant #include of <string.h> (authored by ldionne).
[libc++abi] Remove redundant #include of <string.h>
Tue, Oct 1, 10:54 AM
ldionne committed rL373365: [libc++abi] Remove redundant #include of <string.h>.
[libc++abi] Remove redundant #include of <string.h>
Tue, Oct 1, 10:53 AM
ldionne abandoned D63457: [libc++] Re-export libc++abi as a whole instead of using lists.

Okay, so after speaking to our linker folks a while ago, I decided to drop this.

Tue, Oct 1, 8:56 AM · Restricted Project
ldionne added inline comments to D68275: [libcxx] [test] Query the target platform, not the host one.
Tue, Oct 1, 8:40 AM · Restricted Project
ldionne added a comment to D68269: [libc++abi] Do not define new/delete by default.

It turns out the GCC problem was only due to the linker not resolving the undefined new/delete references in libc++abi to libc++. Adding another -lc++ after -lc++abi fixes the issue.

Tue, Oct 1, 8:36 AM · Restricted Project
ldionne updated the summary of D68269: [libc++abi] Do not define new/delete by default.
Tue, Oct 1, 8:34 AM · Restricted Project
ldionne updated the diff for D68269: [libc++abi] Do not define new/delete by default.

Fix linker error with GCC

Tue, Oct 1, 8:34 AM · Restricted Project