Bug 192875 - [compiler][options][null] Set "Null pointer access" to warning by default
Summary: [compiler][options][null] Set "Null pointer access" to warning by default
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P2 normal (vote)
Target Milestone: 3.4 M3   Edit
Assignee: Maxime Daniel CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-06-15 09:52 EDT by John Arthorne CLA
Modified: 2007-10-29 10:16 EDT (History)
3 users (show)

See Also:
kent_johnson: review+


Attachments
Fix + preliminary changes for test cases (3.95 KB, patch)
2007-10-12 10:33 EDT, Maxime Daniel CLA
no flags Details | Diff
Fix + test cases (5.31 KB, patch)
2007-10-15 04:23 EDT, Maxime Daniel CLA
no flags Details | Diff
Fix + test cases (9.77 KB, patch)
2007-10-17 09:50 EDT, Maxime Daniel CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Arthorne CLA 2007-06-15 09:52:39 EDT
Build: 3.3 RC4

The "Null pointer access" compiler setting is set to "Ignore" by default. Why not enable this by default? As far as I can see, any code with this warning is guaranteed to throw an exception at runtime.
Comment 1 Maxime Daniel CLA 2007-06-15 10:23:48 EDT
John, the decision is with Philippe. My understanding is that, as our confidence in our null analysis support grows, we make incremental efforts to boost its adoption, and that this is a gradual approach. As a first significant step, org.eclipse.jdt.core itself has been using a level of Warning for the 'Null pointer access' setting since the 24th of April, and has no such warning left.
I would still drag your attention to the following caveats:
- our null analysis implementation is subject to drastic constraints that are justified by our continued preference for high performance; this is responsible for more than one outstanding bug that we haven't resolved yet;
- some of these might remain unresolved if we cannot come up with a reasonable solution in terms of cost and performance;
- the last bug I fixed in that area is quite recent.
All in all, while we made progress and while the feature is already useful, we felt it was not mature enough to make it as a default in 3.3 timeframe.

Philippe, would you please comment re. plans as you see them?
Comment 2 John Arthorne CLA 2007-06-15 11:59:02 EDT
Fair enough. My feedback is that I've run it over some large code bases (most of Eclipse source, all of Jazz source), and it found some really good bugs. I didn't find any false positives (although the potential null reference check still has some false positives). It's a catch-22 - you won't get much feedback/testing if it's off by default.
Comment 3 Maxime Daniel CLA 2007-06-18 02:39:54 EDT
Moving to Philippe for further discussion and decision.
Comment 4 Philipe Mulet CLA 2007-10-09 16:57:07 EDT
+1 for enabling it by default in 3.4 stream.
Comment 5 Maxime Daniel CLA 2007-10-10 02:11:59 EDT
Raising priority.
Comment 6 Maxime Daniel CLA 2007-10-12 10:33:34 EDT
Created attachment 80232 [details]
Fix + preliminary changes for test cases

This fix add NullReference to the warning threshold in CompilerOptions, changes the javadoc for JavaCore.defaultOptions, and changes one of the test cases. Tests under way for tests that will be broken by the change (expecting a few at least).
Comment 7 Maxime Daniel CLA 2007-10-15 04:23:20 EDT
Created attachment 80340 [details]
Fix + test cases

Two more changes to tests (the default option being hit by them). Changed two initializations like X x = null; to X x = new X();.
Comment 8 Maxime Daniel CLA 2007-10-15 04:24:12 EDT
Kent, could you please review this?
Comment 9 Olivier Thomann CLA 2007-10-16 00:32:42 EDT
I guess the print usage of the batch compiler should also be updated + the corresponding BatchCompilerTest.
Comment 10 Maxime Daniel CLA 2007-10-17 09:50:50 EDT
Created attachment 80561 [details]
Fix + test cases

Better patch: addresses Olivier's insights (changed the batch compiler help message and a log contents, added tests that illustrate the new default - aka warning is on by default and can be disabled).
Comment 11 Maxime Daniel CLA 2007-10-18 02:30:08 EDT
Released for 3.4M3
Comment 12 Frederic Fusier CLA 2007-10-29 10:16:24 EDT
Verified for 3.4M3 using I20071029-0010 build.