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