[PATCH] D47111: : Implement monotonic_buffer_resource.

Quuxplusone added inline comments. Comment at: include/experimental/memory_resource:489 +memory_resource* __res_; +size_t __next_buffer_size_; +}; Quuxplusone wrote: > I've discovered that Boost.Container does not bother to preserve this state > across c

[PATCH] D47358: : Implement {un, }synchronized_pool_resource.

Quuxplusone planned changes to this revision. Quuxplusone added inline comments. Comment at: src/experimental/memory_resource.cpp:240 +bool allocation_contains(const char *p) const { +// TODO: This part technically relies on undefined behavior. +return allocat

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

Quuxplusone added a comment. @EricWF ping; is this a reasonable solution to LWG 2843 on platforms without aligned new and delete? Repository: rCXX libc++ https://reviews.llvm.org/D47344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D47111: : Implement monotonic_buffer_resource.

Quuxplusone updated this revision to Diff 151983. Quuxplusone added a comment. Minor cosmetic changes. Repository: rCXX libc++ https://reviews.llvm.org/D47111 Files: include/experimental/memory_resource src/experimental/memory_resource.cpp test/libcxx/experimental/memory/memory.resour

[PATCH] D47358: : Implement {un, }synchronized_pool_resource.

Quuxplusone updated this revision to Diff 151984. Quuxplusone added a comment. Herald added a subscriber: mgrang. Bugfix and shrink {un,}synchronized_pool_resource. The old implementation was severely broken in two ways: - It accidentally sometimes trusted the user's `bytes` and `align` argument

[PATCH] D56731: Add -Wimplicit-ctad warning to diagnose CTAD on types with no user defined deduction guides.

Quuxplusone added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2129 +def warn_class_template_argument_deduction_no_user_defined_guides : Warning< + "class template argument deduction for %0 that has no user defined deduction guides" >, + InGroup, Def

[PATCH] D56731: Add -Wimplicit-ctad warning to diagnose CTAD on types with no user defined deduction guides.

Quuxplusone added inline comments. Comment at: test/SemaCXX/cxx1z-class-template-argument-deduction.cpp:426 +// expected-warning@+1 {{class template argument deduction for 'NoExplicit' that has no user defined deduction guides}} +NoExplicit ne(42); + EricWF wrot

[PATCH] D56731: Add -Wimplicit-ctad warning to diagnose CTAD on types with no user defined deduction guides.

Quuxplusone added a comment. > Concretely, I think we could warn if, during template instantiation from a > type-dependent initializer, we select the copy deduction candidate and there > was another viable candidate (the `optional o(x)` case, the `tuple > t{args...}` case, etc), maybe under a n

[PATCH] D56731: Add -Wimplicit-ctad warning to diagnose CTAD on types with no user defined deduction guides.

Quuxplusone added a comment. In D56731#1359190 , @rsmith wrote: > we need to keep in mind while making that decision that a warning that is > either off-by-default or turned off by nearly everyone delivers much less > value Agreed. I would expect the D

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

Quuxplusone updated this revision to Diff 173610. Quuxplusone added a comment. Rebased on master. @EricWF @ldionne ping please? Repository: rCXX libc++ https://reviews.llvm.org/D47344 Files: include/experimental/memory_resource src/experimental/memory_resource.cpp Index: src/experimenta

[PATCH] D47111: : Implement monotonic_buffer_resource.

Quuxplusone updated this revision to Diff 173611. Quuxplusone added a comment. Rebased on master. @ericwf @ldionne ping please? Repository: rCXX libc++ https://reviews.llvm.org/D47111 Files: include/experimental/memory_resource src/experimental/memory_resource.cpp test/libcxx/experime

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

Quuxplusone updated this revision to Diff 173612. Repository: rCXX libc++ https://reviews.llvm.org/D47344 Files: include/experimental/memory_resource src/experimental/memory_resource.cpp Index: src/experimental/memory_resource.cpp ==

[PATCH] D47358: : Implement {un, }synchronized_pool_resource.

Quuxplusone updated this revision to Diff 173613. Quuxplusone added a comment. Herald added a subscriber: libcxx-commits. Rebased on master. @ericwf @ldionne ping please? Repository: rCXX libc++ https://reviews.llvm.org/D47358 Files: include/experimental/memory_resource src/experimental/

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone marked an inline comment as done. Quuxplusone added inline comments. Comment at: docs/LanguageExtensions.rst:1093 library. +* ``__is_trivially_relocatable`` (Clang): Determines whether moving an object + of type ``type``, and then destroying the source object, is

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone marked 3 inline comments as done. Quuxplusone added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013)

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013) rjmccall wrote: > Quuxplusone wrote: > > @rjmc

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013) rjmccall wrote: > Quuxplusone wrote: > > rjmcc

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013) rjmccall wrote: > Quuxplusone wrote: > > rjmcc

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013) rjmccall wrote: > Quuxplusone wrote: > > rjmcc

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added inline comments. Comment at: docs/LanguageExtensions.rst:1096 + equivalent to copying the underlying bytes and then dropping the source object + on the floor. * ``__is_destructible`` (MSVC 2013) dblaikie wrote: > Quuxplusone wrote: > > rjmcc

