Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Apr 24, 2014

https://issues.dlang.org/show_bug.cgi?id=11706

Thanks to some compiler improvements this can be entirely implemented outside the Typedef definition.

@monarchdodra
Copy link
Collaborator

I'm unsure about this change to OriginalType. I think code that uses it assumes enum now, and have forgotten about typedefs. Sure, it originally supported typedef, but that has been deprecated and removed. I'm unsure

In particular, an enum is implicitly castable to the OriginalType. In contrast, Typedef is NOT. By design. The entire idea is to create a brand new type that isn't the original type.

I do agree a trait should be available, but I'm not sure it should be packaged into OriginalType.

... Well, that's my "Shields Up!" reaction anyways.

@monarchdodra
Copy link
Collaborator

I didn't quite check the failing unittest, but the fact they are failing seems like an indication this isn't exactly a trivial change.

@ghost
Copy link
Author

ghost commented Apr 24, 2014

Yeah, I don't quite understand what's going on with the failures. This code works fine in a simplified test-case. I'll investigate.

@ghost
Copy link
Author

ghost commented Apr 24, 2014

I do agree a trait should be available, but I'm not sure it should be packaged into OriginalType.

What's your suggestion for a name. Maybe TypedefType or something?

@ghost ghost changed the title Issue 11706 - std.traits.OriginalType should support std.typecons.Typedef Issue 11706 - Add a TypedefType trait to extract the underlying type of a std.typecons.Typedef Apr 24, 2014
@ghost
Copy link
Author

ghost commented Apr 24, 2014

Updated by implementing TypedefType rather than tweaking OriginalType, which caused issues for some reason.

@monarchdodra
Copy link
Collaborator

Thanks to some compiler improvements this can be entirely implemented outside the Typedef definition.

Nice. Very nice. We need to do the same with Rebindable. I hate that a lot of functions in traits have to import std.typecons just because maybe the current object is a Rebindable...

@monarchdodra
Copy link
Collaborator

Shouldn't this accept any type, and be a no-op for non-Typedefs? It makes generic code much easier. I've "abused" this for OriginalType, that's for sure. It drastically reduces the "condtional bloat":

alias TT = TypedefType!T; //Is T a typedef? Don't care.
static if (is(TT == class)
    ...
else if (is(TT == struct)
   ...
...
}

@ghost
Copy link
Author

ghost commented Apr 26, 2014

no-op for non-Typedefs

No-op? You mean alias itself to the passed-in type?

@ghost
Copy link
Author

ghost commented Apr 26, 2014

Updated with no-op support, with an even cleaner implementation.

@monarchdodra
Copy link
Collaborator

Should I be concerned about:

alias T = TypedefType!(Typedef!int, 75, int, 5, ubyte);

Does that even behave according to spec?

@ghost
Copy link
Author

ghost commented Apr 26, 2014

Does that even behave according to spec?

I'll fix this up. But it looks like TemplateArgsOf will also have to be fixed.

@ghost
Copy link
Author

ghost commented Apr 26, 2014

Filed as Issue 12651. It might be a compiler bug, CC'ed Kenji.

@DmitryOlshansky
Copy link
Member

The linked bug is fixed, so ... is it good to go?

*/
template TypedefType(T)
{
static if (is(T : Typedef!Args, Args...))

Choose a reason for hiding this comment

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

Should match exact Typedef template parameter list.

Copy link
Author

Choose a reason for hiding this comment

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

Oh you mean = Args rather than = Args[0]? I can't remember why I used [0] here, should be fixed.

Choose a reason for hiding this comment

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

I mean that Typedef takes fixed amount of template arguments, not any variadic one.

Copy link
Author

Choose a reason for hiding this comment

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

Ah I thought I had to use the ... syntax.

Choose a reason for hiding this comment

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

No, it is generic pattern matching thing:

struct ZZZ (T1, T2) { }

alias Z = ZZZ!(int, double);

static if (is(Z == ZZZ!(int, T), T))
{
    pragma(msg, T);
}

Anything that can be in template parameter list can go after comma

@mihails-strasuns
Copy link

Ping @AndrejMitrovic

@ghost
Copy link
Author

ghost commented Aug 19, 2014

Updated: Let's see if it passes the tester.

@mihails-strasuns
Copy link

Please add test case for typedef with custom initializer (i.e. Typedef!(int, 42))

@ghost
Copy link
Author

ghost commented Sep 8, 2014

Updated.

@mihails-strasuns
Copy link

Unrelated test failure?

@ghost
Copy link
Author

ghost commented Sep 8, 2014

Yeah I think so.

@mihails-strasuns
Copy link

Ok, I'll mark it for merging, will see if it eventually gets to a green state on its own.

@mihails-strasuns
Copy link

Auto-merge toggled on

mihails-strasuns pushed a commit that referenced this pull request Sep 8, 2014
Issue 11706 - Add a TypedefType trait to extract the underlying type of a std.typecons.Typedef
@mihails-strasuns mihails-strasuns merged commit bf8d579 into dlang:master Sep 8, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants