In the thread "Slow performance compared to C++, ideas?" Manu is arguing that methods should be non-virtual by default. The problem he is having is that developers will forget to declare methods as final.

This issue got me think if there's a way to automatically detect that a method is virtual that shouldn't be.

If one modifies druntime to take advantage out of RTInfo. If I understand everything correctly RTInfo will be instantiated once for each user defined type. With that and the help of a UDA you can check that all methods that should be final really are final. Something like:

struct virtual {} // the UDA

class Foo
{
    void a () {} // static assert, not declared as @virtual
    @virtual void b () {} // ok
    @virtual final void c () {} // static assert, declared as final and @virtual
    final d () {} // ok
}

checkVirtual!(Foo);

import std.typetuple;

void checkVirtual (T) ()
{
    static if (!is(T == class))
        return;

    foreach (m ; __traits(derivedMembers, T))
    {
        alias TypeTuple!(__traits(getAttributes, mixin("T." ~ m))) attrs;
        enum methodName = T.stringof ~ "." ~ m.stringof;

        static if (staticIndexOf!(virtual, attrs) != -1)
        {
            static if (!__traits(isVirtualMethod, mixin("T." ~ m)))
                static assert (false, "Method " ~ methodName ~ " marked as @virtual is not virtual");
        }

        else
        {
            static if (__traits(isVirtualMethod, mixin("T." ~ m)))
                static assert (false, "Method " ~ methodName ~ " is virtual but not marked as @virtual");
        }
    }
}

-- 
/Jacob Carlborg

Jacob Carlborg:

> struct virtual {} // the UDA
>
> class Foo
> {
>     void a () {} // static assert, not declared as @virtual
>     @virtual void b () {} // ok
>     @virtual final void c () {} // static assert, declared as final and @virtual
>     final d () {} // ok
> }
>
> checkVirtual!(Foo);

Is it possible to also write:

checkVirtual!myModuleName;

And verify all the classes in a module?

Bye,
bearophile

On 2013-06-04 12:45, bearophile wrote:

> Is it possible to also write:
>
> checkVirtual!myModuleName;
>
> And verify all the classes in a module?

Not with this version. I tried that first but I couldn't get __traits(allMembers) to work on a module. I think by overriding RTInfo it will be more automatic.

-- 
/Jacob Carlborg

Jacob Carlborg:

>> Is it possible to also write:
>>
>> checkVirtual!myModuleName;
>>
>> And verify all the classes in a module?

Or something like:

checkVirtual!(__MODULE__);


> Not with this version. I tried that first but I couldn't get __traits(allMembers) to work on a module. I think by overriding RTInfo it will be more automatic.

If there are some things that don't give you enough static introspection to perform that, then it's worth adding that missing static introspection to D language.

Bye,
bearophile

On 4 June 2013 18:23, Jacob Carlborg <doob@me.com> wrote:

> In the thread "Slow performance compared to C++, ideas?" Manu is arguing that methods should be non-virtual by default. The problem he is having is that developers will forget to declare methods as final.
>
> This issue got me think if there's a way to automatically detect that a method is virtual that shouldn't be.
>
> If one modifies druntime to take advantage out of RTInfo. If I understand everything correctly RTInfo will be instantiated once for each user defined type. With that and the help of a UDA you can check that all methods that should be final really are final. Something like:
>
> struct virtual {} // the UDA
>
> class Foo
> {
>     void a () {} // static assert, not declared as @virtual
>     @virtual void b () {} // ok
>     @virtual final void c () {} // static assert, declared as final and
> @virtual
>     final d () {} // ok
> }
>
> checkVirtual!(Foo);
>
> import std.typetuple;
>
> void checkVirtual (T) ()
> {
>     static if (!is(T == class))
>         return;
>
>     foreach (m ; __traits(derivedMembers, T))
>     {
>         alias TypeTuple!(__traits(**getAttributes, mixin("T." ~ m)))
> attrs;
>         enum methodName = T.stringof ~ "." ~ m.stringof;
>
>         static if (staticIndexOf!(virtual, attrs) != -1)
>         {
>             static if (!__traits(isVirtualMethod, mixin("T." ~ m)))
>                 static assert (false, "Method " ~ methodName ~ " marked as
> @virtual is not virtual");
>         }
>
>         else
>         {
>             static if (__traits(isVirtualMethod, mixin("T." ~ m)))
>                 static assert (false, "Method " ~ methodName ~ " is
> virtual but not marked as @virtual");
>         }
>     }
> }
>
> --
> /Jacob Carlborg
>

This is a great idea. If final-by-default is ultimately rejected, I'll
definitely try this out.
That said, it only solves half the problem, that is, internal programmers
forgetting to mark their functions final (and assuming they also remember
'checkVirtual!' in their moules).
It offers nothing to address the 3rd party library problem.

On 2013-06-04 13:13, bearophile wrote:

> If there are some things that don't give you enough static introspection
> to perform that, then it's worth adding that missing static
> introspection to D language.

It actually works. My mistake. Here's a version that works with modules as well:

void checkVirtual (alias T) ()
{
    static if (is(T == class))
    {
        foreach (m ; __traits(derivedMembers, T))
        {
            alias TypeTuple!(__traits(getAttributes, mixin("T." ~ m))) attrs;
            enum methodName = T.stringof ~ "." ~ m.stringof;

            static if (staticIndexOf!(virtual, attrs) != -1)
            {
                static if (!__traits(isVirtualMethod, mixin("T." ~ m)))
                    static assert (false, "Method " ~ methodName ~ " marked as @virtual is not virtual");
            }

            else
            {
                static if (__traits(isVirtualMethod, mixin("T." ~ m)))
                    static assert (false, "Method " ~ methodName ~ " is virtual but not marked as @virtual");
            }
        }
    }

    else
    {
        foreach (m ; __traits(allMembers, T))
        {
            mixin("alias " ~ m ~ " member;");
            static if (is(member == class))
                checkVirtual!(member);
        }
    }

}

-- 
/Jacob Carlborg

On 2013-06-04 13:26, Manu wrote:

> This is a great idea. If final-by-default is ultimately rejected, I'll
> definitely try this out.
> That said, it only solves half the problem, that is, internal
> programmers forgetting to mark their functions final (and assuming they
> also remember 'checkVirtual!' in their moules).
> It offers nothing to address the 3rd party library problem.

Yes, exactly. If you override RTInfo in druntime I think it will work automatically. Currently RTInfo looks like this:

template RTInfo(T)
{
    enum RTInfo = null;
}

Override it to look like this:

template RTInfo(T)
{
    enum RTInfo = checkVirtual!(T);
}

If this doesn't work you could create a tool which imports all modules in the source code and calls the function for all modules.

-- 
/Jacob Carlborg

Jacob Carlborg:

> struct virtual {} // the UDA
>
> class Foo
> {
>     void a () {} // static assert, not declared as @virtual
>     @virtual void b () {} // ok
>     @virtual final void c () {} // static assert, declared as final and @virtual
>     final d () {} // ok
> }
>
> checkVirtual!(Foo);

My original purposes for UDAs was to extend the type system, as
you have essentially done here. But to do that well enough the
attributes need to be more active.

An idea:

struct Small(T) if (T.sizeof <= 16) {}

@Small struct Bar {
      int x;
}


Here Small get instantiated with T == Bar, and its constraint
performs the active compile-time test.

Bye,
bearophile

Awesome! But I miss the filename and the line number where the error happened.

On 2013-06-04 15:22, Namespace wrote:
> Awesome! But I miss the filename and the line number where the error
> happened.

Hmm, I wonder if that's possible to get.

-- 
/Jacob Carlborg