EC# parser bug: lost 'new' attribute

[jonathanvdc]

I think I encountered an EC# parser bug while I was implementing abstract/virtual/sealed/override/new in ecsc. It's definitely not a deal-breaker bug, but it is somewhat annoying.

This file

public abstract class Base
{
    public Base() { }
    public virtual int g()
    {
        return 2;
    }
}

public class Derived : Base
{
    public Derived() { }
    public new int g()
    {
        return 3;
    }
}

is parsed as:

@[#public, #abstract] #class(Base, #(), {
    @[#public] #cons(@``, Base, #(), {
        });
    @[#public, #virtual] #fn(#int32, g, #(), {
        #return(2);
    });
});
@[#public] #class(Derived, #(Base), {
    @[#public] #cons(@``, Derived, #(), {
        });
    @[#public] #fn(#int32, g, #(), {
        #return(3);
    });
});

The new attribute seems to be gone. This does not modify the program's behavior, but the missing attribute causes ecsc to report the warning below, despite the fact that g is clearly marked new.

Overrides.cs:15:20: warning: member hiding: method 'g' hides a base method. Consider using the 'new' keyword if hiding was intentional. [-Whidden-member]

    public new int g()
                   ^  
remark: hidden method: Overrides.cs:6:24

    public virtual int g()
                       ^  

I'm assuming that this is the parser's fault.

I don't have much time on my hands at the moment - and this bug is not top priority for me right now - but I wouldn't mind taking a stab at fixing this when I have some spare time.

Anyway, can you by any chance reproduce this bug?

Thanks in advance.

[qwertie]

I can confirm this bug. Test suite oddly missed it. Will fix it tomorrow. On Jun 1, 2016 3:22 AM, "Jonathan Van der Cruysse" notifications@github.com wrote:

I think I encountered an EC# parser bug while I was implementing abstract/ virtual/sealed/override/new in ecsc. It's definitely not a deal-breaker bug, but it is somewhat annoying.

This file

public abstract class Base { public Base() { } public virtual int g() { return 2; } } public class Derived : Base { public Derived() { } public new int g() { return 3; } }

is parsed as:

@[#public, #abstract] #class(Base, #(), { @[#public] #cons(@`, Base, #(), { }); @[#public, #virtual] #fn(#int32, g, #(), {

return(2);

});

}); @[#public] #class(Derived, #(Base), { @[#public] #cons(@`, Derived, #(), { }); @[#public] #fn(#int32, g, #(), {

return(3);

});

});

The new attribute seems to be gone. This does not modify the program's behavior, but the missing attribute causes ecsc to report the warning below, despite the fact that g is clearly marked new.

Overrides.cs:15:20: warning: member hiding: method 'g' hides a base method. Consider using the 'new' keyword if hiding was intentional. [-Whidden-member]

public new int g()
               ^

remark: hidden method: Overrides.cs:6:24

public virtual int g()
                   ^

I'm assuming that this is the parser's fault.

I don't have much time on my hands at the moment - and this bug is not top priority for me right now - but I wouldn't mind taking a stab at fixing this when I have some spare time.

Anyway, can you by any chance reproduce this bug?

Thanks in advance.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/qwertie/ecsharp/issues/36, or mute the thread https://github.com/notifications/unsubscribe/AAr_TYrlGCmUkbfqZ5BHvCQ4e1XLD62Eks5qHIqPgaJpZM4Iq3jC .

[jonathanvdc]

That's awesome. Thanks!

BTW, I also discovered that the new attribute seems to remove trailing attributes, as well. So

public new virtual int g()
{
    return 3;
}

is parsed as

@[#public] #fn(#int32, g, #(), {
    #return(3);
});

But this:

public virtual new int g()
{
    return 3;
}

is parsed as

@[#public, #virtual] #fn(#int32, g, #(), {
    #return(3);
});

Maybe that warrants a test of its own.

[qwertie]

Yeah, I noticed that too, and I'll be adding a fairly large series of tests. Parsing new and word attributes in a recursive-descent parser is much harder than it looks and since I wrote my own code for it (taking over from LLLPG) I've fixed a long series of bugs already.

On Fri, Jun 3, 2016 at 3:36 AM, Jonathan Van der Cruysse < notifications@github.com> wrote:

That's awesome. Thanks!

BTW, I also discovered that the new attribute seems to remove trailing attributes, as well. So

public new virtual int g() { return 3; }

is parsed as

@[#public] #fn(#int32, g, #(), {

return(3);

});

But this:

public virtual new int g() { return 3; }

is parsed as

@[#public, #virtual] #fn(#int32, g, #(), {

return(3);

});

Maybe that warrants a test of its own.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/qwertie/ecsharp/issues/36#issuecomment-223398694, or mute the thread https://github.com/notifications/unsubscribe/AAr_TaziMydfwC_7gAMEY_G6Z_N2P_Gmks5qHzDQgaJpZM4Iq3jC .

• David http://qism.blogspot.com

[qwertie]

OK please pull, should be fixed (source only).

On Fri, Jun 3, 2016 at 7:48 AM, David Piepgrass qwertie256@gmail.com wrote:

Yeah, I noticed that too, and I'll be adding a fairly large series of tests. Parsing new and word attributes in a recursive-descent parser is much harder than it looks and since I wrote my own code for it (taking over from LLLPG) I've fixed a long series of bugs already.

On Fri, Jun 3, 2016 at 3:36 AM, Jonathan Van der Cruysse < notifications@github.com> wrote:

That's awesome. Thanks!

BTW, I also discovered that the new attribute seems to remove trailing attributes, as well. So

public new virtual int g() { return 3; }

is parsed as

@[#public] #fn(#int32, g, #(), {

return(3);

});

But this:

public virtual new int g() { return 3; }

is parsed as

@[#public, #virtual] #fn(#int32, g, #(), {

return(3);

});

Maybe that warrants a test of its own.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/qwertie/ecsharp/issues/36#issuecomment-223398694, or mute the thread https://github.com/notifications/unsubscribe/AAr_TaziMydfwC_7gAMEY_G6Z_N2P_Gmks5qHzDQgaJpZM4Iq3jC .

• David http://qism.blogspot.com

• David http://qism.blogspot.com

[jonathanvdc]

I pulled and ran the tests. EcsParserTests.KeywordAttributes covers my use case, and it passes! Thanks for fixing this bug!

I'll close this issue now, and include the patched version of Loyc.Ecs in the ecsc repo as soon as possible.