[PATCH] D54565: Introduce `-Wc++14-compat-ctad` as a subgroup of `-Wc++14-compat`

Quuxplusone created this revision. Quuxplusone added a reviewer: rsmith. Herald added a subscriber: cfe-commits. Since I am interested in diagnosing this specific C++17ism (but not necessarily other C++17isms) in our codebase going forward. I //think// I got the naming convention right! Personall

[PATCH] D54565: Introduce `-Wc++14-compat-ctad` as a subgroup of `-Wc++14-compat`

Quuxplusone updated this revision to Diff 174150. Quuxplusone added a comment. Fix naming. (Uploaded an earlier diff by accident.) Repository: rC Clang https://reviews.llvm.org/D54565 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td Index: inclu

[PATCH] D54565: Introduce `-Wc++14-compat-ctad` as a subgroup of `-Wc++14-compat`

Quuxplusone updated this revision to Diff 174152. Quuxplusone added a comment. Third time's the charm! Repository: rC Clang https://reviews.llvm.org/D54565 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td Index: include/clang/Basic/DiagnosticSem

[PATCH] D54565: Introduce `-Wc++14-compat-ctad` as a subgroup of `-Wc++14-compat`

Quuxplusone updated this revision to Diff 174449. Quuxplusone added a comment. Added a test case (partly copied from `test/SemaCXX/cxx14-compat.cpp`). On the last warning in the file, I omitted the sketchy part of the warning, which on Godbolt looks like this: https://godbolt.org/z/CB4z99 :7:

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone updated this revision to Diff 174498. Quuxplusone marked 14 inline comments as done. Quuxplusone edited the summary of this revision. Quuxplusone added a comment. Use `[[clang::trivially_relocatable]]` instead of `[[trivially_relocatable]]`. Canonicalize in `QualType::isTriviallyReloca

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added inline comments. Comment at: include/clang/AST/DeclCXX.h:482 +/// and a defaulted destructor. +unsigned IsNaturallyTriviallyRelocatable : 1; + erichkeane wrote: > Quuxplusone wrote: > > erichkeane wrote: > > > Typically we'd have this ca

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added inline comments. Comment at: lib/Parse/ParseDeclCXX.cpp:3811 case ParsedAttr::AT_CXX11NoReturn: + case ParsedAttr::AT_TriviallyRelocatable: return true; erichkeane wrote: > A note for future reviewers, after the C++11 spelling is remove

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone updated this revision to Diff 174513. Quuxplusone added a comment. Implement `[[clang::maybe_trivially_relocatable]]` along the lines suggested by @rjmccall. The idea is that there are two levels of "opt-in-ness." The first level, `[[clang::maybe_trivially_relocatable]]`, means "I wa

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added a comment. In https://reviews.llvm.org/D50119#1302134, @Quuxplusone wrote: > Now that `[[clang::maybe_trivially_relocatable]]` is implemented, we can see > if it does actually simplify the libc++ patch. I will try to get to that > tonight. Here is `deque` implemented with `[

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added a comment. In https://reviews.llvm.org/D50119#1303577, @rjmccall wrote: > In https://reviews.llvm.org/D50119#1303423, @Quuxplusone wrote: > > > In the `unordered_set [[maybe_trivially_relocatable]]` patch, I must wrap > > the attribute in a macro > > `_LIBCPP_MAYBE_TRIVIALLY_R

[PATCH] D54565: Introduce `-Wc++14-compat-ctad` as a subgroup of `-Wc++14-compat`

Quuxplusone added a comment. @lebedev.ri ping? Repository: rC Clang https://reviews.llvm.org/D54565 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47360: Implement as a copy of .

Quuxplusone added a comment. Herald added subscribers: libcxx-commits, jfb. @ericwf ping? (and https://reviews.llvm.org/D47344 https://reviews.llvm.org/D47111 https://reviews.llvm.org/D47358) Repository: rCXX libc++ https://reviews.llvm.org/D47360 _

[PATCH] D54565: Introduce `-Wc++14-compat-ctad` as a subgroup of `-Wc++14-compat`

Quuxplusone added a comment. In https://reviews.llvm.org/D54565#1304740, @rsmith wrote: > In the past, we've been resistant to adding more fine-grained compat > warnings, because we don't want to encourage subsetting the language (which > sounds like exactly what you're trying to do here). We g

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added a comment. In https://reviews.llvm.org/D50119#1303720, @rjmccall wrote: > In https://reviews.llvm.org/D50119#1303662, @Quuxplusone wrote: > > > >> I still believe it is impossible to implement `std::optional` with only > > >> `[[maybe_trivially_relocatable]]`. > > > > > > This

[PATCH] D47358: : Implement {un, }synchronized_pool_resource.

Quuxplusone updated this revision to Diff 175011. Quuxplusone added a comment. Change `1 <<` to `size_t(1) <<` in one last place. @EricWF ping. Repository: rCXX libc++ https://reviews.llvm.org/D47358 Files: include/experimental/memory_resource src/experimental/memory_resource.cpp test

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

Quuxplusone added inline comments. Comment at: test/SemaCXX/static-assert.cpp:111 +static_assert(std::is_same::value, "message"); // expected-error{{static_assert failed due to requirement 'std::is_same::value' "message"}} +static_assert(std::is_const::value, "message");

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

Quuxplusone added a comment. In D50119#1308869 , @rjmccall wrote: > Hmm. I don't remember what we do in this case for `trivial_abi` (which does > need to address it because it's possible to return types like `DestroyBase`). > It looks like we currently

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

Quuxplusone added inline comments. Comment at: test/SemaCXX/static-assert.cpp:111 +static_assert(std::is_same::value, "message"); // expected-error{{static_assert failed due to requirement 'std::is_same::value' "message"}} +static_assert(std::is_const::value, "message");

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

Quuxplusone added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:3061 +#define TYPE_TRAIT_N(Spelling, Name, Key) RecordName == (#Spelling + 2) || +#include "clang/Basic/TokenKinds.def" +#undef TYPE_TRAIT_1 Why do you bother to check a whitelist of "known"

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

Quuxplusone added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:3064 +// If this is a qualified name, expand the template arguments in nested +// qualifiers. +DR->getQualifier()->print(OS, PrintPolicy, true); I don't understand this code enou

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

Quuxplusone added a comment. Looks fine to me; please don't let //me// block this any further. :) Someone else, e.g. @aaron.ballman, should be the one to accept it, though. Comment at: include/clang/AST/NestedNameSpecifier.h:220 + void print(raw_ostream &OS, const PrintingPol

[PATCH] D55270: [Sema] Further improvements to to static_assert diagnostics.

Quuxplusone added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:3062 +public: + FailedBooleanConditionPrinterHelper(const PrintingPolicy &Policy) + : Policy(Policy) {} `explicit` Comment at: lib/Sema/SemaTemplate.cpp:3065 + + ~F

[PATCH] D54565: Introduce `-Wc++14-compat-ctad` as a subgroup of `-Wc++14-compat`

Quuxplusone updated this revision to Diff 176955. Quuxplusone added a comment. Rename `-Wc++14-compat-ctad` to just `-Wctad`, per @rsmith's comment to "move this out of `-Wc++14-compat`." @rsmith ping? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54565/new/ ht

[PATCH] D55270: [Sema] Further improvements to to static_assert diagnostics.

Quuxplusone added inline comments. Comment at: test/PCH/cxx-static_assert.cpp:17 -// expected-error@12 {{static_assert failed "N is not 2!"}} +// expected-error@12 {{static_assert failed due to requirement '1 == 2' "N is not 2!"}} T<1> t1; // expected-note {{in instantiation

[PATCH] D55270: [Sema] Further improvements to to static_assert diagnostics.

Quuxplusone added a subscriber: saar.raz. Quuxplusone added inline comments. Comment at: test/PCH/cxx-static_assert.cpp:17 -// expected-error@12 {{static_assert failed "N is not 2!"}} +// expected-error@12 {{static_assert failed due to requirement '1 == 2' "N is not 2!"}} T<1

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

Quuxplusone added inline comments. Comment at: libcxx/include/memory:1645 -template +template _LIBCPP_INLINE_VISIBILITY ldionne wrote: > Coming at it from a slightly different angle, I would think this is what we > want: > > ``` > template

[PATCH] D47111: : Implement monotonic_buffer_resource.

Quuxplusone added a comment. @ericwf ping? This still needs someone to land it; I don't have commit privs. Thanks! Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47111/new/ https://reviews.llvm.org/D47111 ___ cfe-

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

Quuxplusone updated this revision to Diff 177510. Quuxplusone edited the summary of this revision. Quuxplusone added a comment. Address review comments. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47344/new/ https://reviews.llvm.org/D47344 Files: src/expe

[PATCH] D47111: : Implement monotonic_buffer_resource.

Quuxplusone updated this revision to Diff 177540. Quuxplusone marked 5 inline comments as done. Quuxplusone added a comment. @ericwf ping! Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47111/new/ https://reviews.llvm.org/D47111 Files: include/experimental/m

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

Quuxplusone added inline comments. Comment at: libcxx/include/memory:1645 -template +template _LIBCPP_INLINE_VISIBILITY ldionne wrote: > Quuxplusone wrote: > > ldionne wrote: > > > Coming at it from a slightly different angle, I would think t

[PATCH] D55552: [Sema] Better static assert diagnostics for expressions involving temporaries.

Quuxplusone added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:3089 + else +OS << "("; + for (CXXTemporaryObjectExpr::arg_iterator Arg = Node->arg_begin(), It might be more maintainer-proof to write this as std::pair Braces; i

[PATCH] D55270: [Sema] Further improvements to to static_assert diagnostics.

Quuxplusone added a comment. @courbet: On the cpplang Slack, Peter Feichtinger mentioned that defaulted template arguments should perhaps be treated differently. Is there any way to suppress the `, std::allocator` part of this diagnostic? https://godbolt.org/z/TM0UHc Before your patches: :1

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

Quuxplusone marked an inline comment as done. Quuxplusone added inline comments. Comment at: src/experimental/memory_resource.cpp:29 -public: -~__new_delete_memory_resource_imp() = default; - EricWF wrote: > Why are you removing this? Just removing unnecessar

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

Quuxplusone updated this revision to Diff 189600. Quuxplusone added a comment. Herald added a subscriber: jdoerfert. - Replace the unnecessary destructor in `class __new_delete_resource_imp`. - Add LWG2843 to the list of completed issues (thanks @zoecarver!) Repository: rCXX libc++ CHANGES SI

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

Quuxplusone updated this revision to Diff 189602. Quuxplusone added a comment. Oops, really replace the destructor this time. (Hadn't `git commit`ed.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47344/new/ https://reviews.llvm.org/D47344 Files: src/experimental/memory_resource.cpp

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

Quuxplusone added a comment. In D47344#1423466 , @EricWF wrote: > @Quuxplusone Since the LLVM license has changed since you created this, I > need you to confirm that you accept LLVM's new license. Yes, I accept LLVM's new license. CHANGES SINCE LAST

[PATCH] D47111: : Implement monotonic_buffer_resource.

Quuxplusone updated this revision to Diff 191814. Quuxplusone added a comment. Herald added a subscriber: jdoerfert. Rebased on master. Added `_NOEXCEPT` to the `resource()` method (this is OK per [res.on.exception.handling]/5). @ckennelly ping! Repository: rCXX libc++ CHANGES SINCE LAST AC

[PATCH] D47358: : Implement {un,}synchronized_pool_resource.

Quuxplusone updated this revision to Diff 191815. Quuxplusone added a comment. Herald added a subscriber: jdoerfert. Rebased. Added `_NOEXCEPT` to `upstream_resource()` and `options()` (this is OK per [res.on.exception.handling]/5). Repository: rCXX libc++ CHANGES SINCE LAST ACTION https:/

[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

Quuxplusone added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:486 +/// diagnostics +ConstArgs ConstantArgs; + Please don't use a typedef for this. Follow the style of the surrounding lines. const APValue *ConstantArgs; (The pointer m

[PATCH] D60570: [Sema] Add more tests for the behavior of argument-dependent name lookup

Quuxplusone added inline comments. Comment at: test/CXX/basic/basic.lookup/basic.lookup.argdep/p2-associated-namespaces-classes.cpp:304 +static_assert(f(g3) == 4, "");// FIXME: Also well-formed from the union rule. + // expec

[PATCH] D60570: [Sema] Add more tests for the behavior of argument-dependent name lookup

Quuxplusone added inline comments. Comment at: test/CXX/basic/basic.lookup/basic.lookup.argdep/p2-associated-namespaces-classes.cpp:77 + + // associated class: itself, lambda + namespace X5 { Do you also have a test somewhere to verify that the parameter and r

[PATCH] D60570: [Sema] Add more tests for the behavior of argument-dependent name lookup

Quuxplusone added a comment. As you're making tests for ADL corner cases, you might also consider testing the interactions between ADL and defaulted function parameters, e.g. https://godbolt.org/z/vHnyFl It looks like everyone (except MSVC) already gets that stuff right (or at least portable-be

[PATCH] D60570: [Sema] Add more tests for the behavior of argument-dependent name lookup

Quuxplusone added inline comments. Comment at: test/CXX/basic/basic.lookup/basic.lookup.argdep/p2-associated-namespaces-classes.cpp:304 +static_assert(f(g3) == 4, "");// FIXME: Also well-formed from the union rule. + // expec

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

Quuxplusone added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:2620 + + bool hasExplicitSpecifer() const { +return ExplicitSpecifier.getInt() != ESF_resolved_false || s/Specifer/Specifier/ (and please `git grep` for other instances of the s

[PATCH] D60570: [Sema] Add more tests for the behavior of argument-dependent name lookup

Quuxplusone added a comment. > @Quuxplusone Do you have other objections apart from the template-id issue ? > If not, since D60573 depends on this patch, > I would like to commit this with a comment explaining the issue instead of > the FIXME. Sure, go for it!

[PATCH] D47358: : Implement {un,}synchronized_pool_resource.

Quuxplusone updated this revision to Diff 196575. Quuxplusone added a comment. Rebased on D47111 . Replaced all instances of `__release` with `__release_ptr`, to match what @EricWF did in https://github.com/llvm-mirror/libcxx/commit/8e365750a0299ee0cbef70a3f3492c

[PATCH] D47358: : Implement {un,}synchronized_pool_resource.

Quuxplusone updated this revision to Diff 196699. Quuxplusone added a comment. Add tests that `LIBCPP_ASSERT_NOEXCEPT` the `options` and `upstream_resource` accessors. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47358/new/ https://reviews.llvm.org/D47358 F

[PATCH] D47109: LWG 2969 "polymorphic_allocator::construct() shouldn't pass resource()"

Quuxplusone added a comment. > We also need to mark this off in the status. I can do that in D58879 > or create a separate patch. Good catch! Either is fine with me but I bet a separate patch would be more appropriate. Repository: rCXX libc++ CHANGES SINCE

[PATCH] D52137: Added warning for unary minus used with unsigned type

Quuxplusone added a comment. In https://reviews.llvm.org/D52137#1236053, @rsmith wrote: > I think we can and should do better about false positives here. If you move > this check to SemaChecking, you can produce the warning in a context where > you know what the final type is -- I don't think t

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

Quuxplusone updated this revision to Diff 168169. Quuxplusone edited the summary of this revision. Quuxplusone added a reviewer: ldionne. Quuxplusone added a comment. Herald added a subscriber: libcxx-commits. Rebased on master. Added @ldionne as reviewer. Repository: rCXX libc++ https://revi

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

Quuxplusone added a comment. Can you give more intuition on why classes with no copy/move operations should be forced non-trivial-abi? Let's take this specific example: struct [[clang::trivial_abi]] lock_guard { mutex *m; explicit lock_guard(mutex *m) : m(m) { m->lock(); } ~

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

Quuxplusone added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2953 + "have bases of non-trivial class types|have virtual bases|" + "have __weak fields under ARC|have fields of non-trivial class types}0">; nit: "of non-trivial class

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

Quuxplusone updated this revision to Diff 185225. Quuxplusone added a comment. Rebased on master. @EricWF ping! It's been quite a few months... Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47344/new/ https://reviews.llvm.org/D47344 Files: src/experimental/

[PATCH] D47111: : Implement monotonic_buffer_resource.

Quuxplusone updated this revision to Diff 185229. Quuxplusone added a subscriber: AlisdairM. Quuxplusone added a comment. Rebased on master. @ericwf (cc @AlisdairM) ping! Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47111/new/ https://reviews.llvm.org/D47111

[PATCH] D47358: : Implement {un, }synchronized_pool_resource.

Quuxplusone updated this revision to Diff 185233. Quuxplusone added a subscriber: AlisdairM. Quuxplusone added a comment. Rebased on master. @EricWF (cc @AlisdairM) ping! Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47358/new/ https://reviews.llvm.org/D47358

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

Quuxplusone added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2953 + "have bases of non-trivial class types|have virtual bases|" + "have __weak fields under ARC|have fields of non-trivial class types}0">; ahatanak wrote: > Quuxplus

[PATCH] D57906: [CTU] Do not allow different CPP dialects in CTU

Quuxplusone added a comment. > Consider this STL code: > > template > struct __alloc_traits > #if __cplusplus >= 201103L > : std::allocator_traits<_Alloc> > #endif > { // ... > }; > > > This class template would create ODR errors during merging the two units, > since in o

[PATCH] D37677: [libc++] implement future synchronization using atomic_flag

Quuxplusone added a comment. TODO.txt says "future should use for synchronization." I would have interpreted this as meaning that the line unsigned __state_; should become std::atomic __state_; and appropriate loads and stores used so that `__is_ready()` can be checked without having to

[PATCH] D37538: [libc++] Remove problematic ADL in container implementations.

Quuxplusone added inline comments. Comment at: include/memory:1349 is_same< -decltype(__has_construct_test(declval<_Alloc>(), - declval<_Pointer>(), - declval<_Args>()...)), +

[PATCH] D37958: [libc++] Correctly propagate user-defined lookup_classname().

Quuxplusone added inline comments. Comment at: libcxx/test/std/re/re.traits/lookup_classname_user_defined.pass.cpp:46 +// matches all characters (they are classified as alnum) +std::wstring re1 = L"([[:alnum:]]+)"; +std::regex_search(in, m, std::wregex(re1)); ---

[PATCH] D37958: [libc++] Correctly propagate user-defined lookup_classname().

Quuxplusone added inline comments. Comment at: libcxx/test/std/re/re.traits/lookup_classname_user_defined.pass.cpp:46 +// matches all characters (they are classified as alnum) +std::wstring re1 = L"([[:alnum:]]+)"; +std::regex_search(in, m, std::wregex(re1)); ---

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

Quuxplusone added a comment. Seems low-value at first glance, but I guess I can't argue with those Twitter and codesearch results. Do you have codesearch results for `^2`? https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=%5E+2&search=Search Seems like a lot of false positives (and a lot o

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

Quuxplusone added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10931 + // Do not diagnose hexadecimal literals + if (ExprStr.find("0x") != llvm::StringRef::npos) +return; Can you use `starts_with` (or the LLVM equivalent) in both of these cases? It'l

[PATCH] D65043: [Format] Add C++20 standard to style options

Quuxplusone added a comment. Peanut gallery says: A priori, I don't see any reason for clang-format's `LanguageStandard` options to diverge from Clang's own `-std=` options. It sounds like currently they're very different, and you're proposing to make them basically the same. I think that's a g

[PATCH] D65043: [Format] Add C++20 standard to style options

Quuxplusone added a comment. In D65043#1599220 , @sammccall wrote: > Clang actually defaults to c++14 since clang 6. (CompilerInvocation.cpp near > `CLANG_DEFAULT_STD_CXX`) Today I learned. Thanks! :) > It's important that clang-format works sensibly w

[PATCH] D65043: [Format] Add C++20 standard to style options

Quuxplusone added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:13815 + verifyFormat("co_yield++ i;"); + verifyFormat("co_yield ++i;", Cpp20); + modocache wrote: > Quuxplusone wrote: > > If you're going to test C++11's behavior here, please

[PATCH] D65043: [Format] Add C++20 standard to style options

Quuxplusone added a comment. FWIW, looks unobjectionable to me, except for some nitpicks on the test cases (which are easy to fix so I'm hoping you just will :)). Comment at: clang/unittests/Format/FormatTest.cpp:13780 + // In C++2a, it is interpreted as a prefix increment on

[PATCH] D66332: [clang-format] Fix the bug that joins template closer and > or >>

Quuxplusone added a comment. Drive-by observation: My experiments with https://zed0.co.uk/clang-format-configurator/ show that there is a similar bug where clang-format accidentally produces `>=` via reformatting of `template` `=0>`, `pi` `=3;`, and so on. Is it possible to fix that bug as par

[PATCH] D66332: [clang-format] Fix the bug that joins template closer and > or >>

Quuxplusone added a comment. In D66332#1633749 , @owenpan wrote: > Do you have `SpaceBeforeAssignmentOperators` off? Yes; I mean, that's what I tested in order to produce the buggy behavior. I don't believe anyone should be using that option in producti

[PATCH] D66384: [clang-format] Fix a bug that joins template closer and =

Quuxplusone added a comment. LGTM too, although I would still question whether `Style.SpaceBeforeAssignmentOperators` is providing anyone any benefit at all. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66384/new/ https://reviews.llvm.org/D66384

[PATCH] D66384: [clang-format] Fix a bug that joins template closer and =

Quuxplusone added a comment. LGTM too, although I would still question whether `Style.SpaceBeforeAssignmentOperators` is providing anyone any benefit at all. Comment at: clang/unittests/Format/FormatTest.cpp:6631 + verifyFormat("a = 1;", Style); + verifyFormat("a >>= 1;", St

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

Quuxplusone added a comment. In D66397#1635796 , @xbolva00 wrote: > Now, Chromium disabled this warning :( until we “fix” it. If Chromium is willing to make build-system changes to add `-Wno-xor-as-pow`, then surely they should also be willing to make t

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

Quuxplusone added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10950 + + // Do not diagnose binary literals. + if (ExprStr.find("0b") != llvm::StringRef::npos) I would rather see this whole section as a three-liner: // Do not diagnose binary, octal,

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

Quuxplusone added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10929 +// Do not diagnose if xor keyword is used. +if (ExprStr.find("xor") != llvm::StringRef::npos) + return; xbolva00 wrote: > jfb wrote: > > Doesn't this match any expression tha

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:223 +def note_constexpr_bit_cast_invalid_type : Note< + "cannot constexpr evaluate a bit_cast with a " + "%select{union|pointer|member pointer|volatile|struct with a reference member

[PATCH] D51262: Implement P0553 and P0556

Quuxplusone added inline comments. Comment at: libcxx/include/bit:199 +!is_same_v, char32_t> + > {}; + Given how heavily the code controlled by this trait depends on `numeric_limits<_Tp>`, would it make sense to add something in here about how that

[PATCH] D51262: Implement P0553 and P0556

Quuxplusone added inline comments. Comment at: libcxx/include/bit:378 + const unsigned __retVal = 1u << (__n + __extra); + return (_Tp) (__retVal >> __extra); +} mclow.lists wrote: > mclow.lists wrote: > > Quuxplusone wrote: > > > Why so complicat

[PATCH] D51262: Implement P0553 and P0556

Quuxplusone added inline comments. Comment at: libcxx/include/bit:240 +else if constexpr (sizeof(_Tp) <= sizeof(unsigned long)) +return __clz(static_cast(__t)) + - (numeric_limits::digits - numeric_limits<_Tp>::digits); mclow.lists wrote

[PATCH] D61366: [libcxx] [test] Don't assert that moved-from containers with non-POCMA allocators are empty.

Quuxplusone added a comment. >> Are you not allowed to move the containers elements in this case? > > Correct. The allocator is not POCMA and not equal, so it's functionally the > same as doing `assign(make_move_iterator(begin()), > make_move_iterator(end()))`. In case the clarification helps

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

Quuxplusone added inline comments. Comment at: libcxx/include/memory:1726 +#else // _LIBCPP_HAS_NO_VARIADICS +template +_LIBCPP_INLINE_VISIBILITY vsapsai wrote: > ldionne wrote: > > Here, > > > > ``` > > template > enable_if<__has_construct::value

<    1   2   3   4   5   6   7   >