We need a __traits trait to test for whether one class is derived from another

[dlangBugzillaToGithub]

Jonathan M Davis reported this on 2024-12-06T02:20:06Z

Transferred from https://issues.dlang.org/show_bug.cgi?id=24892

CC List

• Richard (Rikki) Andrew Cattermole
• Dennis

Description

Right now, the normal way to test that one class is derived from another is to do is(T : U), which does not technically test whether T is derived from U. Rather, it tests whether T implicitly converts to U, which happens to be true when T is derived from U. However, it can also be true thanks to alias this, e.g.

```
void main()
{
    auto foo = new Foo;
    A a = foo;

    static assert(is(Foo : A));
}

class A {}

class Foo
{
    A a;

    alias this = a;
}
```

compiles just fine even though Foo is not derived from A. And technically, it's even possible for is(T : U) and is(U : T) to both be true at the same time (whether inheritance is involved or not).

As such, we really need to stop testing for inheritance via testing for implicit conversion. While it works most of the time, it is actually wrong and can cause bugs.

There _might_ be some clever way to test whether one class is derived from another by checking whether alias this exists for the class and whether the alias this implicitly converts to the target type in order to exclude it, but then you have the issue that it's possible for the alias this to convert to one of the class' actual base classes, so excluding it could give the wrong answer. As such, I'm not aware of any way to properly test whether a class is actually derived from another - and if it does exist, it's likely to involve several tests when it should just be a simple question.

As such, I would propose that we add something to __traits to provide this information (either that or add something new to is expressions which explicitly tests for it separate from testing for implicit conversions). For instance, we could have __traits(isDerivedFrom, T, U).

Now, that also raises the question of how interfaces should be handled, since arguably, a class is not derived from an interface (rather, it implements it). So, we could also add something like __traits(implements, T, U) - or we could just have __traits(isDerivedFrom, T, U) handle both classes and interfaces and then use is(U == interface) and is(U == class) to distinguish. That being said, separate traits seems simpler, and it makes it so that you don't accidentally treat a class as an interface or vice versa, so that's what I'd vote for.

In either case, given that we're not planning to get rid of alias this and that it seems to make it impossible to properly test whether one class is derived from another, I really think that we should add a way to explicitly test for inheritance to the language, and then we can start using that instead of is(T : U).

[dlangBugzillaToGithub]

alphaglosined commented on 2024-12-06T02:30:38Z

What I am going to suggest instead of a trait, is to use > and <.

I.e.

``is(Child > Parent)``

``is(Parent < Child)``

This will also work with template parameters:

```d
void func(T > Parent)(T thing) {
}
```

Another benefit of this, is that it can be a direct 1:1 replacement.

[dlangBugzillaToGithub]

dkorpel commented on 2024-12-06T09:04:50Z

Usually when a template function accepts a generic struct/class, the constraints test for capabilities (like having a certain property or operator overload) that the function wants to use on a parameter of that type.

Implicitly converting is an operation that you might want to do. Being derived from a specific class is not an operation on its own. If you do something that only works with derived classes, like array conversion based on covariance, you can test for that specifically:

if(is(const(T)[] : const(U)[])

That would also makes it clear *why* you specifically allow derived classes from `U` but not classes that convert to `U`.

I'm really interested in seeing the function where you want to use this trait, because I suspect it's trying to solve a problem that should be solved elsewhere.

[dlangBugzillaToGithub]

issues.dlang commented on 2024-12-06T09:58:50Z

(In reply to Dennis from comment #2)
> Usually when a template function accepts a generic struct/class, the
> constraints test for capabilities (like having a certain property or
> operator overload) that the function wants to use on a parameter of that
> type.
> 
> Implicitly converting is an operation that you might want to do. Being
> derived from a specific class is not an operation on its own. If you do
> something that only works with derived classes, like array conversion based
> on covariance, you can test for that specifically:
> 
> if(is(const(T)[] : const(U)[])
> 
> That would also makes it clear *why* you specifically allow derived classes
> from `U` but not classes that convert to `U`.
> 
> I'm really interested in seeing the function where you want to use this
> trait, because I suspect it's trying to solve a problem that should be
> solved elsewhere.

1. Determining whether a class is derived from another matters for type introspection in general and not just for template constraints. For instance, if you were trying to wrap D types for an interpreted language implemented in D, you might need to know the class hierarchy so that it could be replicated in that language. And a static if might need to test for whether a class was derived from a specific class in order to do something specific with it that has nothing to do with duck typing (e.g. it could affect which functions you wanted to compile into a templated type).

2. I would actually argue that testing for implicit conversions with template constraints is almost always a mistake. Occasionally, it makes sense, but in general, it causes bugs. A template constraint which tests for an implicit conversion does not actually cause that implicit conversion to happen, which means that unless the templated function's implementation forces the conversion, the type that passed the constraint may or may not even work with the function (best case, you get an error; worst case, you get silent breakage due to the type that the template was instantiated with not actually acting like the type that it would implicitly convert to - or you could get incorrect behavior because the implicit conversion ends up happening in multiple places throughout the function, but the original object is still the one being passed around). So, testing for an implicit conversion and not actually forcing it is just begging for trouble.

And to make matters worse, even if you do force the conversion, that can cause `@safe`ty problems in some cases. isConvertibleToString is a prime example of this. It tests whether a type implicit converts to string, but of course, it doesn't cause the conversion to occur at the call site. So, if a static array of characters is passed in, the template will be instantiated with the static array type and not the dynamic array type, so the static array will be copied into the function. And then if the implicit conversion is done within the function, it will be slicing the local static array, not the static array at the call site. In some cases, this works, but if the dynamic array escapes, then it will end up referring to invalid memory - and this would be invisible barring the usage of scope with DIP 1000. Other implicit conversions could have similar problems. So, in general, the implicit conversion needs to occur at the call site (like it does when a function explicitly takes a dynamic array, and you pass it a static array), but you can't actually force that with templates. So, what needs to happen in the general case is that you simply don't allow the implicit conversion with the template constraint and instead require that the caller do the conversion themselves to ensure that it's done at the call site - just like you have to do right now to pass a static array to a range-based function.

As such, I would argue that it's a bad idea to be testing for whether a class implicitly converts to a base class with a template constraint. It's likely to be far less error-prone than most implicit conversions given that (assuming that alias this is not involved), worst case, you're using a reference that's typed as the derived class and not the base class, which likely won't cause issues. However, if alias this is involved, who knows what's going to happen. The type being passed in could even be a struct rather than a class and end up behaving quite differently from the class reference, particularly if the implicit conversion allocates a new class object, since then you could end up passing the struct around within the function and then allocating a new instance of the class every time the code tries to call a function that's on the class but not on the struct. And you could get similarly weird behavior if it's an alias this on a class instead. It really depends on the actual implementation.

So, IMHO, in general, template constraints that care whether a class is derived from another should actually be testing that it's derived and not testing for an implicit conversion. And if the API is all that matters, then that's what should be tested for and not an implicit conversion.

So, in the general case, I think that it's a mistake to be testing for inheritance via implicit conversion, and I expect that the main reasons that it hasn't been a bigger issue is because alias this fortunately isn't used all that often and because if a function is supposed to take a particular class type, it will probably just take that class type explicitly rather than be templated on the argument type.

But even if there isn't agreement on the idea that testing for implicit conversions with template constraints is almost always a mistake, IMHO, this kind of information is basic information about a type, and it can matter for type introspection outside of template constraints. So, it should be queryable, and the fact that we don't actually have a way to test whether a class is derived from another is a problem.

[dlangBugzillaToGithub]

dkorpel commented on 2024-12-06T23:04:06Z

The first point about it providing new information is false, you can already use std.traits.BaseClassesTuple to find parents.

The second point reasons as follows:

1. Static arrays implicitly converting to slices leads to problems
2. Also, programmers might implement `alias this` functions with bad performance
3. Therefore implicit conversion happening in template functions is bad in general
4. Therefore we should thwart implicit conversion by making our template constraints more strict
5. Therefore we should provide a new construct to accommodate this check

I agree with 1, but that's a separate issue. 
2 is just how generic code works: if you implement an interface in a bad way, the resulting code using that interface will be bad as a consequence. You reap what you sow. 

Regarding 3 and 4: I'm not a fan of alias this with classes, but if we want to get rid of it, we should deprecate it proper. (This has actually been tried but reverted because there wasn't a good migration path.) I don't think we should try to sabotage it by making our templates gatekeep one kind of implicit conversion in favor of another.

Templates constraints should simply ask what they need, which brings me back to the unaddressed question: What actual function do you need it for? It's really hard to reason about handwavy scenarios. When you propose a new feature, there should be at least one real, concrete use case for it.

[dlangBugzillaToGithub]

issues.dlang commented on 2024-12-07T03:15:04Z

(In reply to Dennis from comment #4)
> The first point about it providing new information is false, you can already
> use std.traits.BaseClassesTuple to find parents.

I did not realize that that existed (or more likely, I read through it at some point and then forgot about it), but looking it over, it's an incredibly convoluted solution if you just want to ask whether one class is the base class of another. So, yes, it does provide a solution, taking advantage of a form of is expression that I didn't realize existed, but it's a pretty complicated solution for the simple question. That being said, it should be possible to use that particular is expression in a different way to implement something like isDerivedFrom!(T, U), which would be much better than doing something like getting the list of base classes and then filtering on them, which is what you'd have to do with BaseClassesTuple.

> Templates constraints should simply ask what they need, which brings me back
> to the unaddressed question: What actual function do you need it for? It's
> really hard to reason about handwavy scenarios. When you propose a new
> feature, there should be at least one real, concrete use case for it.

As I said, the ability to know whether one class is derived from another is a basic question of type introspection. I've seen it come up often enough over the years where someone asks how to do it, and they're told to use is(T : U), which as I said, works in many cases but is fundamentally flawed thanks to alias this. This kind of question seems as basic to me as asking whether a function is virtual or whether a type has a particular member. And I already gave an example of where if you need to wrap and replicate D types in another language, then you need to be able to introspect on pretty much everything about the type (including about its base classes). Type introspection is not just about template constraints.

I brought it up, because is(T : U) is wrong, but it's what people are routinely told to do, and I was unaware of any alternative which didn't run afoul of alias this. From the sounds of it, it is at least possible to implement a sane Phobos trait that answers the question, though honestly, I would think that it's basic enough, and the way to do it with is expressions is convoluted enough, that it would make more sense to just have a __traits trait for it. But since there is actually a way to do it, it's certainly a valid argument that it's not worth adding a trait for it to the compiler, though I disagree based on how basic a question this is.

Either way, we need to have a simple solution that we can point to rather than telling folks to use is(T : U), whether that's in __traits, or it's a trait of some kind in Phobos.

[dlangBugzillaToGithub]

dkorpel commented on 2024-12-07T09:57:37Z

So you're unable to provide a single real and concrete code example. That's a shame, because I'm suspicious of those cases where people ask how to check if one class is derived from another, it sounds like classic XY problem. (https://en.wikipedia.org/wiki/XY_problem)