Only a madman would inherit from String

General JUCE discussion
1

Hello! I’m the madman :wink:

JUCE has added final to certain classes like String. It inhibits inheriting from the class. This has broken some of my code, and I haven’t yet found a way to work around it. At the ADC’17 JUCE clinic, I showed an example. @jules, @fabian and I didn’t find a solution straight away, and decided to put it on the forum.

If you can find a solution to the problem below, I’ll buy you a beer :slight_smile:

I have a class called any. It’s similar to std::any or boost::any in that you can put any object into it, it forgets the type, and you can get the object back later on:

any greeting(String("Hello World!"));
// ...
String extracted = greeting.get<String>();

Why not just use std::any? Because I need this:

class Animal {};
class Cat : public Animal {};

any cat((Cat()));
Animal a = cat.get<Animal>();

I need to put in instances of a subclass, and extract just the base part. std::any and boost::any don’t do this.

The any class (simplified):

class any {
public:
    template<typename T>
    explicit any(const T& value)
    : wrapped(std::make_shared<Wrapped<T>>(value))
    {}
    
    template<typename T>
    T get() const
    {
        if (auto ptr = std::dynamic_pointer_cast<T>(wrapped))
            return *ptr;
        
        throw /* type mismatch error */;
    }
    
private:
    struct WrappedBase {
        virtual ~WrappedBase() {}
    };
    
    template<typename T>
    struct Wrapped : WrappedBase, T {
        Wrapped(const T& u) : T(u) {}
    };
    
    std::shared_ptr<WrappedBase> wrapped;
};

By inheriting from T, it can leverage dynamic_cast's upcasting behavior. The Animal/Cat example now works, but the String doesn’t, as String is final. If we don’t inherit from T, how can get() know whether classes are related?

How do we get both examples to work? All ideas and questions are welcome.

2

Does this Animal/Cat example actually compile? I can’t get it to work.

I know it doesn’t solve the problem but couldn’t you work around this by creating a class that contains the String and then get it that way? E.g.

struct StringWrapper
{
    String string;
};

any greeting (StringWrapper ({ "Hello World!" }));
// ...
auto extracted = greeting.get<StringWrapper>().string;
3

Sorry about that, I “simplified” that a bit too much. Please see the updated class above.

The problem with working around is that the class is in my JUCE module. Users of the module might use any type as T. If I was just using it myself with some concrete types, then yes, that would be a way.

There’s ways to special-case String using enable_if and friends…there’s even std::is_final. But I don’t know how to then preserve the dynamic_cast behavior.

4

If you are willing to accept that polymorphic casts on final classes is not allowed, there is a way:

#include <typeinfo>
#include <typeindex>
#include <memory>
#include <type_traits>

class any {
public:
	template<typename T>
	explicit any(const T& value)
		: wrapped(morph<T>(value))
	{}

	template<typename T>
	T get() const
	{
		if (auto ptr = wrapped.get(); ptr != nullptr)
		{
			if (ptr->is_final)
			{
				auto finalBase = static_cast<WrappedFinalBase*>(ptr);
				if (finalBase->index == std::type_index(typeid(T())))
					return *static_cast<T*>(finalBase->memory);
			}
			else if (auto ptr = std::dynamic_pointer_cast<T>(wrapped))
				return *ptr;
		}

		throw std::bad_cast();
	}

private:

	struct WrappedBase
	{
		WrappedBase(bool is_final = false) : is_final(is_final) {}
		const bool is_final = false;

		virtual ~WrappedBase() {}
	};

	template<typename T>
	struct WrappedNormal : public T, WrappedBase {
		WrappedNormal(const T& t) : T(t) {}
	};

	struct WrappedFinalBase : public WrappedBase
	{
		WrappedFinalBase(const std::type_index& ti, void * mem) : WrappedBase{ true }, index(ti), memory(mem) {}
		const std::type_index index;
		void * const memory;
	};

	template<typename T>
	struct WrappedFinal : WrappedFinalBase
	{
		WrappedFinal(const T& u) : t(u), WrappedFinalBase(std::type_index(typeid(T())), &t) {}
		T t;
	};

	template<typename T>
	static WrappedBase * morph(const typename std::enable_if<!std::is_final<T>::value, T>::type & value)
	{
		return new WrappedNormal<T>(value);
	}

	template<typename T>
	static WrappedBase * morph(const typename std::enable_if<std::is_final<T>::value, T>::type & value)
	{
		return new WrappedFinal<T>(value);
	} 

	std::shared_ptr<WrappedBase> wrapped;
};

Test:

class A
{
public:
	virtual ~A() {}
};

class B : A {};
class C : public A {};
class D final : public A {};

int main()
{
	{
		any test{ A() };
		A a = test.get<A>();
	}

	{
		any test{ B() };
		B b = test.get<B>();

		try
		{
			A a = test.get<A>(); // private base class, invalid conversion.
		}
		catch (const std::bad_cast& c)
		{
		}

	}

	{
		any test{ C() };
		A a = test.get<A>();
		C c = test.get<C>();
	}

	{
		any test{ D() };
		D d = test.get<D>();

		try
		{
			A a = test.get<A>(); // final can only specify the exact type.
		}
		catch (const std::bad_cast& c)
		{
		}

	}

	return 0;
}

A final class should implement an abstract class and seal it, thus you would never have been able to slice it to begin with. So unless you did something really wrong, this could be a drop-in replacement for you. In any case, you are now able to solve it on a case-by-case basis, because everything should compile now and be type-safe. With that said, I find it especially concerning your main objective is to allow slicing of polymorphic types.

1 Like
5

Hi @basteln, sorry for not getting back to you sooner. The problem is harder than I initially thought and it took me quite a while to figure out a way which would not add any overhead like juggling with type_ids etc. and work for types marked final.

There is one tiny downside to my solution. You need to tell the compiler which classes are base classes of each other. So something like this:

template <class T> struct BaseClass {};
#define DEFINE_BASE_CLASS(x, y) template<> struct BaseClass<x> { typedef y type; };

// Tell the compiler about your base classes
DEFINE_BASE_CLASS (Cat, Animal);
...

I don’t think you will be able to find a solution without this kind of compile-time metadata (there is no runtime memory increase here - it’s all compile-time). But maybe I’m wrong. In any case it does not add any overhead.

Here’s the solution. The trick is that you mimic the class hierarchy with your Wrapped types as well. So if Cat derives from Animal, you need to have Wrapper<Cat> also derive from Wrapped<Animal>.

class any
{
public:
    template <typename Type>
    any (const Type& what)
          // If Type has a base class Base, then Wrapped<Type> will derive from Wrapper<Base>
        : wrapped (new Storage<Type> (what))
    {}

    template <typename Type>
    Type& get()
    {
        if (Wrapped<Type>* s = dynamic_cast<Wrapped<Type>*> (wrapped.get()))
            return s->get();
        
        throw;
    }

private:
    struct WrappedBase
    {
        virtual ~WrappedBase() {}
    };

    template <typename Type, typename U = void /* this U parameter is a SFINAE trick, it's never really used */>
    struct Wrapped : WrappedBase
    {
        Type& get() { return getStorage(); }
        
        virtual Type& getStorage() = 0;
    };
    
    /* The compiler will always try to use the most specialised template
       However, if Type does not have a base class then the second template argument
       (marked 'U' above) will not be a valid expression and result in a compiler
       error. However, thanks to SFINAE it will not result in a real compiler error.
       The specialisation will simply not be used in this case.
     
       BTW: std::void_t<Type> is a C++17 template which always resolves to void, so
            U will always be void and is never used anywhere anyway. The only reason
            why std::void_t<Type> exists is for this kind of SFINAE trick.
     */
    template <typename Type>
    struct Wrapped<Type, typename my_void_t<typename BaseClass<Type>::type>::type /* <- U */>
       : Wrapped<typename BaseClass<Type>::type>
    {
        typedef typename BaseClass<Type>::type BaseType;
        typedef Wrapped<BaseType> Base;
        
        Type& get() { return static_cast<Type&> (Base::get()); }
    };
    
    template <typename Type>
    struct Storage    : Wrapped<Type>
    {
        Storage (const Type& what) : storage (what) {}
        Type& getStorage() override  { return storage; }
        
    private:
        Type storage;
    };

    std::unique_ptr<WrappedBase> wrapped;
};

The above requires C++17 due to the use of std::void_t, but you can also get it to compile on C++14 by defining std::void_t yourself:

template <typename T> struct my_void_t { typedef void type; };

and replacing the use of void_t in my code above with

template <typename Type>
    struct Wrapped<Type, typename my_void_t<typename BaseClass<Type>::type>::type /* <- U */>
       : Wrapped<typename BaseClass<Type>::type>

Let me know if this works for you.

Edit: Fixed a bug in my code. Hat tip to @Mayae, @jules !

6

When we brainstormed this quickly at the clinic at ADC, I did think “hmm, it’s a tricky case because it’d be nice to allow that automatic cast to sub-type”, and was planning to try to think of some tricks like Fabian did, for ways to implement that casting functionality…

However… Having thought about it more, I agree with some of the concern in comments above that actually the casting problem is difficult because the whole idea is totally broken!

Here’s my best attempt at explaining why… There are two scenarios:

  1. If you’re using the class with non-virtual types, then all is fine. The dynamic_cast definitely shouldn’t be in there because the same type you used to store the object must be used to retrieve it. No problem.

  2. If you’re using it with virtual types, then your constructor that calls std::make_shared is going to be making a COPY of the object you give it a reference to. If your templated type T is a base of the object’s real type, then BAM! it’ll either not compile if the base doesn’t have a copy constructor (this is your best outcome!). Or: if the base class can be copied for some reason, it’ll be horrifically tearing the derived object - very very bad! Or: best case scenario, the tearing luckily doesn’t damage the object, but when you retrieve it, the class you’d ask for can’t be a derived class because the derived bit can’t have been copied, so there’s again no need for the dynamic_cast. But honestly, anything involving virtual classes being used like this is just a horrible bug waiting to happen!

7

@fabian

That’s a quite nice idea, and I like for not being intrusive but that comes with it’s own set of issues, e.g. having to sync class hierachies, and if you have a class hierachy with private inheritance that will not compile (which may or may not be a good thing, other solutions will instead throw bad_cast at runtime).

I guess the biggest downside is the inability to work with multiple inheritance, but again could be a good or bad thing. If we combine the work, we can now have expected behaviour with non-final classes, and for final classes you can opt-in and define some single-inheritance hierachies that you can cast to.

However, if we allow user-provided compile-time meta information, you could actually reimplement the whole multiple inheritance casting system as constexpr tables and ptr offsets - so there is a way - but only a madman would venture there…

Note that your implementation has a bug, you’re only storing the base class storage, not the derived storage, yielding UB when you downcast. Could be fixed with another most-derived type template argument in the Wrapped<> class.

Also, agree wholeheartedly with Jules sentiments.

8

OK I fixed that bug in my code above. Thanks for spotting this @Mayae!

@jules I think this also fixes your issue as the storage is always the most derived class now. However, there is still potential for (human-) error if the user supplies a base-class reference to the any constructor.

Maybe a good way to avoid this is to convert the any constructor to a private move constructor and only allow the user to create an any class with a make_any style static method. Something like this would work:

template <typename T>
struct any_creator                     // any_creator is a friend in any
{
    template <typename... Args>
    static any make (Args... args)     { return any (T (args...)); /* <- using private move constructor */ }
};

// User code:
auto cat = any_creator<Cat>::make();
auto a = cat.get<Animal>();

I think this way there is no way to somehow get UB (as mentioned in your point 2). What do you think?

Edit: Included move semantics to avoid unnecessary copies

9

Thanks for the input! I have a couple of questions…

it’d be nice to allow that automatic cast to sub-type

Might be a typo, but just so we’re on the same page: In my example, I cast to super-type, not sub-type.

  1. If you’re using the class with non-virtual types, then all is fine. The dynamic_cast definitely shouldn’t be in there because the same type you used to store the object must be used to retrieve it.

I’m not sure I understand what you mean…assuming that by virtual you mean polymorphic. Animal and Cat are not polymorphic, but we still need the dynamic_cast to determine whether the requested type is valid.

the same type you used to store the object must be used to retrieve it.

Why does it have to be the same? You could store a Cat and then retrieve an Animal.

If your templated type T is a base of the object’s real type, then BAM!

How do you achieve that? The constructor’s T should be inferred, and if you somehow manage to explicitly force T to be a base type of what you’re actually passing, the slicing would be on your behalf.

10

Like this:

Cat cat;
Animal& animal = cat;
any a (animal);

Now you have a problem as your any class will only have storage for animal. The issue is the copy constructor takes a reference which needn’t be the fully derived type.

Just to be clear: the solution that I posted above is a drop-in replacement for your code (ok, you’ll need to use my DEFINE_BASE_CLASS macro) and solves the problem you had with the final keyword. The slicing issue is completely separate from this and is also a problem with your original code.

To make it impossible to write the above code-snippet, you could use the any_creator trick I mentioned above. Then I don’t see any more problems.

However, I wonder how useful your class would really be. For example, you would expect your any class to also have an assignment operator so that it could change it’s underlying type. Something like this:

template <typename T>
operator=(const T& value)

But here you would have the same copy-constructor problem when trying to implement this and I don’t think my any_creator trick would work here.

11

BTW: std::any does have the copy constructor and the assignment operator but it does not allow you to do those polymorphic conversions you want. For example, this does not work with std::any:

auto a = std::any ((Cat()));
auto* animal = std::any_cast<Animal> (&a); // <- returns a nullptr
auto* cat      = std::any_cast<Cat> (&a); // works!

I think this is an indication that you can’t have both.

12

Thanks everyone for taking the time. I see the problems with my design, I need to step back a little and find a way to avoid this altogether. I learned a lot through the examples you guys posted! :+1: