Bug 245973 - [compiler] Problem irritant cannot exceed 64bit limit
Summary: [compiler] Problem irritant cannot exceed 64bit limit
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.5 M2   Edit
Assignee: Philipe Mulet CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 248781 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-09-02 10:58 EDT by Philipe Mulet CLA
Modified: 2009-12-10 07:16 EST (History)
4 users (show)

See Also:
frederic_fusier: review+

Attachments
Proposed patch (101.49 KB, patch)
2008-09-02 12:08 EDT, Philipe Mulet CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Philipe Mulet CLA 2008-09-02 10:58:55 EDT 
Build 3.5M1

Problem irritants are encoded into a 64bit long; i.e. one bit for each.
There are already 58 bits used today, which means only room left for 6 additional diagnosis.

Need to rework the support the error/warning threshold to accomodate more evolutions in the future.
Comment 1 Philipe Mulet CLA 2008-09-02 12:08:24 EDT 
Created attachment 111495 [details]
Proposed patch

The patch encapsulates errorThreshold/warningThreshold into IrritantSet objects.
These object can store up to 8x29 irritants (reserving 3 higher bits in irritants to denote group; it could later be evolved into 16x28 if needed).

All irritants now are encoded into 32 bit ints, and thus can directly be used in switch (no longer need to split 64 bits into 32 high/low bits).
Comment 2 Philipe Mulet CLA 2008-09-02 12:09:23 EDT 
Frederic - could you conduct a performance run with this one patch, and let me know its impact on performance ?
Comment 3 Philipe Mulet CLA 2008-09-05 06:24:15 EDT 
Released for 3.5M2.
Fixed
Comment 4 Frederic Fusier CLA 2008-09-05 12:14:09 EDT 
Our local performance tests do not show any specific regression for all the tests of FullSourceWorkspaceBuildTests...
Comment 5 Jerome Lanneluc CLA 2008-09-16 05:13:40 EDT 
Verified for 3.5M2 that the patch has been applied and that the tests are still green.
Comment 6 Kent Johnson CLA 2008-09-30 15:03:37 EDT 
*** Bug 248781 has been marked as a duplicate of this bug. ***
Comment 7 Stephan Herrmann CLA 2009-04-13 12:54:54 EDT 
While adopting this change into our variant I found some left-overs
in CompilerOptions which should probably be removed:
 * field OptionToIrritants
 * method long optionKeyToIrritant(String optionName)
I was trying to find a correct and safe type for this map, but actually
the code looks broken here - undecided whether to hold Longs or Integers.
=> should someone try to use this stuff (s)he will end up getting CCEs.

While I like this solution it sure is a more invasive change than what I 
had been using. So this change appears as a barrier when inspecting the 
version history of CompilerOptions: can't really relate old versions
to current state any more..  just a comment ;-)
Comment 8 Dani Megert CLA 2009-12-10 05:51:05 EST 
I didn't review the patch but quickly tried it out:
- the default severity needs to be 'Ignore' i.e. off by default
- the message should be closer to unused variables:
  ==> "The allocated object is never used"
Comment 9 Dani Megert CLA 2009-12-10 07:16:03 EST 
Please ignore the previous comment (wrong bug).