Several places in Phobos I've noticed tests like if(is(T==class)) that check whether T is a class type.  I suspect it is usually meant for these tests to pass if T is an interface as well, but this is not the case.  Doing

  grep 'is(\w\+\s*==\s*class)' *.d

on the Phobos sources reveals many cases where the test for is(T==interface) has most likely been forgotten.  This should be fixed, and I think the best way is to add the following to std.traits:

  template isClass(T)
  {
      enum isClass = is(T == class) || is(T == interface);
  }

Most instances of is(T==class) in Phobos can then be replaced with
isClass!T.

What say you?

-Lars

Vote++.  It's also more readable.

On Thu, May 19, 2011 at 11:11 AM, Lars Tandle Kyllingstad < lars at kyllingen.net> wrote:

> Several places in Phobos I've noticed tests like if(is(T==class)) that check whether T is a class type.  I suspect it is usually meant for these tests to pass if T is an interface as well, but this is not the case.  Doing
>
>  grep 'is(\w\+\s*==\s*class)' *.d
>
> on the Phobos sources reveals many cases where the test for is(T==interface) has most likely been forgotten.  This should be fixed, and I think the best way is to add the following to std.traits:
>
>  template isClass(T)
>  {
>      enum isClass = is(T == class) || is(T == interface);
>  }
>
> Most instances of is(T==class) in Phobos can then be replaced with
> isClass!T.
>
> What say you?
>
> -Lars
>
> _______________________________________________
> phobos mailing list
> phobos at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/phobos
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.puremagic.com/pipermail/phobos/attachments/20110519/e843783b/attachment.html>

Can you give some examples of where it should be fixed?

-Steve




>________________________________
>From: Lars Tandle Kyllingstad <lars at kyllingen.net>
>To: Phobos mailing list <phobos at puremagic.com>
>Sent: Thursday, May 19, 2011 11:11 AM
>Subject: [phobos] isClass!T
>
>Several places in Phobos I've noticed tests like if(is(T==class)) that check whether T is a class type.? I suspect it is usually meant for these tests to pass if T is an interface as well, but this is not the case.? Doing
>
>? grep 'is(\w\+\s*==\s*class)' *.d
>
>on the Phobos sources reveals many cases where the test for is(T==interface) has most likely been forgotten.? This should be fixed, and I think the best way is to add the following to std.traits:
>
>? template isClass(T)
>? {
>? ? ? enum isClass = is(T == class) || is(T == interface);
>? }
>
>Most instances of is(T==class) in Phobos can then be replaced with
>isClass!T.
>
>What say you?
>
>-Lars
>
>_______________________________________________
>phobos mailing list
>phobos at puremagic.com
>http://lists.puremagic.com/mailman/listinfo/phobos
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.puremagic.com/pipermail/phobos/attachments/20110519/ec490617/attachment-0001.html>

Well I kind of dislike that isClass is really isClassOrInterface.

Andrei

On 5/19/11 10:11 AM, Lars Tandle Kyllingstad wrote:
> Several places in Phobos I've noticed tests like if(is(T==class)) that check whether T is a class type.  I suspect it is usually meant for these tests to pass if T is an interface as well, but this is not the case.  Doing
>
>    grep 'is(\w\+\s*==\s*class)' *.d
>
> on the Phobos sources reveals many cases where the test for is(T==interface) has most likely been forgotten.  This should be fixed, and I think the best way is to add the following to std.traits:
>
>    template isClass(T)
>    {
>        enum isClass = is(T == class) || is(T == interface);
>    }
>
> Most instances of is(T==class) in Phobos can then be replaced with
> isClass!T.
>
> What say you?
>
> -Lars
>
> _______________________________________________
> phobos mailing list
> phobos at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/phobos

I ran into it with std.traits.hasMember!(T, "foo"), which always
evaluates to false if T is an interface.
https://github.com/D-Programming-Language/phobos/blob/master/std/traits.d#L1537

Other examples include:

std.conv.emplace(), https://github.com/D-Programming-Language/phobos/blob/master/std/conv.d#L4044 https://github.com/D-Programming-Language/phobos/blob/master/std/conv.d#L4064 https://github.com/D-Programming-Language/phobos/blob/master/std/conv.d#L4158

std.traits.StringTypeOf, https://github.com/D-Programming-Language/phobos/blob/master/std/traits.d#L3181

std.typecons.tuple.toString (I think), https://github.com/D-Programming-Language/phobos/blob/master/std/typecons.d#L508

std.typecons.RefCounted, https://github.com/D-Programming-Language/phobos/blob/master/std/typecons.d#L2272

std.variant.VariantN.opAssign (I think), https://github.com/D-Programming-Language/phobos/blob/master/std/variant.d#L520

-Lars



On Thu, 2011-05-19 at 08:17 -0700, Steve Schveighoffer wrote:
> Can you give some examples of where it should be fixed?
> 
> 
> -Steve
> 
> 
> 
>         ______________________________________________________________
>         From: Lars Tandle Kyllingstad <lars at kyllingen.net>
>         To: Phobos mailing list <phobos at puremagic.com>
>         Sent: Thursday, May 19, 2011 11:11 AM
>         Subject: [phobos] isClass!T
> 
>         Several places in Phobos I've noticed tests like
>         if(is(T==class)) that
>         check whether T is a class type.  I suspect it is usually
>         meant for
>         these tests to pass if T is an interface as well, but this is
>         not the
>         case.  Doing
> 
>           grep 'is(\w\+\s*==\s*class)' *.d
> 
>         on the Phobos sources reveals many cases where the test for
>         is(T==interface) has most likely been forgotten.  This should
>         be fixed,
>         and I think the best way is to add the following to
>         std.traits:
> 
>           template isClass(T)
>           {
>               enum isClass = is(T == class) || is(T == interface);
>           }
> 
>         Most instances of is(T==class) in Phobos can then be replaced
>         with
>         isClass!T.
> 
>         What say you?
> 
>         -Lars
> 
>         _______________________________________________
>         phobos mailing list
>         phobos at puremagic.com
>         http://lists.puremagic.com/mailman/listinfo/phobos
> 
> 
> 
> _______________________________________________
> phobos mailing list
> phobos at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/phobos