Give TypeInfo_Class/TypeInfo_Interface.isBaseOf like C#/Java isAssignableFrom #2770

n8sh · 2019-08-29T00:09:27Z

This is equivalent to C#/Java isAssignableFrom ~~with the argument swapped with the receiver~~. Naming the method "isAssignableFrom" would be more familiar to people coming from C#/Java (and I would find it convenient since it's one fewer difference to remember between languages) but is potentially misleading: "alias this" and overloadable opAssign mean that this would not actually indicate whether values of one type could be assigned to another.

I used the name "isDerivedFrom" since the phrase "derives from" appears in the D online documentation. Swapping the argument order is not ideal, so if someone can think of a sensible name that would work with the same argument order as "isAssignableFrom" please suggest one.

dlang-bot · 2019-08-29T00:09:30Z

Thanks for your pull request, @n8sh!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	20178	enhancement	Add TypeInfo_Class/TypeInfo_Interface.isBaseOf (equivalent to C#/Java isAssignableFrom)

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + druntime#2770"

src/object.d

n8sh · 2019-08-29T00:16:28Z

src/object.d

+    {
+        if (parent is null)
+            return false;
+        if (parent is this || parent.name == this.name)


I use if (parent is this || parent.name == this.name) instead of opEquals because TypeInfo_Class.opEquals isn't marked as any of nogc/nothrow/pure/safe and isn't callable on a const argument or with a const receiver.

Dynamic casting just compares TypeInfo pointers, so if you don't expect copies of TypeInfo being made at runtime, you don't have to compare names. You might also just call rt.cast_._d_isbaseof.

If pointer comparison is good enough for dynamic casting then it is good enough for this purpose too. Thank you for the tip regarding _d_isbaseof.

src/object.d

JinShil · 2019-08-29T02:17:14Z

Seems like a good idea to me. I'm not sure about the implementation though; is name reliable for comparisons? I think the tests could be filled out with more corner cases (e.g. packages, nested types, multiple interface inheritance, etc.)

Geod24 · 2019-08-29T02:50:58Z

What's the use case for this ? Java / C# reflection is primarily runtime, while D's is primarily compile time.

n8sh · 2019-08-29T15:37:54Z

My personal use case was implementing multimethods when the classes involved aren't known at compile time.

PetarKirov · 2019-08-29T15:47:31Z

@n8sh Perhaps rebasing on top of master should fix the error on BuildKite.

n8sh · 2019-08-30T22:29:59Z

Changing to use _d_isbaseof and renaming to isBaseOf with the same argument order as isAssignableFrom.

n8sh · 2019-08-30T22:47:12Z

src/object.d

+        for (auto ti = cast() child; ti !is null; ti = ti.base)
+            if (ti is this)
+                return true;
+        return false;


If there's some case for classes where this gives the wrong answer but _d_isbaseof gives the right answer I haven't found it. Unless such a case can be found I'll leave it here since _d_isbaseof potentially does a lot more work and extern(C) function calls are opaque to the optimizer.

Looking back at this, checking straight up the inheritance chain rather than using _d_isbaseof means that the receiver TypeInfo_Class and argument TypeInfo_Class must represent actual classes rather than being the .info field of some TypeInfo_Interface.

Solution: detect an interface's ClassInfo (TypeInfo_Class that does not represent a class) by checking if the initializer has length zero.

n8sh · 2019-08-30T22:49:17Z

src/rt/cast_.d

@@ -14,6 +14,9 @@
 module rt.cast_;

 extern (C):
+@nogc:
+nothrow:
+pure:


Adding some attributes here so the compiler enforces the same attributes I claim in the private extern(C) declaration in object.d.

thewilsonator · 2020-02-14T08:56:52Z

@n8sh whats the status of this?

n8sh · 2020-02-15T22:58:22Z

@thewilsonator The code is ready but discussion is open regarding whether this should be added.

src/object.d

thewilsonator · 2020-02-15T23:38:15Z

It could also do with a changelog entry.

12345swordy · 2020-02-16T19:28:18Z

I don't see any reason to reject this giving that there are attempts to call c# code from d.

src/object.d

12345swordy · 2020-02-23T22:11:38Z

@n8sh ping. Address rainers comments.

12345swordy · 2020-03-09T16:52:26Z

@thewilsonator Any objections so far?

thewilsonator · 2020-03-10T08:50:41Z

None from me, but it doesn't seem to pass.

12345swordy · 2020-03-26T18:47:49Z

@n8sh the test are not passing, could you rebase?

12345swordy · 2020-03-27T03:17:11Z

@thewilsonator the tests have pass, need to add auto merge label to this as I don't have the authority to do so

Geod24 · 2020-03-28T13:24:14Z

Does this work with shared druntime ?

n8sh · 2020-03-28T17:01:33Z

@Geod24 I don't know the answer to that but it works the same as rt.cast_._d_isbaseof.

Geod24

Sorry for the delay in review. A few nits, otherwise LGTM.
Regarding the shared question, I asked because identity comparison can break in the presence of multiple TypeInfo, which IIRC can happen when using dynamic loading. But if _d_isbaseof does it, it should be fine 🤷‍♂

src/object.d

Geod24 · 2020-03-30T09:56:49Z

src/rt/cast_.d

@@ -101,7 +104,7 @@ int _d_isbaseof2(ClassInfo oc, ClassInfo c, ref size_t offset)
    return false;
 }

-int _d_isbaseof(ClassInfo oc, ClassInfo c)
+int _d_isbaseof(scope ClassInfo oc, scope const ClassInfo c) @safe


You could use tail recursion and make this const, in order to avoid the cast.

I'd rather not mess with the implementation of _d_isbaseof in this PR.

Geod24 · 2020-03-30T09:57:25Z

src/rt/cast_.d

@@ -74,7 +77,7 @@ void* _d_dynamic_cast(Object o, ClassInfo c)
    return res;
 }

-int _d_isbaseof2(ClassInfo oc, ClassInfo c, ref size_t offset)
+int _d_isbaseof2(scope ClassInfo oc, scope const ClassInfo c, scope ref size_t offset) @safe


Ditto recursion

test/typeinfo/src/isbaseof.d

Geod24 · 2020-03-31T03:17:25Z

Ah I found it: #2828

Equivalent to C#/Java isAssignableFrom. Naming the method "isAssignableFrom" would be more familiar to people coming from C#/Java but is potentially misleading: "alias this" and overloadable opAssign mean that this would not actually indicate whether values of one type could be assigned to another. Adding qualifiers to rt.cast_ functions.

12345swordy · 2020-04-06T11:15:50Z

What is the hold up here?

12345swordy · 2020-04-16T15:03:49Z

The objection time limit is up
cc @thewilsonator

n8sh requested review from andralex, MartinNowak and wilzbach as code owners August 29, 2019 00:09

dlang-bot added the Enhancement New functionality label Aug 29, 2019

n8sh commented Aug 29, 2019

View reviewed changes

src/object.d Outdated Show resolved Hide resolved

n8sh commented Aug 29, 2019

View reviewed changes

n8sh force-pushed the issue-20178 branch from b28a893 to 25e93d5 Compare August 29, 2019 00:19

JinShil reviewed Aug 29, 2019

View reviewed changes

src/object.d Outdated Show resolved Hide resolved

JinShil reviewed Aug 29, 2019

View reviewed changes

src/object.d Outdated Show resolved Hide resolved

n8sh force-pushed the issue-20178 branch 3 times, most recently from 976fea2 to e0229ca Compare August 29, 2019 02:41

n8sh force-pushed the issue-20178 branch from e0229ca to d567a8b Compare August 30, 2019 18:54

n8sh force-pushed the issue-20178 branch from d567a8b to ae0d35c Compare August 30, 2019 22:30

n8sh changed the title ~~Give TypeInfo_Class/TypeInfo_Interface.isDerivedFrom like C#/Java isAssignableFrom~~ Give TypeInfo_Class/TypeInfo_Interface.isBaseOf like C#/Java isAssignableFrom Aug 30, 2019

n8sh force-pushed the issue-20178 branch from ae0d35c to ab380aa Compare August 30, 2019 22:33

n8sh commented Aug 30, 2019

View reviewed changes

n8sh force-pushed the issue-20178 branch 2 times, most recently from 125559e to 84d1ed5 Compare August 30, 2019 23:25

n8sh commented Feb 15, 2020

View reviewed changes

src/object.d Outdated Show resolved Hide resolved

n8sh force-pushed the issue-20178 branch from 56bd618 to 0a0736b Compare February 17, 2020 00:47

