Quantcast

Proposal: @NamedParameters AST transform

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Proposal: @NamedParameters AST transform

Peter Niederwieser
I'm proposing to add a @NamedParameters transform to the groovy.transform package, in an attempt to support true named parameters in Groovy. This would add Map-based overloads to annotated methods (or methods of annotated classes). The added methods would delegate to the original methods and handle default parameter values correctly (in any position).

Some details that would need to be clarified:

- Which methods should be processed

A global transform which adds named parameters "everywhere" would be expensive in terms of the number of methods created and might change the behavior of existing code. So a more cautious approach is likely needed. One option is to make @NamedParameters a class-level local transform and allow further tailoring along the lines of @NamedParameters(TargetMethods.PUBLIC). Another idea is to also support a command-line switch (say -DnamedParameters=public). This would make it easy to generate named parameters for a whole project/library. (By the way, is it possible to apply a local transform to all classes in a package? This might be handy.)

- How to handle overloads

We likely need a pragmatic solution here. Something like always delegating to the first encountered overload with the maximum number of parameters. But maybe there is a better solution.

A working prototype for @NamedParameters can be found here: https://github.com/pniederw/groovy-extensions

Let me know what you think.

Cheers,
Peter
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Proposal: @NamedParameters AST transform

Jochen Theodorou
Am 19.01.2012 02:43, schrieb Peter Niederwieser:
> I'm proposing to add a @NamedParameters transform to the groovy.transform
> package, in an attempt to support true named parameters in Groovy. This
> would add Map-based overloads to annotated methods (or methods of annotated
> classes). The added methods would delegate to the original methods and
> handle default parameter values correctly (in any position).
[...]
> A working prototype for @NamedParameters can be found here:
> https://github.com/pniederw/groovy-extensions

first about what it does... I have spend only a short look on it as of
yet, but it seems you basically check the parameters of the map given
in. That means:

@NamedParameters
class A {
   def foo(String a, int b) {1}
}

would become

