Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [cdt-dev] compiler warning policy

It is not discussion for cdt-dev rather for JDT but if compiler does
not understand your code than usually it means it is too complex
than needed, i.e. why doing all this ternary operations to throw away
the result anyway? I.e.
if you write like this compiler would be happy:
  ITargetEnvironment env = getTargetEnvironmentService();
  if (env==null) return;
  IDisassembler disassembler = env.getDisassembler() ;
  if (disassembler == null) return;

On Sat, Sep 11, 2010 at 8:23 PM,  <kirk.beitz@xxxxxxxxx> wrote:
> not to change anyone's mind one way or another about this … i agree with the
> aggressiveness of the extra checks, though i also agree with the annoying
> nature of the redundant/needless checks to quiet them.
> but using this policy, i have found that a true false positive can be
> silenced by code that results in a false negative very easily if one is
> using some sort of accessor that could return null :
>   ITargetEnvironment env = getTargetEnvironmentService();
>   IDisassembler disassembler = (env
> != null ) ? env.getDisassembler() : null;
>   if (disassembler == null) {
>   return;
>   }
>     // and then later
>     if (env.getLongestInstructionLength() ...)
> the last line results in a warning, even though the logic above means the
> code will be exited if env is ever null.
> but before i replaced this code, it contained the following calls, both of
> which actually could have potentially (though not likely) resulted in NPEs :
>
> IDisassembler disassembler = getTargetEnvironmentService().getDisassembler();
>   if (disassembler == null) {
>   return;
>   }
>     // and then later
>     if (getTargetEnvironmentService().getLongestInstructionLength() ...)
>
> my point is really that for this to be more useful and less annoying, the
> compiler not only has to be smarter about figuring out when a potential null
> may not occur, but also when function calls may return null and then be
> immediately dereferenced.
> ++ kirk
>
> On 2010-Sep-2, at 5:36 AM, ext John Cortell wrote:
>
> Fair enough. I randomly selected a CDT project yesterday (I don't remember
> which one) and activated the warning. Most of the reported instances where
> false positives and required needless/redundant checks to quiet them.
> However, I've tried a few more projects this morning at home and am seeing
> some good true positives, so I rescind my recommendation.
>
> At 07:07 PM 9/1/2010, Sergey Prigogin wrote:
>
> -1. I haven't seen any false positives, which required fixes that negatively
> affected code readability or performance. In fact, I've seen very few false
> positives.
>
> -sergey
>
> On Wed, Sep 1, 2010 at 4:48 PM, Alena Laskavaia <elaskavaia.cdt@xxxxxxxxx >
> wrote:
> -1. The warning is very useful if it catch an error before runtime and
> it does most of the time. And you can always write
> a code that won't have this warning, even if cost some redundant
> checks. How many actually warnings we are talking about here?
> You can always set it to ignore on some specific projects (such as
> edc), but I won't do it for cdt.core, etc.
>
> On Wed, Sep 1, 2010 at 5:49 PM, John Cortell <rat042@xxxxxxxxxxxxx> wrote:
>> In http://wiki.eclipse.org/CDT/policy, it's recommended that Potential
>> null
>> pointer access be set to 'warning'. The default is 'ignore'. I think the
>> usefulness of this warning is too debatable to make it a recommendation.
>> The
>> compiler's ability to detect a possible null pointer use is too weak and
>> enabling  the warning results in many false positives, requiring
>> developers
>> to either add needless checks to the code, live with the warnings, or
>> disable them at too large a scope (method). If no one objects, I'll remove
>> that recommendation from the WIKI page.
>>
>> John
>>
>>
>> _______________________________________________
>> cdt-dev mailing list
>> cdt-dev@xxxxxxxxxxx
>> https://dev.eclipse.org/mailman/listinfo/cdt-dev
>>
>>
> _______________________________________________
> cdt-dev mailing list
> cdt-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/cdt-dev
>
>
> _______________________________________________
> cdt-dev mailing list
> cdt-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/cdt-dev
>
> _______________________________________________
> cdt-dev mailing list
> cdt-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/cdt-dev
>
>


Back to the top