Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Could you please specify what paper you're implementing in the description?
test/std/containers/associative/set/set.cons/deduct.pass.cpp | ||
---|---|---|
43 ↗ | (On Diff #188674) | clang-format any new files please. |
Rebased on master.
@EricWF ping?
(Is "CTAD for container types" similar to "polymorphic memory resources" in the bucket of "things libc++ doesn't want to implement for engineering reasons," and if so, could you guys go on record with that? It would be less dysfunctional than the current model.)
No, it is not like that.
I've just been quite busy. Please ping this weekly to help it gain attention. I'll try to take a look at it this week.
A couple of minor nits.
Same comment about __identify as in the other review. You need answer it in one place, only.
test/std/containers/associative/multiset/multiset.cons/deduct.pass.cpp | ||
---|---|---|
88 ↗ | (On Diff #195977) | A comment here "braces instead of paren" would be nice. |
104 ↗ | (On Diff #195977) | These should all be const |
Almost there. A couple nits, a few consts, and landing D58587, and we're done.
include/set | ||
---|---|---|
862 ↗ | (On Diff #195977) | Any reason that you're not using __iter_value_type here? (Besides that it is declared in D58587) |
1365 ↗ | (On Diff #195977) | same comment re: __iter_value_type |
test/std/containers/associative/set/set.cons/deduct.pass.cpp | ||
94 ↗ | (On Diff #195977) | Please change to std::allocator<long> When we tighten up the constraints on set, it won't break this test. |
Review comments. (Add "braces instead of parens" comment; add const to int expected_s[] throughout)
Update the tests to no longer test P1518R0 (although it is still supported by the code); this is the same thing I did in my last edit to D58587.
@mclow.lists ping! I think this one is now ready to land, unless I missed something.