lexer bug

[jnfoster]

This program compiles successfully using p4test

package Pck();
extern void f<x>();
Pck() main;

while this program fails,

package Pck();
header x {}
extern void f<x>();
Pck() main;

with an error:

syntax error, unexpected TYPE
extern void f<x
              ^
error: 1 errors encountered, aborting compilation

Typically type parameters are binders so one wouldn't expect a prior declaration to affect whether the declaration of f is syntactically valid or not.

[jnfoster]

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

extern x f<x>();

[mihaibudiu]

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.

[jnfoster]

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

[ChrisDodd]

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.

[jnfoster]

Ooh, that is devious.

-N

On Wed, Oct 19, 2016 at 12:20 PM, Chris Dodd notifications@github.com
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
.

[mihaibudiu]

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

[ChrisDodd]

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 notifications@github.com
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
.

[jnfoster]

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

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

[ChrisDodd]

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.

[jnfoster]

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 notifications@github.com
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
.

[mihaibudiu]

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.

[jnfoster]

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);
}

and so on, all of which are totally sensible. Hence, my preference for the
warning.

On Wed, Oct 19, 2016 at 4:24 PM, mbudiu-vmw notifications@github.com
wrote:

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.

—
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/ABwi0kEdocExfxhOPnDQn6gPWzYK0Kviks5q1qaagaJpZM4KaQim
.

[jnfoster]

@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...

[mihaibudiu]

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.

[jnfoster]

Aha. I see it now. Thanks.