[libcxx-commits] [PATCH] D112510: [libc++] P0433R2: add the remaining deduction guides.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/docs/Status/Cxx17.rst:43
 
-   .. [#note-P0433] P0433: So far, only the ``<string>``, sequence containers, container adaptors and ``<regex>`` portions of P0433 have been implemented.
+   .. [#note-P0433] P0433: The only part not fully implemented is the requirement that certain deduction guides should not participate in overload resolution when given incorrect template arguments.
    .. [#note-P0607] P0607: The parts of P0607 that are not done are the ``<regex>`` bits.
----------------
Quuxplusone wrote:
> I assume you've basically done the work already to figure out //which// guides you're talking about; could you specify that exact list right here? (Or even better, make a second PR that just fixes them ;))
I think @var-const 's intent was to come back and fix them in a future PR. The guides he's talking about here are things like SFINAEing `unordered_map` guides away when the provided iterator is not an input iterator. We have a couple such guides that are not 100% correctly implemented.


================
Comment at: libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp:40-65
+    {
+//  optional(const T&);
+    const int& source = 5;
+    std::optional opt(source);
+    ASSERT_SAME_TYPE(decltype(opt), std::optional<int>);
+    assert(static_cast<bool>(opt));
+    assert(*opt == 5);
----------------
Quuxplusone wrote:
> FWIW, these three new tests strike me as redundant with line 35 (which itself — pre-existing — is already redundant with line 27).
IMO those are all relevant. The `const T&` one makes sure that we handle references correctly, the `T*` one that we handle pointers correctly, and the `T[]` one that arrays will cause the deduction guide to deduce an optional pointer.

Sure, when you know how it's implemented you realize that it's obvious it's going to work, but if you don't know how the deduction guide is implemented, IMO these test cases are useful.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112510/new/

https://reviews.llvm.org/D112510