-
-
Notifications
You must be signed in to change notification settings - Fork 610
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TypeInfo.init -> .initializer (part 2) #6550
Conversation
|
Regarding the tester failures: dlang/druntime#1766 (comment) |
Oh, you mean dmd - yeah... gdc is unaffected. :-) |
|
I know it's ugly, but perhaps (note: untested): static if(__traits(compiles, {auto p = TypeInfo.initializer})) auto p = TypeInfo.initializer;
else auto p = TypeInfo.init;This is a very special and very odd case. I think we are justified in doing something like this in this one spot, even if it lasts forever! One thing you could also do is create a UFCS function to create an initializer "property" when it doesn't already exist. Something like: static if(!__traits(compiles, ... /*same idea as above*/)
auto initializer(TypeInfo t)
{
return t.init;
} |
f3b3e81 to
327932a
Compare
Done. Figuring out why it segfaulted at first wasn't fun. I'd rather update the compilers in the testers than forever having to deal with bugs that have long been fixed. |
|
Looks good I think.
Probably true, but having something that is robust like this is a step forward either way. |
src/root/rmem.d
Outdated
|
|
||
| // TypeInfo.initializer for compilers older than 2.070 | ||
| static if(!__traits(hasMember, TypeInfo, "initializer")) | ||
| private const(void[]) initializer(T : TypeInfo)(const T t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why this has to be a template? If it does have to be a template, you can probably leave the attributes off as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why this has to be a template?
Reason is in the commit message: It's a template because old ClassInfo uses a member byte[] init; instead of overriding the method.
It could also be an overload for ClassInfo, but that would be longer and the template way seems a bit more robust.
If it does have to be a template, you can probably leave the attributes off as well.
I guess so. I'd leave them in, though. The function needs all those attributes in order to call TypeInfo.init. I prefer spelling them out in such cases. But I can remove them, of course, if you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reason is in the commit message
Hah, didn't even notice that, as you have to hover over the message to see the whole thing. I love asking already exactly answered questions 😝
Looking at history, looks like the init change for ClassInfo was fixed in April of 2015. Hard to believe the testers are using such an old compiler...
But I can remove them, of course, if you want.
No worries, I don't have commit rights here anyway. I'll leave it up to the DMD maintainers to decide.
327932a to
3d3a0c2
Compare
TypeInfo.init is deprecated and is going to to be removed.
It's a template because old ClassInfo uses a member `byte[] init;` instead of overriding the method.
|
TypeInfo.init is deprecated and is going to to be removed.
This blocks dlang/druntime#1766.