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

Quuxplusone added a comment. @rsmith ping! @thakis: You said, //"From a user point of view, I have no idea what "CTAD" means, and I sometimes work on a C++ compiler."// Did you mean "I think the command-line option should be named something more verbose than `-W[c++14-compat-]ctad`"? Or "I don

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

Quuxplusone added a comment. @EricWF ping! Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47344/new/ https://reviews.llvm.org/D47344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

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

Quuxplusone added a comment. LGTM. Tiny style suggestions, which I won't mind if you ignore. Comment at: include/clang/AST/Type.h:995 void getAsStringInternal(std::string &Str, - const PrintingPolicy &Policy) const { -return getAsStringInternal(

[PATCH] D55933: [Sema] Do not print default template parameters.

Quuxplusone added inline comments. Comment at: lib/AST/TypePrinter.cpp:173 + // information about whether default template parameters were actually + // written. + switch (QT.getTypePtr()->getTypeClass()) { FWIW, I don't understand this comment's terminology.

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

Quuxplusone added a comment. @rsmith ping! It's been a week since the last ping... Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54565/new/ https://reviews.llvm.org/D54565 ___ cfe-commits mailing list cfe-commits@lis

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

Quuxplusone added a comment. In D54565#1339454 , @thakis wrote: > Since I don't know what it means, I don't know if I want a diagnostic for it > :-) Fair point. ;) But the full text of the diagnostic message does say "class template argument deduction

[PATCH] D55933: [Sema] Do not print default template parameters.

