You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@groovy.apache.org by Paul King <pa...@asert.com.au> on 2024/04/08 11:56:06 UTC
Potential enhancement to type checking extensions
Hi folks,
I was putting together a blog post on supporting Multiversal Equality
with Groovy. It's an optional feature in Scala 3:
https://docs.scala-lang.org/scala3/book/ca-multiversal-equality.html
Using the example (involving print and audio books) from the above
link, we can write an ad-hoc type checking extension something like
this:
beforeMethodCall { call ->
if (call.methodAsString == 'equals') {
lhsType = getType(call.objectExpression)
rhsType = getType(call.arguments[0])
if (lhsType != rhsType &&
lhsType != classNodeFor(PrintedBook) &&
rhsType != classNodeFor(AudioBook)) {
addStaticTypeError("Invalid equality check: $lhsType.name
!= $rhsType.name", call)
handled = true
}
}
}
This turns on strict equality checking except for the particular
ad-hoc exclusion listed for print and audio books. So the following
would be true:
42.equals('foo') // STC error: int and String aren't compatible
aBook.equals(pBook) // STC error: due to the order we gave in the exclusion
pBook.equals(aBook) // compiles and returns true due to equals
definition (not shown)
The blog discusses how we could make it less ad-hoc if we wanted to by
introducing a CanEqual marker interface and so forth, but that's
off-topic for just now.
What I wanted to show is the same examples but using the '==' and '!='
operators, since that would be the typical Groovy style for this
scenario. Unfortunately, using the type checking extension DSL doesn't
currently work for binary operators. The swap from '==' to the
'equals' method call occurs well after type checking is finished. The
same would apply to operators eventually falling back to 'compareTo'.
You can still make it work by not using the DSL and writing your own
type checking extension, but that's significantly more work.
Our options seem to be:
(1) not trying to make this work
(2) modify operators to method call expressions earlier (might remove
some optimization steps)
(3) tweak StaticTypeCheckingVisitor#visitBinaryExpression to support
before/after method call hooks for known cases like equals/compareTo
with a pretend method call
(4) alter the TypeCheckingExtension interface with before/after binary
expression calls.
Note that while the multiversal equality example has some interest, it
isn't really what I am trying to fix here. It just shows a hole in the
type checking extension DSL support that I'd like to fix if we can
find a nice fix. If someone wanted to make it a type error to compare
'int's and 'float's (say), they would face the same issue but with
compareTo not equals.
Does anyone have strong opinions on this before I start having a play
and seeing what might work? In particular, a preference for option 3
or 4?
Cheers, Paul.
Re: Potential enhancement to type checking extensions
Posted by MG <mg...@arscreat.com>.
@Both potential tweaks that I looked at did use the more general
approach: Assumed as much, just wanted to emphasize this, before I
argued that maybe we don't need this at all.
I think my argument holds for the more general example you (or Scala)
gives, I don't know if this is worth the effort, it still seems like an
academic problem to me.
Cheers,
mg
On 01/05/2024 23:37, Paul King wrote:
>> if this is to be supported, I would still argue for the more general approach.
> Both potential tweaks that I looked at did use the more general approach.
Re: Potential enhancement to type checking extensions
Posted by Paul King <pa...@asert.com.au>.
Thanks mg, comments inline.
On Thu, May 2, 2024 at 2:51 AM MG <mg...@arscreat.com> wrote:
>
> Hi Paul,
>
> if this is to be supported, I would still argue for the more general approach.
Both potential tweaks that I looked at did use the more general approach.
> However, having scanned the blog post, it seems to me the case for having this kind of support is quite weak.
This thread wasn't really about adding support for multiversal
equality per se. It was about potential extensions to the type
checking DSL to allow extensions like multiversal equality (or others)
to be more easily written.
If we were going to consider multiversal equality, we'd discuss that
in a separate thread, and yes, like Scala I would suggest making it
opt-in.
> E.g. change of color code library leading to assertion failing for the wrong reason:
>
> Yes, you will need to change the library everywhere and remove the old library, anything else will lead to a world of pain.
I won't repeat the arguments for multiversal equality from the Scala
docs which the blog post referenced, but the argument is that having
multiversal equality support allows the compiler to better check that
indeed you have changed the "color library" everywhere. The Scala docs
just use in their example a type changed from S to T, so it might not
be as obvious as changing libraries, but rather a single class change
somewhere. In both cases, the compiler would flag the difference. If
you don't want the compiler to flag such a difference, you could write
your code using the old style equals method instead.
> If a step-by-step replacement cannot be avoided, you need to keep the two worlds strictly separated by design...
>
> ...which includes not simply switching the return type of a method, if that method will be called from both worlds !
>
> On the other hand having these kinds of checks in place could potentially lead to problems when using Object collections which use some kind of "already in collection check" using equality, which then might throw if one wants to add an object which does not allow for comparison with most other objects.
>
> In a sense it feels like having this would violate some long held assumptions regarding Object to Object comparability, for little gain, which begs the question if it is practical / woth it.
> From my own experience I can say that over > 40 years of programming in a multitude of (mostly statically typed) languages, I have never wished for a feature like that.
>
> ...and this is coming from someone who is big on static typing & type checks where possible, for readability and avoiding some trivial runtime errors :-)
Agreed on most points but we're straying into discussing multiversal
equality again rather than the type checking extensions DSL.
Multiversal equality was just an example since it appeared recently in
one of my social media feeds and folks seemed interested in it. The
point about Groovy's type checking extensions is that because they are
extensible, we don't need to add this to the language - folks can
easily have an adhoc extension themselves if they want this feature.
If lots of folks were interested in it, we could certainly consider
making an opt-in extension but that is for another thread.
> Cheers,
> mg
>
>
> On 01/05/2024 13:40, Paul King wrote:
>
> Just to come back to this discussion. The existing functionality is
> what I ended up using in this blog post:
>
> https://groovy.apache.org/blog/multiversal-equality
>
> In that post, you can see two code fragments that start with
> "afterVisitMethod" and contain code using the type checking extension
> DSL. I was really hoping to make those examples much smaller but
> didn't get too far.
>
> First, I created a "checkingVisitor" method that takes a code block to
> visit and a map of closures, e.g. visitBinaryExpression: { existing
> code }. This saved a small handful of lines.
>
> I also played around with just making "binaryExpression" methods as
> peers to "afterVisitMethod" but you needed several of those (e.g. also
> bitwiseNegation and declaration expressions to cover the cases we have
> in our existing checkers) and before and after variants. This added
> quite some complexity.
>
> To cut a long story short, I have left the functionality as is for
> now. There is still scope for making improvements in the future, but
> the need for the change now seemed less compelling to me after this
> little exercise. Let me know if you have differing thoughts.
>
> Cheers, Paul.
>
>
> On Tue, Apr 9, 2024 at 1:21 PM Paul King <pa...@asert.com.au> wrote:
>
> I also think option 4 is the right way to go. Option 3 has the benefit
> that both the operator and method call could be covered just by
> handling the method call (since the former is converted into the
> latter). Option 4 might require you to cover both cases in your type
> extension - but that might also be what is desired. Looking at Scala,
> it seems that they only apply Multiversal Equality when using the
> operator, not when using the equals method.
>
> Cheers, Paul.
>
> On Mon, Apr 8, 2024 at 11:28 PM Jochen Theodorou <bl...@gmx.org> wrote:
>
> On 08.04.24 13:56, Paul King wrote:
> [...]
>
> What I wanted to show is the same examples but using the '==' and '!='
> operators, since that would be the typical Groovy style for this
> scenario. Unfortunately, using the type checking extension DSL doesn't
> currently work for binary operators. The swap from '==' to the
> 'equals' method call occurs well after type checking is finished. The
> same would apply to operators eventually falling back to 'compareTo'.
>
> the replacements may be even on ScriptBytecodeAdapter.
>
> You can still make it work by not using the DSL and writing your own
> type checking extension, but that's significantly more work.
>
> Our options seem to be:
> (1) not trying to make this work
> (2) modify operators to method call expressions earlier (might remove
> some optimization steps)
> (3) tweak StaticTypeCheckingVisitor#visitBinaryExpression to support
> before/after method call hooks for known cases like equals/compareTo
> with a pretend method call
> (4) alter the TypeCheckingExtension interface with before/after binary
> expression calls.
>
> [...]
>
> Does anyone have strong opinions on this before I start having a play
> and seeing what might work? In particular, a preference for option 3
> or 4?
>
> Doing that only for special cases does not sound right to me. I would be
> for option 4... is there anything speaking against that?
>
> bye Jochen
>
>
Re: Potential enhancement to type checking extensions
Posted by MG <mg...@arscreat.com>.
Hi Paul,
1. if this /is /to be supported, I would still argue for the more
general approach.
2. However, having scanned the blog post, it seems to me the case for
having this kind of support is quite weak.
1. E.g. change of color code library leading to assertion failing
for the wrong reason:
1. Yes, you will need to change the library everywhere and
remove the old library, anything else will lead to a world
of pain.
2. If a step-by-step replacement cannot be avoided, you need to
keep the two worlds strictly separated by design...
1. ...which includes not simply switching the return type
of a method, if that method will be called from both
worlds !
3. On the other hand having these kinds of checks in place could
potentially lead to problems when using Object collections which use
some kind of "already in collection check" using equality, which
then might throw if one wants to add an object which does not allow
for comparison with most other objects.
1. In a sense it feels like having this would violate some long
held assumptions regarding Object to Object comparability, for
little gain, which begs the question if it is practical / woth it.
2. From my own experience I can say that over > 40 years of
programming in a multitude of (mostly statically typed)
languages, I have never wished for a feature like that.
4. ...and this is coming from someone who is big on static typing &
type checks where possible, for readability and avoiding some
trivial runtime errors :-)
Cheers,
mg
On 01/05/2024 13:40, Paul King wrote:
> Just to come back to this discussion. The existing functionality is
> what I ended up using in this blog post:
>
> https://groovy.apache.org/blog/multiversal-equality
>
> In that post, you can see two code fragments that start with
> "afterVisitMethod" and contain code using the type checking extension
> DSL. I was really hoping to make those examples much smaller but
> didn't get too far.
>
> First, I created a "checkingVisitor" method that takes a code block to
> visit and a map of closures, e.g. visitBinaryExpression: { existing
> code }. This saved a small handful of lines.
>
> I also played around with just making "binaryExpression" methods as
> peers to "afterVisitMethod" but you needed several of those (e.g. also
> bitwiseNegation and declaration expressions to cover the cases we have
> in our existing checkers) and before and after variants. This added
> quite some complexity.
>
> To cut a long story short, I have left the functionality as is for
> now. There is still scope for making improvements in the future, but
> the need for the change now seemed less compelling to me after this
> little exercise. Let me know if you have differing thoughts.
>
> Cheers, Paul.
>
>
> On Tue, Apr 9, 2024 at 1:21 PM Paul King<pa...@asert.com.au> wrote:
>> I also think option 4 is the right way to go. Option 3 has the benefit
>> that both the operator and method call could be covered just by
>> handling the method call (since the former is converted into the
>> latter). Option 4 might require you to cover both cases in your type
>> extension - but that might also be what is desired. Looking at Scala,
>> it seems that they only apply Multiversal Equality when using the
>> operator, not when using the equals method.
>>
>> Cheers, Paul.
>>
>> On Mon, Apr 8, 2024 at 11:28 PM Jochen Theodorou<bl...@gmx.org> wrote:
>>> On 08.04.24 13:56, Paul King wrote:
>>> [...]
>>>> What I wanted to show is the same examples but using the '==' and '!='
>>>> operators, since that would be the typical Groovy style for this
>>>> scenario. Unfortunately, using the type checking extension DSL doesn't
>>>> currently work for binary operators. The swap from '==' to the
>>>> 'equals' method call occurs well after type checking is finished. The
>>>> same would apply to operators eventually falling back to 'compareTo'.
>>> the replacements may be even on ScriptBytecodeAdapter.
>>>
>>>> You can still make it work by not using the DSL and writing your own
>>>> type checking extension, but that's significantly more work.
>>>>
>>>> Our options seem to be:
>>>> (1) not trying to make this work
>>>> (2) modify operators to method call expressions earlier (might remove
>>>> some optimization steps)
>>>> (3) tweak StaticTypeCheckingVisitor#visitBinaryExpression to support
>>>> before/after method call hooks for known cases like equals/compareTo
>>>> with a pretend method call
>>>> (4) alter the TypeCheckingExtension interface with before/after binary
>>>> expression calls.
>>>>
>>> [...]
>>>> Does anyone have strong opinions on this before I start having a play
>>>> and seeing what might work? In particular, a preference for option 3
>>>> or 4?
>>> Doing that only for special cases does not sound right to me. I would be
>>> for option 4... is there anything speaking against that?
>>>
>>> bye Jochen
>>>
Re: Potential enhancement to type checking extensions
Posted by Paul King <pa...@asert.com.au>.
Just to come back to this discussion. The existing functionality is
what I ended up using in this blog post:
https://groovy.apache.org/blog/multiversal-equality
In that post, you can see two code fragments that start with
"afterVisitMethod" and contain code using the type checking extension
DSL. I was really hoping to make those examples much smaller but
didn't get too far.
First, I created a "checkingVisitor" method that takes a code block to
visit and a map of closures, e.g. visitBinaryExpression: { existing
code }. This saved a small handful of lines.
I also played around with just making "binaryExpression" methods as
peers to "afterVisitMethod" but you needed several of those (e.g. also
bitwiseNegation and declaration expressions to cover the cases we have
in our existing checkers) and before and after variants. This added
quite some complexity.
To cut a long story short, I have left the functionality as is for
now. There is still scope for making improvements in the future, but
the need for the change now seemed less compelling to me after this
little exercise. Let me know if you have differing thoughts.
Cheers, Paul.
On Tue, Apr 9, 2024 at 1:21 PM Paul King <pa...@asert.com.au> wrote:
>
> I also think option 4 is the right way to go. Option 3 has the benefit
> that both the operator and method call could be covered just by
> handling the method call (since the former is converted into the
> latter). Option 4 might require you to cover both cases in your type
> extension - but that might also be what is desired. Looking at Scala,
> it seems that they only apply Multiversal Equality when using the
> operator, not when using the equals method.
>
> Cheers, Paul.
>
> On Mon, Apr 8, 2024 at 11:28 PM Jochen Theodorou <bl...@gmx.org> wrote:
> >
> > On 08.04.24 13:56, Paul King wrote:
> > [...]
> > > What I wanted to show is the same examples but using the '==' and '!='
> > > operators, since that would be the typical Groovy style for this
> > > scenario. Unfortunately, using the type checking extension DSL doesn't
> > > currently work for binary operators. The swap from '==' to the
> > > 'equals' method call occurs well after type checking is finished. The
> > > same would apply to operators eventually falling back to 'compareTo'.
> >
> > the replacements may be even on ScriptBytecodeAdapter.
> >
> > > You can still make it work by not using the DSL and writing your own
> > > type checking extension, but that's significantly more work.
> > >
> > > Our options seem to be:
> > > (1) not trying to make this work
> > > (2) modify operators to method call expressions earlier (might remove
> > > some optimization steps)
> > > (3) tweak StaticTypeCheckingVisitor#visitBinaryExpression to support
> > > before/after method call hooks for known cases like equals/compareTo
> > > with a pretend method call
> > > (4) alter the TypeCheckingExtension interface with before/after binary
> > > expression calls.
> > >
> > [...]
> > >
> > > Does anyone have strong opinions on this before I start having a play
> > > and seeing what might work? In particular, a preference for option 3
> > > or 4?
> >
> > Doing that only for special cases does not sound right to me. I would be
> > for option 4... is there anything speaking against that?
> >
> > bye Jochen
> >
Re: [EXT] Re: Potential enhancement to type checking extensions
Posted by "Milles, Eric (TR Technology) via dev" <de...@groovy.apache.org>.
For me, adding hooks into visitBinaryExpression is the way to go – I think that is your option 3. The type-checker could be looking up the method name associated with the binary operator and then checking as a method call. It looks like all that is missing and getResultType indicates that all comparisons are boolean. The actual conversion from binary expression to method call expression happens later in BinaryExpressionTransformer.
Not sure if adding a method to the TypeCheckingExtension is better versus using beforeMethodCall, onMethodSelection and afterMethodCall.
In order to override the equals to perform as you expected, did you need to drop into dynamic mode? Or were you able to get it to go, but the types did not check properly in some cases?
________________________________
From: Paul King <pa...@asert.com.au>
Sent: Monday, April 8, 2024 10:21 PM
To: dev@groovy.apache.org <de...@groovy.apache.org>
Subject: [EXT] Re: Potential enhancement to type checking extensions
External Email: Use caution with links and attachments.
I also think option 4 is the right way to go. Option 3 has the benefit
that both the operator and method call could be covered just by
handling the method call (since the former is converted into the
latter). Option 4 might require you to cover both cases in your type
extension - but that might also be what is desired. Looking at Scala,
it seems that they only apply Multiversal Equality when using the
operator, not when using the equals method.
Cheers, Paul.
On Mon, Apr 8, 2024 at 11:28 PM Jochen Theodorou <bl...@gmx.org> wrote:
>
> On 08.04.24 13:56, Paul King wrote:
> [...]
> > What I wanted to show is the same examples but using the '==' and '!='
> > operators, since that would be the typical Groovy style for this
> > scenario. Unfortunately, using the type checking extension DSL doesn't
> > currently work for binary operators. The swap from '==' to the
> > 'equals' method call occurs well after type checking is finished. The
> > same would apply to operators eventually falling back to 'compareTo'.
>
> the replacements may be even on ScriptBytecodeAdapter.
>
> > You can still make it work by not using the DSL and writing your own
> > type checking extension, but that's significantly more work.
> >
> > Our options seem to be:
> > (1) not trying to make this work
> > (2) modify operators to method call expressions earlier (might remove
> > some optimization steps)
> > (3) tweak StaticTypeCheckingVisitor#visitBinaryExpression to support
> > before/after method call hooks for known cases like equals/compareTo
> > with a pretend method call
> > (4) alter the TypeCheckingExtension interface with before/after binary
> > expression calls.
> >
> [...]
> >
> > Does anyone have strong opinions on this before I start having a play
> > and seeing what might work? In particular, a preference for option 3
> > or 4?
>
> Doing that only for special cases does not sound right to me. I would be
> for option 4... is there anything speaking against that?
>
> bye Jochen
>
Re: Potential enhancement to type checking extensions
Posted by Paul King <pa...@asert.com.au>.
I also think option 4 is the right way to go. Option 3 has the benefit
that both the operator and method call could be covered just by
handling the method call (since the former is converted into the
latter). Option 4 might require you to cover both cases in your type
extension - but that might also be what is desired. Looking at Scala,
it seems that they only apply Multiversal Equality when using the
operator, not when using the equals method.
Cheers, Paul.
On Mon, Apr 8, 2024 at 11:28 PM Jochen Theodorou <bl...@gmx.org> wrote:
>
> On 08.04.24 13:56, Paul King wrote:
> [...]
> > What I wanted to show is the same examples but using the '==' and '!='
> > operators, since that would be the typical Groovy style for this
> > scenario. Unfortunately, using the type checking extension DSL doesn't
> > currently work for binary operators. The swap from '==' to the
> > 'equals' method call occurs well after type checking is finished. The
> > same would apply to operators eventually falling back to 'compareTo'.
>
> the replacements may be even on ScriptBytecodeAdapter.
>
> > You can still make it work by not using the DSL and writing your own
> > type checking extension, but that's significantly more work.
> >
> > Our options seem to be:
> > (1) not trying to make this work
> > (2) modify operators to method call expressions earlier (might remove
> > some optimization steps)
> > (3) tweak StaticTypeCheckingVisitor#visitBinaryExpression to support
> > before/after method call hooks for known cases like equals/compareTo
> > with a pretend method call
> > (4) alter the TypeCheckingExtension interface with before/after binary
> > expression calls.
> >
> [...]
> >
> > Does anyone have strong opinions on this before I start having a play
> > and seeing what might work? In particular, a preference for option 3
> > or 4?
>
> Doing that only for special cases does not sound right to me. I would be
> for option 4... is there anything speaking against that?
>
> bye Jochen
>
Re: Potential enhancement to type checking extensions
Posted by MG <mg...@arscreat.com>.
Even without the deep insight into the inner workings of Groovy you guys
have, I would tend to agree with Jochen here:
1. Option 4 looks to be the clean, most flexible & general option.
2. Specialized support for equality that handles or makes handling
easier for both equality operator and equals could maybe be supplied
in a TypeCheckingExtension sub-class or a helper class... (?)
Cheers,
mg
On 08/04/2024 15:28, Jochen Theodorou wrote:
>
>> Our options seem to be:
>> (1) not trying to make this work
>> (2) modify operators to method call expressions earlier (might remove
>> some optimization steps)
>> (3) tweak StaticTypeCheckingVisitor#visitBinaryExpression to support
>> before/after method call hooks for known cases like equals/compareTo
>> with a pretend method call
>> (4) alter the TypeCheckingExtension interface with before/after binary
>> expression calls.
>>
> [...]
>>
>> Does anyone have strong opinions on this before I start having a play
>> and seeing what might work? In particular, a preference for option 3
>> or 4?
>
> Doing that only for special cases does not sound right to me. I would be
> for option 4... is there anything speaking against that?
>
> bye Jochen
>
Re: Potential enhancement to type checking extensions
Posted by Jochen Theodorou <bl...@gmx.org>.
On 08.04.24 13:56, Paul King wrote:
[...]
> What I wanted to show is the same examples but using the '==' and '!='
> operators, since that would be the typical Groovy style for this
> scenario. Unfortunately, using the type checking extension DSL doesn't
> currently work for binary operators. The swap from '==' to the
> 'equals' method call occurs well after type checking is finished. The
> same would apply to operators eventually falling back to 'compareTo'.
the replacements may be even on ScriptBytecodeAdapter.
> You can still make it work by not using the DSL and writing your own
> type checking extension, but that's significantly more work.
>
> Our options seem to be:
> (1) not trying to make this work
> (2) modify operators to method call expressions earlier (might remove
> some optimization steps)
> (3) tweak StaticTypeCheckingVisitor#visitBinaryExpression to support
> before/after method call hooks for known cases like equals/compareTo
> with a pretend method call
> (4) alter the TypeCheckingExtension interface with before/after binary
> expression calls.
>
[...]
>
> Does anyone have strong opinions on this before I start having a play
> and seeing what might work? In particular, a preference for option 3
> or 4?
Doing that only for special cases does not sound right to me. I would be
for option 4... is there anything speaking against that?
bye Jochen