[PATCH] D133249: [libc++] Documents details of the pre-commit CI.

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM w/ comments. Thanks a lot for writing this! Comment at: clang/www/hacking.html:279 + + Some changes in Clang affect https://libcxx.llvm.org";>libc++, + for example:

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

ldionne added inline comments. Comment at: libcxx/CMakeLists.txt:421 +if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE) + set(default_install_path "${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1") +else() ldionne wrote: > Instead, I'd do th

[PATCH] D121141: [Clang] Add `-funstable` flag to enable unstable and experimental features: follow-up fixes

ldionne commandeered this revision. ldionne edited reviewers, added: egorzhdan; removed: ldionne. ldionne added a comment. Herald added a subscriber: MaskRay. Thanks a lot for the fixes @egorzhdan! I think this looks pretty good. Since LLVM 15 is coming up and we'd like to have this in its final

[PATCH] D128927: [libc++] Always build c++experimental.a

ldionne created this revision. ldionne added a reviewer: Mordante. Herald added subscribers: Enna1, abrachet, arichardson, mgorny. Herald added a project: All. ldionne requested review of this revision. Herald added projects: clang, Sanitizers, libc++. Herald added subscribers: libcxx-commits, Sani

[PATCH] D128927: [libc++] Always build c++experimental.a

ldionne added a subscriber: mstorsjo. ldionne added a comment. @mstorsjo I might need some assistance figuring out what to do with the Windows parts of this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128927/new/ https://reviews.llvm.org

[PATCH] D128927: [libc++] Always build c++experimental.a

ldionne updated this revision to Diff 442378. ldionne marked 2 inline comments as done. ldionne added a comment. Address review comments and try to fix some CI issues Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128927/new/ https://reviews.llvm.or

[PATCH] D128927: [libc++] Always build c++experimental.a

ldionne added inline comments. Comment at: libcxx/docs/ReleaseNotes.rst:226 + experimental features provided by it. Note that vendors are encouraged to ship the + experimental library now that the compiler provides an ergonomic way to use it. Mordante wrote:

[PATCH] D129198: [clang] Correct the macOS version that supports aligned allocation

ldionne created this revision. ldionne added a reviewer: ahatanak. Herald added a project: All. ldionne requested review of this revision. Herald added projects: clang, libc++. Herald added subscribers: libcxx-commits, cfe-commits. Herald added a reviewer: libc++. After checking the libc++abi.dyli

[PATCH] D125683: [runtimes] Replace LIBCXX_ENABLE_STATIC_ABI_LIBRARY & friends by a new LIBCXX_CXX_ABI choice

ldionne updated this revision to Diff 442549. ldionne marked 2 inline comments as done. ldionne added a comment. Rebase and address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125683/new/ https://reviews.llvm.org/D125683 Files:

[PATCH] D129198: [clang] Correct the macOS version that supports aligned allocation

ldionne accepted this revision as: libc++. ldionne added a comment. I looked at the CI failures and they look like flukes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129198/new/ https://reviews.llvm.org/D129198 _

[PATCH] D129198: [clang] Correct the macOS version that supports aligned allocation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4722b5d1330a: [clang] Correct the macOS version that supports aligned allocation (authored by ldionne). Repository: rG LLVM Github Monorepo CHANG

[PATCH] D128927: [libc++] Always build c++experimental.a

ldionne updated this revision to Diff 442667. ldionne added a comment. Rebase onto main and temporarily disable experimental tests on Windows Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128927/new/ https://reviews.llvm.org/D128927 Files: clang

[PATCH] D128927: [libc++] Always build c++experimental.a

