Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Templated TypeInfo #3174

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Templated TypeInfo #3174

wants to merge 19 commits into from

Conversation

andralex
Copy link
Member

@andralex andralex commented Jul 28, 2020

Starting small. This duplicates the implementation in https://github.com/dlang/druntime/blob/master/src/rt/typeinfo/ti_int.d. The idea is, of course, that the templated implementation will replace implementations for all scalar types. For testing purposes I restricted it to int.

The thing works if typeid(int) lowers to RTTypeid!(int). The use of eponymous templates elegantly hides the type name.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @andralex!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

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#3174"

@andralex
Copy link
Member Author

I took upon painting the bikeshed by changing the name from RTTypeid to __typeid. Reasons:

  • A value with a capitalized name breaks the convention used everywhere in druntime and phobos
  • The whole thing is awkward to type - was that RTTypeID or RTTypeId or...?
  • The moment someone sees __typeid they'll figure a connection with typeid. With RTTypeid, they'll have to run to the manual.

@wilzbach
Copy link
Member

__typeid

__ is reserved for compiler internals like auto-generated variables. For runtime template hooks we typically use _d_.

@n8sh
Copy link
Member

n8sh commented Jul 28, 2020

__typeid

__ is reserved for compiler internals like auto-generated variables. For runtime template hooks we typically use _d_.

Wouldn't this be okay for the same reason as __cmp, __move_post_blt, __switch_error, etc?

@andralex
Copy link
Member Author

Added statically-typed versions of all functions, so if people write typeid(T).func in generic code, an efficient version is picked up.

@UplinkCoder
Copy link
Member

Mind the 32bit platforms.
hash_t is 32bit there.

posix.mak Show resolved Hide resolved
@andralex
Copy link
Member Author

andralex commented Aug 1, 2020

Per discussions with @WalterBright, it turns out it's not reasonably feasible to do an incremental replacement. The plan instead is to develop __typeid separately from the built-in typeid, and replace uses of the latter with the former. At the very end we adjust typeid to lower to __typeid.

Currently no support for struct, class, and interface.

src/object.d Outdated Show resolved Hide resolved
src/object.d Outdated Show resolved Hide resolved
@andralex andralex changed the title Templated TypeInfo for int Templated TypeInfo Aug 1, 2020
src/object.d Outdated Show resolved Hide resolved
{
return __typeid!U1.argTypes(arg1, arg2);
}
else static if (__traits(isStaticArray, T) || __traits(isAssociativeArray, T))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, is expression should be able to match static array, and associative arrays as well, for example:
is(T : U[size], U, size_t size) || is(T : K[V], K, V)

assert(p);
p.__postblit();
}
else static if (__traits(isStaticArray, T) && is(typeof(&UnqualifiedType.init[0].__postblit)))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may trip on static arrays with 0 length (although they don't make much sense, they are allowed in D right now). An alternative would be to extract next type from static array using is expression, and use it's init value: is(T : V[size], V, size_t size) && is(typeof(V.init.__postblit))

assert(p);
p.__dtor();
}
else static if (__traits(isStaticArray, T) && is(typeof(&UnqualifiedType.init[0].__dtor)))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue with 0 sized static array.

return 1;
}

static string toString() @nogc nothrow pure @safe { return T.stringof; }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to [https://dlang.org/spec/property.html#stringof](Properties page) stringof returns just name of Type, not fully qualified name of Type, therefore if there are two Dog classes one in test module and another in impl module both would be printed as just Dog (hence no way to differentiate between test and impl Dogs). It is also different from what toString for original TypeInfo is returning, which is fully qualified name. Something as fullyQualifiedName from std.traits should be implemented, although, to note, fullyQualifiedName is still not compliant with toString from TypeInfo, since it has tendency to shorten eponymous templates, while TypeInfo doesn't, in other words TypeInfo.toString for Dog!int would return smth like: "Dog!int.Dog" while fullyQualifiedName from std.traits would return just: "Dog!int"

return T.sizeof;
}

alias tsize = tsizeImpl;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it is better to rename tsizeImpl to tsize?

assert(sa1[] !is sa2[]);

ti = __typeid!(S[3]);
assert(ti.getHash(&sa1) == ti.getHash(&sa2));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice coverage tests

}
return (d1 == d2) ? 0 : ((d1 < d2) ? -1 : 1);
return result;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this refactoring objectively better? If it isn't, I find it significantly more difficult to understand.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's supposed to be faster

@dlang-bot dlang-bot added Needs Rebase needs a `git rebase` performed Needs Work stalled labels May 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants