4

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.

Improve this question
3
  • 6
    why don't you make the constructor explicit (to avoid undesirable implicit conversion)? godbolt.org/z/zdY6P831z Commented 1 hour ago
  • st(const st *source) should handle the source == nullptr situation. Either by throwing, or behaving like st() constructor. Commented 25 mins ago
  • 1
    @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. Commented 18 mins ago

2 Answers 2

Reset to default
6

Implicit conversion from pointer to object is indeed recipe for surprise. Its too simple to make a typo and get code that silenty 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.

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

isocpp.github.io/CppCoreGuidelines/…
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.
@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.
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.
-3

Unfortunately the accepted 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 350. I will not go deep on 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_locactuion::current().function()); }; }; //! st= st::~st(){ //release all resources std::println("{:s}", std::source_locactuion::current().function()); }; }; //! ~st()

Not following the rule 350 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 use to make it verbos:

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 senarios.

Improve this answer

6 Comments

if you refer to other answer, please do so correctly. My answer does mention the possibility to provide a copy constructor (among other alternatives).
Ineed I didnt identify the missing copy constructor as the "main problem" but thats not what you write.
Why do they need to provide a copy constructor when the intent is to pass a const reference? There should be no copying here.
What is wrong with the implicitly defined copy constructor and copy assignment operator? How do they differ from your user-provided alternatives?
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.
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.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.