groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ocs@ocs" <...@ocs.cz>
Subject Re: suggestion: ImplicitSafeNavigation annotation
Date Tue, 14 Aug 2018 13:07:18 GMT
Andres,

> On 14 Aug 2018, at 2:31 PM, Andres Almiray <aalmiray@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 <ocs@ocs.cz <mailto:ocs@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.
> 
> 


Mime
View raw message