ldionne marked an inline comment as done. ldionne added a comment. Thanks @mstorsjo! Regarding `_LIBCPP_EXPERIMENTAL_FUNC_VIS`, yes I think it would make sense to use a different visibility macro for symbols that we know are provided only as part of a static library. I would not call it `_LIBCP

[PATCH] D128927: [libc++] Always build c++experimental.a

ldionne updated this revision to Diff 443252. ldionne marked an inline comment as done. ldionne added a comment. Windows fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128927/new/ https://reviews.llvm.org/D128927 Files: clang/cmake/caches/

[PATCH] D128927: [libc++] Always build c++experimental.a

ldionne updated this revision to Diff 443342. ldionne added a comment. Rebase onto main and remove references to -funstable since the design is still changing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128927/new/ https://reviews.llvm.org/D128

[PATCH] D128465: [llvm] cmake config groundwork to have ZSTD in LLVM

ldionne added inline comments. Comment at: llvm/CMakeLists.txt:441 +set(LLVM_ENABLE_ZSTD "ON" CACHE STRING "Use zstd for compression/decompression if available. Can be ON, OFF, or FORCE_ON") + This broke builds that do not specify `LLVM_ENABLE_ZSTD=OFF` explic

[PATCH] D128927: [libc++] Always build c++experimental.a

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbb939931a1ad: [libc++] Always build c++experimental.a (authored by ldionne). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

ldionne accepted this revision as: libc++, libc++abi, ldionne. ldionne added a comment. I'm happy with this from the libc++/libc++abi side of things. You can ignore the failing CI job, it's been addressed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D131274: [clang][Darwin] Re-apply "Always set the default C++ Standard Library to libc++"

ldionne created this revision. ldionne added a reviewer: leonardchan. Herald added a project: All. ldionne requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Newer SDKs don't even provide libstdc++ headers, so it's effectively never va

[PATCH] D131274: [clang][Darwin] Re-apply "Always set the default C++ Standard Library to libc++"

ldionne added a comment. @leonardchan Does this fix the Fuchsia issues you were seeing? I think I understand what went wrong and I think this version of the patch should address the issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131274/new/

[PATCH] D131274: [clang][Darwin] Re-apply "Always set the default C++ Standard Library to libc++"

ldionne updated this revision to Diff 450771. ldionne added a comment. Herald added a reviewer: JDevlieghere. Fix additional tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131274/new/ https://reviews.llvm.org/D131274 Files: clang/lib/Drive

[PATCH] D131274: [clang][Darwin] Re-apply "Always set the default C++ Standard Library to libc++"

ldionne updated this revision to Diff 451287. ldionne added a comment. Fix more tests that failed on internal bots (but I can't reproduce the failure locally for some reason) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131274/new/ https://review

[PATCH] D106339: Add support to generate Sphinx DOCX documentation

ldionne requested changes to this revision. ldionne added a comment. This revision now requires changes to proceed. What's the benefit of having docx documentation? We generate HTML documentation, which ends up in the website, and that seems strictly superior to generating docx. What do you need

[PATCH] D106394: [clang][pp] adds '#pragma include_instead'

ldionne added a comment. This is quite interesting, I'd love to be able to use this in libc++ (and suggest that my peers use this in Apple's libc)! In D106394#2892832 , @sammccall wrote: > Eventually this seems like a reasonable thing to want for user c

[PATCH] D103395: PR45879: Keep evaluated expression in LValue object

ldionne added a comment. Gentle ping. I can't review this, but if someone can, it would be awesome. https://bugs.llvm.org/show_bug.cgi?id=45879 is breaking some libc++ tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103395/new/ https://review

[PATCH] D120727: [libc++] Overhaul how we select the ABI library

ldionne added a comment. In D120727#3372594 , @mstorsjo wrote: > FWIW I think D116689 interacts with this > somewhat. I think D116689 is useful for > the fixes I want to do wrt libcxx

[PATCH] D121078: Replace links to archived mailing lists by links to Discourse forums

ldionne accepted this revision. ldionne added a comment. LGTM, thanks for fixing the documentation quirks in libc++/libunwind! And sorry this went under my Radar last week :-). @tonic Is this OK with you? I don't want to override your "request for changes" by committing this. Repository: rG

[PATCH] D122232: [clang][NFC] Refactor logic for picking standard library on Apple

ldionne created this revision. ldionne added a reviewer: egorzhdan. Herald added a project: All. ldionne requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Flip the logic around: always default to libc++ except on older platforms, instead of d

[PATCH] D122232: [clang][NFC] Refactor logic for picking standard library on Apple

This revision was automatically updated to reflect the committed changes. Closed by commit rG80e66a05b6fa: [clang][NFC] Refactor logic for picking standard library on Apple (authored by ldionne). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122232/n

[PATCH] D122874: [clang] Add GNU spelling for no_unqiue_address attribute

ldionne added a comment. I actually think that `[[no_unique_address]]` and `__attribute__((no_unique_address))` should be equivalent. Similarly, we should be able to do `[[msvc::no_unique_address]]` and `__attribute__((msvc::no_unique_address))`, and both should be equivalent. Which one we pic

[PATCH] D122931: [CMake][compiler-rt] Support for using in-tree libc++

ldionne added a comment. I like this, however IMO it would be better to instead have something like COMPILER_RT_CXX_LIBRARY="libc++|system-libc++|libstdc++|..." That way, this can be extended to support other C++ libraries without having to add new boolean options, and without forcing folks t

[PATCH] D123655: [clang-tidy] Add portability-std-allocator-const-t check

ldionne added a comment. In D123655#3448472 , @Eugene.Zelenko wrote: > Shouldn't this check be enabled only when libstdc++ is used? No, because we would like to enforce this in libc++ as well! Thanks @MaskRay for pursuing this! Repository: rG LLVM

[PATCH] D123655: [clang-tidy] Add portability-std-allocator-const-t check

ldionne added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/portability-std-allocator-const-t.rst:11 + +libstdc++ never supports ``std::allocator`` and containers using them. +Since GCC 8.0 (`PR48101 `),

[PATCH] D103395: PR45879: Keep evaluated expression in LValue object

ldionne added a comment. Hello, I know I'm not helping move this patch along because I'm not the right person to review this. However, I'd like to kindly reiterate that fixing this would be helpful for libc++. I just had to re-XFAIL one of our tuple tests because of this :-) Repository: rG

[PATCH] D103395: PR45879: Keep evaluated expression in LValue object

ldionne added a comment. @rsmith Thanks a bunch! And thanks @sepavloff for trying out this approach too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103395/new/ https://reviews.llvm.org/D103395 ___ cf

[PATCH] D119351: Update all LLVM documentation mentioning runtimes in LLVM_ENABLE_PROJECTS

ldionne created this revision. Herald added subscribers: libcxx-commits, ayermolo, arphaman, mgorny. Herald added a reviewer: sscalpone. Herald added a reviewer: rafauler. Herald added a reviewer: Amir. Herald added a reviewer: maksfb. Herald added projects: libunwind, Flang. Herald added a reviewe

[PATCH] D119351: Update all LLVM documentation mentioning runtimes in LLVM_ENABLE_PROJECTS

ldionne updated this revision to Diff 407500. ldionne marked 3 inline comments as done. ldionne added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119351/new/ https://reviews.llvm.org/D119351 Files: README.md

[PATCH] D119351: Update all LLVM documentation mentioning runtimes in LLVM_ENABLE_PROJECTS

ldionne added inline comments. Comment at: README.md:71 + mlir, openmp, polly, or pstl. ``LLVM_ENABLE_RUNTIMES`` can include any of + libcxx, libcxxabi, libunwind, or compiler-rt. tianshilei1992 wrote: > FWIW, OpenMP can be put into either of

[PATCH] D119351: Update all LLVM documentation mentioning runtimes in LLVM_ENABLE_PROJECTS

ldionne updated this revision to Diff 407631. ldionne marked 2 inline comments as done. ldionne added a comment. Address review comments. I'm going to commit this and cherry-pick onto release/14.x Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D11935

[PATCH] D119351: Update all LLVM documentation mentioning runtimes in LLVM_ENABLE_PROJECTS

ldionne added inline comments. Comment at: bolt/docs/OptimizingClang.md:228-236 $ CPATH=${TOPLEV}/stage1/install/bin/ -$ cmake -G Ninja ${TOPLEV}/llvm-project/llvm -DLLVM_TARGETS_TO_BUILD=X86 \ +$ cmake -G Ninja -S ${TOPLEV}/llvm-project/llvm -B ${TOPLEV}/stage2-prof-gen \ +

[PATCH] D119351: Update all LLVM documentation mentioning runtimes in LLVM_ENABLE_PROJECTS

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4ae83bb2b1e2: Update all LLVM documentation mentioning runtimes in LLVM_ENABLE_PROJECTS (authored by ldionne). Repository: rG LLVM Github Monorepo

[PATCH] D119351: Update all LLVM documentation mentioning runtimes in LLVM_ENABLE_PROJECTS

ldionne marked an inline comment as done. ldionne added inline comments. Comment at: llvm/docs/GettingStarted.rst:1225 +* -DLLVM_ENABLE_RUNTIMES + Set this equal to the runtimes you wish to compile (e.g. libcxx, libcxxabi, etc.) Arfrever wrote: > One space i

[PATCH] D119918: [CMake] Rename TARGET_TRIPLE to LLVM_TARGET_TRIPLE

ldionne accepted this revision. ldionne added a comment. LGTM from libc++/libc++abi's perspective. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119918/new/ https://reviews.llvm.org/D119918 ___ cfe-commi

[PATCH] D121838: Generalize "check-all" umbrella targets, use for check-clang-tools

ldionne resigned from this revision. ldionne added a comment. I think @phosek is the right person to look at this. I looked at it and it seems fine, but I don't know how the runtimes tests are setup well enough to spot an issue if there were one. Nit: I'd suggest rebasing and re-uploading the p

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

ldionne added a subscriber: ahatanak. ldionne added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4078-4082 + if (!Args.getLastArg(options::OPT_fsized_deallocation, + options::OPT_fno_sized_deallocation)) +Opts.SizedDeallocation

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

ldionne added a comment. (BTW I strongly support this patch, I just think we should do it properly on all platforms from the start :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

ldionne added a comment. In D112921#3473632 , @pcwang-thead wrote: > In D112921#3473022 , @ldionne wrote: > >> (BTW I strongly support this patch, I just think we should do it properly on >> all platforms from t

[PATCH] D124708: Fix "the the" typo in documentation and user facing strings

ldionne accepted this revision. ldionne added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124708/new/ https://reviews.llvm.org/D124708 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D120727: [libc++] Overhaul how we select the ABI library

ldionne updated this revision to Diff 427721. ldionne added a comment. Rebase onto main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120727/new/ https://reviews.llvm.org/D120727 Files: clang/cmake/caches/CrossWinToARMLinux.cmake libcxx/CMakeL

[PATCH] D128927: [libc++] Always build c++experimental.a

ldionne added a comment. In D128927#3648616 , @fhahn wrote: > It looks like this is causing macOS builds to fail on GreenDragon. Please > take a look and refer the commit if it takes longer to resolve, as the bot > has been red for a while now. Thanks

[PATCH] D121141: [Clang] Add `-funstable` flag to enable unstable and experimental features: follow-up fixes

ldionne updated this revision to Diff 444709. ldionne marked an inline comment as done. ldionne added a comment. Herald added subscribers: pmatos, asb, abrachet, atanasyan, jrtc27, aheejin, jgravelle-google, sbc100, sdardis, dschuff, emaste. - Rebase - Rename to -fexperimental-library - Add tests

[PATCH] D121141: [Clang] Add `-funstable` flag to enable unstable and experimental features: follow-up fixes

ldionne added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:276 + +Enable unstable and experimental language and library features. + MaskRay wrote: > The doc is auto-generated by tablegen > (https://discourse.llvm.org/t/clang-driver-optio

[PATCH] D121141: [Clang] Add `-funstable` flag to enable unstable and experimental features: follow-up fixes

ldionne marked an inline comment as done. ldionne added a comment. In D121141#3652616 , @MaskRay wrote: > LGTM. > > It may be related, @urnathan wants to add `-std=c++{current,future}` to GCC > and may have opinions on the option name. Interesting! @urn

[PATCH] D121141: [Clang] Add `-funstable` flag to enable unstable and experimental features: follow-up fixes

ldionne updated this revision to Diff 444753. ldionne added a comment. Address review comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121141/new/ https://reviews.llvm.org/D121141 Files: clang/docs/ClangCommandLineReference.rst clang/inc

[PATCH] D129048: Rewording the "static_assert" to static assertion

ldionne added a comment. Most of the libc++ CI testing happens on BuildKite, and we only trigger the libc++ buildkite pipeline if files in `libcxx/` (and a few others) have changed. This wasn't the case here. If you want to re-land without breaking libc++, you'd have to go through libc++'s `.ve

[PATCH] D121141: [Clang] Add `-funstable` flag to enable unstable and experimental features: follow-up fixes

ldionne added inline comments. Comment at: clang/include/clang/Driver/Options.td:1186 -defm unstable : BoolFOption<"unstable", - LangOpts<"Unstable">, DefaultFalse, - PosFlag, +defm experimental_library : BoolFOption<"experimental-library", + LangOpts<"ExperimentalLibrary">,

[PATCH] D121141: [Clang] Add `-fexperimental-library` flag to enable unstable and experimental features: follow-up fixes

ldionne added a subscriber: mstorsjo. ldionne added a comment. The `experimental-library-flag` test is currently failing on Windows because on Windows, `-stdlib=libc++` seems to be ignored and we don't add `-lc++` or `-lc++experimental`. Does someone understand how things are supposed to work w

[PATCH] D128927: [libc++] Always build c++experimental.a

ldionne added a comment. The weird part here is that you're configuring libc++, but you are building neither the static nor the shared library. I don't understand why you do that, and that may hide some other more fundamental issue in your setup. Anyways, I think the issue should be resolved by

[PATCH] D128927: [libc++] Always build c++experimental.a

This revision was automatically updated to reflect the committed changes. Closed by commit rG7300a651f5f3: [libc++] Re-apply "Always build c++experimental.a"" (authored by ldionne). Changed prior to commit: https://reviews.llvm.org/D128927?vs=443344&id=445822#toc Repository: rG LLVM Github M

