Re: [cdt-dev] compiler warning policy

[Alena Laskavaia <elaskavaia.cdt@xxxxxxxxx>]

damn, you typing faster!

On Sat, Sep 11, 2010 at 8:35 PM, Sergey Prigogin
<eclipse.sprigogin@xxxxxxxxx> wrote:
> Slightly longer but arguably more readable would be:
> ITargetEnvironment env = getTargetEnvironmentService();
>     if (env == null) {
> return;
>     }
>     IDisassembler disassembler = env.getDisassembler();
>     if (disassembler == null) {
> return;
>     }
>     // and then later
>     if (env.getLongestInstructionLength() ...)
> I'm pretty sure that the compiler will be happy with this code.
> -sergey
> On Sat, Sep 11, 2010 at 5: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
>>
>
>
> _______________________________________________
> cdt-dev mailing list
> cdt-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/cdt-dev
>
>