Possible issues with deduction guides from p1518r2

[jbcoe]

Does https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1518r2.html somehow also apply to indirect?

I.e. should it be:

- constexpr indirect(allocator_arg_t, const Allocator& a, const indirect& other);
+ constexpr indirect(allocator_arg_t, const type_identity_t<Allocator>& a, const indirect& other);

- constexpr indirect(allocator_arg_t, const Allocator& a, indirect&& other);
+ constexpr indirect(allocator_arg_t, const type_identity_t<Allocator>& a, indirect&& other);

I am not sure, but I got reminded of P1518 and at least wanted to mention it.

Possibly related: microsoft/STL#4060

We're currently consistent with tuple: https://eel.is/c++draft/tuple.tuple which might be the right way to be.

Taken from conversation with @Ukilele on another issue #497

[Ukilele]

There is a difference between std::tuple and std::indirect:
For std::tuple, the template parameter Alloc is a parameter of the constructor. For std::indirect, the template parameter Allocator is a parameter of the class. For std::tuple the templat parameter Alloc must be deduced from an argument that a user passes to the constructor call. But for std::indirect the Allocator type can/will be deduced from the const indirect& other (the last parameter of the constructor of std::indirect).

So the scenario would be the following, taken from p1518:

https://value_type.compiler-explorer.com/z/cPWzTGb87

using xyz::indirect;
using PA = std::pmr::polymorphic_allocator<int>;

int main()
{
    std::pmr::monotonic_buffer_resource mr;
    std::pmr::polymorphic_allocator<int> a = &mr;
    indirect<int, PA> pi(std::allocator_arg, a);

    auto i1 = indirect<int, PA>(pi);
    auto i2 = indirect<int, PA>(std::allocator_arg, a, pi);
    auto i3 = indirect<int, PA>(std::allocator_arg, &mr, pi);

    auto di1 = indirect(pi);
    auto di2 = indirect(std::allocator_arg, a, pi);
    auto di3 = indirect(std::allocator_arg, &mr, pi); //Error here
}

The line auto di3 = indirect(std::allocator_arg, &mr, pi) fails with an error about failing class template argument deduction.
And I think we would want that line to work and compile and deduce di3 to be indirect<int, PA>. Hence, I tend to think that we would want to adopt the changes from P1518. and apply the diff using type_identity_t.

[Quuxplusone]

(Author of P1518 here.) Yep, I agree with @Ukilele's latest comment:

    auto pi = indirect<int, PA>();
    auto di1 = indirect(pi);  // OK
    auto di2 = indirect(std::allocator_arg, a, pi);  // OK
    auto di3 = indirect(std::allocator_arg, &mr, pi);  // error, but should be OK

So basically P3019r11's signatures

constexpr indirect(const indirect& other);
constexpr indirect(allocator_arg_t, const Allocator& a, const indirect& other);
constexpr indirect(indirect&& other) noexcept;
constexpr indirect(allocator_arg_t, const Allocator& a, indirect&& other) noexcept(see below);

should be amended to

constexpr indirect(const indirect& other);
constexpr indirect(allocator_arg_t, const type_identity_t<Allocator>& a, const indirect& other);
constexpr indirect(indirect&& other) noexcept;
constexpr indirect(allocator_arg_t, const type_identity_t<Allocator>& a, indirect&& other) noexcept(see below);

and likewise for polymorphic.

N.B.: All the other constructors, e.g.

template <class U=T>
 explicit constexpr indirect(allocator_arg_t, const Allocator& a, U&& u);

