4

I just realized this code compiles fine, but has undefined behavior and (naturally) crashes at runtime:

#include <map>
#include <memory>
#include <utility>

int main(int argc, char *argv[])
{
    std::pair<std::unique_ptr<int>, int> a(&argc, 0);
    std::map<std::unique_ptr<int>, int> m; m.insert(std::make_pair(&argc, argc));
}

This was a bit of a shocker for me because I just naturally assumed the explicit nature of std::unique_ptr<T>'s T* constructor would prevent me from converting a T * to std::unique_ptr<T> by accident, but it appears this safeguard no longer functions inside some wrappers that forward to constructors, like std::pair.

So I guess I'm wondering a few things:

  • Is there a good mitigation for this? Is this a well-known problem?

  • Are these intentional, or should they be seen as defects in the standard?

  • If it's a defect, what is the correction that should be made? Can it be made at the library level, or does it require a language change? (e.g., does it require overloadability by explicit-ness?)

Bear in mind std::unique_ptr<int> is just an example; it's not always obvious that the code is wrong, and it rapidly becomes more and more difficult to anticipate potentially dangerous conversions the more the code is templated, so I'm really wondering how people deal with this.

user541686
  • 205,094
  • 128
  • 528
  • 886
  • I would say it is the drawback of the forwarding constructor, which do explicit construction. – Jarod42 Mar 19 '21 at 11:59
  • one could argue that you are explicitly creating a `std::pair, int>` (not convinced myself) – 463035818_is_not_an_ai Mar 19 '21 at 12:01
  • @Jarod42: Yeah, so I'm wondering if it's just a risk we need to take or if the situation can be improved somehow. Right now I'm almost tempted to `delete` that constructor in my local implementation, but it would break so much code that works fine, and I haven't even tried to check what other types suffer from similar issues... – user541686 Mar 19 '21 at 12:01
  • @largest_prime_is_463035818: I suppose you could *try* to begin to argue that for the first case (I'm not convinced either) but in the second case it sure seems implicit to the user. – user541686 Mar 19 '21 at 12:08

2 Answers2

1

You are calling an explicit constructor of std::pair<std::unique_ptr<int>, int>, it has inherited it's explicitness from std::unique_ptr<int>'s constructor.

It is much the same as std::unique_ptr<int> p(&argc);. You have to be vigilant about it.

Containers own their elements, insert and emplace have to construct values. insert and emplace are not constructors or conversion operators, they can't be marked explicit.

There has to be a balance between "not by accident" and "obtuse to do deliberately". Having forwarding constructors (that are conditionally explicit from C++20) is the choice the committee made.

Caleth
  • 52,200
  • 2
  • 44
  • 75
  • Just a note, "you" in the second line is `std::map`, not me. – user541686 Mar 19 '21 at 12:09
  • @user541686 yes, I've expanded upon why inserting into containers must mean construction – Caleth Mar 19 '21 at 12:18
  • So right now your answer is basically "Yeah it's deliberate, it sucks, just be careful", which I guess is alright, but not particularly enlightening (I basically figured as much). It would be super helpful if you could expand on how to address this in practice, not solely for me but also for future readers, e.g.: What are other instances of this problem do you know of that people should watch out for? (Like the `emplace` you mentioned. What about `tuple`? Any other types/methods? Is the list small enough to enumerate?) Is there a practice to mitigate this, esp. in templates? Any insights? – user541686 Mar 19 '21 at 21:24
  • @user541686 anything that is forwarding arguments to constructors is likely to be like this – Caleth Mar 20 '21 at 10:04
0

Containers’ emplace methods are the most notable example of this; yes, it’s intenational.

alagner
  • 3,448
  • 1
  • 13
  • 25
  • The thing is `emplace` literally means "call the constructor explicitly", so it's *expected* to do this. And so you can know to avoid `emplace` where possible, and to be extra careful when you see `emplace` anywhere. Whereas here the conversions seem completely unexpected and unintentional. – user541686 Mar 19 '21 at 12:07