[PATCH] D121141: [Clang] Add `-fexperimental-library` flag to enable unstable and experimental features: follow-up fixes

ldionne marked 12 inline comments as done. ldionne added a comment. In D121141#3658946 , @phosek wrote: > That's correct, I implemented the support for `-stdlib=libc++` on MSVC > targets in D101479 , but that change caus

[PATCH] D121141: [Clang] Add `-fexperimental-library` flag to enable unstable and experimental features: follow-up fixes

ldionne updated this revision to Diff 445859. ldionne marked 2 inline comments as done. ldionne added a comment. Address most review feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121141/new/ https://reviews.llvm.org/D121141 Files: clan

[PATCH] D121141: [Clang] Add `-fexperimental-library` flag to enable unstable and experimental features: follow-up fixes

ldionne added a comment. Windows seems to have been "fixed" (a band-aid really), and the Linux failure seems unrelated, so I'll ship this now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121141/new/ https://reviews.llvm.org/D121141

[PATCH] D121141: [Clang] Add `-fexperimental-library` flag to enable unstable and experimental features: follow-up fixes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGca495e36c1b4: [clang] Add a new flag -fexperimental-library to enable experimental library… (authored by ldionne). Repository: rG LLVM Github Mono

[PATCH] D121141: [Clang] Add `-fexperimental-library` flag to enable unstable and experimental features: follow-up fixes