Quuxplusone added inline comments. Comment at: test/SemaCXX/static-assert-cxx17.cpp:109 +template +void foo7() { + static_assert(X()); courbet wrote: > Quuxplusone wrote: > > None of these new test cases actually involve the default template argument. > > > >

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

Quuxplusone added a comment. > - using CTAD on arbitrary class templates that weren't designed for it > creates a source compatibility problem that the class template author has no > control over nor say in > - using CTAD on class templates that were designed for it does not create a > new sour

[PATCH] D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop

Quuxplusone added inline comments. Comment at: include/clang/Sema/Sema.h:5337 ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC, - bool DiscardedValue = false, + bool WarnOnDiscardedValue = false,

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

Quuxplusone added a comment. @EricWF ping! Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47344/new/ https://reviews.llvm.org/D47344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

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

Quuxplusone updated this revision to Diff 179710. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47344/new/ https://reviews.llvm.org/D47344 Files: src/experimental/memory_resource.cpp Index: src/experimental/memory_resource.cpp ==

[PATCH] D47111: : Implement monotonic_buffer_resource.

Quuxplusone updated this revision to Diff 179718. Quuxplusone added a comment. Updated and addressed review comments. `test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp` is now XFAILed on OSX platforms, using code copied-and-p

[PATCH] D47111: : Implement monotonic_buffer_resource.

Quuxplusone added a comment. @EricWF ping! and oops, I never submitted these comments, I guess. Comment at: include/experimental/memory_resource:427 +{ +static const size_t __default_buffer_capacity = 1024; +static const size_t __default_buffer_alignment = 16; -

[PATCH] D55933: [Sema] Do not print default template parameters.

Quuxplusone added inline comments. Comment at: lib/AST/TypePrinter.cpp:179 +// `X` gets canonicalized to `X`. We disable +// canonicalization so that `printTag()`` can see the template parameters as +// written. Nit: there's an extra ` on this line.

[PATCH] D55933: [Sema] Do not print default template parameters.

Quuxplusone added inline comments. Comment at: lib/AST/TypePrinter.cpp:179 +// `X` gets canonicalized to `X`. We disable +// canonicalization so that `printTag()`` can see the template parameters as +// written. courbet wrote: > Quuxplusone wrote: >

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

Quuxplusone added inline comments. Comment at: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:196 test_ctor_under_alloc(); + test_ctor_with_different_value_type(); } vsapsai wrote: > Quuxplusone wrote: > > I suggest tha

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

Quuxplusone added inline comments. Comment at: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:161 + { +// Though types are different, initialization can be done with `memcpy`. +int32_t array[1] = {-1}; I might add "

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

Quuxplusone added inline comments. Comment at: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:161 + { +// Though types are different, initialization can be done with `memcpy`. +int32_t array[1] = {-1}; vsapsai wrote

[PATCH] D47111: : Implement monotonic_buffer_resource.

Quuxplusone updated this revision to Diff 219756. Quuxplusone added a comment. Herald added a subscriber: dexonsmith. Rebased on master. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47111/new/ https://reviews.llvm.org/D47111 Files: include/experimental/mem

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

Quuxplusone updated this revision to Diff 219758. Quuxplusone added a comment. Rebased on master. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47358/new/ https://reviews.llvm.org/D47358 Files: include/experimental/memory_resource src/experimental/memory_

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

Quuxplusone added a comment. In D65043#1669410 , @sammccall wrote: > (For actual coroutine support, treating `co_return` and `co_yield` like > `return` everywhere might make sense) I agree, with one nit. `co_return` should be treated like `return`, but

[PATCH] D67897: Fix __is_signed builtin

Quuxplusone added a comment. But `std::is_signed_v` needs to yield `false`. Isn't it cleaner to leave the compiler builtin matching the library type-trait, so that the library doesn't have to check for integral types separately? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D67897: Fix __is_signed builtin

Quuxplusone added a comment. In D67897#1678392 , @rsmith wrote: > In D67897#1678388 , @Quuxplusone > wrote: > > > But `std::is_signed_v` needs to yield `false`. > > > It should yield `true`; the spec says "If is_­a

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

Quuxplusone added a comment. Please add test cases showing the intended behavior for - when the copy constructor is explicitly defaulted but the copy assignment operator is {implicitly defaulted, implicitly deleted, user-provided} - when the copy assignment operator is explicitly defaulted but t

[PATCH] D45591: Clean carriage returns from lib/ and include/. NFC.

Quuxplusone created this revision. Herald added subscribers: cfe-commits, krytarowski, emaste. Clean carriage returns from lib/ and include/. NFC. (I have to make this change locally in order for `git diff` to show sane output after I edit a file, so I might as well ask for it to be committed. I

[PATCH] D45601: Warn on bool* to bool conversion

Quuxplusone added inline comments. Comment at: test/CXX/expr/expr.unary/expr.unary.op/p6.cpp:18 // -- pointer, bool b6 = !&b4; // expected-warning{{address of 'b4' will always evaluate to 'true'}} +// expected-warning@-1 {{comparing 'bool *' as a boolean}} T

[PATCH] D45643: [Failing one test] Reword [-Wreturn-type] messages to "non-void x does not return a value"

Quuxplusone added inline comments. Comment at: bindings/python/tests/cindex/test_diagnostics.py:18 self.assertEqual(tu.diagnostics[0].spelling, -'control reaches end of non-void function') +'non-void function does not return a value') --

[PATCH] D44948: Add diagnostic -Waggregate-ctors, "aggregate type has user-declared constructors"

Quuxplusone abandoned this revision. Quuxplusone added a comment. This patch's new home is https://github.com/Quuxplusone/clang/commits/aggregate-ctors — which is where I should have put it to begin with. Repository: rC Clang https://reviews.llvm.org/D44948 __

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

Quuxplusone added a comment. I don't understand the spelling of this option. You spell it `-wtest` (because rjmccall suggested that spelling); but for my money it should be spelled `-Wno-self-assign-nonfield`. Also, if you go this route, you miss the true positive reported by someone on the oth

[PATCH] D45601: Warn on bool* to bool conversion

Quuxplusone added inline comments. Comment at: lib/Sema/SemaChecking.cpp:2623 +QualType QT = Pointer->getType()->getPointeeType(); +if (!QT.isNull() && QT->isBooleanType()) + // Warn on bool* to bool conversion. ef

[PATCH] D45601: Warn on bool* to bool conversion

Quuxplusone added inline comments. Comment at: lib/Sema/SemaChecking.cpp:2623 +QualType QT = Pointer->getType()->getPointeeType(); +if (!QT.isNull() && QT->isBooleanType()) + // Warn on bool* to bool conversion. Qu

[PATCH] D43322: Diagnose cases of "return x" that should be "return std::move(x)" for efficiency

arthur.j.odwyer added a comment. Sorry, I responded via email but I guess Phabricator didn't pick it up for some reason. See below. Repository: rC Clang https://reviews.llvm.org/D43322 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:

[PATCH] D43322: Diagnose cases of "return x" that should be "return std::move(x)" for efficiency

Quuxplusone added a comment. In https://reviews.llvm.org/D43322#1083075, @arthur.j.odwyer wrote: > Sorry, I responded via email but I guess Phabricator didn't pick it up for > some reason. > See below. And then Phab *still* didn't pick up the important part... Okay, I'll try pasting it here.

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

Quuxplusone added a comment. I'm pretty sure that the C++03 standard doesn't permit the implementation to call any `construct` method here, even if it wanted to. And this adds a couple hundred LOC to get that (non-standard) behavior. Is there a specific use-case that you're trying to enable by

[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:1479 +struct __has_construct_missing +: false_type +{ vsapsai wrote: > vsapsai wrote: > > vsapsai wrote: > > > erik.pilkington wrote: > > > > vsapsai wrote: > > > > > erik.pilkington wrote:

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

Quuxplusone added a comment. Herald added a subscriber: ldionne. @EricWF ping! Repository: rCXX libc++ https://reviews.llvm.org/D47344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

[PATCH] D47111: : Implement monotonic_buffer_resource.

Quuxplusone updated this revision to Diff 153881. Repository: rCXX libc++ https://reviews.llvm.org/D47111 Files: include/experimental/memory_resource src/experimental/memory_resource.cpp test/libcxx/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_ge

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

Quuxplusone updated this revision to Diff 153882. Herald added subscribers: ldionne, christof. Repository: rCXX libc++ https://reviews.llvm.org/D47358 Files: include/experimental/memory_resource src/experimental/memory_resource.cpp test/libcxx/experimental/memory/memory.resource.pool/me

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

Quuxplusone added inline comments. Comment at: src/experimental/memory_resource.cpp:29 +static bool is_aligned_to(void *ptr, size_t align) +{ +void *p2 = ptr; EricWF wrote: > Wait, can't this be written `reinterpret_cast(ptr) % align == 0`? Yes on sane platfo

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

Quuxplusone updated this revision to Diff 154041. Quuxplusone added a comment. Updated to no longer check "size != 0". Also rolled in some drive-by cosmetic refactoring that I'd done later in my branch: these functions aren't in a public header and don't need to be uglified. Repository: rCXX

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

Quuxplusone added inline comments. Comment at: src/experimental/memory_resource.cpp:48 +#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION +if (__size != 0 && !is_aligned_to(result, __align)) { +_VSTD::__libcpp_deallocate(result, __align); Quuxplusone w

[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:1479 +struct __has_construct_missing +: false_type +{ vsapsai wrote: > Quuxplusone wrote: > > vsapsai wrote: > > > vsapsai wrote: > > > > vsapsai wrote: > > > > > erik.pilkington wrote: > >

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

Quuxplusone added inline comments. Comment at: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp:60 + + void construct(pointer p, const value_type& val) + { Per my comments on D48342, I think it would be fun to add a te

[PATCH] D47111: : Implement monotonic_buffer_resource.

Quuxplusone marked 7 inline comments as done. Quuxplusone added a comment. > 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 should be in the same f

[PATCH] D47111: : Implement monotonic_buffer_resource.

Quuxplusone updated this revision to Diff 154050. Quuxplusone marked 2 inline comments as done. Quuxplusone added a comment. Massive changes based on Eric's feedback. Moved constructors and destructors of `monotonic_buffer_resource` into the header so they can be completely inlined. Renamed `str

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

Quuxplusone added inline comments. Comment at: libcxx/include/memory:1470 +decltype(_VSTD::declval<_Alloc>().construct(_VSTD::declval<_Pointer>(), +_VSTD::declval<_Args>())), +void I think you sh

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

Quuxplusone added inline comments. Comment at: libcxx/include/memory:1470 +decltype(_VSTD::declval<_Alloc>().construct(_VSTD::declval<_Pointer>(), +_VSTD::declval<_Args>())), +void vsapsai wrote:

[PATCH] D47111: : Implement monotonic_buffer_resource.

Quuxplusone updated this revision to Diff 154486. Quuxplusone added a comment. Oops, my previous diff had a stray `)` in it, somehow. Repository: rCXX libc++ https://reviews.llvm.org/D47111 Files: include/experimental/memory_resource src/experimental/memory_resource.cpp test/libcxx/ex

[PATCH] D49112: [Sema] Implement -Wmemset-transposed-args

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:662 +def note_suspicious_sizeof_memset_silence : Note< + "%select{parenthesize the third argument|cast the second argument to 'int'}0 to silence">; + If it were my c

[PATCH] D49114: Add a clang-tidy check for "magic numbers"

Quuxplusone added a comment. The cult of "no magic numbers" is horrible and we should be trying to //deprogram// its adherents, not create a whole new generation of them. I would be happy if this clang-tidy patch were quickly abandoned. //But//, it's just a clang-tidy check — it's easy for peop

[PATCH] D49120: [libc++] P0898R3 2 of 12: Implement header

Quuxplusone added inline comments. Comment at: include/concepts:175 +template +_LIBCPP_CONCEPT_DECL Same = __same_impl<_Tp, _Up> && __same_impl<_Up, _Tp>; + Peanut gallery asks: From lines 166-171 it looks awfully like `__same_impl<_Tp, _Up>` is true if and onl

[PATCH] D49120: [libc++] P0898R3 2 of 12: Implement header

Quuxplusone added inline comments. Comment at: include/concepts:175 +template +_LIBCPP_CONCEPT_DECL Same = __same_impl<_Tp, _Up> && __same_impl<_Up, _Tp>; + CaseyCarter wrote: > Quuxplusone wrote: > > Peanut gallery asks: From lines 166-171 it looks awfully like

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

Quuxplusone added a comment. In https://reviews.llvm.org/D48753#1157695, @EricWF wrote: > 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. I think it's being proposed under "quality of i

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

Quuxplusone added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:11 + + double circleArea = 3.1415926535 * radius * radius; + 0x8000- wrote: > Eugene.Zelenko wrote: > > JonasToth wrote: > > > This example is good, but righ

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

Quuxplusone added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:11 + + double circleArea = 3.1415926535 * radius * radius; + 0x8000- wrote: > Quuxplusone wrote: > > 0x8000- wrote: > > > Eugene.Zelenko wrote: > > > > J

[PATCH] D49112: [Sema] Implement -Wmemset-transposed-args

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:662 +def note_suspicious_sizeof_memset_silence : Note< + "%select{parenthesize the third argument|cast the second argument to 'int'}0 to silence">; + aaron.ballman w

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

Quuxplusone created this revision. Quuxplusone added reviewers: EricWF, mclow.lists, erik.pilkington, vsapsai. Herald added subscribers: cfe-commits, ldionne, christof. Inspired by Volodymyr's work on https://reviews.llvm.org/D48753, I've taken it upon myself to refactor the static member functio

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

Quuxplusone updated this revision to Diff 155473. Quuxplusone added a comment. Move the functions from `` to ``, since that's their only caller. Uniform treatment of the pointer/iterator parameters; discover that the difference between "copy_forward" and "copy_range_forward" was that the former

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

Quuxplusone added inline comments. Comment at: include/charconv:244 +static _LIBCPP_INLINE_VISIBILITY char const* +read(char const* __p, char const* __ep, type& __a, type& __b) +{ mclow.lists wrote: > lichray wrote: > > mclow.lists wrote: > > > Same c

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

Quuxplusone updated this revision to Diff 155791. Quuxplusone added a comment. Implement similar cosmetic cleanup to https://reviews.llvm.org/D47111, but for the pool resources this time. I think the argument for keeping do_allocate and do_deallocate in the .cpp file is stronger for these guys t

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

Quuxplusone updated this revision to Diff 155796. Quuxplusone marked 4 inline comments as done. Quuxplusone added a comment. Address @vsapsai's review comments. Repository: rCXX libc++ https://reviews.llvm.org/D49317 Files: include/memory include/vector test/libcxx/containers/sequences

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

Quuxplusone added inline comments. Comment at: include/vector:298 +__copy_construct_forward(_Alloc& __a, _Iter __begin1, _Iter __end1, + _Ptr& __begin2, _CopyViaMemcpy) +{ vsapsai wrote: > Why does this function use `_CopyViaMemcpy` and no

[PATCH] D49112: [Sema] Implement -Wmemset-transposed-args

Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:7984 + if (isa(ThirdArg) && + cast(ThirdArg)->getValue() == 0) { +WarningKind = 0; > Suppress the diagnostic in cases like `memset(ptr, 0xff, PADDING)`, when > `PADDING` is

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

Quuxplusone added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6425 + "available on %2 %3 or newer">; def note_silence_unligned_allocation_unavailable : Note< "if you supply your own aligned allocation functions, use " I observe th

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

Quuxplusone added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.h:52 + + const std::vector IngnoredValuesInput; + std::vector IngnoredValues; `Ignored`. Please spellcheck throughout. Comment at: docs/clang-tidy/checks/

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

Quuxplusone updated this revision to Diff 156210. Quuxplusone edited the summary of this revision. Quuxplusone added a comment. Remove some incorrect `noexcept` from ``. I noticed this because the calls to `__clang_call_terminate` showed up on Godbolt. But the pessimization is still there even w

[PATCH] D68681: [libc++][test] Miscellaneous MSVC cleanups

Quuxplusone added inline comments. Comment at: test/std/containers/unord/unord.map/unord.map.cnstr/deduct.pass.cpp:70 +#pragma warning(disable: 4244) // '%s': conversion from '%s' to '%s', possible loss of data +#endif // TEST_COMPILER_C1XX + Alternatively, if

[PATCH] D68681: [libc++][test] Miscellaneous MSVC cleanups

Quuxplusone added inline comments. Comment at: test/std/containers/unord/unord.map/unord.map.cnstr/deduct.pass.cpp:70 +#pragma warning(disable: 4244) // '%s': conversion from '%s' to '%s', possible loss of data +#endif // TEST_COMPILER_C1XX + CaseyCarter wrote:

[PATCH] D51741: [coro]Pass rvalue reference for named local variable to return_value

Quuxplusone added a comment. In D51741#1701757 , @aaronpuchert wrote: > This change breaks the following code that worked before: > > task f(MoveOnly &value) { > co_return value; > } > This patch is heavily heavily merge-conflicted by P1825

[PATCH] D51741: [coro]Pass rvalue reference for named local variable to return_value

Quuxplusone added inline comments. Comment at: cfe/trunk/lib/Sema/SemaCoroutine.cpp:857-859 // FIXME: If the operand is a reference to a variable that's about to go out // of scope, we should treat the operand as an xvalue for this overload // resolution. ---

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaCoroutine.cpp:869 if (E) { -auto NRVOCandidate = this->getCopyElisionCandidate(E->getType(), E, CES_AsIfByStdMove); -if (NRVOCandidate) { - InitializedEntity Entity = - InitializedEntity::

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

Quuxplusone added a comment. Oh, and can you please make sure there are test cases for all the various cases covered in P1155 ? Specifically, I would expect all three of the following test cases to compile successfully. It l

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

Quuxplusone added a subscriber: lewissbaker. Quuxplusone added a comment. One more test to add: struct Widget { task foo() && { co_return *this; // IIUC this should call return_value(Widget&), not return_value(Widget&&) } }; Comment at: clang/lib/Se

[PATCH] D47111: : Implement monotonic_buffer_resource.

Quuxplusone marked 11 inline comments as done. Quuxplusone added inline comments. Comment at: include/experimental/memory_resource:427 +static _LIBCPP_CONSTEXPR const size_t __default_buffer_capacity = 1024; +static _LIBCPP_CONSTEXPR const size_t __default_buffer_alignmen

[PATCH] D80079: [clang-format] [NFC] isCpp() is inconsistently used to mean both C++ and Objective C, add language specific isXXX() functions

Quuxplusone added inline comments. Comment at: clang/include/clang/Format/Format.h:1632 + bool isCppOnly() const { return Language == LK_Cpp; } + bool isObjectiveC() const { return Language == LK_ObjC; } + bool isCpp() const { return isCppOnly() || isObjectiveC(); } --

[PATCH] D79800: [Sema] Remove default values for arguments prior to a parameter pack if the pack is used

Quuxplusone added inline comments. Comment at: clang/test/CXX/drs/dr7xx.cpp:225 template void f(int i = 0, T ...args) {} void ff() { f(); } Is this even supposed to compile? The only valid specializations of `f` require `T...` to be an empty pack, which viol

[PATCH] D79800: [Sema] Remove default values for arguments prior to a parameter pack if the pack is used

Quuxplusone added inline comments. Comment at: clang/test/CXX/drs/dr7xx.cpp:225 template void f(int i = 0, T ...args) {} void ff() { f(); } rjmccall wrote: > Quuxplusone wrote: > > Is this even supposed to compile? The only valid specializations of `f` > > r

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

Quuxplusone added a comment. For GTest and GMock specifically, it would be a good medium-term idea to try to upstream patches into those libraries. I did eventually have success upstreaming fixes for `-Wdeprecated` into GTest, for example: https://github.com/google/googletest/pull/2815 Disablin

[PATCH] D79714: [Diagnostics] Restore -Wdeprecated warning when user-declared copy assignment operator is defined as deleted (PR45634)

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:158 +def DeprecatedCopy : DiagGroup<"deprecated-copy", [DeprecatedCopyUserProvided]>; +def DeprecatedCopyDtor : DiagGroup<"deprecated-copy-dtor", [DeprecatedCopyDtorUserProvided]>; def

[PATCH] D76572: Replace `T(x)` with `reinterpret_cast(x)` everywhere it means reinterpret_cast. No functional change

Quuxplusone marked an inline comment as done. Quuxplusone added a comment. @DiggerLin @ahatanak @espindola @reames Gentle ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76572/new/ https://reviews.llvm.org/D76572

[PATCH] D76572: Replace `T(x)` with `reinterpret_cast(x)` everywhere it means reinterpret_cast. No functional change

Quuxplusone added subscribers: kparzysz, Bigcheese. Quuxplusone added a comment. Ah. Then I'm not sure who else to ping, or even which pieces remain in need of review. CODE_OWNERS.TXT isn't being very helpful here... @kparzysz for HexagonCommonGEP.cpp ("Hexagon backend")? @Bigcheese for Binary.h

[PATCH] D77598: Integral template argument suffix and cast printing

Quuxplusone added inline comments. Comment at: clang/test/SemaTemplate/matrix-type.cpp:77 void use_matrix_3(matrix &m) {} -// expected-note@-1 {{candidate template ignored: deduced type 'matrix<[...], 5UL - 2, 5UL>' of 1st parameter does not match adjusted type 'matrix<[...], 5

[PATCH] D82728: [clang] Add -Wsuggest-override

Quuxplusone added a comment. In D82728#2133720 , @dblaikie wrote: > (the presence of at least one "override" being a signal the user intended to > use override and missed some [...] I'm in favor of `-Wsuggest-override`, and would definitely use it if it

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

Quuxplusone added a comment. FWIW, I think the example you gave is **correct** for GCC to warn on. You said: class Base { virtual void foo(); // to be overridden void foo(int); // implemented in terms of foo() }; foo(int) is hidden in derived classes. So if someone derives from Base

[PATCH] D81920: [clangd] Change FSProvider::getFileSystem to take CurrentWorkingDirectory

Quuxplusone added inline comments. Comment at: clang-tools-extra/clangd/support/FSProvider.h:37 + /// This is an overload instead of an optional to make implicit string -> + /// StringRef conversion possible. + virtual llvm::IntrusiveRefCntPtr Re how to fix th

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

Quuxplusone added a comment. In D82617#2117441 , @sammccall wrote: > In D82617#2117086 , @Quuxplusone > wrote: > > > FWIW, I think the example you gave is **correct** for GCC to warn on. > > > Everything the warnin

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

Quuxplusone added a comment. > what would be really useful is to clarify your position on whether the > warning in clang is delivering value I have no special information on that. Clearly someone thought it was adding value when they added it (way back in 2009; https://github.com/llvm/llvm-pro

[PATCH] D82736: [clangd] Rename FS.view(None) to FS.viewWithDefaultCWD()

Quuxplusone created this revision. Quuxplusone added reviewers: sammccall, kadircet, dblaikie. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Fixes an instance of `-Woverloaded-virtual` on GCC. Clarifies the purpose of t

[PATCH] D82736: [clangd] Rename FS.view(None) to FS.viewWithDefaultCWD()

Quuxplusone added a comment. In D82736#2119262 , @sammccall wrote: > `viewWithDefaultCWD` suggests to me the default has some semantics which > don't exist, if using this API "shape" I'd substitute `Arbitrary` here I'm naturally 100% fine with that. I c

[PATCH] D82736: [clangd] Rename FS.view(None) to FS.viewWithDefaultCWD()

Quuxplusone abandoned this revision. Quuxplusone added a comment. Superseded by D82793 . :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82736/new/ https://reviews.llvm.org/D82736 ___

[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

Quuxplusone added inline comments. Comment at: test/clang-tidy/bugprone-map-subscript-operator-lookup.cpp:48 + int getter() const; + void setter(int); +}; Besides mgehre's request for `auto&` test cases, I'd like to see some test cases involving these member f

[PATCH] D78511: [Driver][doc] Document option -mtune as a no-op. NFC.

Quuxplusone added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:2738 +Accepts any value for compatability reasons with GCC, thus not performing any CPU type specific tuning. + "Accepts any value, for compatibility with GCC. Does not per

[PATCH] D78785: Fix x86/x86_64 calling convention for _ExtInt

Quuxplusone added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:2804 +return; + } + Asking silly questions because I don't stand a chance of understanding either the code or the tests... In the x86-64 ABI, an ordinary 16-byte `struct` would

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

Quuxplusone added inline comments. Comment at: llvm/include/llvm/Support/BinaryStreamRef.h:124 - bool operator==(const RefType &Other) const { -if (BorrowedImpl != Other.BorrowedImpl) + friend bool operator==(const RefType &Self, const RefType &Other) { +if (Self.Borr

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

Quuxplusone added inline comments. Comment at: clang/docs/DiagnosticsReference.rst:2853 +-Wcompare-no-parentheses + s/-no-/-op-/ Comment at: clang/docs/DiagnosticsReference.rst:9885 -Also controls `-Wbitwise-

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

Quuxplusone added a comment. Btw, this is an awesome patch! I'm looking forward to getting to use it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85097/new/ https://reviews.llvm.org/D85097 ___ cfe-comm

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:14010 + << Bop->getSourceRange() << OpLoc; + SuggestParentheses(Self, Bop->getOperatorLoc(), + Self.PDiag(diag::note_precedence_silence) vabridgers wrote: > vabri

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:14047 +static bool isComparisonOpSamePrecedence(BinaryOperatorKind Opc) { + switch (Opc) { Same precedence as what? I think this should just be called `isRelationalOperator`, and I woul

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6139 +def note_compare_seperate_sides : Note< + "seperate the expression into two clauses to give it the intended mathematical meaning">; + Both in the string and in

[PATCH] D85545: [Diagnostics] Diagnose missing comma in string array initialization

Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:6916 +for (unsigned i = 0; i < numConcat; ++i) + if (SL->getStrTokenLoc(i).isMacroID()) { +hasMacro = true; I wonder if perhaps the warning should trigger only

<    1   2   3   4   5   6   7   >