class A {
   def foo(String a, int b) {1}
   def foo(Map m){
     if (!m.containsKey("a") throw MissingNamedParameterException()
     if (!m.containsKey("b") throw MissingNamedParameterException()
     foo(m.a,m.b)
   }
}

Well, you use TernaryExpression instead my if-else here, but
conceptually this makes no big difference. Do I see that right?

> Some details that would need to be clarified:
>
> - Which methods should be processed
>
> A global transform which adds named parameters "everywhere" would be
> expensive in terms of the number of methods created and might change the
> behavior of existing code. So a more cautious approach is likely needed. One
> option is to make @NamedParameters a class-level local transform and allow
> further tailoring along the lines of @NamedParameters(TargetMethods.PUBLIC).
> Another idea is to also support a command-line switch (say
> -DnamedParameters=public). This would make it easy to generate named
> parameters for a whole project/library. (By the way, is it possible to apply
> a local transform to all classes in a package? This might be handy.)

I am against a global transform for that kind of thing, because of the
other overload problem.

> - How to handle overloads
>
> We likely need a pragmatic solution here. Something like always delegating
> to the first encountered overload with the maximum number of parameters. But
> maybe there is a better solution.

Delegating simply to the first one found, or the maximum and such things
can be surprising to the users, I think those are no good solutions. If
you find an overload, then throw a compilation error. In short: don't
allow overloads.

Of course that almost automatically forbids a usage as global transform,
because too much code would no longer compile.

As for the package transform.... I actually don't know, but if not we
should enable that.

bye blackdrag

--
Jochen "blackdrag" Theodorou - Groovy Project Tech Lead
blog: http://blackdragsview.blogspot.com/
german groovy discussion newsgroup: de.comp.lang.misc
For Groovy programming sources visit http://groovy-lang.org


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Proposal: @NamedParameters AST transform

Peter Niederwieser
Jochen Theodorou wrote
first about what it does... I have spend only a short look on it as of
yet, but it seems you basically check the parameters of the map given
in. That means:

@NamedParameters
class A {
   def foo(String a, int b) {1}
}

would become

class A {
   def foo(String a, int b) {1}
   def foo(Map m){
     if (!m.containsKey("a") throw MissingNamedParameterException()
     if (!m.containsKey("b") throw MissingNamedParameterException()
     foo(m.a,m.b)
   }
}

Well, you use TernaryExpression instead my if-else here, but
conceptually this makes no big difference. Do I see that right?
That's correct, except that default values are also handled. In other words,

class A {
  def foo(String a, int b = 3) { 1 }
}

becomes

class A {
  def foo(String a, int b = 3) { 1 }
  def foo(Map m) {
    foo(
      m.containsKey("a") ? m["a"] : throwMissingNamedParameter("foo", "a"),
      m.containsKey("b") ? m["b"] : 3
    )
  }
}

With named parameters, default values become much more useful than they are with positional parameters.

Jochen Theodorou wrote
I am against a global transform for that kind of thing, because of the
other overload problem.
In general I agree. I'd prefer to have zero global transforms in the Groovy distribution. (The existing ones cause some headaches, e.g. in IDEA). However, named parameters are most interesting for public APIs, and should be an either-or decision. In other words, either my API supports named parameters, or it doesn't. It makes less sense to enable them for some classes/methods and not for others (how should the user know?). Given that we can't enable them by default (which I believe is true due to the pragmatic approach), it would be great to have an easy way to enable them for the whole compilation unit or a set of packages.

Jochen Theodorou wrote
Delegating simply to the first one found, or the maximum and such things
can be surprising to the users, I think those are no good solutions. If
you find an overload, then throw a compilation error. In short: don't
allow overloads.
When named parameters are enabled per method, the issue won't arise (except if multiple overloads are enabled, in which case we can certainly throw an exception). When named parameters are enabled per class/package/compilation unit, I wouldn't want to throw an exception. If we can't find a clear enough strategy to handle overloads, then I'd just skip them. One could still select a particular overload by annotating the method.

Thinking more about this, what if base class and derived class are compiled separately? Does Groovy require the derived class to be recompiled when the base class changes (e.g. a method is added), or does it handle this case fine (which would actually pose a problem for overload handling)?

Cheers,
Peter
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Proposal: @NamedParameters AST transform

Jochen Theodorou
Am 24.01.2012 21:56, schrieb Peter Niederwieser:
[...]

> That's correct, except that default values are also handled. In other words,
>
> class A {
>    def foo(String a, int b = 3) { 1 }
> }
>
> becomes
>
> class A {
>    def foo(String a, int b = 3) { 1 }
>    def foo(Map m) {
>      foo(
>        m.containsKey("a") ? m["a"] : throwMissingNamedParameter("foo", "a"),
>        m.containsKey("b") ? m["b"] : 3
>      )
>    }
> }
>
> With named parameters, default values become much more useful than they are
> with positional parameters.

ok, good... but if I have for example foo(Map m, int b=3) I will run
into problems with your helper method, right?

Just looking at the method signature I note the following:
* overloads are possible only using default arguments
* some single methods like foo(Map) are not possible anymore

those speak against a global transform, because they are a restriction
of the language. And if you for example have an abstract super class
that forces you to use overloads or forces you to implement a Map taking
method, you are kind of screwed. Even more so if there is already a
public final foo(Map) method in the super class. That should be rare,
but certainly is possible.

Now mentioning super classes I just noticed, that it starts getting
really interesting if you subclass A to add another foo method. I mean,
you overwrite the old Map method then, making former valid calls
invalid, because your new Map method then uses the new foo signature?

[..]

> Jochen Theodorou wrote
>>
>> Delegating simply to the first one found, or the maximum and such things
>> can be surprising to the users, I think those are no good solutions. If
>> you find an overload, then throw a compilation error. In short: don't
>> allow overloads.
>
> When named parameters are enabled per method, the issue won't arise (except
> if multiple overloads are enabled, in which case we can certainly throw an
> exception). When named parameters are enabled per class/package/compilation
> unit, I wouldn't want to throw an exception. If we can't find a clear enough
> strategy to handle overloads, then I'd just skip them. One could still
> select a particular overload by annotating the method.

yes, the problem though is that this approach is a bit nontransparent to
the user. Though skipping the transformation for that method name is
certainly better than just taking one to whatever criteria.

Enabling them on a per compilation unit is currently not possible. That
is infrastructure we first have to make. We have our compilation
customizers, but for those you have to call a method on the
CompilationUnit instance. If you want that transform for the regular
user, then this is not really what you want. though having him to
compile with for example:

groovyc --customizers=groovyx.transform.NamedParameters

is also not all that nice actually.

Maybe per package level is wide enough as scope?

> Thinking more about this, what if base class and derived class are compiled
> separately? Does Groovy require the derived class to be recompiled when the
> base class changes (e.g. a method is added), or does it handle this case
> fine (which would actually pose a problem for overload handling)?

Groovy requires recompilation of the derived class if you change the
hierarchy for example. It normally does not require recompilation if you
add a method.

bye blackdrag

--
Jochen "blackdrag" Theodorou - Groovy Project Tech Lead
blog: http://blackdragsview.blogspot.com/
german groovy discussion newsgroup: de.comp.lang.misc
For Groovy programming sources visit http://groovy-lang.org


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Proposal: @NamedParameters AST transform

Peter Niederwieser
Jochen Theodorou wrote
ok, good... but if I have for example foo(Map m, int b=3) I will run
into problems with your helper method, right?

Just looking at the method signature I note the following:
* overloads are possible only using default arguments
* some single methods like foo(Map) are not possible anymore
It's a prototype, and I haven't cared about any of the special cases yet.

Jochen Theodorou wrote
those speak against a global transform, because they are a restriction
of the language. And if you for example have an abstract super class
that forces you to use overloads or forces you to implement a Map taking
method, you are kind of screwed. Even more so if there is already a
public final foo(Map) method in the super class. That should be rare,
but certainly is possible.
I certainly don't want the transform to be applied by default. But it might be handy to have a way to apply it for a whole compilation unit. If necessary, this can wait.

Jochen Theodorou wrote
groovyc --customizers=groovyx.transform.NamedParameters

is also not all that nice actually.

Maybe per package level is wide enough as scope?
If --customizers is already implemented, it would be nice to support it. Per-package activation is certainly an alternative. If neither of them is available today, we could start with per-class and see how it goes.

--
Peter Niederwieser
Principal Engineer, Gradleware
http://gradleware.com
Creator, Spock Framework
http://spockframework.org
Twitter: @pniederw
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Proposal: @NamedParameters AST transform

Jochen Theodorou
Am 25.01.2012 14:45, schrieb Peter Niederwieser:

>
> Jochen Theodorou wrote
>>
>> ok, good... but if I have for example foo(Map m, int b=3) I will run
>> into problems with your helper method, right?
>>
>> Just looking at the method signature I note the following:
>> * overloads are possible only using default arguments
>> * some single methods like foo(Map) are not possible anymore
>>
>
> It's a prototype, and I haven't cared about any of the special cases yet.

sure, no problem... but I wonder if the problem can be solved with this
approach in a way that is nice to have. for named arguments I see only
two ways to implement... one is to use runtime information though
annotations and let it influence the method selection accordingly, the
other is with a helper method. We could of course give the helper a
different name to avoid the Map taking method problem... but then the
runtime needs again to know about it and in subclasses we will have
again the logic destroyed easily. The runtime approach is surely more
robust, but has different problems.

[...]

> Jochen Theodorou wrote
>>
>> groovyc --customizers=groovyx.transform.NamedParameters
>>
>> is also not all that nice actually.
>>
>> Maybe per package level is wide enough as scope?
>
> If --customizers is already implemented, it would be nice to support it.
> Per-package activation is certainly an alternative. If neither of them is
> available today, we could start with per-class and see how it goes.

it is not implemented yet, no. when Cedric added customizers we talked
about the command line thing, but we wanted to wait and see if people
really need that before we go that step.

bye blackdrag

--
Jochen "blackdrag" Theodorou - Groovy Project Tech Lead
blog: http://blackdragsview.blogspot.com/
german groovy discussion newsgroup: de.comp.lang.misc
For Groovy programming sources visit http://groovy-lang.org


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Proposal: @NamedParameters AST transform

Cédric Champeau
Also when we talked with Rémi Forax last week, he told us that JDK 8
would add the ability to add metadata to method arguments, which could
greatly help us defining "true" named parameters (for example, combining
annotations to method parameters, then adding bytecode metadata to
method calls).

Cédric

Le 25/01/2012 15:47, Jochen Theodorou a écrit :

> Am 25.01.2012 14:45, schrieb Peter Niederwieser:
>>
>> Jochen Theodorou wrote
>>>
>>> ok, good... but if I have for example foo(Map m, int b=3) I will run
>>> into problems with your helper method, right?
>>>
>>> Just looking at the method signature I note the following:
>>> * overloads are possible only using default arguments
>>> * some single methods like foo(Map) are not possible anymore
>>>
>>
>> It's a prototype, and I haven't cared about any of the special cases
>> yet.
>
> sure, no problem... but I wonder if the problem can be solved with
> this approach in a way that is nice to have. for named arguments I see
> only two ways to implement... one is to use runtime information though
> annotations and let it influence the method selection accordingly, the
> other is with a helper method. We could of course give the helper a
> different name to avoid the Map taking method problem... but then the
> runtime needs again to know about it and in subclasses we will have
> again the logic destroyed easily. The runtime approach is surely more
> robust, but has different problems.
>
> [...]
>> Jochen Theodorou wrote
>>>
>>> groovyc --customizers=groovyx.transform.NamedParameters
>>>
>>> is also not all that nice actually.
>>>
>>> Maybe per package level is wide enough as scope?
>>
>> If --customizers is already implemented, it would be nice to support it.
>> Per-package activation is certainly an alternative. If neither of
>> them is
>> available today, we could start with per-class and see how it goes.
>
> it is not implemented yet, no. when Cedric added customizers we talked
> about the command line thing, but we wanted to wait and see if people
> really need that before we go that step.
>
> bye blackdrag
>


--
Cédric Champeau
SpringSource - A Division Of VMware
http://www.springsource.com/
http://twitter.com/CedricChampeau


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Proposal: @NamedParameters AST transform

Guillaume Laforge-4
I'd be inclined to wait and see what will come up with JDK 8 to make a final call on this topic (rather than implementing something in a sub-optimal way when something will be supported natively in later JDKs)

On Wed, Jan 25, 2012 at 15:53, Cédric Champeau <[hidden email]> wrote:
Also when we talked with Rémi Forax last week, he told us that JDK 8 would add the ability to add metadata to method arguments, which could greatly help us defining "true" named parameters (for example, combining annotations to method parameters, then adding bytecode metadata to method calls).

Cédric

Le 25/01/2012 15:47, Jochen Theodorou a écrit :

Am 25.01.2012 14:45, schrieb Peter Niederwieser:

Jochen Theodorou wrote

ok, good... but if I have for example foo(Map m, int b=3) I will run
into problems with your helper method, right?

Just looking at the method signature I note the following:
* overloads are possible only using default arguments
* some single methods like foo(Map) are not possible anymore


It's a prototype, and I haven't cared about any of the special cases yet.

sure, no problem... but I wonder if the problem can be solved with this approach in a way that is nice to have. for named arguments I see only two ways to implement... one is to use runtime information though annotations and let it influence the method selection accordingly, the other is with a helper method. We could of course give the helper a different name to avoid the Map taking method problem... but then the runtime needs again to know about it and in subclasses we will have again the logic destroyed easily. The runtime approach is surely more robust, but has different problems.

[...]
Jochen Theodorou wrote

groovyc --customizers=groovyx.transform.NamedParameters

is also not all that nice actually.

Maybe per package level is wide enough as scope?

If --customizers is already implemented, it would be nice to support it.
Per-package activation is certainly an alternative. If neither of them is
available today, we could start with per-class and see how it goes.

it is not implemented yet, no. when Cedric added customizers we talked about the command line thing, but we wanted to wait and see if people really need that before we go that step.

bye blackdrag



--
Cédric Champeau
SpringSource - A Division Of VMware
http://www.springsource.com/
http://twitter.com/CedricChampeau



---------------------------------------------------------------------
To unsubscribe from this list, please visit:

  http://xircles.codehaus.org/manage_email





--
Guillaume Laforge
Groovy Project Manager
Head of Groovy Development at SpringSource
http://www.springsource.com/g2one
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Proposal: @NamedParameters AST transform

Cédric Champeau
In reply to this post by Peter Niederwieser

Jochen Theodorou wrote
groovyc --customizers=groovyx.transform.NamedParameters

is also not all that nice actually.

Maybe per package level is wide enough as scope?

If --customizers is already implemented, it would be nice to support it.
Per-package activation is certainly an alternative. If neither of them is
available today, we could start with per-class and see how it goes.

--customizers is not implemented yet, but such an option would be very poor as compared to what can be done with a compiler configuration. For example, some AST transformations require parameters, or you could also want to add default imports. For that, I am more thinking of a compiler configuration file which would be nothing more than a groovy script allowing you to update the CompilerConfiguration before the compilation process is launched. For example:

compilerconfig.groovy

config.with {
    addCompilationCustomizers (new ASTTransformationCustomizer(NamedParameters))
}

which could be shorten a little more with implicit "with":

addCompilationCustomizers (new ASTTransformationCustomizer(NamedParameters))

Another advantage I see is that those configuration files could be reused by the Groovy Console too.

What do you think?
Loading...