Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[proposal] Syntactic sugar for constructors #129

Closed
cole-barefoot opened this issue Mar 24, 2017 · 9 comments
Closed

[proposal] Syntactic sugar for constructors #129

cole-barefoot opened this issue Mar 24, 2017 · 9 comments

Comments

@cole-barefoot
Copy link
Contributor

This proposal refines Item 1 in #125.

Problem

Names of parser/control definitions and instances must be distinct. When a definition is instantiated only once, this leads to boilerplate code in the beginning of a block:

// Control definitions.
control c1(...) { ... }
control c2(...) { ... }
control c3(...) { ... }

control main_c(...) {
    // Control instantiations.
    c1() do_c1;
    c2() do_c2;
    c3() do_c3;
    apply {
        do_c1.apply(...);
        do_c2.apply(...);
        do_c3.apply(...);
    }
}

V1Switch<H, M>(..., main_c(), ...) main;

For large programs, eg. switch.p4, there may be many controls instantiated once in a top-level control, and the list of control instantiations---with names like "do_X"---can be quite long.

This example uses controls, but the same problem arises for parsers.

Proposal

Add syntactic sugar to ease the case where a parser/control is only instantiated once. With no constructor parameters, this:

control c1(...) { ... } 

is considered equivalent to this:

control c1_t(...)() { ... }
c1_t() c1;

where the name c1_t is not used elsewhere in the program.

Examples

Under this proposal, there would be three ways to define parsers/controls.

// No constructor arguments, so c1 can be used as a literal in later controls.
control c1(...) { ... } 

// With constructor arguments, objects have to be instantiated by data constructor.
control c2(...)() { ... }
control c3(...)(...) { ... }

control c4 ( ... ) {
    c2() my_c2;
    c3(...) my_c3;
    apply {
        c1.apply(...);
        my_c2.apply(...);
        my_c3.apply(...);
    }
}

The first example would now look like:

// Control definitions.
control c1(...) { ... }
control c2(...) { ... }
control c3(...) { ... }

control main_c(...) {
    apply {
        c1.apply(...);
        c2.apply(...);
        c3.apply(...);
    }
}

V1Switch<H, M>(..., main_c(), ...) main;
@mihaibudiu
Copy link
Contributor

mihaibudiu commented Mar 26, 2017

I would rephrase this the other way around, which has the exact same effect: it is the application of a type which introduces a new instantiation. I.e.

control callee() { ... }

control caller() {
   apply { callee.apply(); }
}

is the same as

control caller() {
   @name("callee") callee() calle_instance;
   apply { callee_instance.apply(); }
}

callee_instance is a fresh name.
Note that the @name annotation ensures that the control-plane API remains unchanged.

The proposal has also to explain what happens when a control is invoked twice:

control caller() {
   apply { callee.apply(); callee.apply(); }
}

This could be interpreted in two ways (or it could be forbidden):

control caller() {
   callee() calle_instance;
   apply { callee_instance.apply(); callee_instance.apply(); }
}

or

control caller() {
   callee() calle_instance1;
   callee() calle_instance2;
   apply { callee_instance1.apply(); callee_instance2.apply(); }
}

@sethfowler
Copy link

I haven't thought through your suggestion, Mihai, but regarding Cole's original proposal:

This might feel a bit less magic if there were explicit syntax to specify whether a control needs to be instantiated. I'm not a fan of the terminology, but static has pretty wide acceptance and understanding at this point. One can imagine writing the control definitions in your proposal example as:

static control c1(...) { ... }
static control c2(...) { ... }
static control c3(...) { ... }

I expect the vast majority of controls to be "static" in this sense, so perhaps it would be better to require some kind of more explicit syntax to indicate that the control does need to be instantiated, but it's not obvious to me what would be readable in that case, especially not without larger changes in the language. (class control or control class seem to be in the right ballpark, but they imply more language features than we actually offer at present.)

@mihaibudiu
Copy link
Contributor

To be honest I don't really understand what static is supposed to mean.
everything has to be instantiated to exist in P4. It's just that instantiations (aka constructors) are all evaluated at compile-time.

The annotation approach has multiple other benefits:

  • it does not pollute the grammar with additional syntax
  • it applies to all kinds of elements that accept annotations, e.g., controls, parsers, actions, tables, keys, etc.

@jnfoster
Copy link
Collaborator

I appreciate the effort to eliminate syntactic confusion, but I agree with @mbudiu-vmw. I wouldn't want to use a keyword like static that is already used to mean various things in other languages, and if we did add it, I would want it to apply uniformly to declarations not just to controls.

@cole-barefoot
Copy link
Contributor Author

I'd like to propose tabling this and revisiting for a later version of the language. There are two reasons. First, the implementation turns out to be tricky, because we need to allow type names to be used as instances but also as types. For example:

control sugar(...) { ... }
control my_constructor(...)(sugar s) // "sugar" used as a type name
{ ... }
my_constructor(sugar) c; // "sugar" used as an instance

Second, after working through some examples and writing a PR for this change against the spec, I find the syntax more confusing than convenient. Type names refer to types, except when () are omitted at the type definition, after which that type name also refers to an instance of that type.

We could surely make this work. However, in the short term, I would rather err on the side of a simpler language and implementation---with slightly more verbose P4 programs---rather than risk introducing bugs for a convenience feature.

@mihaibudiu
Copy link
Contributor

This is a useful proposal, so we can simplify it.
Don't change any rules for declaring controls, but just allow a control to be applied directly, without instantiation. This will cover 90% of the use cases, and it looks a lot like P4-14.

@cole-barefoot
Copy link
Contributor Author

I like this idea. I'll amend the PR for the spec and finish the implementation.

mihaibudiu pushed a commit to mihaibudiu/p4c-clone that referenced this issue Apr 27, 2017
cole-barefoot added a commit that referenced this issue Apr 27, 2017
Add language defining the constructor syntactic sugar in #129.
ChrisDodd pushed a commit to p4lang/p4c that referenced this issue Apr 28, 2017
* Fix for issue #537

* Implement spec proposal p4lang/p4-spec#129

* Make it work for parsers too
@jnfoster
Copy link
Collaborator

jnfoster commented May 8, 2017

Can we close this now?

@jnfoster
Copy link
Collaborator

jnfoster commented May 8, 2017

Closing after discussion with Cole.

@jnfoster jnfoster closed this as completed May 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants