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