[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-openjfx-dev
Subject: Re: eclipse warnings
From: Nir Lisker <nlisker () gmail ! com>
Date: 2023-12-05 8:54:55
Message-ID: CA+0ynh94V4UvuMbVw31FG=J7S2iqSGqDT2a3itE5-pCmVB_YSQ () mail ! gmail ! com
[Download RAW message or body]
There's already ongoing work for this in
https://bugs.openjdk.org/browse/JDK-8297300. John provided some fixes, I
need to get back to it.
On Mon, Dec 4, 2023 at 9:53 PM Johan Vos <johan.vos@gluonhq.com> wrote:
> Also, these commits often affect many files at once (in scattered
> locations), and that makes backports harder.
>
> - Johan
>
> On Mon, Dec 4, 2023 at 6:14 PM Kevin Rushforth <kevin.rushforth@oracle.com>
> wrote:
>
>> We did a few of these sort of cleanup fixes a year or so ago.
>>
>> In general, this sort of cleanup *might* be useful, but also causes some
>> code churn and takes review cycles to ensure that there is no unintentional
>> side effect.
>>
>> The last two might be OK cleanup tasks, but I wouldn't make them a high
>> priority. Worth noting is that a seemingly redundant null check or
>> instanceof check is not always a bad thing, so I wouldn't clean up all of
>> them.
>>
>> The first group is the more interesting one. In some cases a potential
>> null access can highlight actual bugs. However, I oppose any automated
>> solution for these, since adding a null check where you don't expect a null
>> (even if you IDE thinks it might be possible) can hide the root cause of a
>> problem.
>>
>> We aren't going to enforce these, though, so you'll likely need to
>> configure your IDE to be less picky.
>>
>> -- Kevin
>>
>>
>> On 12/4/2023 8:34 AM, Andy Goryachev wrote:
>>
>> Dear colleagues:
>>
>>
>>
>> Imported the openjfx project into another workspace with a more stringent
>> error checking and discovered a few issues:
>>
>>
>>
>> - potential null pointer access: 295
>> - unnecessary cast or instanceof: 190
>> - redundant null check: 61
>>
>>
>>
>> Do we want to clean these up?
>>
>>
>>
>> -andy
>>
>>
>>
>>
>>
[Attachment #3 (text/html)]
<div dir="ltr">There's already ongoing work for this in <a \
href="https://bugs.openjdk.org/browse/JDK-8297300">https://bugs.openjdk.org/browse/JDK-8297300</a>. \
John provided some fixes, I need to get back to it.</div><br><div \
class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Dec 4, 2023 at \
9:53 PM Johan Vos <<a \
href="mailto:johan.vos@gluonhq.com">johan.vos@gluonhq.com</a>> \
wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Also, \
these commits often affect many files at once (in scattered locations), and that \
makes backports harder.<div><br></div><div>- Johan</div></div><br><div \
class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Dec 4, 2023 at \
6:14 PM Kevin Rushforth <<a href="mailto:kevin.rushforth@oracle.com" \
target="_blank">kevin.rushforth@oracle.com</a>> wrote:<br></div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"><u></u>
<div>
We did a few of these sort of cleanup fixes a year or so ago.<br>
<br>
In general, this sort of cleanup *might* be useful, but also causes
some code churn and takes review cycles to ensure that there is no
unintentional side effect.<br>
<br>
The last two might be OK cleanup tasks, but I wouldn't make them a
high priority. Worth noting is that a seemingly redundant null check
or instanceof check is not always a bad thing, so I wouldn't clean
up all of them.<br>
<br>
The first group is the more interesting one. In some cases a
potential null access can highlight actual bugs. However, I oppose
any automated solution for these, since adding a null check where
you don't expect a null (even if you IDE thinks it might be
possible) can hide the root cause of a problem.<br>
<br>
We aren't going to enforce these, though, so you'll likely need to
configure your IDE to be less picky.<br>
<br>
-- Kevin<br>
<br>
<br>
<div>On 12/4/2023 8:34 AM, Andy Goryachev
wrote:<br>
</div>
<blockquote type="cite">
<div>
<p class="MsoNormal"><span style="font-family:"Iosevka Fixed \
SS16"">Dear colleagues:<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Iosevka Fixed \
SS16""><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Iosevka Fixed \
SS16"">Imported the openjfx project into another workspace with a more
stringent error checking and discovered a few \
issues:<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Iosevka Fixed \
SS16""><u></u> <u></u></span></p> <ul style="margin-top:0in" type="disc">
<li style="margin-left:0in"><span style="font-family:"Iosevka Fixed \
SS16"">potential null pointer access: 295<u></u><u></u></span></li>
<li style="margin-left:0in"><span style="font-family:"Iosevka Fixed \
SS16"">unnecessary cast or instanceof: 190<u></u><u></u></span></li>
<li style="margin-left:0in"><span style="font-family:"Iosevka Fixed \
SS16"">redundant null check: 61<u></u><u></u></span></li>
</ul>
<p class="MsoNormal"><span style="font-family:"Iosevka Fixed \
SS16""><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Iosevka Fixed \
SS16"">Do we want to clean these up?<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Iosevka Fixed \
SS16""><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Iosevka Fixed \
SS16"">-andy<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Iosevka Fixed \
SS16""><u></u> <u></u></span></p> </div>
</blockquote>
<br>
</div>
</blockquote></div>
</blockquote></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic