You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@netbeans.apache.org by Jan Lahoda <la...@gmail.com> on 2018/06/08 04:08:47 UTC

[patch] NETBEANS-889: handling lambdas in NPECheck

Hi,

I'd like to ask for a review of a possible fix for NETBEANS-889
<https://issues.apache.org/jira/browse/NETBEANS-889>.

The testcase there is:
---
    String value = "";
    void nullDereference(){
        Runnable run = ()->{
            value = null;
        };
        run.run();//false positive
    }
---

The reason is that the lambda (method) is not handled as a full method, so
the changes from inside the lambda affect the outer context. The proposed
patch handles lambdas as methods.

Originally, visitMethod was filling variable2State with default values for
parameters and fields. It would be possible to extend that for lambdas, but
that seems unnecessary, as if a variable is not in the variable2State map
when used, the default is picked up automatically. See
NPECheck.VisitorImpl.visitIdent.

Proposed patch:
https://issues.apache.org/jira/secure/attachment/12927013/NETBEANS-889-2.diff

Feedback is welcome!

Thanks,
    Jan

Re: [patch] NETBEANS-889: handling lambdas in NPECheck

Posted by Jan Lahoda <la...@gmail.com>.
On Fri, Jun 8, 2018 at 6:46 AM, Emilian Bold <em...@protonmail.ch>
wrote:

> I wonder why all those lines with variable2State.put are not needed
> anymore.
>
> The old code called super.visitMethod which I guess would recurse and
> visit everything and call variable2State.put eventually.
>
> So, why were they put in there to begin with? For lambdas?
>

To be honest, I don't recall (even after looking at the logs). Currently,
it feels unnecessary - the default values will be looked up automatically
in visitIdentifier and visitMemberSelect if there is no value for the given
variable in the variable2State map.

Jan


>
> As usual, it's a pleasure to see unit test.
>
> --emi
>
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
>
> On 8 June 2018 7:08 AM, Jan Lahoda <la...@gmail.com> wrote:
>
> > Hi,
> >
> > I'd like to ask for a review of a possible fix for NETBEANS-889
> >
> > https://issues.apache.org/jira/browse/NETBEANS-889.
> >
> > The testcase there is:
> >
> >
> > --------------------------
> >
> >     String value = "";
> >     void nullDereference(){
> >         Runnable run = ()->{
> >
> >             value = null;
> >         };
> >         run.run();//false positive
> >     }
> >
> >
> > The reason is that the lambda (method) is not handled as a full method,
> so
> >
> > the changes from inside the lambda affect the outer context. The proposed
> >
> > patch handles lambdas as methods.
> >
> > Originally, visitMethod was filling variable2State with default values
> for
> >
> > parameters and fields. It would be possible to extend that for lambdas,
> but
> >
> > that seems unnecessary, as if a variable is not in the variable2State map
> >
> > when used, the default is picked up automatically. See
> >
> > NPECheck.VisitorImpl.visitIdent.
> >
> > Proposed patch:
> >
> > https://issues.apache.org/jira/secure/attachment/
> 12927013/NETBEANS-889-2.diff
> >
> > Feedback is welcome!
> >
> > Thanks,
> >
> > Jan
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@netbeans.incubator.apache.org
> For additional commands, e-mail: dev-help@netbeans.incubator.apache.org
>
> For further information about the NetBeans mailing lists, visit:
> https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
>
>
>
>

Re: [patch] NETBEANS-889: handling lambdas in NPECheck

Posted by Emilian Bold <em...@protonmail.ch>.
I wonder why all those lines with variable2State.put are not needed anymore.

The old code called super.visitMethod which I guess would recurse and visit everything and call variable2State.put eventually.

So, why were they put in there to begin with? For lambdas?

As usual, it's a pleasure to see unit test.

--emi

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐

On 8 June 2018 7:08 AM, Jan Lahoda <la...@gmail.com> wrote:

> Hi,
> 
> I'd like to ask for a review of a possible fix for NETBEANS-889
> 
> https://issues.apache.org/jira/browse/NETBEANS-889.
> 
> The testcase there is:
> 
> 
> --------------------------
> 
>     String value = "";
>     void nullDereference(){
>         Runnable run = ()->{
>     
>             value = null;
>         };
>         run.run();//false positive
>     }
>     
> 
> The reason is that the lambda (method) is not handled as a full method, so
> 
> the changes from inside the lambda affect the outer context. The proposed
> 
> patch handles lambdas as methods.
> 
> Originally, visitMethod was filling variable2State with default values for
> 
> parameters and fields. It would be possible to extend that for lambdas, but
> 
> that seems unnecessary, as if a variable is not in the variable2State map
> 
> when used, the default is picked up automatically. See
> 
> NPECheck.VisitorImpl.visitIdent.
> 
> Proposed patch:
> 
> https://issues.apache.org/jira/secure/attachment/12927013/NETBEANS-889-2.diff
> 
> Feedback is welcome!
> 
> Thanks,
> 
> Jan


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@netbeans.incubator.apache.org
For additional commands, e-mail: dev-help@netbeans.incubator.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists