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 <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 > >
