[PATCH] D48909: [clang-doc] Update BitcodeReader to use llvm::Error

2018-07-03 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:13 #include "llvm/ADT/Optional.h" #include "llvm/Support/raw_ostream.h" Please include Support/Error.h and utility Comment at: clang-tools-extra

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

2018-07-03 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF requested changes to this revision. EricWF added a comment. This revision now requires changes to proceed. Actually, I spotted a couple of issues. Requesting changes. Comment at: src/experimental/memory_resource.cpp:29 +static bool is_aligned_to(void *ptr, size_t align)

[PATCH] D48908: [clang-doc] Pass over function-internal declarations

2018-07-03 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-doc/Mapper.cpp:32 + // Skip function-internal decls. + if (const auto *F = D->getParentFunctionOrMethod()) +return true; Type is not easily deducible, so please don't use auto. http

r336240 - PR33924: merge local declarations that have linkage of some kind within

2018-07-03 Thread Richard Smith via cfe-commits
Author: rsmith Date: Tue Jul 3 19:25:38 2018 New Revision: 336240 URL: http://llvm.org/viewvc/llvm-project?rev=336240&view=rev Log: PR33924: merge local declarations that have linkage of some kind within merged function definitions; also merge functions with deduced return types. This seems like

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

2018-07-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits
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()"

2018-07-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits
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()"

2018-07-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits
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] D48894: [AST] Rename some Redeclarable functions to reduce confusion

2018-07-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 154044. MaskRay added a comment. Rename `Next` to `Link` as per rsmith Repository: rC Clang https://reviews.llvm.org/D48894 Files: include/clang/AST/Redeclarable.h Index: include/clang/AST/Redeclarable.h ===

[PATCH] D47111: : Implement monotonic_buffer_resource.

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

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

2018-07-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits
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.

2018-07-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits
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.

2018-07-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits
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.

2018-07-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits
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] D47111: : Implement monotonic_buffer_resource.

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

<    1   2