ldionne added a comment. I saw that this change broke `llvm-clang-x86_64-sie-ubuntu-fast`, and I am following up with the bot owner to understand what's special about their target. I'm not being notified of any other failures so far. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D121141: [Clang] Add `-fexperimental-library` flag to enable unstable and experimental features: follow-up fixes

ldionne added a comment. I just got notified of a failure on `clang-ppc64-aix`. I'll take a look first thing tomorrow morning. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121141/new/ https://reviews.llvm.org/D121141

[PATCH] D129048: Rewording the "static_assert" to static assertion

ldionne added a comment. In D129048#3668594 , @philnik wrote: > Also, please wait for #libc approval > next time. This, x1000. We go through the trouble of having excellent pre-commit testing and automatic review g

[PATCH] D129048: Rewording the "static_assert" to static assertion

ldionne added a comment. The libc++ CI runs pre-commit only, and it runs on all commits that touch something under `libcxx/`, `libcxxabi/`, `runtimes/` and `libunwind/`. In particular, it won't trigger if you make changes to `clang/` only -- that would create too much traffic and concretely it

[PATCH] D128927: [libc++] Always build c++experimental.a

ldionne added a comment. In D128927#3665748 , @hans wrote: > In D128927#3662659 , @ldionne wrote: > >> The weird part here is that you're configuring libc++, but you are building >> neither the static nor the sha

[PATCH] D129048: Rewording the "static_assert" to static assertion

ldionne added a comment. In D129048#3671568 , @aaron.ballman wrote: > FWIW, I've convinced myself that I agree with you here that the burden > probably should have been on libc++ maintainers in this case. libc++ almost > feels more like a downstream co

[PATCH] D120727: [libc++] Overhaul how we select the ABI library

ldionne updated this revision to Diff 428108. ldionne added a comment. Herald added a project: libc++abi. Herald added a reviewer: libc++abi. Try addressing CI issues. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120727/new/ https://reviews.llvm.o

[PATCH] D120727: [libc++] Overhaul how we select the ABI library

ldionne updated this revision to Diff 428361. ldionne added a comment. Rebase to poke CI. Any interest in reviewing this @phosek @mstorsjo? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120727/new/ https://reviews.llvm.org/D120727 Files: clang/c

[PATCH] D120727: [libc++] Overhaul how we select the ABI library

ldionne updated this revision to Diff 428658. ldionne marked 2 inline comments as done. ldionne added a comment. Rebase and address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120727/new/ https://reviews.llvm.org/D120727 Files:

[PATCH] D120727: [libc++] Overhaul how we select the ABI library

ldionne added inline comments. Comment at: libcxxabi/CMakeLists.txt:201 endif() +message(STATUS "Using libc++abi testing configuration: ${LIBCXXABI_TEST_CONFIG}") set(LIBCXXABI_TEST_PARAMS "" CACHE STRING mstorsjo wrote: > Unrelated to the rest of the patch? I

[PATCH] D120727: [libc++] Overhaul how we select the ABI library

ldionne marked 5 inline comments as done. ldionne added inline comments. Comment at: libcxx/CMakeLists.txt:250 + +set(LIBCXX_SUPPORTED_ABI_LIBRARIES none libcxxabi system-libcxxabi libcxxrt libstdc++ libsupc++ vcruntime) +set(LIBCXX_CXX_ABI "${LIBCXX_DEFAULT_ABI_LIBRARY}" CACHE

[PATCH] D120727: [libc++] Overhaul how we select the ABI library

ldionne updated this revision to Diff 429069. ldionne marked 2 inline comments as done. ldionne added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120727/new/ https://reviews.llvm.org/D120727 Files: clang/cma

[PATCH] D120727: [libc++] Overhaul how we select the ABI library

ldionne accepted this revision as: libc++. ldionne marked an inline comment as done. ldionne added inline comments. This revision is now accepted and ready to land. Comment at: libcxx/CMakeLists.txt:250 + +set(LIBCXX_SUPPORTED_ABI_LIBRARIES none libcxxabi system-libcxxabi libcxxr

[PATCH] D120727: [libc++] Overhaul how we select the ABI library

This revision was automatically updated to reflect the committed changes. Closed by commit rGa80e65e00ada: [libc++] Overhaul how we select the ABI library (authored by ldionne). Changed prior to commit: https://reviews.llvm.org/D120727?vs=429069&id=429209#toc Repository: rG LLVM Github Monor

[PATCH] D125683: [runtimes] Replace LIBCXX_ENABLE_STATIC_ABI_LIBRARY & friends by a new LIBCXX_CXX_ABI choice

ldionne created this revision. ldionne added reviewers: phosek, mstorsjo. Herald added subscribers: abrachet, mgorny. Herald added a project: All. ldionne requested review of this revision. Herald added projects: clang, Sanitizers, libc++, libc++abi, LLVM. Herald added subscribers: llvm-commits, li

[PATCH] D125683: [runtimes] Replace LIBCXX_ENABLE_STATIC_ABI_LIBRARY & friends by a new LIBCXX_CXX_ABI choice

ldionne added a comment. In D125683#3515675 , @mstorsjo wrote: > It looks like this patch still lacks compatibility handling for all external > users that are setting this option, which need to be able to set the option > in this way at least until afte

[PATCH] D125683: [runtimes] Replace LIBCXX_ENABLE_STATIC_ABI_LIBRARY & friends by a new LIBCXX_CXX_ABI choice

ldionne added a comment. In D125683#3516911 , @mstorsjo wrote: > I guess that's better than silently (well, cmake warns, but it's easy to > miss) no longer doing what it used to, but it's not ideal IMO. I do my > continuous build testing with a build in

[PATCH] D125683: [runtimes] Replace LIBCXX_ENABLE_STATIC_ABI_LIBRARY & friends by a new LIBCXX_CXX_ABI choice

ldionne updated this revision to Diff 430051. ldionne added a comment. Don't remove the old options yet for backwards compatibility. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125683/new/ https://reviews.llvm.org/D125683 Files: clang/cmake/ca

[PATCH] D125683: [runtimes] Replace LIBCXX_ENABLE_STATIC_ABI_LIBRARY & friends by a new LIBCXX_CXX_ABI choice

ldionne added inline comments. Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:124 set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "") -set(RUNTIMES_${target}_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "") set(RUNTIMES_${target}_LIBCXX_ABI_VERSION

[PATCH] D125683: [runtimes] Replace LIBCXX_ENABLE_STATIC_ABI_LIBRARY & friends by a new LIBCXX_CXX_ABI choice

ldionne updated this revision to Diff 430779. ldionne marked 3 inline comments as done. ldionne added a comment. Herald added a subscriber: arichardson. Address some comments (but not @phosek's yet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125

[PATCH] D125683: [runtimes] Replace LIBCXX_ENABLE_STATIC_ABI_LIBRARY & friends by a new LIBCXX_CXX_ABI choice

ldionne added inline comments. Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:124 set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "") -set(RUNTIMES_${target}_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "") set(RUNTIMES_${target}_LIBCXX_ABI_VERSION

[PATCH] D125683: [runtimes] Replace LIBCXX_ENABLE_STATIC_ABI_LIBRARY & friends by a new LIBCXX_CXX_ABI choice

ldionne added inline comments. Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:124 set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "") -set(RUNTIMES_${target}_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "") set(RUNTIMES_${target}_LIBCXX_ABI_VERSION

[PATCH] D66364: Diagnose use of _Thread_local as an extension when appropriate

ldionne added a comment. In D66364#1637570 , @aaron.ballman wrote: > In D66364#1635863 , @ldionne wrote: > > > In D66364#1635814 , @aaron.ballman > > wrote: > > > > > [ ...

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

ldionne commandeered this revision. ldionne edited reviewers, added: EricWF; removed: ldionne. ldionne added a comment. Herald added a subscriber: jkorous. Taking over because Eric is on vacation. I think everything has been addressed at this point. @scanon do you see anything else that needs to

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

This revision was automatically updated to reflect the committed changes. Closed by commit rL370891: [libc++] Add `__truncating_cast` for safely casting float types to integers (authored by ldionne, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed pri

[PATCH] D87459: [libcxx][test] ostream{,buf}_iterator::difference_type changes in C++20

ldionne updated this revision to Diff 291213. ldionne added a comment. Implement the changes for libc++. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87459/new/ https://reviews.llvm.org/D87459 Files: libcxx/include/iterator libcxx/test/std/it

[PATCH] D87459: [libcxx][test] ostream{,buf}_iterator::difference_type changes in C++20

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG71a16e40f78a: [libcxx] ostream{,buf}_iterator::difference_type changes in C++20 (authored by ldionne). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D31413: [libc++] Use __attribute__((init_priority(101))) to ensure streams get initialized early

ldionne commandeered this revision. ldionne added a reviewer: EricWF. ldionne added a comment. Herald added subscribers: dexonsmith, jkorous. I did some work on trying to reproduce this a while ago but never got to reproducing it on macOS. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D3

[PATCH] D31413: [libc++] Use __attribute__((init_priority(101))) to ensure streams get initialized early

ldionne updated this revision to Diff 292254. ldionne added a comment. Herald added a project: libc++. Herald added a subscriber: libcxx-commits. Herald added a reviewer: libc++. Added a test. I can't reproduce the issue, so I don't know whether the test is useful or not. Please help with that!

[PATCH] D31413: [libc++] Use __attribute__((init_priority(101))) to ensure streams get initialized early

ldionne added a comment. In D31413#2277118 , @eastig wrote: > In D31413#2277070 , @ldionne wrote: > >> Added a test. I can't reproduce the issue, so I don't know whether the test >> is useful or not. Please help wi

[PATCH] D31413: [libc++] Use __attribute__((init_priority(101))) to ensure streams get initialized early

ldionne updated this revision to Diff 292261. ldionne added a comment. Add tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D31413/new/ https://reviews.llvm.org/D31413 Files: libcxx/src/iostream.cpp libcxx/test/std/input.output/iostream.obj

[PATCH] D31413: [libc++] Use __attribute__((init_priority(101))) to ensure streams get initialized early

ldionne added inline comments. Comment at: libcxx/test/std/input.output/iostream.objects/init.pass.cpp:15 +// +// This test works by checking that 'std::{cin/cout,cerr}' is the same in a +// static object constructor and in the main function. It dumps the memory of --

[PATCH] D31413: [libc++] Use __attribute__((init_priority(101))) to ensure streams get initialized early

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG39faf428164a: [libc++] Ensure streams are initialized early (autho

[PATCH] D31413: [libc++] Use __attribute__((init_priority(101))) to ensure streams get initialized early

ldionne added a subscriber: hans. ldionne added a comment. @hans Is there still time to cherry-pick this to the 11 release? In D31413#2277198 , @eastig wrote: > I don't mind. > Just to check, in PR28954 it is mentioned the solution based on > 'init_prio

[PATCH] D12689: [libc++][static linking] std streams are not initialized prior to their use in static object constructors

ldionne commandeered this revision. ldionne added a reviewer: eastig. ldionne added a comment. Herald added subscribers: dexonsmith, jkorous. This is superseded by https://reviews.llvm.org/D31413. Closing to clear up the review queue. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12689/

[PATCH] D12689: [libc++][static linking] std streams are not initialized prior to their use in static object constructors

ldionne added a comment. In D12689#2277225 , @eastig wrote: > Abandoned in favour of https://reviews.llvm.org/D31413 Sorry, we had a race condition :-) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12689/new/ https://reviews.llvm.org/D12689 __

[PATCH] D31413: [libc++] Use __attribute__((init_priority(101))) to ensure streams get initialized early

ldionne added a comment. In D31413#2277630 , @smeenai wrote: > What was the conclusion for the comments about the priority level (100 vs. > 101)? My understanding is that values below `101` are literally not allowed: <...>/llvm/libcxx/src/iostream.cpp

<    1   2   3   4   5   6   7   8   >