lexer bug about p4c HOT 15 CLOSED

The example can be made even more devious by changing f's return type to x:

extern x f<x>();

from p4c.

The second problem isn't any harder than the first one.
Unfortunately the grammar becomes ambiguous if we allow a name instead of a nonTypeName for a typeParameter. @ChrisDodd: do you have any suggestion on how to fix this?
Java has a really ugly syntax for this issue.

from p4c.

I see a few options:

1. Accept this "bug".
2. Continue massaging the lexer and parser, if possible. I defer to @ChrisDodd on this.
3. Adopt a different syntax for type parameters to methods -- e.g., Java's hideous (I agree) notation or maybe Scala's with square brackets.
4. Rely on type inference exclusively, as in C#. I don't know how this would affect programs though -- I can easily imagine there are scenarios where we need to specify specific type arguments.
5. Switch to something more powerful like a GLR parser?

-N

from p4c.

The problematic case that's really hard to parse is

header x {}
extern foo {
    foo<x>(x a);  // a templated constructor
}

We can disallow redeclaring types in this one case (allowing redeclaring types as type parameters in other contexts) with a fairly small grammar change; this case is problematic as you don't know that this is a ctor decl with a typeParameterList and not a (return) type with a typeArgumentList until you get to the ( after the type list.

from p4c.

Ooh, that is devious.

-N

On Wed, Oct 19, 2016 at 12:20 PM, Chris Dodd [email protected]
wrote:

The problematic case that's really hard to parse is

header x {}
extern foo {
foo(x a); // a templated constructor
}

We can disallow redeclaring types in this one case (allowing redeclaring
types as type parameters in other contexts) with a fairly small grammar
change; this case is problematic as you don't know that this is a ctor decl
with a typeParameterList and not a (return) type with a typeArgumentList
until you get to the ( after the type list.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#108 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABwi0kpsWSS5scGShz6MFy3epNdhl0Rbks5q1m2RgaJpZM4KaQim
.

from p4c.

Actually we don't allow type parameters on constructors; if you need type parameters on a constructor, they have to be on the entire object.

extern E {  E<T>();  } // illegal
extern E<T> { E(); } // legal

from p4c.

Well, the implementation certainly allows it -- the rule for
methodPrototype at line 526 for the constructor includes optional type
parameters. Remove those and the conflict goes away.

On Wed, Oct 19, 2016 at 1:14 PM, mbudiu-vmw [email protected]
wrote:

Actually we don't allow type parameters on constructors; if you need type
parameters on a constructor, they have to be on the entire object.

extern E { E(); } // illegal
extern E { E(); } // legal

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#108 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AD4c8XPOoCismrA1NeTe0bX8_t_fs0ENks5q1nougaJpZM4KaQim
.

from p4c.

As does the spec (although presumably this was generated from the Bison grammar)...

methodPrototype
    : functionPrototype ';'
    | TYPE optTypeParameters '(' parameterList ')' ';' 
    ;

from p4c.

The proposed fix causes one test failure -- p4_16_errors/generic_e/p4, which is just:

extern X<D>
{
   void f<D>(in D d); // bug: D shadows D
}

The comment is correct in that there are two Ds in two different scopes, and the inner one shadows the outer one, but I'm not certain that it should be an error. It was previously giving a syntax error, which is definitely wrong (this is syntactically correct), but should it be a semantic error to have shadowing here? If so, we need a specific semantic check.

from p4c.

If type parameter declarations are binders, I might emit a warning but
would not treat this as an error...

-N

On Wed, Oct 19, 2016 at 4:17 PM, Chris Dodd [email protected]
wrote:

The proposed fix causes one test failure -- p4_16_errors/generic_e/p4,
which is just:

extern X
{
void f(in D d); // bug: D shadows D
}

The comment is correct in that there are two Ds in two different scopes,
and the inner one shadows the outer one, but I'm not certain that it should
be an error. It was previously giving a syntax error, which is definitely
wrong (this is syntactically correct), but should it be a semantic error to
have shadowing here? If so, we need a specific semantic check.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#108 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABwi0uXPNTOnuq9-YyOwqIQWdzfx-TM4ks5q1qUmgaJpZM4KaQim
.

from p4c.

Since extern declarations are part of the architecture, and they contain nothing but method declarations, I think that shadowing a type parameter is a really a bug in the library's design, which will confuse users. It's bad style, and it should be discouraged strongly.

from p4c.

Another view is that for all binders we morally work up to alpha
equivalence -- that is, this snippet is equivalent to

extern X
{

void f(in D d);
}

or equivalently

extern X
{

void f(in E d);
}

or equivalently

extern X
{

void f(in B d);
}

from p4c.

@mbudiu-vmw when you have a moment could you clarify "Actually we don't allow type parameters on constructors; if you need type parameters on a constructor, they have to be on the entire object." We should fix the spec if this is what's wanted...

from p4c.

At least a partial fix has made it to the spec: the grammar does not allow type parameters on a constructor. I don't know if the text emphasizes it.

from p4c.

Aha. I see it now. Thanks.

from p4c.