C++ Logo

std-proposals

Advanced search

Re: Add a specialized "Extension" concept to the inheritance syntax

From: Mark A. Gibbs <indi.in.the.wired_at_[hidden]>
Date: Fri, 17 May 2019 19:15:57 -0400
> class Base
> {
> public:
>     tail_extensible void setup()
>     {
>         // do some setup
>     }
>
>    head_extensible void cleanup()
>    {
>       // do some cleanup
>    }
> };
>
> class Derived
> {
> public:
>     void setup() extension
>     {
>         // Base::setup() is called automatically
>         // do my own setup
>     }
>
>     void cleanup() extension
>     {
>         // do my own cleanup
>         // Base::cleanup is called automatically
>     }
> };

It seems a little unnecessarily restrictive to specify in the base class
whether derived classes must "head" extend or "tail" extend. I might
want to do some extra work /before/ the base class's set up. For
example, the base class might have some core functionality, and I might
want a class that does some checking before using it... which means I
want to do the checks /before/ Base::setup(). Having that decision
forced by the base class seems to defy the whole point of extensibility.

So perhaps it might be better if it looked something like this:

class Base
{
public:
    extensible void setup()
    {
        // do some setup
    }

   extensible void cleanup()
   {
      // do some cleanup
   }
};

class Derived : public Base
{
public:
    void setup() extension(head)
    {
        // Base::setup() is called automatically
        // do my own setup
    }

    void cleanup() extension(tail)
    {
        // do my own cleanup
        // Base::cleanup is called automatically
    }
};

But in that case, there's no need for a new "extensible" keyword. That's
just "virtual".

class Base
{
public:
    virtual void setup()
    {
        // do some setup
    }

   virtual void cleanup()
   {
      // do some cleanup
   }
};

class Derived : public Base
{
public:
    void setup() extension(head)
    {
        // Base::setup() is called automatically
        // do my own setup
    }

    void cleanup() extension(tail)
    {
        // do my own cleanup
        // Base::cleanup is called automatically
    }
};

But do I really want the "extension" declarations as part of the
/interface/? That doesn't seem kosher. Whether the base class function
is called first, last, in the middle, or at all seems to be an
implementation detail. Thus, the right place to put it seems to be
within the function body:

class Base
{
public:
    virtual void setup()
    {
        // do some setup
    }

   virtual void cleanup()
   {
      // do some cleanup
   }
};

class Derived : public Base
{
public:
    void setup() extension
    {
        do_base_call(s)_here;
        // Base::setup() is called automatically
        // do my own setup
    }

    void cleanup() extension
    {
        // do my own cleanup
        do_base_call(s)_here;
        // Base::cleanup is called automatically
    }
};

Now we no longer need the "extension" keyword, because it's just "override".

What about the "do_base_call(s)_here" thing? Because with what we've
ended up with, it's just another spelling of
"Base::setup()" or "Base::cleanup()" depending on the context.

There /might/ be some additional benefits, like automatically forwarding
function arguments, and maybe you could say all base class functions
with the same signature would be called in forward or reverse standard
order (that is, the order used during construction or destruction,
respectively)... but even those benefits come with a whole new slew of
caveats and difficulties. (For example, if there are multiple functions
to be called, you can't simply use perfect forwarding, because the first
call would move away all the arguments. You could end up with a slew of
invisible, expensive copies. And what happens if something throws
between two base calls? And if there are multiple base classes what
happens if not all the base classes have the required override
signature? What if they have variants that "work" - like
Base1::setup(int) and Base2::setup(long)? And so on and so forth.)

The killer problem with this idea, I think, is this:

struct Base1
{
    virtual void setup();
    virtual void cleanup();
   
    // ...
};

struct Base2
{
    virtual void setup();
    virtual void cleanup();
   
    // ...
};

struct Base3
{
    virtual void setup();
    virtual void cleanup();
   
    // ...
};

struct Derived : Base1, Base2, Base3
{
    void setup() override
    {
        do_base_calls_here;
       
        /* The above is expanded by the compiler to:
        Base1::setup();
        Base2::setup();
        Base3::setup();
        */
       
        // any other set up
    }
   
    // ...
};

What happens if Base2::setup() throws? Base1's setup is already
complete, so we probably need to call Base1::cleanup() or we're going to
get a resource leak of some kind. How can the "do_base_calls_here"
keyword know to do that?

You can get a similar but even more nefarious problem with only two
bases and noexcept functions like this:

struct Base1
{
    virtual void setup(std::string) noexcept;
    virtual void cleanup();
   
    // ...
};

struct Base2
{
    virtual void setup(std::string) noexcept;
    virtual void cleanup();
   
    // ...
};

struct Derived : Base1, Base2
{
    void setup(std::string s) noexcept override
    {
        do_base_calls_here;
       
        /* Expanded to:
        Base1::setup(s);
        Base2::setup(s);
        */
       
        // etc.
    }
};

s is copied twice, and the second copy might throw even though the
actual function calls are all noexcept.

Also, the second copy might not even be necessary. If the programmer
knows s won't be needed any more, they could manually write the second
call as "Base2::setup(std::move(s));". So this construct isn't only
dangerous, it's inefficient.

Consider what you'd have to write today(-ish, using P0052's <scope>) for
the above:

struct Base1
{
    virtual void setup(std::string);
    virtual void cleanup();
};

struct Base2
{
    virtual void setup(std::string);
    virtual void cleanup();
};

struct Derived : Base1, Base2
{
    void setup(std::string s) override
    {
        Base1::setup(s);
        auto f1 = std::make_scope_fail([&]{ Base1::cleanup(); });
        Base2::setup(std::move(s)); // with or without move
        auto f2 = std::make_scope_fail([&]{ Base2::cleanup(); });
       
        // etc.
    }
};

Granted, it's a bit verbose and boilerplate-y, so how would you express
all that with all its attendant complexity in a simpler way?

Even if you try to avoid most of the complications by restricting this
to single inheritance and using /all/ noexcept functions...:

struct base
{
    virtual void setup(std::string) noexcept;
    virtual void cleanup() noexcept;
};

struct derived : base
{
    void setup(std::string s) noexcept override
    {
        extension; // or whatever keyword or syntax
       
        /* Expands to:
        base::setup(s); // s is copied, might throw
        */
       
        // etc., but if there are any errors, is base cleanup done?
    }
};

... you can still end up with a surprise potential throw. Plus you still
need to worry about doing the cleanup.

Even in just the single inheritance case, this is what you'd need:

// Not assuming this is a template, just writing it as one
// to denote unknown argument count and types.
auto derived::setup(Args&&... args) /*noexcept?*/ -> ReturnType
{
    // Arguments have to be copied, moved, or just referenced
    // depending on whether they're needed later:
    auto ret = base::setup(args...);
   
    // Maybe check the return type and if it shows some
    // failure state, exit immediately:
    if (is_failure(ret))
        return ret;
   
    // Register some kind of cleanup function in case
    // of future errors (but what happens if *that*
    // throws???):
    auto onfail = std::make_scope_fail([&]{ base::cleanup(); });
   
    // [[ ACTUAL EXTENSION CODE GOES HERE ]]
}

You need to pack all that complexity and customizeability into this:

auto derived::setup(Args&&... args) /*noexcept?*/ -> ReturnType
{
    // ??? <- something here
   
    // [[ ACTUAL EXTENSION CODE GOES HERE ]]
}

And that's just for the /single/ inheritance case.

That seems a tall order, but good luck!


Received on 2019-05-17 18:17:49