[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

EricWF added inline comments. Comment at: lib/Sema/SemaOverload.cpp:9177 +// specified in C++2a [over.match.oper]p1.10. +if (RewrittenCandidateTieBreaker && ICS1.isStandard() && +ICS2.isStandard() && ICS1.Standard.getRank() == ICR_Exact_Match && T

[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

EricWF updated this revision to Diff 145398. EricWF added a comment. Generalize `CXXRewrittenExpr` to contain only a "representation" of the original expression, and the fully checked rewritten expression. By "representation" i mean to imply that we can't actually build and check a full represe

[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

EricWF updated this revision to Diff 145419. EricWF added a comment. - Further generalize `CXXRewrittenExpr` and get it working with `TreeTransform.h`. https://reviews.llvm.org/D45680 Files: include/clang/AST/Expr.h include/clang/AST/ExprCXX.h include/clang/AST/RecursiveASTVisitor.h in

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF added inline comments. Comment at: lib/AST/ExprConstant.cpp:8829 + return EvaluateComparisonBinaryOperator(Info, E, OnSuccess, [&]() { +return ExprEvaluatorBaseTy::VisitBinaryOperator(E); + }); rsmith wrote: > EricWF wrote: > > rsmith wrote: > > > It

[PATCH] D45680: [C++2a] Implement operator<=> Part 2: Operator Rewritting and Overload Resolution.

EricWF updated this revision to Diff 145643. EricWF added a comment. - Rebase on master. I think the correct direction to head with this patch is to start removing the bits of the implementation which evaluate and build rewritten expressions during overload resolution. I'll submit such an updat

[PATCH] D45680: [C++2a] Implement operator<=> Part 2: Operator Rewritting and Overload Resolution.

EricWF added a comment. I'm still not sure how to best generate a diff that best ignores white spaces changes caused by clang-format or my editor. I'll work on removing existing whitespace changes. https://reviews.llvm.org/D45680 ___ cfe-commits m

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF marked 10 inline comments as done. EricWF added inline comments. Comment at: include/clang/AST/SourceLocExprScope.h:39 +//static_assert(foo() == __LINE__); +return OldVal == nullptr || isa(DefaultExpr) || + !OldVal->isInSameContext(EvalContext); -

[PATCH] D48694: [libc++abi] Limit libc++ header search to specified paths

EricWF added a comment. LGTM, but I'm a bit confused. You seem to argue that no system places C++ headers on the default search paths, but also that it would be a problem if such a system did. Why wouldn't the conclusion be true? That is, although C++ includes aren't normally found along the de

[PATCH] D48694: [libc++abi] Limit libc++ header search to specified paths

EricWF added a comment. > For whether it makes sense to search for includes in the system path, that > boils down to @ldionne's question of whether building libc++abi against a > system-installed libc++ is supported. I actually don't know the answer to > that myself ... @dexonsmith and @EricW

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF planned changes to this revision. EricWF added a comment. Previously this patch performed and leaked a lot of `StringLiteral` temporaries. I think I have a better solution for that problem, but this patch isn't it. Updating patch incoming. https://reviews.llvm.org/D37035

[PATCH] D45131: [AST] Refactor UnaryTransformType into TransformTraitType supporting non-unary transforms

EricWF marked 13 inline comments as done. EricWF added inline comments. Comment at: lib/AST/ItaniumMangle.cpp:3250-3251 - mangleType(T->getBaseType()); + for (auto Ty : T->getArgs()) +mangleType(Ty); } rsmith wrote: > EricWF wrote: > > EricWF wrote: > >

[PATCH] D45131: [AST] Refactor UnaryTransformType into TransformTraitType supporting non-unary transforms

EricWF updated this revision to Diff 154005. EricWF marked 2 inline comments as done. EricWF added a comment. Address main review comments. The `TransformTraitType` is now built early, and the `DeclSpec` refers to it and not the list of argument types. Additionally, the mangling formulation `u

[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM. I'll add a test when I commit this. Repository: rCXX libc++ https://reviews.llvm.org/D47344 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"

EricWF requested changes to this revision. EricWF added a comment. This revision now requires changes to proceed. Actually, I spotted a couple of issues. Requesting changes. Comment at: src/experimental/memory_resource.cpp:29 +static bool is_aligned_to(void *ptr, size_t align)

[PATCH] D47111: : Implement monotonic_buffer_resource.

EricWF added a comment. In https://reviews.llvm.org/D47111#1114284, @Quuxplusone wrote: > Refactor to remove unused fields from the header structs. > > Before this change, `sizeof(monotonic_buffer_resource) == 56` and the > overhead per allocated chunk was `40` bytes. > After this change, `size

[PATCH] D47111: : Implement monotonic_buffer_resource.

EricWF added a comment. In https://reviews.llvm.org/D47111#1151714, @Quuxplusone wrote: > > I'll take a pass at the tests tomorrow. Some pointers in general: > > > > - Tests should be named after the component they test, not how they're > > testing it. > > - All the tests for a single component

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

EricWF added a comment. Are there any tests which actually exercise the new behavior? Comment at: libcxx/include/memory:1665 +(is_same::type> >::value +|| is_same >::value +|| !__has_construct::value) && I'm not sure

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

EricWF added a comment. @rsmith Ping. This needs to land before the next branch for release. https://reviews.llvm.org/D45015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17

EricWF commandeered this revision. EricWF edited reviewers, added: lebedev.ri; removed: EricWF. EricWF added a comment. Hijacking with permission. Repository: rL LLVM https://reviews.llvm.org/D45179 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D45179: [libc++] Add _LIBCPP_ENABLE_NODISCARD and _LIBCPP_NODISCARD_EXT to allow pre-C++2a [[nodiscard]]

EricWF updated this revision to Diff 154724. EricWF retitled this revision from "[libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17" to "[libc++] Add _LIBCPP_ENABLE_NODISCARD and _LIBCPP_NODISCARD_EXT to allow pre-C++2a [[nodiscard]]". EricWF edited the summary of

[PATCH] D45179: [libc++] Add _LIBCPP_ENABLE_NODISCARD and _LIBCPP_NODISCARD_EXT to allow pre-C++2a [[nodiscard]]

EricWF added a comment. A couple other notes: - Add an application of `_LIBCPP_NODISCARD_EXT` to `get_temporary_buffer` to demonstrate usage. https://reviews.llvm.org/D45179 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm

[PATCH] D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets

EricWF added a comment. I'm working on this! I'll be taking what is hopefully a final pass at the tests today. https://reviews.llvm.org/D46845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

EricWF added a comment. Why are we doing this? I can't find the language in the C++03 specification that requires us to call an allocators `construct` method if it's present. https://reviews.llvm.org/D48753 ___ cfe-commits mailing list cfe-commits

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

EricWF added a reviewer: ldionne. EricWF added a subscriber: ldionne. EricWF added a comment. Adding @ldionne as an observer, since he's knee-deep in the visibility mess right now. After re-evaluating, I think I was being overly cautious the last time I looked at this. I think this patch should

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

EricWF added a reviewer: dexonsmith. EricWF added a comment. Ping. Are there any more reviewers I should add to this? https://reviews.llvm.org/D45015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D68756: [libc++][test] Change IsSmallObject's calculation for std::any's small object buffer

EricWF accepted this revision. EricWF added inline comments. This revision is now accepted and ready to land. Comment at: libcxx/test/support/any_helpers.h:30 {}; template bool containsType(std::any const& a) { LGTM. Repository: rG LLVM Github Monorepo

[PATCH] D78223: [clang-tidy] performance-range-for-copy only for copy.

EricWF created this revision. EricWF added reviewers: fowles, shuaiwang. EricWF added a project: clang-tools-extra. Herald added subscribers: mgehre, xazax.hun. Currently the range-for-copy incorrectly suggests changing a by-value loop var to a reference to avoid copies even when: (1) A co

[PATCH] D78323: [clang] Fix invalid comparator in tablegen

EricWF created this revision. EricWF added reviewers: fowles, bkramer, sdesmalen. Herald added a project: clang. The current version of the comparator does not introduce a strict weak ordering. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D78323 Files: clang/utils/TableGen/

[PATCH] D78323: [clang] Fix invalid comparator in tablegen

EricWF added a comment. In D78323#1987596 , @sdesmalen wrote: > LGTM, did this comparison introduce any non-determinism? Potentially, but I observed the issue using libc++'s debug mode. I'm unsure if the current inputs would have caused libc++ to read pa

[PATCH] D78323: [clang] Fix invalid comparator in tablegen

EricWF closed this revision. EricWF added a comment. af2968e37f4c95846ffe287b64a4fcd72c765bee Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78323/new/ https://reviews.llvm.org/D

[PATCH] D40259: [libcxx] LWG2993: reference_wrapper

EricWF requested changes to this revision. EricWF added inline comments. This revision now requires changes to proceed. Comment at: include/__functional_base:396 +!is_same<__uncvref_t<_Up>, reference_wrapper>::value +>::type, bool _IsNothrow = noexcept(__bind(_VSTD::d

[PATCH] D40415: [libcxx] Define istream_iterator equality comparison operators out-of-line

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM. Thanks. https://reviews.llvm.org/D40415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listi

[PATCH] D41148: [libcxx] implement declarations based on P0214R7.

EricWF added inline comments. Comment at: libcxx/include/experimental/simd:1 +#ifndef _LIBCPP_EXPERIMENTAL_SIMD +#define _LIBCPP_EXPERIMENTAL_SIMD Please add the license header similar to `experimental/filesystem`. Comment at: libcxx/include/ex

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

EricWF created this revision. EricWF added reviewers: mclow.lists, ldionne. Herald added subscribers: libcxx-commits, dexonsmith, christof. This is needed anytime we need to clamp an arbitrary floating point value to an integer type. Repository: rCXX libc++ https://reviews.llvm.org/D66836 F

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

EricWF marked 10 inline comments as done. EricWF added inline comments. Comment at: include/math.h:1556 +_LIBCPP_BEGIN_NAMESPACE_STD +template Seems odd this is the only thing in this file inside the standard namespace. > Are we moving towards writing `std::__helper` instead

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

EricWF updated this revision to Diff 217700. EricWF added a comment. Address review comments. I think this is good to go. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66836/new/ https://reviews.llvm.org/D66836 Files: include/math.h test/libcxx/numerics/clamp_to_integral.pass.cpp

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

EricWF marked an inline comment as done. EricWF added inline comments. Comment at: include/math.h:1573 + enum { _Bits = numeric_limits<_IntT>::digits - numeric_limits<_FloatT>::digits }; + static const _IntT value = numeric_limits<_IntT>::max() >> _Bits << _Bits; +}; -

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

EricWF updated this revision to Diff 218149. EricWF marked an inline comment as done. EricWF added a comment. Address review comments. - Document that NaN isn't a supported input. - Fix spelling mistake. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66836/new/ https://reviews.llvm.org/

[PATCH] D35297: [Sema] Fix operator lookup to consider local extern declarations.

EricWF added a comment. @rsmith Ping. This should be super simple to review. https://reviews.llvm.org/D35297 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35297: [Sema] Fix operator lookup to consider local extern declarations.

EricWF updated this revision to Diff 108845. EricWF added a comment. - Address issues in inline comments. The patch should be ready to go now :-) https://reviews.llvm.org/D35297 Files: lib/Sema/SemaLookup.cpp test/SemaCXX/overloaded-operator.cpp Index: test/SemaCXX/overloaded-operator.cpp

[PATCH] D35297: [Sema] Fix operator lookup to consider local extern declarations.

EricWF added a comment. @rsmith Should this be considered for the `5.0` release? https://reviews.llvm.org/D35297 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36070: [coroutines] Evaluate the operand of void `co_return` expressions.

EricWF created this revision. Previously Clang incorrectly ignored the expression of a void `co_return`. This patch addresses that bug. I'm not quite sure if I got the code-gen right, but this patch is at least a start. https://reviews.llvm.org/D36070 Files: lib/CodeGen/CGCoroutine.cpp t

[PATCH] D18174: Fix libcxx build on musl

EricWF requested changes to this revision. EricWF added a comment. This revision now requires changes to proceed. This patch is not OK, since it affects way more that just MUSL libc. Also can you please provide a reduced reproducer as two why `pthread_mutex_t mut = PTHREAD_MUTEX_INITIALIZER` is

[PATCH] D36070: [coroutines] Evaluate the operand of void `co_return` expressions.

EricWF added inline comments. Comment at: test/CodeGenCoroutines/coro-ret-void.cpp:30 +coro1 f2() { + co_return(void) A{}; +} rsmith wrote: > This seems like an extremely confusing way to format this statement. Maybe > move the space left a bit? Urg. `git clang

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

EricWF requested changes to this revision. EricWF added a comment. This revision now requires changes to proceed. Hmm. So the documentation is wrong, but not "fully wrong". If you simply try applying this patch and compiling with GCC you'll still encounter a bunch of instances of `warning: type

[PATCH] D35863: Use the allocator's pointers for deallocation in `std::list`

EricWF requested changes to this revision. EricWF added a comment. This revision now requires changes to proceed. First, this patch would need tests before continuing, Could you please them? The list tests live under `test/std/containers/sequences/list`. Let me know if you have any questions or

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF created this revision. This patch implements the source location builtins `__builtin_LINE(), `__builtin_FUNCTION()`, `__builtin_FILE()` and `__builtin_COLUMN()`. These builtins are needed to implement [`std::experimental::source_location`](https://rawgit.com/cplusplus/fundamentals-ts/v2/

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF updated this revision to Diff 112268. EricWF marked 4 inline comments as done. EricWF added a comment. - Address inline comments. - Fix calls to class call operators. https://reviews.llvm.org/D37035 Files: docs/LanguageExtensions.rst include/clang/AST/Decl.h include/clang/AST/Expr.

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF marked an inline comment as done. EricWF added a comment. In https://reviews.llvm.org/D37035#849574, @rsmith wrote: > I don't like the model of conditionally rebuilding the default initializer / > default argument if it contains one of these builtins; it seems more > heavy-handed than ne

[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

EricWF added a comment. Due to some complexities libc++ has dealing with the allocator, as well some optimizations it would be a serious undertaking to restructure libc++ to properly construct the types in the hash containers, This change would be greatly simpler and much appreciated. Reposito

[PATCH] D61364: [libcxx] [test] Fix incorrect allocator::construct assertions in associative container tests

EricWF commandeered this revision. EricWF edited reviewers, added: BillyONeal; removed: EricWF. EricWF added a comment. Herald added a project: All. Fixing this requires a bunch of container overall in the containers that I don't have the time for right now. So I'm going to go ahead and close thi

[PATCH] D69062: Resolve LWG issue 2426

EricWF commandeered this revision. EricWF edited reviewers, added: zoecarver; removed: EricWF. EricWF added a comment. Herald added a reviewer: libc++. Herald added a project: All. In D69062#1711610 , @ldionne wrote: > If we want to mark the LWG issue as b

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

EricWF added inline comments. Comment at: libcxx/include/new:229-237 +#if !defined(_LIBCPP_CXX03_LANG) +using __libcpp_max_align_t = max_align_t; +#else +union __libcpp_max_align_t { + void * __f1; + long long int __f2; + long double __f3; rsmith wrote: > Is t

[PATCH] D74871: Fix interaction between -fdiscard-value-names and LLVM Bitcode

EricWF added a comment. This seems wrong to me. Discarding value names is the default and has been forever in non-assert builds. Why is this not a bug in w/e is handling the IR? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74871/new/ https://revi

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

EricWF added a subscriber: ldionne. EricWF added a comment. @ldionne Since this has the possibility of breaking existing users of `std::max_align_t`, can you test this against your c++03 codebases? @joerg I still don't understand why you need this change. Why can't we provide the name in C++03

[PATCH] D67052: Add reference type transformation builtins

EricWF added a comment. LGTM once all of the inline comments are addressed. After that, this is ready to land. Comment at: clang/lib/AST/ItaniumMangle.cpp:3412 break; + case UnaryTransformType::RemoveReferenceType: +Out << "3err"; zoecarv

[PATCH] D41223: [libc++] Fix PR35491 - std::array of zero-size doesn't work with non-default constructible types.

EricWF added a comment. In D41223#1899394 , @jtbandes wrote: > The lack of `_LIBCPP_CONSTEXPR_AFTER_CXX14` on the `array` > specialization's `begin()`, `end()`, and other methods seems to be a bug: > https://stackoverflow.com/questions/60462569/empty-std

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

EricWF requested changes to this revision. EricWF added a comment. This revision now requires changes to proceed. I've already stated my disapproval of this patch. Libc++ has never and will never provide nor value C++03 conformance. Moving backwards to C++03 is inconsistent with the libraries gen

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

EricWF added a comment. I'll be picking this up seriously this week. I'm currently getting an internal version of this clang-tidy reviewed, and my plan was to submit it upstream after because I didn't see this patch until now (bad email filters). I think we can unify the two checks, Thanks for w

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

EricWF requested changes to this revision. EricWF added a comment. This revision now requires changes to proceed. - This check should suggest fixes. It knows what function is actually being resolved, and it has enough information to create a minimally qualified name that resolves it. - This ch

[PATCH] D67588: Add builtin trait for add/remove cv (and similar)

EricWF added a comment. LGTM other than the inline comments. I'll take a second pass at this once they're addressed. Let's land the patch this week! Comment at: clang/include/clang/Sema/DeclSpec.h:419 static bool isTypeRep(TST T) { return (T == TST_typename || T == TST

[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM. The weird template test case might is derived from an old failure that I believe was addressed in Clang, but regression tests are fine by me. The macro expansion fix is correct and corre

[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

EricWF closed this revision. EricWF added a comment. Committed in 020ed6713d889a95f8c98d7725c87b458d99f6b3 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72484/new/ https://reviews.llvm.org/D72484

[PATCH] D73245: Don't define std::max_align_t if not used in C++03 mode

EricWF requested changes to this revision. EricWF added a comment. This revision now requires changes to proceed. the library has never had a C++03 mode, and I am against adding one now. I want libc++ to move away from C++03, not towards it CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D43047: [Builtins] Overload __builtin_operator_new/delete to allow forwarding to usual allocation/deallocation functions.

EricWF updated this revision to Diff 139347. EricWF marked an inline comment as done. EricWF added a comment. - Merge with upstream. - Add requested assertion. https://reviews.llvm.org/D43047 Files: include/clang/Basic/Builtins.def include/clang/Basic/DiagnosticSemaKinds.td include/clang/

[PATCH] D43047: [Builtins] Overload __builtin_operator_new/delete to allow forwarding to usual allocation/deallocation functions.

EricWF added inline comments. Comment at: lib/Sema/SemaExprCXX.cpp:3458 + } + TheCall->getCallee()->setType(OperatorNewOrDelete->getType()); + rsmith wrote: > It would be nice to assert that the callee you're setting the type of is an > ImplicitCastExpr doing

[PATCH] D43047: [Builtins] Overload __builtin_operator_new/delete to allow forwarding to usual allocation/deallocation functions.

This revision was automatically updated to reflect the committed changes. Closed by commit rC328134: [Builtins] Overload __builtin_operator_new/delete to allow forwarding to usual… (authored by EricWF, committed by ). Repository: rC Clang https://reviews.llvm.org/D43047 Files: include/clang

[PATCH] D43047: [Builtins] Overload __builtin_operator_new/delete to allow forwarding to usual allocation/deallocation functions.

This revision was automatically updated to reflect the committed changes. Closed by commit rL328134: [Builtins] Overload __builtin_operator_new/delete to allow forwarding to usual… (authored by EricWF, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://rev

[PATCH] D40218: [Clang] Add __builtin_launder

EricWF marked an inline comment as done. EricWF added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:1947-1948 +const auto *Record = ArgTy->getAsCXXRecordDecl(); +if (CGM.getCodeGenOpts().StrictVTablePointers && Record && +Record->isDynamicClass()) +

[PATCH] D40218: [Clang] Add __builtin_launder

EricWF marked 2 inline comments as done. EricWF added inline comments. Comment at: test/CodeGenCXX/builtin-launder.cpp:93-96 +/// The test cases in this namespace technically need to be laundered according +/// to the language in the standard (ie they have const or reference sub

[PATCH] D40218: [Clang] Add __builtin_launder

EricWF updated this revision to Diff 139363. EricWF marked an inline comment as done. EricWF added a comment. - Launder types with subobjects of dynamic class type. - Improve diagnostic selection using `llvm::Optional`. - Add comment about LTO ABI concerns to test file. - Merge with master. http

[PATCH] D44915: [coroutines] Fix invalid source range in co_await call expressions.

EricWF created this revision. EricWF added reviewers: GorNishanov, rsmith, vsk, aaron.ballman. Herald added a subscriber: modocache. Currently an invalid source range is generated for the member call expressions of `co_await`. The end location of the call expression is the `co_await` token loc,

[PATCH] D44865: [libc++] Implement P0608R1 - A sane variant converting constructor

EricWF added a comment. Has this paper been adopted into the standard yet? Comment at: include/variant:1097 +{ +static void test(); +}; I see no need to stop using `operator()` in favor of a static function. Is there a reason? Also this would have to be `

[PATCH] D44865: [libc++] Implement P0608R1 - A sane variant converting constructor

EricWF added inline comments. Comment at: include/variant:1109 +#define _LIBCPP_VARIANT_BOOLEAN_CONVERSION(bool_type)\ +template\ EricWF wrote: > I would like to see a version of this patch tha

[PATCH] D44915: [coroutines] Fix invalid source range in co_await call expressions.

This revision was automatically updated to reflect the committed changes. Closed by commit rC328606: [coroutines] Fix invalid source range in co_await call expressions. (authored by EricWF, committed by ). Repository: rC Clang https://reviews.llvm.org/D44915 Files: lib/Sema/SemaCoroutine.cp

[PATCH] D44865: [libc++] Implement P0608R1 - A sane variant converting constructor

EricWF added inline comments. Comment at: include/variant:1109 +#define _LIBCPP_VARIANT_BOOLEAN_CONVERSION(bool_type)\ +template\ lichray wrote: > EricWF wrote: > > EricWF wrote: > > > I would

[PATCH] D44865: [libc++] Implement P0608R1 - A sane variant converting constructor

EricWF added inline comments. Comment at: include/variant:1109 +#define _LIBCPP_VARIANT_BOOLEAN_CONVERSION(bool_type)\ +template\ lichray wrote: > EricWF wrote: > > lichray wrote: > > > EricWF

[PATCH] D45013: Generate warning when over-aligned new call is required but not supported.

EricWF created this revision. EricWF added reviewers: vsapsai, mclow.lists. Herald added a subscriber: christof. Currently libc++ silently ignores over-aligned allocation requests made through __libcpp_allocate when aligned new/delete is not available. This patch uses the `diagnose_if` at

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

EricWF created this revision. EricWF added reviewers: rsmith, vsapsai, erik.pilkington, ahatanak. Libc++ needs to know when aligned allocation is supported by clang, but is otherwise unavailable at link time. This patch adds a predefined macro to allow libc++ to do that. IDK if using a predefin

[PATCH] D44865: [libc++] Implement P0608R1 - A sane variant converting constructor

EricWF added a comment. This LGTM. However, we should add tests for `const bool` and `volatile bool` before committing. Also I would like @mclow.lists input about applying this DR early since LWG hasn't commented on it yet. Repository: rCXX libc++ https://reviews.llvm.org/D44865 ___

[PATCH] D45128: [libcxx][test] Silence -Wself-assign diagnostics

EricWF added inline comments. Comment at: test/std/language.support/support.types/byteops/and.assign.pass.cpp:30 -static_assert(noexcept(b &= b), "" ); +static_assert(noexcept(b &= (std::byte &)b), "" ); Should Clang really be warning when the expres

[PATCH] D45131: [AST] Refactor UnaryTransformType into TransformTraitType supporting non-unary transforms

EricWF created this revision. EricWF added reviewers: rsmith, aaron.ballman, majnemer. Herald added a subscriber: klimek. This patch refactors `UnaryTransformType` into `TransformTraitType` which accepts an arbitrary number of input arguments, which is needed to support upcoming traits such as `

[PATCH] D45013: Generate warning when over-aligned new call is required but not supported.

EricWF added a comment. In https://reviews.llvm.org/D45013#1052600, @vsapsai wrote: > Does it make sense to have a warning for `__libcpp_deallocate` and > `get_temporary_buffer` too? Not sure about deallocate as you have allocated > memory already and allocation has a warning. So for deallocati

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

EricWF added a comment. One thing I would like to see in this patch is ensuring the warning doesn't get generated when the expression appears in unevaluated contexts, such as `decltype` and `noexcept`. The warning is fundamentally about dataflow, but this doesn't apply to unevaluated expression

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

EricWF added a comment. In https://reviews.llvm.org/D44883#1055018, @Quuxplusone wrote: > @EricWF, is it important IYO that this warning not trigger in unevaluated > contexts even for non-dependently-typed variables? > This is the case that seems to be coming up in practice in libc++ tests, but

[PATCH] D44773: [CMake] Use custom command and target to install libc++ headers

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. This LGTM modulo requested changes. There's a section in `NOTES.TXT` about the steps required for adding a header. Please update that to mention listing it in CMake. Commen

[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17

EricWF added a comment. I'm also in favor of making this opt-out instead of opt-in, except for a handful of functions like `unique_ptr::release` which may yield false positives. However, for functions where discarding the return value is always a bug, I don't see an issue in libc++ emitting a w

[PATCH] D45128: [libcxx][test] Silence -Wself-assign diagnostics

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM. Perhaps we could add a comment for the first such occurrence of the cast in each file explaining it. Repository: rCXX libc++ https://reviews.llvm.org/D45128 __

[PATCH] D40218: [Clang] Add __builtin_launder

EricWF added a comment. Ping. https://reviews.llvm.org/D40218 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

EricWF added a comment. Ping. I would like to get a way for libc++ to detect this case before the next release. Repository: rC Clang https://reviews.llvm.org/D45015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D44411: [libc++] Fix Container::insert(value_type const&) tests

EricWF added a comment. Have you verified that we're not losing test coverage here? That is, are you sure we still have tests for the rvalue overloads in other test files? Comment at: libcxx/test/std/containers/associative/multiset/insert_cv.pass.cpp:41 +const VT v3(3); +

[PATCH] D32411: [libcxx] Provide #include_next alternative for MSVC

EricWF added a comment. Herald added a subscriber: christof. I spoke to STL on the MSVC team a while ago, and he stated that if we produced a paper describing why we need `#include_next` and the rational behind it, and they would pass that on to the front-end team. They didn't guarantee that the

[PATCH] D45121: [coroutines] Add noop_coroutine to

EricWF added inline comments. Comment at: include/experimental/coroutine:262 +struct noop_coroutine_promise {}; + This whole thing should be wrapped in a `__has_builtin(__builtin_coro_noop)` so the header still compiles with older clang versions. ===

[PATCH] D43509: [libcxx][cmake] Remove libatomic temporarily from CMAKE_REQUIRED_LIBRARIES when configuring

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM. I would modify the comment to mention that LLVM's configuration may already be adding `libatomic` Repository: rCXX libc++ https://reviews.llvm.org/D43509

[PATCH] D45121: [coroutines] Add noop_coroutine to

EricWF added inline comments. Comment at: include/experimental/coroutine:288 + +coroutine_handle() { + this->__handle_ = __builtin_coro_noop(); Can `__builtin_coro_noop` produce a constant expression? https://reviews.llvm.org/D45121

[PATCH] D45121: [coroutines] Add noop_coroutine to

EricWF added inline comments. Comment at: include/experimental/coroutine:294 + +inline _LIBCPP_ALWAYS_INLINE +noop_coroutine_handle noop_coroutine() _NOEXCEPT { GorNishanov wrote: > lewissbaker wrote: > > EricWF wrote: > > > This should just be `_LIBCPP_INLINE_VI

[PATCH] D45121: [coroutines] Add noop_coroutine to

EricWF added inline comments. Comment at: include/experimental/coroutine:294 + +inline _LIBCPP_ALWAYS_INLINE +noop_coroutine_handle noop_coroutine() _NOEXCEPT { GorNishanov wrote: > EricWF wrote: > > GorNishanov wrote: > > > lewissbaker wrote: > > > > EricWF wrot

[PATCH] D45121: [coroutines] Add noop_coroutine to

EricWF accepted this revision. EricWF added a comment. Yeah, LGTM. https://reviews.llvm.org/D45121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45131: [AST] Refactor UnaryTransformType into TransformTraitType supporting non-unary transforms

EricWF marked 10 inline comments as done. EricWF added inline comments. Comment at: include/clang/AST/Type.h:3990 /// The untransformed type. - QualType BaseType; + SmallVector ArgTypes; rsmith wrote: > You can't store types with non-trivial destructors on

[PATCH] D45131: [AST] Refactor UnaryTransformType into TransformTraitType supporting non-unary transforms

EricWF marked 4 inline comments as done. EricWF added inline comments. Comment at: lib/Parse/ParseDeclCXX.cpp:1077-1087 + auto DiagSelect = [&]() -> Optional { +if (Info.MinArity && Info.MinArity == Info.MaxArity && +Info.MinArity != Args.size()) + return DiagIn

<    1   2   3   4   5   6   7   8   9   10   >