[PATCH] D50421: [libcxx] [test] Remove assertion that includes and .

2018-08-07 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: test/std/utilities/template.bitset/includes.pass.cpp:11 +// test that includes , and #include template void test_typedef() {} grammar nit: comma https://reviews.llvm.org/D50421 ___

[PATCH] D41217: [Concepts] Concept Specialization Expressions

2018-08-11 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: include/clang/Sema/Sema.h:7134 + // We are substituting template arguments into a constraint expression. + ConstraintSubstitution } Kind; Missing a trailing comma here. Comment at: t

[PATCH] D47111: : Implement monotonic_buffer_resource.

2018-08-13 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. Long-delayed ping! Repository: rCXX libc++ https://reviews.llvm.org/D47111 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-08-14 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 160642. Quuxplusone marked an inline comment as done. Quuxplusone added a comment. Rebased, and ping! Repository: rC Clang https://reviews.llvm.org/D50119 Files: docs/LanguageExtensions.rst include/clang/AST/DeclCXX.h include/clang/AST/Type.h

[PATCH] D50805: Don't warn on returning the address of a label

2018-08-16 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. > I think that, ultimately, this warning isn't worth the code complexity in > clang to support it. The code complexity is //at most// 11 lines (the 11 lines removed by this patch), which really isn't much. But there's an interface-complexity argument to be made, f

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

2018-08-17 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 161190. Quuxplusone added a comment. A type with no move-constructor, and some non-defaulted copy-constructors (even if it *also* has some defaulted copy-constructors) should not be considered naturally trivially relocatable. The paper already said this,

[PATCH] D41284: [Concepts] Associated constraints infrastructure.

2018-08-17 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/var-template-decl.cpp:10 + +template requires bool(U()) +int B::A = int(U()); For my own edification, could you explain whether, given #define BOOL bool

[PATCH] D41284: [Concepts] Associated constraints infrastructure.

2018-08-18 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/var-template-decl.cpp:10 + +template requires bool(U()) +int B::A = int(U()); saar.raz wrote: > Quuxplusone wrote: > > For my own edification, could you explai

[PATCH] D41284: [Concepts] Associated constraints infrastructure.

2018-08-19 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/var-template-decl.cpp:10 + +template requires bool(U()) +int B::A = int(U()); saar.raz wrote: > saar.raz wrote: > > Rakete wrote: > > > Quuxplusone wrote:

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

2018-08-21 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 161866. Quuxplusone added a comment. Update to match the wording in D1144R0 draft revision 13. There's no longer any need to fail when we see a mutable member; on the other hand, we *always* need to perform the special member lookup to find out if our cl

[PATCH] D51135: Rename has{MutableFields, VolatileMember} to has{Mutable, Volatile}Subobject. NFC.

2018-08-22 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone created this revision. Herald added a subscriber: cfe-commits. Rename `HasMutableFields` to `HasMutableSubobject` for accuracy. This bitflag is set whenever the record type has a mutable subobject, regardless of whether that subobject is a direct member, or a member of some b

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

2018-05-08 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. > Can you post the diagnostic exactly as it appears in the compiler output? I > am surprised that it would appear here. It should appear here only if the > standard library's implicit conversion from std::map<...>::iterator to > std::map<...>::const_iterator were im

[PATCH] D46806: Remove unused code from __functional_base. NFC.

2018-05-13 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone created this revision. Quuxplusone added a reviewer: EricWF. Herald added a subscriber: cfe-commits. `__user_alloc_construct_impl` is used by , but this `__user_alloc_construct` is never used. Repository: rCXX libc++ https://reviews.llvm.org/D46806 Files: include/__functional_

[PATCH] D46807: Rename test_memory_resource.hpp -> test_experimental_memory_resource.hpp. NFC.

2018-05-13 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone created this revision. Quuxplusone added reviewers: EricWF, mclow.lists. Herald added subscribers: cfe-commits, christof. This makes room for a new "test_memory_resource.hpp", very similar to the old one, but testing the C++17 header instead of the experimental header. Repository:

[PATCH] D45766: [Sema] Add -Wno-self-assign-overloaded

2018-05-14 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. In https://reviews.llvm.org/D45766#1097797, @ksu.shadura wrote: > Thank you for the test example! I got your point, but I wanted to ask if it > should be like this for commutative operations? > In our case it is actually matrix, and subtraction of matrices is not >

[PATCH] D46602: [clang-tidy] Store checks profiling info as YAML files

2018-05-16 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: clang-tidy/tool/ClangTidyMain.cpp:188 +format to stderr. When this option is passed, +these per-TU profiles are instead stored as YAML.)"), + cl::value_desc("prefix"), Dri

[PATCH] D47090: Implement C++17 .

2018-05-18 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. > I would prefer if we completed (according to > the current standard, not the LFTS spec), and then moved it. > Would you be willing to do that instead? Let me see if I understand. libc++'s `` differs from C++17 `` in at least these ways: (A) It's missing `monoton

[PATCH] D47090: Implement C++17 .

2018-05-19 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: include/__memory_resource_base:196 + typename __uses_alloc_ctor< + _T1, polymorphic_allocator&, _Args1... + >::type() >> (B) It's got different semantics around uses-allocat

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

2018-05-19 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone created this revision. Quuxplusone added a reviewer: EricWF. Herald added a subscriber: cfe-commits. In the TS, `uses_allocator` construction for `pair` tried to use an allocator type of `memory_resource*`, which is incorrect because `memory_resource*` is not an allocator type. LWG 2

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

2018-05-19 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 147676. Quuxplusone added a comment. Herald added a subscriber: christof. Fix two more tests hiding in test/libcxx/, and give -U999 context. Repository: rCXX libc++ https://reviews.llvm.org/D47109 Files: include/experimental/memory_resource test

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

2018-05-19 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. In https://reviews.llvm.org/D47109#1105680, @EricWF wrote: > This LGTM. Let's land it and see if anybody complains. Sounds good to me. I don't have commit privs so could you land it for me? (Besides, I'm happy for you to get the blame if it *does* break someone's co

[PATCH] D46806: Remove unused code from __functional_base. NFC.

2018-05-19 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 147677. Quuxplusone added a comment. @EricWF: would you mind landing these two drive-by fixes while you're at it? Repository: rCXX libc++ https://reviews.llvm.org/D46806 Files: include/__functional_base include/experimental/memory_resource Inde

[PATCH] D46806: Remove unused code from __functional_base. NFC.

2018-05-19 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: include/experimental/memory_resource:99 // 8.5, memory.resource -class _LIBCPP_TEMPLATE_VIS memory_resource +class _LIBCPP_TYPE_VIS memory_resource { ...although maybe I don't understand the rules here. For example

[PATCH] D47111: Implement monotonic_buffer_resource in

2018-05-19 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone created this revision. Quuxplusone added a reviewer: EricWF. Herald added a subscriber: cfe-commits. (https://reviews.llvm.org/D47090 included this in ``; at Eric's request, I've split this out into its own patch applied to the existing `` instead.) Repository: rCXX libc++ https

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

2018-05-22 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: include/charconv:89 +_LIBCPP_BEGIN_NAMESPACE_STD + +enum class _LIBCPP_ENUM_VIS chars_format lichray wrote: > mclow.lists wrote: > > lichray wrote: > > > EricWF wrote: > > > > We need to hide these names when `_LIBCP

[PATCH] D47067: Update NRVO logic to support early return

2018-05-22 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: lib/Sema/SemaDecl.cpp:12760 // to deduce an implicit return type. - if (FD->getReturnType()->isRecordType() && - (!getLangOpts().CPlusPlus || !FD->isDependentContext())) + if (!FD->getReturnType()->isScalarT

[PATCH] D47067: Update NRVO logic to support early return

2018-05-24 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: lib/Sema/SemaDecl.cpp:12760 // to deduce an implicit return type. - if (FD->getReturnType()->isRecordType() && - (!getLangOpts().CPlusPlus || !FD->isDependentContext())) + if (!FD->getReturnType()->isScalarT

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

