[jira] [Comment Edited] (GROOVY-7522) TupleConstructor shouldn't overwrite existing constructors

["Paul King (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/GROOVY-7522?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14641533#comment-14641533 ] 

Paul King edited comment on GROOVY-7522 at 7/25/15 10:54 AM:
-------------------------------------------------------------

Your understanding is correct that this is an edge case; the problem occurs because you are providing one empty constructor. You are also correct in your assessment that we can't change from {{> 1}} to {{> 0}} because of stubs.

In theory, we could make it smarter to write as you say "just the missing constructors". But it turns out to be a little more complex than you might expect. The tuple constructor generated makes use of Groovy's optional args. Then, in later compilation phases such constructors are replaced by the multiple equivalent Java constructors. E.g.:
{code}
@TupleConstructor
class Person { String first, last; int age }
{code}
would get this chain of generated constructors:
{code}
Person(String first, String last, int age)
Person(String first, String last)
Person(String first)
Person()
{code}
To work out what the missing ones are, you'd have to pre-calculate all of the final list of generated constructors for not only the @TupleConstructor generated one but any existing explicit ones using default parameters. Then there is the added complication of when there is a chain with an explicit duplicate in the middle. It's not clear whether the code generated for the missing variant(s) should call the code within the explicitly provided variant(s) or behave more like what the original tuple generated version would behave? Often, using {{force}} can help in some of the edge cases but not yours - it does the opposite of what you'd want.

This annotation was written prior to annotation aliases. Another potential solution to making such transforms smarter is to split it into two parts which run in different phases of the compiler. TupleConstructor would then become the alias to the other two parts. Having said that, I can't think of an obvious split that would help here.


was (Author: paulk):
Your understanding is correct that this is an edge case; the problem occurs because you are providing one empty constructor. You are also correct in your assessment that we can't change from {{> 1}} to {{> 0}} because of stubs.

In theory, we could make it smarter to write as you say "just the missing constructors". But it turns out to be a little more complex than you might expect. The tuple constructor generated makes use of Groovy's optional args. Then, in later compilation phases such constructors are replaced by the multiple equivalent Java constructors. E.g.:
{code}
@TupleConstructor
class Person { String first, last; int age }
{code}
would get this chain of generated constructors:
{code}
Person(String first, String last, int age)
Person(String first, String last)
Person(String first)
Person()
{code}
To work out what the missing ones are, you'd have to pre-calculate all of the final list of generated constructors for not only the @TupleConstructor generated one but any existing explicit ones using default parameters. Then there is the added complication of when there is a chain with an explicit duplicate in the middle. Should the next one in the chain that is missing and should be provided call the explicit one or be generated in such a way to act like the original tuple generated version would behave? Often, using {{force}} can help in some of the edge cases but not yours - it does the opposite of what you'd want.

This annotation was written prior to annotation aliases. Another potential solution to making such transforms smarter is to split it into two parts which run in different phases of the compiler. TupleConstructor would then become the alias to the other two parts. Having said that, I can't think of an obvious split that would help here.

> TupleConstructor shouldn't overwrite existing constructors
> ----------------------------------------------------------
>
>                 Key: GROOVY-7522
>                 URL: https://issues.apache.org/jira/browse/GROOVY-7522
>             Project: Groovy
>          Issue Type: Bug
>            Reporter: Keegan Witt
>
> {{@TupleConstructor}} should not overwrite existing constructors.  For example, this should work, but doesn't currently
> {code:java}
> assert new Cat("Mr. Bigglesworth").name == null  // fails
> @groovy.transform.TupleConstructor
> class Cat {
>   String name
>   int age
>   Cat(String name) {}
> }
> {code}
> Why aren't the {{includes}}/{{excludes}} annotation elements a sufficient workaround?  Because one might want all the other combinations {{@TupleConstructor}} provides, but still have their own implementation for a subset of the combinations.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)