Unexpected conversion from pointer to object during function call

Question

This piece of code is a synthetic example extracted from a real world app.

This code doesn't make much sense, its only purpose is illustration and everything that is not relevant has been stripped.

#include <iostream>
class st
{
public:
 int m = 1;
 st() {
 std::cout << "st()\n";
 };
 st(const st *source) {
 m = source->m;
 std::cout << "st(const st *source) " << m << "\n";
 };
};
void Foo(const st& bar) {
 std::cout << "Foo" << "\n";
}
int main() {
 st foo;
 const st* p = &foo;
 Foo(p); //(1)
}

Output

st()
st(const st *source) 1
Foo

The problem here is when I call the void Foo(const st& bar) function with a pointer to st as in the line with comment //(1) following happens:

As there is no Foo function that takes a st* as argument, a temporary object is created via the st(const st *) constructor and then a reference to this temporary object is passed to Foo. The code works as expected, but it took me a while to figure out why the st constructor was called.

I just wonder how this could have been prevented. I suppose either:

• void Foo(const st& bar) should rather be void Foo(const st *bar)
• or there shouldn't be a st(const st *source) constructor but rather a st(const st & source) constructor.
• or are there some C++ attributes that may generate warnings in such cases?

I'm using the C++20 standard.

• Oersted: why don't you make the constructor explicit (to avoid undesirable implicit conversion)? godbolt.org/z/zdY6P831z
• Eljay: st(const st *source) should handle the source == nullptr situation. Either by throwing, or behaving like st() constructor.
• Jabberwocky: @Eljay yes, but as I wrote this is minimal code that only illustrates the proble, nothing more. Just edited the question to make that clear.

Answer 1

Implicit conversion from pointer to object is indeed recipe for surprise. It's too simple to make a typo and get code that silently does the wrong thing.

It is generally recommended to declare constructors that participate in conversions as explicit (see eg cppcoreguidelines) to avoid implicit conversions:

 explicit st(const st *source) {
 m = source->m;
 cout << "st(const st *source) " << m << "\n";
 };

Then Foo(p); is an error while you had to write Foo(st{p}); for the explicit conversion.

Live Demo

Alternatively, you can consider to use a named method rather than a constructor for the conversion:

 static st from_pointer(const st *source) {
 // ...
 }

Depends on why you need the conversion and how it is used. If you are looking for a way to keep the conversion implicit but at the same time prevent Foo(p) from compiling, I don't think this can be done.

Addressing your list of alternatives:

void Foo(const st& bar) should rather be void Foo(const st *bar)

I cannot tell. Does Foo allow nullptr or otherwise invalid pointers to be passed? If no, use a reference.

or there shouldn't be a st(const st *source) constructor but rather a st(const st & source) constructor.

That would be a copy constructor. If it makes sense for st to define a copy constructor, go for it. All other code that wants to copy a st will use that same copy constructor. If that helps to remove the converting constructor even better.

or are there some C++ attributes that may generate warnings in such cases?

explicit is not new and it generates an error. I am not aware of (new) attributes that generate a warning.

• Oersted: isocpp.github.io/CppCoreGuidelines/…
• Daniel Langr: Nitpicking: "It is generally recommended to declare converting constructors as explicit". According to the C++23 standard, a constructor cannot be explicit and converting at the same time: timsong-cpp.github.io/cppwp/n4950/class.conv.ctor#1.sentence-2. But it's true that the current draft no longer contains this definition: eel.is/c++draft/class.conv.ctor#1.sentence-1.
• 463035818_is_not_an_ai: @DanielLangr I already stumbled upon the wording. I remember that at some point the meaning of "converting constructor" changed to include also constructors with more than 1 argument. The core guideline on the other hand explicitly talks about 1 argument constructor that one should make explicit, which I suppose makes sense.
• 463035818_is_not_an_ai: hm no. I think also for >1 argument constructors explicit is useful (perhaps a little bit less than for 1 argument constructors, but only just a little) godbolt.org/z/zKWYz7nrn.
• Oersted: I thought that the explicit keyword was recommended for constructors that have exactly one argument without default value (so it can have more than one arguments)
• 463035818_is_not_an_ai: sounds reasonable, now I am not sure if this is what they mean when the guideline says "Single-argument constructors"
• Oersted: not sure either but as far as I remember, it was Scott Meyers recommandation.
• Jabberwocky: I really wonder why this answer has been downvotesd. It was exactly what I was looking for. It answers my question to 100%.
• 463035818_is_not_an_ai: I am in favor of the reasoning outlined here meta.stackoverflow.com/a/285777/4117728. If the downvoter wanted to leave constructive feedback, they'd leave a comment, but they are not obliged to do so.

Answer 2

Unfortunately, this answer missed the main problem. The OP needs a copy constructor:

st::st(st const& src)
: m{src.m} //constructor init list
{ std::println("{:s}", std::source_locactuion::current().function()); };

Note the use of constructor initializer list. It's important for classes with expensive resources or const members.

The other notice is the rule 3/5/0. I will not go deep into that, but since you have a copy constructor, you will need an assignment operator to match, and a destructor:

auto& st::operator=(st const& src){
 m = src.m;
 std::println("{:s}", std::source_location::current().function()); };
}; //! st=
st::~st(){
 //release all resources
 
 std::println("{:s}", std::source_location::current().function()); };
}; //! ~st()

Not following the rule 3/5/0 is going to cost severe resource bugs (use after free, double free, leak) in future.

The constructor in original snippet is never used in C++ programming. In corner cases where it might make sense, a tag can be used to make it verbose:

struct st_tag_t{};
constexpr st_tage_t st_tag;
explicit st::st(st_tag_t, st const*);

This eliminates unintended use of this constructor in all scenarios.

• 463035818_is_not_an_ai: if you refer to other answer, please do so correctly. My answer does mention the possibility to provide a copy constructor (among other alternatives).
• 463035818_is_not_an_ai: Ineed I didnt identify the missing copy constructor as the "main problem" but thats not what you write.
• user229044: Why do they need to provide a copy constructor when the intent is to pass a const reference? There should be no copying here.
• Daniel Langr: What is wrong with the implicitly defined copy constructor and copy assignment operator? How do they differ from your user-provided alternatives?
• user229044: Note that, if I make all the changes you're suggesting (correcting several typos) to the OP's code, their code still does the exact same thing (calls st(st *source)) and ignores all of your additions. If you have a specific fix here, I think you need to provide additional compilable code that demonstrates how your suggestion applies to OP.
• 463035818_is_not_an_ai: in the given example there is no way to have "severe resource bugs". I think I know what you refer to but the rule of 3/5/0 seems to be not relevant here. The rule says that if you write one of the three then probably you need to write the others too and you must consider to do so. I'd assume OP did that. There is no need for a custom destructor nor assignment operator even in presence of your proposed copy constructor, suggesting they are strictly needed for correctness is just wrong.