2018-05-24 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. > Sounds good to me. I don't have commit privs so could you land it for me? @EricWF ping! Repository: rCXX libc++ https://reviews.llvm.org/D47109 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

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

2018-05-24 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone created this revision. Quuxplusone added a reviewer: EricWF. Herald added subscribers: cfe-commits, JDevlieghere. https://github.com/cplusplus/draft/commit/6216651aada9bc2f9cefe90edbde4ea9e32251ab `new_delete_resource().allocate(n, a)` has basically three permissible results: - Retur

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

2018-05-24 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 148540. Quuxplusone added a comment. Oops. If we do get an underaligned result, let's not leak it! Repository: rCXX libc++ https://reviews.llvm.org/D47344 Files: src/experimental/memory_resource.cpp Index: src/experimental/memory_resource.cpp ===

[PATCH] D47111: : Implement monotonic_buffer_resource.

2018-05-24 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 148542. Quuxplusone retitled this revision from "Implement monotonic_buffer_resource in " to ": Implement monotonic_buffer_resource.". Quuxplusone added 1 blocking reviewer(s): EricWF. Quuxplusone added a comment. Fix one visibility macro. Repository:

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

2018-05-24 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone created this revision. Quuxplusone added a reviewer: EricWF. Herald added a subscriber: cfe-commits. Split out from https://reviews.llvm.org/D47090. This patch is based on top of (depends on) https://reviews.llvm.org/D47111, but I'm posting it now for review. Repository: rCXX libc

[PATCH] D47090: Implement C++17 .

2018-05-24 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone abandoned this revision. Quuxplusone added a comment. I'm re-submitting this as a series of smaller patches that first bring `` up to date with C++17, and then copy it over to ``. In order, these smaller patches are: https://reviews.llvm.org/D46806 https://reviews.llvm.org/D47109 ht

[PATCH] D46807: Rename test_memory_resource.hpp -> test_experimental_memory_resource.hpp. NFC.

2018-05-24 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone abandoned this revision. Quuxplusone added a comment. This is now part of https://reviews.llvm.org/D47360. Repository: rCXX libc++ https://reviews.llvm.org/D46807 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.ll

[PATCH] D47067: Update NRVO logic to support early return

2018-05-25 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. Just commenting to say that this LGTM and I have no further nitpicks. I have verified that I cannot detect any change in the behavior of `-Wpessimizing-move` or `-Wreturn-std-move` due to this change (and I //can// successfully detect the additional copy-elision du

[PATCH] D47111: : Implement monotonic_buffer_resource.

2018-05-28 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: include/experimental/memory_resource:429 +size_t __capacity_; +size_t __alignment_; +size_t __used_; EricWF wrote: > I can't imagine we'll need more than 1 byte to represent the alignment. Even assuming f

[PATCH] D47111: : Implement monotonic_buffer_resource.

2018-05-28 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 148843. Quuxplusone added a comment. Add `override`; disintegrate the unit test; adopt some tests from Eric's https://reviews.llvm.org/D27402. Also fix one QOI issue discovered by Eric's tests: if the user passes an `initial_size` to the constructor, the

[PATCH] D47111: : Implement monotonic_buffer_resource.

2018-05-28 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 148845. Quuxplusone added a comment. 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, `sizeof(monotonic_buffer_

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

2018-05-29 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 149006. Quuxplusone added a comment. Rebase and update the diff. Repository: rCXX libc++ https://reviews.llvm.org/D47358 Files: include/experimental/memory_resource src/experimental/memory_resource.cpp test/std/experimental/memory/memory.resour

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

2018-05-29 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: test/std/experimental/memory/memory.resource.pool/pool_options.pass.cpp:11 +// UNSUPPORTED: c++98, c++03, c++11, c++14 +// UNSUPPORTED: apple-clang-7 + This test comes from Eric's D27402. The default-initialization

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

2017-10-09 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. I'm also much out of my depth here, but I'm skeptical. You're changing the comments in the code from essentially saying "This workaround helps people with broken code" to essentially saying "This indispensable functionality helps people like me who use dlopen()." A

[PATCH] D51200: Introduce per-callsite inline intrinsics

2018-08-24 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8236 +def err_argument_to_inline_intrinsic_builtin_call : Error< + "argument to %0 must not be a builtin call">; +def err_argument_to_inline_intrinsic_pdotr_call : Error< I

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

2018-09-06 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. @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-commits

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

2018-07-21 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 156684. Quuxplusone added a comment. Fix the "zero-byte allocation might be misaligned" issue by just never trying to allocate zero bytes — try to allocate 1 byte instead. Repository: rCXX libc++ https://reviews.llvm.org/D47344 Files: include/expe

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

2018-07-21 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone marked 5 inline comments as done. 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(re

[PATCH] D49771: CodeGen: use non-zero memset when possible for automatic variables

2018-07-24 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: test/CodeGen/init.c:202 + union U { char c; int i; }; + struct S { union U u; int i; }; + struct S arr[5] = { { {0xF0}, 0xF0F0F0F0 }, { {0xF0}, 0xF0F0F0F0 }, { {0xF0}, 0xF0F0F0F0 }, { {0xF0}, 0xF0F0F0F0 }, { {0xF0}, 0xF0F0F0F0 },

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

2018-07-27 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone marked an inline comment as done. Quuxplusone added inline comments. Comment at: include/vector:296 +_LIBCPP_INLINE_VISIBILITY +inline void +__copy_construct_forward(_Alloc& __a, _Iter __begin1, _Iter __end1, ldionne wrote: > Do you really need `inlin

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

2018-07-27 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. @ldionne: I don't know if your "LGTM" is necessarily sufficient to commit this or not; but either way, I don't have commit privs, so could I ask you (or someone else) to commit this on my behalf? Thanks! Repository: rCXX libc++ https://reviews.llvm.org/D49317

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

2018-07-30 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: include/vector:318 +} +} + Marshall writes: > Instead, we should be writing simple loops that the compiler can optimize > into a call to memcpy if it chooses. Having calls to memcpy in the code paths > makes it

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

2018-07-30 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: include/vector:318 +} +} + mclow.lists wrote: > Quuxplusone wrote: > > Marshall writes: > > > Instead, we should be writing simple loops that the compiler can optimize > > > into a call to memcpy if it chooses.

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

2018-07-30 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. @mclow.lists: Well, anyway, is this pure refactoring acceptable, or would you prefer to keep these helpers inside `allocator_traits` until a better solution to constexpr-memcpy can be invented? Repository: rCXX libc++ https://reviews.llvm.org/D49317 _

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

2018-07-30 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. In https://reviews.llvm.org/D49317#1180852, @ldionne wrote: > After thinking about this for some more, I'm not sure this patch is worth > doing in its current form. The minimal patch for allowing specializations of > `allocator_traits` would be: > > 1. move the `__m

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

2018-07-30 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. @lichray: I'm interested in merging this patch into my libc++ fork, but the latest update seems to be missing a ton of files. For example `charconv_test_helpers.h` is included by one of the tests, but does not exist. Also there's a lot of boilerplate missing: you ou

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

2018-07-31 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone created this revision. Quuxplusone added a reviewer: rsmith. Quuxplusone added a project: clang. Herald added a subscriber: cfe-commits. This is the compiler half of C++ proposal 1144 "Object relocation in terms of move plus destroy," as seen on https://godbolt.org/g/zUUAVW and https

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

2018-08-01 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 158613. Quuxplusone added a comment. Address feedback from @Rakete. Fix wrong ordering of 'class|struct|...' in the diagnostic. Add `union` test cases. Correctly drop the attribute whenever it is diagnosed as inapplicable. Repository: rC Clang h

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

2018-08-01 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2942 + "not %select{move-constructible|destructible}2" + >; + > Might be useful to add a note explaining why the type isn't trivially > relocatable instead of the general

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

2018-08-01 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 158615. Quuxplusone added a comment. clang-format. Repository: rC Clang https://reviews.llvm.org/D50119 Files: compiler-explorer-llvm-commit.sh docs/LanguageExtensions.rst include/clang/AST/DeclCXX.h include/clang/Basic/Attr.td include/clan

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

2018-08-02 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 158822. Quuxplusone marked 20 inline comments as done. Quuxplusone added a comment. Further removal of dead code based on @Rakete's feedback. Repository: rC Clang https://reviews.llvm.org/D50119 Files: docs/LanguageExtensions.rst include/clan

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

2018-08-02 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: compiler-explorer-llvm-commit.sh:1 +# This is the commit of LLVM that we're currently based on. +git reset --hard 1fa19f68007cd126a04448093c171f40e556087e Rakete wrote: > What's this file? A mistake? Yeah, it's p

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

2018-08-02 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone marked an inline comment as done. Quuxplusone added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp:6091 + for (auto *F : Record->fields()) { +if (F->isMutable()) { Rakete wrote: > Can you move this in `ActOnFields`? That way we avoid

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

2018-08-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 159073. Quuxplusone marked 8 inline comments as done. Quuxplusone added a comment. Correctly drop the attribute from non-relocatable instantiations of class templates (even though they don't print the diagnostic). Repository: rC Clang https://reviews

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

2018-08-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp:6187 +Record->dropAttr(); + } else if (Record->needsImplicitMoveConstructor() && + Record->defaultedMoveConstructorIsDeleted()) { Rakete wrote: > Rakete w

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

2018-02-14 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone created this revision. Quuxplusone added a project: clang. Herald added a subscriber: cfe-commits. This patch adds two new diagnostics, which are off by default: **-Wreturn-std-move** Diagnose cases of `return x` or `throw x`, where `x` is the name of a local variable or parameter,

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

2018-02-15 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone marked an inline comment as done. Quuxplusone added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:23 +def warn_return_std_move : Warning< + "adding 'std::move' here would select a better conversion sequence">, + InGroup, DefaultIgnore;

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

2018-02-15 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 134514. Quuxplusone added a comment. Reword the diagnostic per rsmith's suggestions. Still TODO: - figure out how to detect whether this is a return-stmt or throw-expression - figure out how to generate the appropriate fixit Repository: rC Clang http

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

2018-02-16 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 134779. Quuxplusone added a comment. Add fixits. Repository: rC Clang https://reviews.llvm.org/D43322 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaExprCXX.cpp

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

2018-02-19 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 134999. Quuxplusone edited the summary of this revision. Repository: rC Clang https://reviews.llvm.org/D43322 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaExprC

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

2018-02-19 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. @rsmith and/or @rtrieu, please take another look? All my TODOs are done now: there are fixits, and the wording of the diagnostic changes if it's a "throw" instead of a "return", and the wording has been updated per Richard Smith's suggestions. I have one very mino

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

2018-02-19 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 135005. Quuxplusone added a comment. Removed a redundant check for LValueReferenceType in the CWG1579 codepath. (In that branch, we know that standard C++ *did* perform the copy-to-move transformation, so obviously we can't have had an lvalue reference t

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

2018-02-22 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 135484. Quuxplusone added a reviewer: rsmith. Quuxplusone added a subscriber: Rakete. Quuxplusone added a comment. Eliminate a couple of `auto` per comment by @Rakete. Repository: rC Clang https://reviews.llvm.org/D43322 Files: include/clan

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

2018-02-23 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. In https://reviews.llvm.org/D43322#1017190, @lebedev.ri wrote: > Somewhat related thing i have noticed: https://godbolt.org/g/ow58JV IIUC, you're asking whether it would be possible to detect instances of return take(mysharedptr); and rewrite them into return

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

2018-02-26 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. @rtrieu please take a look? Repository: rC Clang https://reviews.llvm.org/D43322 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41316: [libcxx] Allow random_device to be built optionally

2017-12-16 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. I keep seeing patches go by for other targets where they're just implementing `random_device` for their target. It doesn't *have* to be based on an actual /dev/urandom. You can see all the other options under #ifdefs in this file: getentropy, /dev/random, nacl_secur

[PATCH] D41316: [libcxx] Allow random_device to be built optionally

2018-01-02 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. @weimingz: Since your platform supports `srand(0)`, is it possible to look at how your platform implements `srand(0)` and "inline" that implementation into `random_device`? That seems like it would be more in keeping with the other ifdefs in this file. I'm confiden

[PATCH] D41740: [clang-tidy] Adding a new bugprone check for streaming objects of type int8_t or uint8_t

2018-01-04 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: clang-tidy/bugprone/StreamInt8Check.cpp:44 +} else if (Name == "int8_t") { +diag(Offender->getLocStart(), "streaming int8_t"); +break; I don't know clang-tidy style either, but might i

[PATCH] D43898: Preliminary refactoring in service of -Wreturn-std-move. NFC.

2018-02-28 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone created this revision. Quuxplusone added a reviewer: rtrieu. Herald added a subscriber: cfe-commits. This patch is extracted from https://reviews.llvm.org/D43322, which adds a new diagnostic `-Wreturn-std-move`. This patch here is just the non-functional parts of that patch. It pulls

[PATCH] D43898: Preliminary refactoring in service of -Wreturn-std-move. NFC.

2018-02-28 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 136380. Quuxplusone added a comment. Add a block comment for function `TryMoveInitialization`. Repository: rC Clang https://reviews.llvm.org/D43898 Files: include/clang/Sema/Sema.h lib/Sema/SemaExprCXX.cpp lib/Sema/SemaStmt.cpp lib/Sema/SemaT

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

2018-02-28 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: lib/Sema/SemaStmt.cpp:2937 +static void AttemptMoveInitialization(Sema& S, + const InitializedEntity &Entity, rtrieu wrote: > I have a few concerns about this function. The cod

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

2018-02-28 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 136383. Quuxplusone added a comment. Rename some functions and parameters. Rebase onto https://reviews.llvm.org/D43898. Repository: rC Clang https://reviews.llvm.org/D43322 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/Diagn

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

2018-02-28 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5639 + "local variable %0 will be copied despite being %select{returned|thrown}1 by name">, + InGroup, DefaultIgnore; +def note_add_std_move : Note< I would like some gui

[PATCH] D43766: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std::make_unique

2018-03-01 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:21 +: MakeSmartPtrCheck(Name, Context, "std::make_unique"), + MinimumLanguageVersion(Options.get("MakeUniqueLanguageVersion", + getDefaultMinim

[PATCH] D43898: Preliminary refactoring in service of -Wreturn-std-move. NFC.

2018-03-02 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. @rtrieu please take a look? Also, I would need someone to commit this for me, as I don't have commit privs. Repository: rC Clang https://reviews.llvm.org/D43898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41316: [libcxx] Allow random_device to be built optionally

2018-03-05 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: test/std/numerics/rand/rand.device/lit.local.cfg:1 +# Disable all of the random device tests if the correct feature is not available. +if 'libcpp-has-no-random-device' in config.available_features: EricWF wrote: > T

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

2018-03-05 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. @rtrieu (and perhaps @rsmith) ping? The action items I need help with are: - Review and land the refactoring patch https://reviews.llvm.org/D43898 (I don't have commit privs) - Ideally, test compiling a bunch of (e.g. Google) code with https://reviews.llvm.org/D433

[PATCH] D44143: Create properly seeded random generator check

2018-03-06 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: docs/clang-tidy/checks/cert-properly-seeded-random-generator.rst:26 +std::random_device dev; +std::mt19937 engine3(dev()); // Good + } Seeding MT19937 with a single 32-bit integer is //not// "Good". It makes

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

2018-03-07 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. Btw, I'm going to be talking about this patch tonight at 7 in Palo Alto. :) https://www.meetup.com/ACCU-Bay-Area/events/248040207/ https://docs.google.com/presentation/d/18ZRnedocXSQKn9Eh67gGv-ignReHfRD7vj_dxrra1kc/ Repository: rC Clang https://reviews.llvm.org/D4

[PATCH] D43898: Preliminary refactoring in service of -Wreturn-std-move. NFC.

2018-03-09 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. @rtrieu ping? Repository: rC Clang https://reviews.llvm.org/D43898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43898: Preliminary refactoring in service of -Wreturn-std-move. NFC.

2018-03-10 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone marked 4 inline comments as done. Quuxplusone added inline comments. Comment at: lib/Sema/SemaStmt.cpp:2970 + FunctionDecl *FD = Step.Function.Function; + if (isa(FD)) { +// C++14 [class.copy]p32: rtrieu wrote: > Use early exit here:

[PATCH] D43898: Preliminary refactoring in service of -Wreturn-std-move. NFC.

2018-03-10 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 137920. Quuxplusone marked an inline comment as done. Quuxplusone added a comment. Addressed @rtrieu's comments. @rtrieu, please take another look at this and https://reviews.llvm.org/D43322? Thanks! Repository: rC Clang https://reviews.llvm.org/D43

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

2018-03-10 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 137921. Quuxplusone added a comment. Rebased on https://reviews.llvm.org/D43898 after addressing @rtrieu's latest comments over there. Repository: rC Clang https://reviews.llvm.org/D43322 Files: include/clang/Basic/DiagnosticGroups.td include/cl

[PATCH] D43898: Preliminary refactoring in service of -Wreturn-std-move. NFC.

2018-03-13 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. @rtrieu thanks! I don't have commit privileges; could I ask you to commit this on my behalf? (Or if not, please say no, and I'll know to go looking for someone else to ask.) Once/if https://reviews.llvm.org/D43898 hits master, I'll rebase https://reviews.llvm.org

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

2018-03-14 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 138457. Quuxplusone added a comment. Boldly add `-Wreturn-std-move` to `-Wmove` (and thus also to `-Wall`). The backward-compatibility-concerned diagnostic, `-Wreturn-std-move-in-c++11`, is *not* appropriate for `-Wmove`; but I believe this main diagnosti

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

2018-03-15 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:388 def PessimizingMove : DiagGroup<"pessimizing-move">; +def ReturnStdMoveInCXX11 : DiagGroup<"return-std-move-in-c++11">; +def ReturnStdMove : DiagGroup<"return-std-move">;

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

2018-03-15 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 138594. Quuxplusone added a comment. Rebase on master, now that the refactoring https://reviews.llvm.org/D43898 has been pushed (thanks @rtrieu!) Repository: rC Clang https://reviews.llvm.org/D43322 Files: include/clang/Basic/DiagnosticGroups.td

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

2018-03-18 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. @rtrieu ping! I've rebased this on the pure refactoring you committed for me last week. Repository: rC Clang https://reviews.llvm.org/D43322 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.o

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

2018-06-02 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 149611. Quuxplusone added a comment. - Split up the unit tests. - Refactor to shrink the memory layout of the resource object itself. Before this patch `sizeof(unsynchronized_pool_resource)==48`. After this patch `sizeof(unsynchronized_pool_resource) ==

[PATCH] D46806: Remove unused code from __functional_base. NFC.

2018-06-05 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 150067. Quuxplusone edited the summary of this revision. Quuxplusone added a comment. Also, `` doesn't need a full definition of `std::tuple`; just the forward declaration in `<__tuple>` will suffice. Repository: rCXX libc++ https://reviews.llvm.org/

[PATCH] D47360: Implement as a copy of .

2018-06-05 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone planned changes to this revision. Quuxplusone added a comment. Once the dependencies (https://reviews.llvm.org/D47111 and https://reviews.llvm.org/D47358) are merged, I need to update the unit tests in this patch to reflect the unit tests that were committed. Right now, the unit tes

[PATCH] D47111: : Implement monotonic_buffer_resource.

2018-06-10 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: include/experimental/memory_resource:489 +memory_resource* __res_; +size_t __next_buffer_size_; +}; I've discovered that Boost.Container does not bother to preserve this state across calls to `release()`. If

[PATCH] D47111: : Implement monotonic_buffer_resource.

2018-06-13 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 151112. Quuxplusone added a comment. Herald added a subscriber: christof. - Shrink monotonic_buffer_resource, and make release() not "leak" allocation size. Repeatedly calling allocate() and release() in a loop should not blow up. I originally thought

  1   2   3   4   5   6   7   >