Andres, > On 14 Aug 2018, at 2:31 PM, Andres Almiray <aalmi...@gmail.com> wrote: > This is a good example of a feature that can be experimented with as an > external AST transformation, there's no need to add it to core just yet.
Not quite, alas. I am experimenting with it for years; it sort of works, but is far from easy or reliable. To implement the non-NPE null-propagating behaviour solely at the ASTT side is extremely difficult; you have to cope with all the expressions explicitly, you have to manage a number of ugly special and side cases (e.g., AttributeExpression overrides safe as read-only and trying to set it on throws; an attempt to set the safe attribute for a super.whatever causes a verify error; null?.is(null) does not work reasonably, etc...) 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(foo)) explicitly; (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). For all I know, this probably would not work properly with @CompileStatic (which I do not use at all myself, but others do frequently). 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. > Advantages of such approach: > - faster development/release cycle. > - can target specific Groovy version to begin with. > - may break compatibility until feature works as expected. Completely agreed; that's why I am suggesting this only after I used the ASTT/metaclass approach for years — and also when a major version with lots of potential compatibility breaks is about to come soon anyway. > Adding this feature to core in an early stage (conception) is too early IMHO. > Remember that @TailCall started life as an external AST xform, it was added > to core when it became mature enough :-) Quite: what I am suggesting is that it's the right time now to add it to core, alleviating the need of the [*] hacks :) Thanks and all the best, OC > ------------------------------------------- > Java Champion; Groovy Enthusiast > JCP EC Associate Seat > http://andresalmiray.com <http://andresalmiray.com/> > http://www.linkedin.com/in/aalmiray <http://www.linkedin.com/in/aalmiray> > -- > What goes up, must come down. Ask any system administrator. > There are 10 types of people in the world: Those who understand binary, and > those who don't. > To understand recursion, we must first understand recursion. > > On Tue, Aug 14, 2018 at 1:28 PM, ocs@ocs <o...@ocs.cz <mailto:o...@ocs.cz>> > wrote: > Gentlemen, > > some NPE-related problems of today brought me to re-interate one of my older > suggestions. > > We have the so-called “safe navigation”[*], which in some cases allows a null > to be propagated out of an expression instead of throwing a NPE. At the > moment, it can be triggered for a particular sub-expression (like > property/method-call and, as of 3, newly also indexing) using a question mark > (e.g., “foo?.bar()” or “foo?[bar]”). > > Do please correct me if I am wrong, but far as I know, there still are > expressions which do not allow the “safe mode”, e.g., arithmetic (“a+b” etc). > Furthermore, there are cases when one simply wants a bigger block of code to > contain only null-propagating expressions and never NPE; in such case, using > the question mark syntax is both inconvenient and error-prone (for it is very > easy to forget one of the lot of question marks needed in such a code, and > then get an uncaught unwanted NPE). > > For these reasons, I would suggest adding a new annotation, whose name might > be e.g., “ImplicitSafeNavigation”; it would simply force a null-propagation > to be implicitly and automatically used for *all* expressions in the > annotated scope, i.e., NPE would never be thrown for them; for example: > > === > @ImplicitSafeNavigation class Foo { > static foo(a,b,c,d,e) { > a.bar+b*c[d]<<e.bax() // just e.g.; would work with *any* expression which > NPEs today > } > } > assert null == Foo.foo(null,null,null,null,null) > === > > I wonder whether this enhancement would be possible to implement in some > forthcoming Groovy release? Myself, I believe it would help tremendously. > > If feasible, then it is for a further discussion whether in the scope of this > annotation > (a) a safe-navigation syntax (“foo?.bar”) should be ignored as superfluous; > (b) or, whether in this scope it should reverse the behaviour to trigger an > NPE anyway; > (c) or, whether it should be ignored as (a), and aside of that it would be > worth the effort (and technically possible) to add another syntax to force > NPE over a particular sub-expression (e.g., “foo!.bar”). > > Thanks and all the best, > OC > > [*] The name might not be quite apt, for propagating a null is not inherently > safer than NPEing; those are simply two different approaches, both of which > serve best in different circumstances. A better name would be something like > “null-propagating” or “non-NPE” mode, I guess. Myself, I don't think we > should change the name though, for all are used to it. > >