Skip to content
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

Implement issue# 16744: TypeOf template helper. #4920

Closed
wants to merge 1 commit into from

Conversation

jmdavis
Copy link
Member

@jmdavis jmdavis commented Nov 24, 2016

typeof cannot be used directly with templates such as staticMap, because
we don't have the template equivalent of lambda functions. TypeOf is
therefore a template wrapper around typeof so that it can be used in
contexts that require a template.

typeof cannot be used directly with templates such as staticMap, because
we don't have the template equivalent of lambda functions. TypeOf is
therefore a template wrapper around typeof so that it can be used in
contexts that require a template.
@PetarKirov
Copy link
Member

How about:

/++
    Returns:
        The type of the expression `exp` or `T`
        if the argument is already a type.

    Useful when combined with higher-order templates
    such as $(REF, staticMap, std, meta).
 +/
alias TypeOf(alias exp) = typeof(exp);

/// ditto
alias TypeOf(T) = T;

unittest
{
    import std.meta : AliasSeq, staticMap;

    static assert(is(TypeOf!1 == int));
    static assert(is(TypeOf!"hello" == string));
    static assert(is(TypeOf!(17 + 42) == int));

    int[] arr;
    static assert(is(TypeOf!arr == int[]));

    static assert(is(staticMap!(TypeOf, 1, true, "hello") ==
        AliasSeq!(int, bool, string)));

    struct S {}
    static assert(is(staticMap!(TypeOf, 1, true, int, bool, short, S.init, S) ==
        AliasSeq!(int, bool, int, bool, short, S, S)));
}

In other words:

  • Short templated alias and enum declarations don't need the full template syntax
  • Add identity template overload so you can get the types of a heterogeneous alias sequences, that include types in addition to expressions

@jmdavis
Copy link
Member Author

jmdavis commented Nov 24, 2016

It doesn't return anything, so having a "returns" section just seems wrong to me. The docs what it does, and the examples show it.

I could reduce it to the one-liner, and if folks want me to, that's fine, but I don't see much benefit. It works either way.

Regardless, I completely disagree with having it work with stuff that typeof doesn't work with. The whole point was to turn typeof into a template. And in general, mixing types and expressions in the same AliasSeq is a bad idea except when you're using it to as a template argument list, in which case, you wouldn't be doing stuff like staticMap on it, and something like TypeOf would not be appropriate.

@PetarKirov
Copy link
Member

PetarKirov commented Nov 25, 2016

It doesn't return anything, so having a "returns" section just seems wrong to me. The docs what it does, and the examples show it.

I don't have a strong opinion about this, though the templates in std.meta are often called meta-functions and naturally each function returns something. E.g. type functions (like std.traits.Unqual) accept types and return types. Similarly, templateNot is meta-function that takes a template and returns a template. Yes, that's accomplished through an alias declaration, but that's just a syntax detail. I think the function metaphor makes the most sense from the users' point of view.

I could reduce it to the one-liner, and if folks want me to, that's fine, but I don't see much benefit. It works either way.

In general, I believe that code should pay for itself. I.e. there shouldn't be any unnecessary code. Shorter code is most often easier to read and understand.

Regardless, I completely disagree with having it work with stuff that typeof doesn't work with. The whole point was to turn typeof into a template. And in general, mixing types and expressions in the same AliasSeq is a bad idea except when you're using it to as a template argument list, in which case, you wouldn't be doing stuff like staticMap on it, and something like TypeOf would not be appropriate.

I disagree. A generic library should not impose arbitrary restrictions on its users. This is like saying that Alias should support not support types, except for built-in ones, because you can already alias them without the need for Alias. It may not sound immediately useful, but comes up in generic code often enough, that I think it would be useful. Consider for example:

static assert(SizeOf!(3 + 4) == 4);
static assert(SizeOf!int == 4);
static assert(SizeOf!"asd" == 2 * size_t.sizeof);
static assert(SizeOf!'c' == 1);

template SizeOf(T...)
    if (T.length == 1)
{
    enum SizeOf = TypeOf!(T[0]).sizeof;
}

Why would the writer of SizeOf need to special case types from expressions? This is a problem that is better fixed in one place, rather than duplicating boilerplate everywhere.

@jmdavis
Copy link
Member Author

jmdavis commented Nov 25, 2016

I disagree. A generic library should not impose arbitrary restrictions on its users.

There is nothing arbitrary about the restriction. typeof gives you the type of its argument. TypeOf does the same. It makes no sense to get the type of a type.

@dlang-bot dlang-bot removed the stalled label Jun 22, 2017
@wilzbach wilzbach added the @andralex Approval from Andrei is required label Jul 10, 2017
@MetaLang
Copy link
Member

MetaLang commented Jul 21, 2017

I agree that we should not change how typeof works, as that would likely be a huge breaking change. The best we can do is add a helper template like this.

Ping @andralex so we can finally get this merged.

Also @jmdavis please rebase.

@@ -77,6 +77,7 @@
* $(LREF StringTypeOf)
* $(LREF AssocArrayTypeOf)
* $(LREF BuiltinTypeOf)
* $(LREF TypeOf)
Copy link
Member

Choose a reason for hiding this comment

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

Please change to Typeof so it matches the built-in operator.

Copy link
Member Author

Choose a reason for hiding this comment

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

That wouldn't match the naming conventions or how other templates in std.traits are named.

Helper template so that `typeof can be used with templates taking other
templates (such as $(REF, staticMap, std, meta)).
+/
template TypeOf(alias exp)
Copy link
Member

Choose a reason for hiding this comment

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

This is better implemented as @ZombineDev described.

Copy link
Member Author

Choose a reason for hiding this comment

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

Have TypeOf work on types even though typeof doesn't work on types? As I said, I don't think that it makes any sense to get the type of a type, and I'm not going to make that change.

@@ -5074,6 +5075,31 @@ template BuiltinTypeOf(T)
else static assert(0);
}

/++
Helper template so that `typeof can be used with templates taking other
templates (such as $(REF, staticMap, std, meta)).
Copy link
Member

Choose a reason for hiding this comment

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

Plenty of other templates in this module use the word "returns", so please add a Returns: section here, as well as Params:. See https://github.com/jmdavis/phobos/blob/95112dc7cabe31ab3b63679d0c8a283d6dfef496/std/traits.d#L2034 for an example.

}

///
unittest
Copy link
Member

Choose a reason for hiding this comment

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

Please add a unit test or two for types as well.

@andralex
Copy link
Member

How often do we need staticMap with typeof in the standard library? The savings should be considerable; otherwise we can just leave the one-liner as an example.

@jmdavis
Copy link
Member Author

jmdavis commented Jul 21, 2017

How often do we need staticMap with typeof in the standard library? The savings should be considerable; otherwise we can just leave the one-liner as an example.

I don't follow. Do you want the implementation changed in some way, or do you disagree with the fact that staticMap is used in an example? Being able to use this template with something like staticMap is pretty much the whole reason to create it. If you're not passing this template to another template like staticMap, then you'd just use typeof directly.

@andralex
Copy link
Member

@jmdavis the simplest question is: since it's a trivial one-liner, why not just ask the user to do it if they have a need? We need to have a case for high frequency of use.

Also: copying typeof 100% seems a dubious goal. It's like doing our very best to not be creative. How about we do something better and more general that includes what typeof does, and also more wonderful things? I'm thinking e.g. TypesOf!(a, b, c) returns an AliasSeq consisting of the types of a, b, and c. If you only pass one expression, you get its type.

MetaLang added a commit that referenced this pull request Jul 29, 2017
This an alternative implementation to #4920
MetaLang added a commit that referenced this pull request Jul 29, 2017
This an alternative implementation to #4920
MetaLang added a commit that referenced this pull request Jul 29, 2017
This an alternative implementation to #4920
MetaLang added a commit that referenced this pull request Jul 29, 2017
This an alternative implementation to #4920
MetaLang added a commit that referenced this pull request Jul 29, 2017
This an alternative implementation to #4920
MetaLang added a commit that referenced this pull request Jul 29, 2017
This an alternative implementation to #4920
MetaLang added a commit that referenced this pull request Jul 29, 2017
This an alternative implementation to #4920
MetaLang added a commit that referenced this pull request Jul 29, 2017
This an alternative implementation to #4920
MetaLang added a commit that referenced this pull request Jul 29, 2017
This an alternative implementation to #4920
@PetarKirov
Copy link
Member

Closing in favor of #5662

@PetarKirov PetarKirov closed this Jul 31, 2017
MetaLang added a commit that referenced this pull request Oct 18, 2017
Previous PR: #5662. I'm resubmitting this after it was reverted in #5664.

This an alternative implementation to #4920.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@andralex Approval from Andrei is required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants