Jochen,
> On 14 Aug 2018, at 6:34 PM, Jochen Theodorou <[email protected]> wrote:
> [...]
>> For the moment, the best solution — far as I have been able to ascertain —
>> consists of [*]
>> (a) at launch, setting own DelegatingMetaClass subclass for a
>> Null.metaclass; it essentially would return null from invokeMethod, but
>> still needs to process special cases (like e.g., null.is
>> <http://null.is>(foo)) explicitly;
>
> so for general Groovy... how are we supposed to recognized in a transform if
> "is" has been replaced?
It is definitely highly arguable, but in the very specific case of
“null?.is(null)“, which currently returns an absurd (though technically
understandable) false value, I would rather suggest to break the backward
compatibility.
I can't really see anyone whose code would actually use "?.is", and moreover,
rely on that “null?.is(null)“ is valid and returns null (instead of true). Is
there such a person in the sweet world?
Anyway, the point is, I'd say, rather at the unimportant side, for I believe
is() is being won't be widely used anyway, given we got === and !=== now.
Thus, even if we decide that breaking backward compatibility even in this
slightly insane case is a big no-no, we just can keep the current behaviour of
“null?.is(null)==null“ unchanged. There's no real harm; new code would simply
use === instead, and old one would work as before.
>> (b) since the above for some godforsaken reason does not work for property
>> access at all, still implement an ASTT with a ClassCodeExpressionTransformer
>> to set expression.safe=true for get/setProperty, guarding explicitly against
>> the known problematic cases (e.g., super.getProperty).
>
> are you saying x?.foo will NPE if x is null? Or that x?.getFoo() will NPE in
> that case? Not sure how to read your comment.
Provided only (a) “the Null.metaclass; returning null from invokeMethod” is
used and no ASTT, then yes, it would NPE. Easiest thing on earth to try:
===
262 /tmp> <q.groovy
class q {
static main(args) {
// ExpandoMetaClass.enableGlobally() // I thought this is needed; seems
not (though my application use it anyway)
def mc=new OCSNMC(org.codehaus.groovy.runtime.NullObject)
mc.initialize()
org.codehaus.groovy.runtime.NullObject.metaClass=mc
println "null.foo() is OK: ${null.foo()==null}"
println "null.foo we won't see: ${null.foo==null}"
}
}
class OCSNMC extends DelegatingMetaClass {
OCSNMC(Class clazz){
super(clazz)
}
Object invokeMethod(Object object, String methodName, Object[] arguments) {
null
}
}
263 /tmp> /usr/local/groovy-2.4.15/bin/groovy q
null.foo() is OK: true
Caught: java.lang.NullPointerException: Cannot get property 'foo' on null object
java.lang.NullPointerException: Cannot get property 'foo' on null object
at q.main(q.groovy:9)
at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
264 /tmp>
===
>> For all I know, this probably would not work properly with @CompileStatic
>> (which I do not use at all myself, but others do frequently).
>
> the result type could be a problem... Worth to check.
Definitely :)
>> Trust me, been there, done that. I am pretty darn sure it would be
>> /infinitely/ easier and, what's important, more reliable in the core with an
>> explicit compiler support.
>
> How about making a small github project to dump the current state there?
Not that easy, for my code is mixed up with other ASTT and runtime stuff; but
I'll try to make some simple proof-of-concept ASAP and send here.
Thanks and all the best,
OC