rainers reviewed Feb 21, 2020

View reviewed changes

src/object.d Outdated Show resolved Hide resolved

src/object.d Show resolved Hide resolved

n8sh force-pushed the issue-20178 branch 6 times, most recently from 283cfaf to 8008f6f Compare February 27, 2020 18:13

n8sh force-pushed the issue-20178 branch 2 times, most recently from 6d6a791 to 7d20e09 Compare March 7, 2020 17:07

thewilsonator approved these changes Mar 10, 2020

View reviewed changes

n8sh force-pushed the issue-20178 branch 2 times, most recently from 7c636d0 to 23dea06 Compare March 26, 2020 20:07

Geod24 approved these changes Mar 31, 2020

View reviewed changes

n8sh force-pushed the issue-20178 branch from 23dea06 to bc10972 Compare March 31, 2020 16:51

n8sh added the 72h no objection -> merge The PR will be merged if there are no objections raised. label Apr 12, 2020

n8sh added the auto-merge label Apr 16, 2020

dlang-bot merged commit cd643e2 into dlang:master Apr 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Give TypeInfo_Class/TypeInfo_Interface.isBaseOf like C#/Java isAssignableFrom #2770

Give TypeInfo_Class/TypeInfo_Interface.isBaseOf like C#/Java isAssignableFrom #2770

n8sh commented Aug 29, 2019 •

edited

dlang-bot commented Aug 29, 2019 •

edited

n8sh Aug 29, 2019

rainers Aug 29, 2019

n8sh Aug 30, 2019

JinShil commented Aug 29, 2019 •

edited

Geod24 commented Aug 29, 2019

n8sh commented Aug 29, 2019

PetarKirov commented Aug 29, 2019

n8sh commented Aug 30, 2019

n8sh Aug 30, 2019

n8sh Feb 16, 2020

n8sh Mar 7, 2020

n8sh Aug 30, 2019 •

edited

thewilsonator commented Feb 14, 2020

n8sh commented Feb 15, 2020

thewilsonator commented Feb 15, 2020

12345swordy commented Feb 16, 2020

12345swordy commented Feb 23, 2020

12345swordy commented Mar 9, 2020 •

edited

thewilsonator commented Mar 10, 2020

12345swordy commented Mar 26, 2020

12345swordy commented Mar 27, 2020 •

edited

Geod24 commented Mar 28, 2020

n8sh commented Mar 28, 2020

Geod24 left a comment

Geod24 Mar 30, 2020

n8sh Mar 31, 2020

Geod24 Mar 30, 2020

Geod24 commented Mar 31, 2020

12345swordy commented Apr 6, 2020

12345swordy commented Apr 16, 2020

Give TypeInfo_Class/TypeInfo_Interface.isBaseOf like C#/Java isAssignableFrom #2770

Give TypeInfo_Class/TypeInfo_Interface.isBaseOf like C#/Java isAssignableFrom #2770

Conversation

n8sh commented Aug 29, 2019 • edited

dlang-bot commented Aug 29, 2019 • edited

Bugzilla references

Testing this PR locally

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JinShil commented Aug 29, 2019 • edited

Geod24 commented Aug 29, 2019

n8sh commented Aug 29, 2019

PetarKirov commented Aug 29, 2019

n8sh commented Aug 30, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

n8sh Aug 30, 2019 • edited

Choose a reason for hiding this comment

thewilsonator commented Feb 14, 2020

n8sh commented Feb 15, 2020

thewilsonator commented Feb 15, 2020

12345swordy commented Feb 16, 2020

12345swordy commented Feb 23, 2020

12345swordy commented Mar 9, 2020 • edited

thewilsonator commented Mar 10, 2020

12345swordy commented Mar 26, 2020

12345swordy commented Mar 27, 2020 • edited

Geod24 commented Mar 28, 2020

n8sh commented Mar 28, 2020

Geod24 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Geod24 commented Mar 31, 2020

12345swordy commented Apr 6, 2020

12345swordy commented Apr 16, 2020

n8sh commented Aug 29, 2019 •

edited

dlang-bot commented Aug 29, 2019 •

edited

JinShil commented Aug 29, 2019 •

edited

n8sh Aug 30, 2019 •

edited

12345swordy commented Mar 9, 2020 •

edited

12345swordy commented Mar 27, 2020 •

edited