[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

2023-04-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne requested changes to this revision. ldionne added a comment. This revision now requires changes to proceed. Thanks for working on this! This generally LGTM, with a few comments. You should be able to add tests in a way similar to `clang/test/Driver/darwin-header-search-libcxx.cpp`. The i

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

2023-04-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:413-414 + // Including the path to the just-built libc++.dylib if libc++ is bootstrapped + // and /bin/../lib/libc++.dylib is a valid path + I would reword this to: ``` // If

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

2023-04-17 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a subscriber: arphaman. ldionne added inline comments. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:431 + // libc++.dylib in the toolchain. + if ((!Args.hasArg(options::OPT_nostdinc, options::OPT_nostdlibinc, +options::OPT_nostdincxx)) &&

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

2023-05-25 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:431 + // libc++.dylib in the toolchain. + if ((!Args.hasArg(options::OPT_nostdinc, options::OPT_nostdlibinc, +options::OPT_nostdincxx)) && arphaman wrote: > ld

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

2023-06-01 Thread Louis Dionne via Phabricator via cfe-commits
ldionne requested changes to this revision. ldionne added inline comments. This revision now requires changes to proceed. Comment at: clang/test/Driver/darwin-header-search-libcxx.cpp:119 // RUN: -nostdinc++ \ -// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

2023-06-01 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments. Comment at: clang/test/Driver/darwin-header-search-libcxx.cpp:95 // Make sure that using -nostdinc, -nostdinc++ or -nostdlib will drop both the toolchain // C++ include path and the sysroot one. Comment at:

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

2023-06-01 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. This LGTM w/ comments applied. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148266/new/ https://reviews.llvm.org/D148266 ___

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

2023-06-01 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments. Comment at: clang/test/Driver/darwin-header-search-libcxx.cpp:105 // RUN: -nostdinc \ // RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \ // RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ ldion

[PATCH] D135341: [clang] adds `__reference_constructs_from_temporary`

2023-02-07 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. I've been looking at implementing `reference_constructs_from_temporary` & friends and this would be sweet to have. Is this blocked on something specific? Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:397 +def err_reserved_identifier_fo

[PATCH] D135341: [clang] adds `__reference_constructs_from_temporary`

2023-02-10 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In D135341#4111884 , @cjdb wrote: > In D135341#4111673 , @ldionne wrote: > >> I've been looking at implementing `reference_constructs_from_temporary` & >> friends and this would be sweet

[PATCH] D142604: [Clang] Fix __VA_OPT__ implementation so that it treats the concatenation of a non-placemaker token and placemaker token as a non-placemaker token

2023-02-15 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. I suspect the libc++ failure is because the patch is not rebased onto `main`. I remember seeing those issues a while back. I think it's unlikely to be this patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142604/new/ https://reviews.llvm.org/D142604 _

[PATCH] D149752: WIP: Debug symlink creation

2023-05-05 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 519824. ldionne added a comment. Use tarball Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149752/new/ https://reviews.llvm.org/D149752 Files: clang/foo libcxx/utils/ci/buildkite-pipeline-clang.yml libcx

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

2023-05-10 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision as: libc++abi. ldionne added a comment. This revision is now accepted and ready to land. Code changes in libc++ and libc++abi LGTM. I am neutral on whether the diagnostic is worth adding, but don't consider libc++ and libc++abi as blockers for this patch. ==

[PATCH] D148945: [clang] Remove workaround for old LLVM_ENABLE_PROJECTS=libcxx build

2023-04-21 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. Herald added a project: All. ldionne requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. We don't support the LLVM_ENABLE_PROJECTS=libcxx build anymore (and have not for over a year), so it should be fine

[PATCH] D148945: [clang] Remove workaround for old LLVM_ENABLE_PROJECTS=libcxx build

2023-04-21 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added subscribers: phosek, Ericson2314, aaron.ballman. ldionne added a comment. CCing some folks that were watching D132324 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148945/new/ https://reviews.llvm.o

[PATCH] D148945: [clang] Remove workaround for old LLVM_ENABLE_PROJECTS=libcxx build

2023-04-24 Thread Louis Dionne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5084ba395e48: [clang] Remove workaround for old LLVM_ENABLE_PROJECTS=libcxx build (authored by ldionne). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148945

[PATCH] D149553: [clang] Use -std=c++23 instead of -std=c++2b

2023-05-02 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. Thanks for doing this! No objection, will let Aaron give the thumbs up. Comment at: clang/test/Parser/cxx2b-label.cpp:1 -// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx2b -std=c++2b -Wpre-c++2b-compat %s +// RUN: %clang_cc1 -fsyntax-only -verify=

[PATCH] D149752: WIP: Debug symlink creation

2023-05-03 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 519218. ldionne added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Poke CI Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149752/new/ https://reviews.llvm.org/D149752 Files:

[PATCH] D149752: WIP: Debug symlink creation

2023-05-04 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 519476. ldionne added a comment. Herald added subscribers: libcxx-commits, arichardson. Herald added a project: libc++. Herald added a reviewer: libc++. Check file type Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D149752: WIP: Debug symlink creation

2023-05-04 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 519483. ldionne added a comment. Make sure we run the Clang pipeline always Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149752/new/ https://reviews.llvm.org/D149752 Files: clang/foo libcxx/utils/ci/build

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

2022-12-12 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. This LGTM. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2084-2089 + // On Apple platforms, C and C++ Standard Library headers are not provided + // with the base sy

[PATCH] D139938: [clang] Don't spuriously pass -stdlib=libc++ to CC1 on Darwin

2022-12-13 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. ldionne added a reviewer: arphaman. Herald added a project: All. ldionne requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Previously, we would be passing down -stdlib=libc++ from the Driver to CC1 whene

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

2022-12-13 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. You should make sure that the CI is passing before merging! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136315/new/ https://reviews.llvm.org/D136315 ___ cfe-commits mailing lis

[PATCH] D114903: [clang] Support array placement new in constant expressions

2022-01-31 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added subscribers: Quuxplusone, mumbleskates, egorzhdan. ldionne added a comment. Gentle ping -- I suspect this may be too naive, but it's a start. It would be awesome to ship this in LLVM 14, since it would allow us to unblock libc++ work on a couple features. Repository: rG LLVM Gi

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

2022-03-01 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. Herald added a subscriber: arichardson. ldionne requested review of this revision. Herald added projects: clang, libc++. Herald added subscribers: libcxx-commits, cfe-commits. Herald added a reviewer: libc++. This patch overhauls how we pick up the ABI library. Inste

[PATCH] D120160: [Clang] Add `-funstable` flag to enable unstable and experimental features

2022-03-01 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added subscribers: libc++, libc++ vendors, rsmith, CaseyCarter. ldionne added a comment. Oops, this seems to have happened while I was on vacation. I wanted to have a wider discussion here before shipping the patch. Let's have it now and we can make post-commit changes depending on where

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

2022-03-01 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 412191. 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/CMake

[PATCH] D120907: [docs] Add PowerPC release notes for LLVM 14

2022-03-03 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM for libc++. I assume this is targeting `release/14.x` only. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120907/new/ https://reviews.llv

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

2022-03-04 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. Herald added a project: All. Shall we land this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119918/new/ https://reviews.llvm.org/D119918 ___ cfe-commits mailing list cfe-commi

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

2022-03-07 Thread Louis Dionne via Phabricator via cfe-commits
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 M

[PATCH] D101166: This commit tries to revert Clang commit 40beb1f84a to see if that's the cause of failures in the Runtimes build.

2021-04-23 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. Herald added a subscriber: arichardson. ldionne requested review of this revision. Herald added projects: clang, libc++. Herald added subscribers: libcxx-commits, cfe-commits. Herald added a reviewer: libc++. I'm creating a Phab review for this to take advantage of t

[PATCH] D101166: This commit tries to revert Clang commit 40beb1f84a to see if that's the cause of failures in the Runtimes build.

2021-04-23 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 340050. ldionne added a comment. Try running a full bisect Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101166/new/ https://reviews.llvm.org/D101166 Files: libcxx/utils/ci/buildkite-pipeline.yml libcxx/ut

[PATCH] D101166: This commit tries to revert Clang commit 40beb1f84a to see if that's the cause of failures in the Runtimes build.

2021-04-23 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 340077. ldionne added a comment. Smaller bisect Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101166/new/ https://reviews.llvm.org/D101166 Files: libcxx/test/std/utilities/template.bitset/bitset.members/left

[PATCH] D101166: This commit tries to revert Clang commit 40beb1f84a to see if that's the cause of failures in the Runtimes build.

2021-04-23 Thread Louis Dionne via Phabricator via cfe-commits
ldionne abandoned this revision. ldionne added a comment. Fixed by 3b71de41cc7c79ca03b0d3a5d7fd383abebf4167 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101166/new/ https://re

[PATCH] D101598: [clang][Sema] adds `[[clang::no_address]]` attribute

2021-04-30 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. Just for posterity, what we discussed is that since there is a list of addressable functions in the standard, we should explore adding a warning to Clang that fires whenever somebody takes the address of a function in namespace `std`, except if it's an addressable funct

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

2022-01-03 Thread Louis Dionne via Phabricator via cfe-commits
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 be

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

2021-12-13 Thread Louis Dionne via Phabricator via cfe-commits
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 _

[PATCH] D115685: [NFC] Fix typos in release notes

2021-12-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115685/new/ https://reviews.llvm.org/D115685 ___

[PATCH] D112221: Mark ATOMIC_VAR_INIT and ATOMIC_FLAG_INIT as deprecated

2021-12-15 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a subscriber: tambre. ldionne added a comment. Sorry, I seemed to have missed this entirely. Since this is touching only `` and not libc++'s ``, I think it would actually be OK not to make any changes at all inside libc++. Nothing should break. We should separately implement P0883

[PATCH] D112221: Mark ATOMIC_VAR_INIT and ATOMIC_FLAG_INIT as deprecated

2021-12-15 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. @tambre Any appetite for a libc++ patch? :) Otherwise, let me know and I'll put it in my list of things to fix up. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112221/new/ https://reviews.llvm.org/D112221 ___ cfe-co

[PATCH] D112221: Mark ATOMIC_VAR_INIT and ATOMIC_FLAG_INIT as deprecated

2021-12-16 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In D112221#3196740 , @tambre wrote: > In D112221#3195955 , @ldionne wrote: > >> @tambre Any appetite for a libc++ patch? :) >> >> Otherwise, let me know and I'll put it in my list of thing

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

2021-12-29 Thread Louis Dionne via Phabricator via cfe-commits
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 ___ cfe-commits mailing list cfe-commi

[PATCH] D46443: [libc++] Add missing cstdalign header

2021-03-30 Thread Louis Dionne via Phabricator via cfe-commits
ldionne requested changes to this revision. ldionne added a comment. Trying to summarize the discussion here for the author: 1. Please add a `` header to libc++ that does `#include_next ` and then defines both `__alignof_is_defined` and `__alignas_is_defined`. 2. Please re-add a test checking f

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

2021-03-31 Thread Louis Dionne via Phabricator via cfe-commits
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} ${COMPILER_RT_INSTALL_PATH}/

[PATCH] D89013: [libcxx] Support per-target __config_site in per-target runtime build

2021-03-31 Thread Louis Dionne via Phabricator via cfe-commits
ldionne requested changes to this revision. ldionne added a comment. This revision now requires changes to proceed. I'm personally not so concerned with the Clang driver side of things, but primarily with adding more complexity to the libc++ build. Shouldn't we be driving things from the runtime

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

2021-04-01 Thread Louis Dionne via Phabricator via cfe-commits
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 ${LIBCXX_INSTALL_HE

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

2021-04-06 Thread Louis Dionne via Phabricator via cfe-commits
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 ___

[PATCH] D100057: Remove warning "suggest braces" for aggregate initialization of an empty class with an aggregate base class.

2021-04-08 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. FWIW, I think this is acceptable, but I think even better would be to make it clear in the Standard that this is acceptable. Having spoken with a few folks about this, it doesn't appear to be specified very clearly. CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D89013: [libcxx] Support per-target __config_site in per-target runtime build

2021-04-13 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. I agree the design is clean from the "what we ship" perspective. I really like the idea of shipping the whole library in a generic spot and only having one file embody all the target specific configuration of the library. I think it's a very elegant way to handle the co

[PATCH] D89013: [libcxx] Support per-target __config_site in per-target runtime build

2021-04-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. Well, one way to see things is that this patch only makes yet another step in a direction that we're already rather far in (in terms of CMake complexity). And as I said, I think the separation of `__config_site` in its own platform-specific directory makes a lot of sens

[PATCH] D89013: [libcxx] Support per-target __config_site in per-target runtime build

2021-04-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. As discussed offline, I think this is fine. This layout seems to be closest to what other compilers use. @phosek said he would send out an RFC to make the multiarch layout become the defaul

[PATCH] D89013: [libcxx] Support per-target __config_site in per-target runtime build

2021-04-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. Oh, and can you make sure the Runtimes build passes CI before shipping this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89013/new/ https://reviews.llvm.org/D89013 ___ cfe-comm

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

2021-04-15 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In D45639#2383754 , @smeenai wrote: > Just following up on this, cos I'm curious :) I have 12.1 now, and I still > only see the C++ headers in the toolchain and not in any of the SDKs. Look in Xcode 12.5 beta 3, you should see li

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

2021-04-16 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. In D45639#2693706 , @phosek wrote: > It's depends on the order: whichever comes first wins. The default order of > paths that the driver uses is (1)

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

2021-04-21 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In D45639#2703913 , @JDevlieghere wrote: > This breaks `TestAppleSimulatorOSType.py ` on GreenDragon. First failed > build: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/31346/ > [...] > > Based on your description abo

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

2021-04-21 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In D45639#2706589 , @dexonsmith wrote: > I'm not sure I'm totally following, but just want to double check that the > tests won't somehow use the libc++ from the SDK instead of ToT? I guess the > test uses `-nostdinc++` or someth

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

2021-01-11 Thread Louis Dionne via Phabricator via cfe-commits
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) - -

[PATCH] D50739: Clean up macros to detect underling C library functionality

2021-01-11 Thread Louis Dionne via Phabricator via cfe-commits
ldionne commandeered this revision. ldionne edited reviewers, added: mclow.lists; removed: ldionne. ldionne added a comment. This revision now requires review to proceed. Herald added a subscriber: jkorous. Commandeering to close as I don't think this applies anymore. Repository: rG LLVM Githu

[PATCH] D35388: [libc++] Give extern templates default visibility on gcc

2021-01-12 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. Can you rebase onto `main` and re-upload the diff? It will trigger CI. If the CI passes, this is good to go. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D35388/new/ https://reviews.llvm.org

[PATCH] D93922: Mangle `__alignof__` differently than `alignof`.

2021-01-12 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:4009 +// mangling. Previously, it used a special-cased nonstandard extension. +if (Context.getASTContext().getLangOpts().getClangABICompat() >= +LangOptions::ClangABI::Ver11) {

[PATCH] D35388: [libc++] Give extern templates default visibility on gcc

2021-01-12 Thread Louis Dionne via Phabricator via cfe-commits
ldionne removed a reviewer: EricWF. ldionne added a subscriber: EricWF. ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. In D35388#2493696 , @smeenai wrote: > It's been a long time since I've contrib

[PATCH] D93922: Mangle `__alignof__` differently than `alignof`.

2021-01-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision as: libc++abi. ldionne added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:4009 +// mangling. Previously, it used a special-cased nonstandard extension. +if (Context.getASTContext().getLangOpts().getClangABICompat() >= +

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

2021-01-19 Thread Louis Dionne via Phabricator via cfe-commits
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

[PATCH] D91630: [Parse] Add parsing support for C++ attributes on using-declarations

2021-01-19 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. Gentle ping! Can we merge this? I'd love to move forward with https://reviews.llvm.org/D90257. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91630/new/ https://reviews.llvm.org/D91630 ___ cfe-commits mailing list cfe

[PATCH] D95055: [clang] Don't look into for C++ headers if they are found alongside the toolchain

2021-01-20 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. ldionne added reviewers: tstellar, dexonsmith. Herald added a subscriber: jkorous. ldionne requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently, Clang looks for libc++ headers alongside the installation d

[PATCH] D95055: [clang] Don't look into for C++ headers if they are found alongside the toolchain

2021-01-20 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. I would like to cherry-pick this change onto LLVM 11.1.0, if there is still time to do so. It is breaking Clang when the `sysroot` contains headers for libc++. Is it too late? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D93922: Mangle `__alignof__` differently than `alignof`.

2021-01-21 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In D93922#2509931 , @jyknight wrote: > Was the "group" libc++abi accept intended to be an accept for the whole > revision? Or should still ping for review from rsmith or someone else? Sorry, I should have been clearer. I just wan

[PATCH] D112030: [docs] Remove Makefile.sphinx files

2021-10-18 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. We don't use them in libc++, fine by me to remove if the website documentation builders don't use them (IDK how that's set up). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

2021-10-21 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In D111283#3056748 , @rsmith wrote: > Do you have examples showing what this does in practice for errors in > real-world code? I'm concerned that stripping back to the common type sugar > may produce an unintuitive result in som

[PATCH] D42225: libcxx: Provide overloads for basic_filebuf::open() et al that take wchar_t* filenames on Windows.

2021-08-24 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added subscribers: mstorsjo, ldionne. ldionne added a comment. Herald added a subscriber: libcxx-commits. @pcc @mstorsjo Are we aware of anyone using these extensions? I would like to suggest that we either remove this extension if it's not useful, or make it unconditional (not only on W

[PATCH] D42225: libcxx: Provide overloads for basic_filebuf::open() et al that take wchar_t* filenames on Windows.

2021-08-24 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a subscriber: CaseyCarter. ldionne added a comment. In D42225#2963190 , @mstorsjo wrote: > In D42225#2962348 , @ldionne wrote: > >> @pcc @mstorsjo Are we aware of anyone using these extensions? >> >> I

[PATCH] D63270: [clangd] Add include-mapping for C symbols.

2021-08-27 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. Herald added a subscriber: usaxena95. Herald added a project: clang-tools-extra. This is awesome, but the script doesn't work anymore with `-language=cpp` -- is there any interest in looking into that? The C++ Standard Library has changed quite a bit recently and it woul

[PATCH] D108696: [Coroutines] [Frontend] Lookup in std namespace first

2021-09-03 Thread Louis Dionne via Phabricator via cfe-commits
ldionne reopened this revision. ldionne added a comment. This revision is now accepted and ready to land. Based on the commit description, I don't understand this change at all. Why do we want to tweak the name lookup just for `std::coroutine`? Yes, we do have an action item to finish coroutines

[PATCH] D108696: [Coroutines] [Frontend] Lookup in std namespace first

2021-09-03 Thread Louis Dionne via Phabricator via cfe-commits
ldionne reopened this revision. ldionne added a comment. This revision is now accepted and ready to land. (woops, this closed the rev when I committed the revert) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108696/new/ https://reviews.llvm.org/D1

[PATCH] D109408: [libcxxabi] NFC: fix incorrect indentation of braces

2021-09-07 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. FWIW, I dislike that we don't indent stuff at all inside namespaces -- often I find it useful to indent things inside short-lived namespaces. But let's go for simplicity and consistency. Di

[PATCH] D109411: [clang] Enable the special enable_if_t diagnostics for libc++'s _EnableIf as well.

2021-09-08 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. This shouldn't be necessary anymore since we won't be using `_EnableIf` in libc++ going forward. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109411/new/ https://reviews.llvm.org/D109411 _

[PATCH] D109411: [clang] Enable the special enable_if_t diagnostics for libc++'s _EnableIf as well.

2021-09-08 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. Actually, scratch that, we could tweak this patch so it accepts `__enable_if_t` as well (that will be the new spelling for `enable_if_t` for pre-C++14 places going forward. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109

[PATCH] D109411: [clang] Enable the special enable_if_t diagnostics for libc++'s __enable_if_t as well.

2021-09-08 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. This seems reasonable to me. Comment at: clang/lib/Sema/SemaTemplate.cpp:3514 /// Determine whether this alias template is "enable_if_t". +/// libc++ >=14 uses "__enable_i

[PATCH] D109460: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

2021-09-08 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision. ldionne added a reviewer: arphaman. ldionne requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. On Apple platforms, the base system does not contain headers (e.g. in /usr/include). As a result, one needs to point

[PATCH] D109460: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

2021-09-08 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. Herald added a subscriber: ormris. @arphaman This is a WIP patch -- I'm trying to see whether this idea is good or just bad from the start. To summarize, people have been complaining (e.g. here ) that Clang on Darwin doesn't

<    3   4   5   6   7   8