should stay as they are (unless I'm brain-farting here). For example, the ctor above will be used for indirect(allocator_arg, pa, 42), which will deduce U=int but be unable to deduce T, so CTAD will fail — we never get around to caring what Allocator might be deduced as. ...Well, actually, you have a deduction guide for that: indirect(allocator_arg, pa, 42) will use the deduction guide and deduce indirect<int, decltype(pa)>. Even so, we can't handle indirect(allocator_arg, &mr, 42) in any event. (Because &mr is of type memory_resource*, which is not an allocator type, and there are an infinity of candidate allocator types it might enjoy converting to. Without an existing indirect<T,Allocator> object in the argument list, we can't guess what Allocator ought to be.)

[jbcoe]

Thanks @Quuxplusone @Ukilele

This looks like an unambiguous problem with a clear solution.

Can this solution be applied later without it being a breaking change? If not I’ll need to contact LWG and LEWG to broaden the scope of our discussion for Hagenburg and update our draft. If the fix can be applied later, perhaps the two of you would like to write a short paper and present it? I don’t want to deny others the opportunity to share their work and take credit.

[Ukilele]

Regarding Arthur's comment:

and likewise for polymorphic.

Yes, indeed for polymorphic a similar change should be applied to these two constructors:

constexpr polymorphic(allocator_arg_t, const Allocator& a, const polymorphic& other);
constexpr polymorphic(allocator_arg_t, const Allocator& a, polymorphic&& other) noexcept(see below);

Answering your comment, Jon:

Can this solution be applied later without it being a breaking change?

Yes. Similarly as p1518 was retrospectively applied to all the containers in the STL. All it does is: Make CTAD, which does not compile today, compile.

I don't think it's an important feature and would prefer to see indirect & polymorphic rather sooner without this specific CTAD than later. I don't mind if anybody else writes the paper. Otherwise, I would pick it up (alone or in a joint effort). I will not make it to any of the ISO meetings in 2025 so the time frame would be slightly unclear.

[Quuxplusone]

I've emailed the LWG reflector just now. My experience in LWG leads me to believe that all you have to do is issue a P3019R13 with this tiny patch; I don't think that will cause it to miss plenary even in Hagenberg, let alone miss the C++26 train. All you have to do is say "oops, yeah, consistency with CTAD was my intent, here's the two-line patch" and you'll be fine.

(Preferably, say it by replying in that LWG reflector thread.)

[jbcoe]

While we can hold off changes to the draft while we wait for a reply from the relevant C++ Standards Committee folk, is anyone inclined to make additions/changes for type deduction to the reference implementation and tests? We can hide them behind a macro if needed.

Update: I've tried and failed (so far) to add the extra deduction guides for indirect. When defining:

template <typename Alloc, typename Value>
indirect(std::allocator_arg_t, Alloc, Value) -> 
  indirect<Value, typename std::allocator_traits<Alloc>::template rebind_alloc<Value>>;

template <typename Alloc, typename Value>
indirect(std::allocator_arg_t, std::type_identity_t<Alloc>, const indirect<Value, Alloc>&) ->
  indirect<Value, Alloc>;

The first deduction guide is always preferred.

This issue looks potentially related: llvm/llvm-project#57646.

This stack overflow post also looks related https://stackoverflow.com/questions/77258993/can-a-user-defined-deduction-guide-be-constrained-by-a-trailing-requires-clause

[Ukilele]

I've implemented it for indirect and think, I did it right: Ukilele@4b61112

Good that you found: llvm/llvm-project#57646. I circumvented the issue by using std::enable_if here.

I guess one outcome/finding of this is, that we have to constraint this deduction guide

template <class Allocator, class Value>
indirect(allocator_arg_t, Allocator, Value) -> indirect<Value,
         typename allocator_traits<Allocator>::template rebind_alloc<Value>>;

in such a way, that it only participates in overload resolution if Value is not xyz::indirect. Otherwise, the line:

  xyz::indirect copyDestination(std::allocator_arg, &mr, input_indirect);

would favor that explicit deduction guide and fail at allocator_traits<Allocator>, because Allocator would be e.g. std::pmr::memory_resource* (or std::nullptr_t as in my unit test here);

Note that I have issues with polymorphic, WIP.

[Quuxplusone]

@Ukilele: Hm, I had not considered that aspect. I see now that indirect isn't so much like vector or queue as it is like reverse_iterator or optional — it's simply 100% unsafe to use any of its constructors with CTAD. (See my previous writings such as 1, 2.) So while I still think it's appropriate to add the type_identity_ts as we established originally — because that's consistent with the rest of the STL — I'm suddenly much less interested in trying to make CTAD "work" for indirect and would even be well-disposed toward a proposal that tried to "disable" CTAD for indirect (except that such a proposal would require some very novel technique and thus be inappropriate for P3019).

https://value_type.compiler-explorer.com/z/jYYreqnPh

    auto i = 42;
    auto j = indirect(42);
    auto k = indirect<int, A<int>>(42);

    std::same_as<indirect<int, A<int>>>
    auto di = indirect(std::allocator_arg, a, i);

    std::same_as<indirect<indirect<int>, A<indirect<int>>>> 
    auto dj = indirect(std::allocator_arg, a, j);

    std::same_as<indirect<int, A<int>>> 
    auto dk = indirect(std::allocator_arg, a, k);

[jbcoe]

Thanks @Quuxplusone . I’m not sure that I see why deduction guides for indirect are ‘unsafe’.

Could you give an explicit example? I can’t quite piece together the problem from your references and it would be good to get all the necessary information here in one place.

[Quuxplusone]

Could you give an explicit example?

I don't have any "from the Book" example (like I do for reverse_iterator), because I haven't looked hard at indirect or how we expect it to be used in real life. But a really simple toy example is, like,
https://godbolt.org/z/cqsaTo9j3

template<class T>
void mything(T t) {
  auto copy = xyz::indirect(t); // 0
  assert(*copy == t); // 1
  assert(sizeof(*copy) == sizeof(t)); // 2
}

int main() {
    mything(42);  // 1 OK, 2 OK
    mything(xyz::indirect<int>());  // 1 OK, 2 boom!
}

The issue here is "obvious"; but since it bites only for certain template-type-inputs and since the point of error (line 2) is separated visually from the point of the bug (the CTAD use on line 0), I consider that the best guideline is never to write something like line 0 at all.

[jbcoe]

Thanks @Quuxplusone. I’m afraid it’s still not quite obvious to me.

Are we any worse than optional?

Would you recommend removing CTAD and adding ‘make_indirect’?