1

I'm using a third-party class with (only) constructor as follows

class foo  // cannot be altered
{
  public:
    explicit foo(std::istream&);
  ...
};

and the documentation of which suggests the following approach

std::ifstream from("file.txt");
foo obj(from);
from.close();

I cannot alter foo and want to use as member of another class

class bar
{
     foo obj;                           // must not be altered
   public:
     explicit
     bar(std::string const&filename)    // must not be altered
       : obj(std::ifstream(filename))   // error: no matching constructor
   {}
   ...
};

except that this does not work, since the temporary std::ifstream created from the filename is not guaranteed to live long enough to construct the foo obj and hence cannot be converted to a std::istream& (the situation would be different, if foo::foo() accepted a const std::istream&).

So my question: can I make the constructor of bar to work without changing the design of bar (for example, bar::bar() to take a std::istream&, or bar to hold a std::unique_ptr<foo> instead of a foo or by adding data members to bar)?

Walter
  • 44,150
  • 20
  • 113
  • 196
  • 2
    Have `bar` take a `std::istream&` and pass that to `obj`? – NathanOliver Aug 20 '20 at 13:07
  • @NathanOliver that would alter the design of `bar` in an unwanted way – Walter Aug 20 '20 at 13:08
  • If `foo::foo` accepted a `const istream&`, you would have undefined behaviour instead. Not an improvement, in my opinion. – molbdnilo Aug 20 '20 at 13:09
  • @molbdnilo Can you elaborate on that? I thought that this is well defined behaviour: a temporary will live as long as const refs to it, no? – Walter Aug 20 '20 at 13:10
  • "... without changing the design of `bar`" do you mean " ... without changing the design of `foo`" ? – 463035818_is_not_an_ai Aug 20 '20 at 13:10
  • apparently not, then I dont understand the question :/ – 463035818_is_not_an_ai Aug 20 '20 at 13:11
  • @idclev463035818 I meant what I said: no change to `foo` at all, no change to the basic design of `bar`: same constructor interface, same members. – Walter Aug 20 '20 at 13:11
  • If you don't want `bar` to hold onto the ifstream object, there you have to take it in in the constructor. `foo` requires an lvalue and the only way to create one on the fly is to use `new` but if you don't hold onto the pointer then you have a memory leak. – NathanOliver Aug 20 '20 at 13:12
  • does wrapping `bar` in a `moo` count as "changing the design" ? – 463035818_is_not_an_ai Aug 20 '20 at 13:12
  • @Walter They ;ive as long as the first binding, which is the constructor parameter. If `foo` doesn't store the stream, but only uses it for construction, it would be fine - *if* you could read from a const stream, but you can't. – molbdnilo Aug 20 '20 at 13:13
  • @NathanOliver What if I have an object that keeps a `mutable std::ifstream` and allows `const` type conversion to an `std::ifstream&` -- would that be okay? – Walter Aug 20 '20 at 13:14
  • 1
    @Walter Where are you going to keep that? At that point you should just have a `ifstream` member in `bar`. – NathanOliver Aug 20 '20 at 13:15
  • @NathanOliver keep it nowhere, but use a temporary of that object type and pass it to the constructor of `foo` -- at least the clang compiler gives no errors anymore. – Walter Aug 20 '20 at 13:17
  • 1
    @Walter That doesn't work. Temporary lifetime extension only works with function local references. Passing a temporary to a constructor where the object holds onto a reference to it does not extend the lifetime and will leave you with a dangling reference. – NathanOliver Aug 20 '20 at 13:19

2 Answers2

2

Your design constraints are impossible to satisfy. The safest one to relax is to hold onto a std::ifstream in bar.

class bar
{
     std::ifstream objs_stream;         // must be declared before obj
     foo obj;                           // must not be altered
   public:
     explicit
     bar(std::string const&filename)    // must not be altered
       : objs_stream(filename), obj(objs_stream)
   {}
   ...
};

Another option would be to submit a patch to the third party class:

class foo
{
  public:
    explicit foo(std::istream&);
    explicit foo(std::istream&& is) : foo(is) {}
  ...
};

If foo has a copy or move constructor, you could

foo make_foo(const std::string& filename)
{
    std::ifstream is(filename);
    return foo(is);
}

class bar
{
     foo obj;                           // must not be altered
   public:
     explicit
     bar(std::string const&filename)    // must not be altered
       : obj(make_foo(filename))
   {}
   ...
};
Caleth
  • 52,200
  • 2
  • 44
  • 75
0

I came up with the following possibility (which compiles w/o errors).

class bar
{
    foo obj;                           // must not be altered
    struct tmp {
       mutable std::ifstream s;
       tmp(std::string const&f) : s(f) {}
       operator std::istream&() const { return s; }
    };
  public:
    explicit
    bar(std::string const&filename)    // must not be altered
      : obj(tmp(filename))
  {}
  ...
};

Is this okay? I.e. will tmp::s live until the construction of obj returns (which according to the documentation of foo, see edited question, is sufficient)?

Walter
  • 44,150
  • 20
  • 113
  • 196
  • 1
    This will leave you with a dangling reference and `foo`'s use of the stream will have undefined behavior. The compiler is not required to diagnose this. – NathanOliver Aug 20 '20 at 13:23
  • @NathanOliver can you elaborate on this? Note that `obj` does not hold onto the reference/pointer to its constructor's argument (see edited question). – Walter Aug 20 '20 at 13:24
  • 1
    As soon as `obj(tmp(filename))` finishes, `s` from `tmp` is destroyed which means that the stream reference `obj` has is referring to an object that no longer exists. If `obj` needs to use the stream after it's constructed, then that uses will use a dangling reference which is UB. If `obj` only use the stream in the constructor then you're safe, but that requires everyone working on this to know that, so it imposes documentation requirements. – NathanOliver Aug 20 '20 at 13:28
  • @NathanOliver *If obj needs to use the stream after it's constructed* it doesn't. I had hoped that was made clear in the question. I know that the design of `foo` is poor, but I cannot do anything about that. – Walter Aug 20 '20 at 13:31