[Lldb-commits] [PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

ldionne added a comment. (sorry, no idea why it says "this revision is now accepted and ready to land on my behalf, I was just removing the libc++ review group to clear our review queue) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112374/new/

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

ldionne added a comment. Herald added subscribers: wangpc, gysit, Dinistro, hoy, bviyer, wlei, jplehr, PiotrZSL, luke, sunshaoce. Herald added a reviewer: springerm. Herald added a reviewer: kiranchandramohan. Herald added a project: clang-format. Herald added reviewers: rymiel,

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

ldionne commandeered this revision. ldionne added a reviewer: t-tye. ldionne added a comment. Herald added a subscriber: jplehr. Herald added a project: All. [GitHub PR transition cleanup] This has been open for 2 years with multiple subscribers able to see this discussion, but there doesn't

[Lldb-commits] [PATCH] D158863: Implement the monolithic CI pipeline in the monorepo

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcf1a3d93581f: Implement the monolithic CI pipeline in the monorepo (authored by ldionne). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D158863: Implement the monolithic CI pipeline in the monorepo

ldionne updated this revision to Diff 554420. ldionne marked an inline comment as done. ldionne added a comment. Address review comments. I did some testing using GH PRs and this should work, but some tweaks might be necessary. After discussing with Mikhail, I'll merge this now and we can make

[Lldb-commits] [PATCH] D158863: Implement the monolithic CI pipeline in the monorepo

ldionne marked an inline comment as done. ldionne added inline comments. Comment at: .ci/generate-buildkite-pipeline-premerge:163 + +if [[ ! ${SPECIFIC_PIPELINE_AVAILABLE} -eq 1 ]]; then + # Figure out which projects need to be built on each platform goncharov

[Lldb-commits] [PATCH] D158863: Implement the monolithic CI pipeline in the monorepo

ldionne added a comment. Herald added a subscriber: JDevlieghere. Sorry for spamming everyone, I'm trying to ensure that the CI will work with all the sub-projects in the monorepo once we move to GH PRs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D158863: Implement the monolithic CI pipeline in the monorepo

ldionne updated this revision to Diff 554016. ldionne marked an inline comment as done. ldionne added a comment. Herald added a reviewer: bollu. Herald added subscribers: cfe-commits, libc-commits, openmp-commits, libcxx-commits, lldb-commits, Sanitizers, Enna1, yota9, ayermolo, jvesely. Herald

[Lldb-commits] [PATCH] D142007: [NFC] Fix "form/from" typos

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM, thanks! I don't think you need to wait for other owners to approve before landing this, this is pretty clearly an improvement. Repository: rG LLVM Github Monorepo CHANGES SINCE

[Lldb-commits] [PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

ldionne added a comment. LGTM (sorry, it looks like I approved on behalf of all libc++ vendors but that wasn't my intention). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137724/new/ https://reviews.llvm.org/D137724

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

ldionne accepted this revision. ldionne added a comment. Thanks for the fixes. LGTM for `libcxx/`, `libcxxabi/` and `libunwind/`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137338/new/ https://reviews.llvm.org/D137338

[Lldb-commits] [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

[Lldb-commits] [PATCH] D132608: [CMake] Clean up CMake binary dir handling

ldionne added a comment. I think I like the spirit of this change, which seems to be to move us closer to `CMAKE_foo` variables and further from `LLVM_foo` variables for equivalent functionality. I have a comment, but this essentially LGTM. The libc++ CI failures (in particular the

[Lldb-commits] [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

[Lldb-commits] [PATCH] D134878: Update developer policy on potentially breaking changes

ldionne accepted this revision. ldionne added inline comments. Comment at: llvm/docs/DeveloperPolicy.rst:140 + +* After the change has been committed to the repository, the potentially + disruptive changes described in the release notes should be posted to the

[Lldb-commits] [PATCH] D134878: Update developer policy on potentially breaking changes

ldionne added a comment. Thanks for working on this! FWIW, this more or less standardizes what we've been doing in libc++ for the past ~1.5 years and it's been pretty low effort for us to do. And putting my vendor hat on, it's been extremely useful for me to track down potential issues when

[Lldb-commits] [PATCH] D133618: Adapt LLDB dataformatters for libcxx change D129386

ldionne accepted this revision. ldionne added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:1004 + auto dataobj = GetChildMemberWithName( + valobj, {ConstString("__data_"),

[Lldb-commits] [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`

[Lldb-commits] [PATCH] D127444: [libc++] Use ABI tags instead of internal linkage to provide per-TU insulation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd2e86866be0f: [libc++] Re-apply the use of ABI tags to provide per-TU insulation (authored by ldionne). Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D127444: [libc++] Use ABI tags instead of internal linkage to provide per-TU insulation

ldionne updated this revision to Diff 443066. ldionne added a comment. Herald added projects: LLDB, LLVM. Herald added subscribers: llvm-commits, lldb-commits. Band-aid for the LLDB issue, and also include 6148c79a in the

[Lldb-commits] [PATCH] D128694: [lldb/Dataformatters] Adapt C++ std::string dataformatter for D128285

ldionne added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:637 if (location_sp->GetName() == g_size_name) - location_sp = short_sp->GetChildAtIndex(3, true); + location_sp = short_sp->GetChildAtIndex(2, true); if

[Lldb-commits] [PATCH] D123580: [libc++] Use bit field for checking if string is in long or short mode

ldionne added inline comments. Comment at: libcxx/utils/gdb/libcxx/printers.py:192 class StdStringPrinter(object): """Print a std::string.""" philnik wrote: > labath wrote: > > philnik wrote: > > > dblaikie wrote: > > > > philnik wrote: > > > > > jgorbe

[Lldb-commits] [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:

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

ldionne requested changes to this revision. ldionne added a comment. This revision now requires changes to proceed. Thanks for doing this! We need to fix a few undefined references, though. Comment at: libcxx/docs/index.rst:223 * `libcxx-commits Mailing List`_ * `libcxx-dev

[Lldb-commits] [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 ___

[Lldb-commits] [PATCH] D116224: Revert "[amdgpu] Enable selection of `s_cselect_b64`."

ldionne commandeered this revision. ldionne added a reviewer: david-salinas. ldionne added a comment. Yeah, I think you messed something up with your git commands. I'm going to commandeer and abandon this revision to get it out of the libc++ review queue since it's been 10 days since this has

[Lldb-commits] [PATCH] D116351: Update Bug report URL to Github Issues

ldionne accepted this revision as: libc++, libunwind. ldionne added a comment. Libc++ and libunwind changes LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116351/new/ https://reviews.llvm.org/D116351 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D115566: Quote some more destination paths with variables

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. This looks reasonable to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115566/new/ https://reviews.llvm.org/D115566

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

ldionne removed projects: libc++, libunwind. ldionne removed reviewers: libunwind, libc++, ldionne. ldionne added a comment. Herald added projects: libc++, libunwind. Herald added a reviewer: libc++. Herald added a reviewer: libunwind. Removing from the runtimes review queue -- please ping me

[Lldb-commits] [PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM from the libc++ perspective. Unsubscribing to reduce spam, please ping me on Discord if you need further input. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

ldionne added a comment. Herald added a project: Flang. If there is no plan to move forward with generating `.docx` documentation, can we please abandon this review? I'm trying to clean up libc++'s review queue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D107717: [LLVM][CMake][NFC] Resolve FIXME: Rename LLVM_CMAKE_PATH to LLVM_CMAKE_DIR throughout the project

ldionne accepted this revision. ldionne added a comment. This LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107717/new/ https://reviews.llvm.org/D107717 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [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

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

ldionne added a comment. In D100581#2793926 , @dblaikie wrote: > In D100581#2793775 , @ldionne wrote: > >> Hello! There are still some false positives, for example this one is >> breaking the libc++ CI: >>

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

ldionne added a comment. Hello! There are still some false positives, for example this one is breaking the libc++ CI: https://buildkite.com/llvm-project/libcxx-ci/builds/3530#8679608a-200b-4a48-beb4-a9e9924a8848 Would it be possible to either fix this quickly or revert the change until the

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

ldionne accepted this revision as: libc++, libc++abi. ldionne added a comment. LGTM for libcxx and libcxxabi. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99484/new/ https://reviews.llvm.org/D99484 ___

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

ldionne added inline comments. Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:66 install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}" -DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dstdir} +DESTINATION

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

ldionne added a subscriber: phosek. ldionne added a comment. I am generally OK with the libcxx and libcxxabi changes. Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:389 get_compiler_rt_target(${arch} target) -set(${install_dir}

[Lldb-commits] [PATCH] D94374: [CMake] Remove dead code setting policies to NEW

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94374/new/ https://reviews.llvm.org/D94374

[Lldb-commits] [PATCH] D94374: [CMake] Remove dead code setting policies to NEW

ldionne requested changes to this revision. ldionne added a comment. This revision now requires changes to proceed. Except for the google-benchmark nit, LGTM. Thanks a lot for the cleanup! Comment at: libcxx/utils/google-benchmark/CMakeLists.txt:3 - -project (benchmark) -

[Lldb-commits] [PATCH] D89813: [CMake][NFC] Limit the use of uppercase_CMAKE_BUILD_TYPE

ldionne added a comment. Are generator expressions expanded in `if()`? If so, we could at least change those to `if ("$" STREQUAL "DEBUG")`. This would preserve the case-insensitivity while solving the problem you were concerned about, and make the code easier to read. Repository: rG LLVM

[Lldb-commits] [PATCH] D89813: [CMake][NFC] Limit the use of uppercase_CMAKE_BUILD_TYPE

ldionne added a comment. I quite like this. We should strive to use the same name as CMake uses for its build types, i.e. `Debug`, `Release`, etc. If I configure CMake with `-DCMAKE_BUILD_TYPE=DEBUG`, is `CMAKE_BUILD_TYPE` normalized back to `Debug` by CMake, or is it defined to `DEBUG`? If

[Lldb-commits] [PATCH] D87051: scan-build-py: fix multiprocessing error

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. From the docs: > `multiprocessing.freeze_support()` > Add support for when a program which uses multiprocessing has been frozen to > produce a Windows executable. (Has been tested with

[Lldb-commits] [PATCH] D86616: [cmake] Make gtest include directories a part of the library interface

ldionne added a comment. In D86616#2241936 , @labath wrote: > In D86616#2238946 , @ldionne wrote: > >> LGTM, but I'm not an owner for any of the projects touched by this change. > > I picked you because you seemed

[Lldb-commits] [PATCH] D86616: [cmake] Make gtest include directories a part of the library interface

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM, but I'm not an owner for any of the projects touched by this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86616/new/

[Lldb-commits] [PATCH] D84748: [cmake] Make gtest macro definitions a part the library interface

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. Can you confirm that all the targets that need to "link" (in the CMake sense) against gtest use the gtest CMake target? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [Differential] D78648: [CMake] Bump CMake minimum version to 3.13.4

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGafa1afd4108d: [CMake] Bump CMake minimum version to 3.13.4 (authored by ldionne). Herald added subscribers: llvm-commits, msifontes, sstefan1,

[Lldb-commits] [PATCH] D78648: [CMake] Bump CMake minimum version to 3.13.4

ldionne added a comment. Herald added subscribers: llvm-commits, msifontes, sstefan1, jurahul, stephenneuendorffer, aartbik. Herald added projects: MLIR, LLVM. Okay, the previous patch has landed and no issues have come up, so I'm going to move forward with this patch now. Repository: rG

[Lldb-commits] [PATCH] D78648: [CMake] Bump CMake minimum version to 3.13.4

This revision was automatically updated to reflect the committed changes. Closed by commit rGafa1afd4108d: [CMake] Bump CMake minimum version to 3.13.4 (authored by ldionne). Changed prior to commit: https://reviews.llvm.org/D78648?vs=259300=279895#toc Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D78648: [CMake] Bump CMake minimum version to 3.13.4

ldionne created this revision. Herald added subscribers: libcxx-commits, openmp-commits, lldb-commits, Sanitizers, cfe-commits, frgossen, grosul1, jvesely, Joonsoo, liufengdb, lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst, shauheen, jpienaar, rriddle, mehdi_amini, lebedev.ri,

[Lldb-commits] [PATCH] D78648: [CMake] Bump CMake minimum version to 3.13.4

ldionne added a comment. This review is for the upcoming CMake upgrade once we branch off the LLVM 11 release branch. I won't apply it until after the branch has been created, which is around July. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D64395: [CMake] Avoid libcxxabi dependency when building LLDB from the monorepo on macOS by default

ldionne added a comment. In D64395#1575999 , @sgraenitz wrote: > Thanks for taking a look. > > In D64395#1575967 , @ldionne wrote: > > > When is `LLDBConfig.cmake` used? > > > Very early in the LLDB config process:

[Lldb-commits] [PATCH] D64395: [CMake] Avoid libcxxabi dependency when building LLDB from the monorepo on macOS by default

ldionne added a comment. When is `LLDBConfig.cmake` used? I'm trying to understand when the `LIBCXX_ENABLE_SHARED=OFF` setting will come into play. Will it influence whether `libc++.dylib` is built in the monorepo whenever you also happen to build LLDB? Comment at: