Comment 1 Dirk Baeumer 2004-10-19 05:05:34 EDT

The downside of adding these warning is that users which used another Java 
compiler will be faced with lots of new warnings when they switch to Eclipse 
which might let them think that there is something wrong with the Eclipse Java 
compiler. 

From a programmer point of view I second the request.

Comment 2 Philipe Mulet 2004-10-19 07:53:43 EDT

Unused local variables are sometimes useful for debugging purpose, they hold a
value on can inspect (I remember I was optimizing them out at compile-time, and
people jumped on me for this - this is why you have it as an option nowadays).

Increasing the severity level of some optional diagnosis is something we should
look into.

Wondering if we shouldn't think of compiler profiles, where you would control
group of options instead.
Could imagine:  level-0/stick to JLS
                level-1/flag unused constructs
                level-2/common mistakes
                level-3/...

?

Comment 3 Philipe Mulet 2005-04-07 08:01:47 EDT

Deferring post 3.1

Comment 4 Philipe Mulet 2005-05-30 05:31:37 EDT

reopening for consideration within 3.1

Comment 5 Philipe Mulet 2005-05-30 05:32:22 EDT

Frederic : pls comment on results of your investigations on changing compiler
defaults.

Comment 6 Frederic Fusier 2005-05-30 11:14:54 EDT

Created attachment 21965 [details]
Patch to implement this default changes

Note that this patch also includes changes for default on following options:
 - CompilerOptions.OPTION_ReportFieldHiding: Ignore -> Warning
 - CompilerOptions.OPTION_ReportLocalVariableHiding: Ignore -> Warning
 - JavaCore.COMPILER_PB_INVALID_JAVADOC: Ignore -> Warning
 - JavaCore.COMPILER_PB_INVALID_JAVADOC_TAGS__DEPRECATED_REF: Enabled->Disabled

 - JavaCore.COMPILER_PB_INVALID_JAVADOC_TAGS__NOT_VISIBLE_REF:
Enabled->Disabled

Comment 7 Frederic Fusier 2005-05-30 11:23:00 EDT

First performance results show that:
1) for batch compiler impact of these new default optoins seems to be negligible
   (ie. less than our tests measure error rate (~2%)),
2) for builder impact is around 5% worst mainly due to the fact that javadoc are 
   parsed.

Comment 8 Frederic Fusier 2005-05-30 11:30:11 EDT

Philippe,

I intend to modify test testFullBuildDefault() in FullSourceWorkspaceBuildTests
to explain expected failure by adding following line:
setComment(Performance.EXPLAINS_DEGRADATION_COMMENT, "J2SE 5.0 support + default
warnings change since 3.0 (typically parse of Javadoc comments)");

Please validate, thx

Comment 9 Philipe Mulet 2005-05-30 11:35:50 EDT

"J2SE 5.0 support + additional warnings (e.g. validate Javadoc comments)"

Comment 10 Philipe Mulet 2005-05-30 11:40:22 EDT

Need to think about compromise between perf degradation and out of the box
experience improvement... 

Comment 11 Philipe Mulet 2005-05-30 11:48:26 EDT

Interacted with Erich on consequences. We agreed to leave javadoc turned off,
relying on documentation to mention it as a good tip.

Comment 12 Frederic Fusier 2005-05-31 07:58:13 EDT

Created attachment 22020 [details]
New patch for this functionality

Finally, here are options default change with this patch:
 - CompilerOptions.OPTION_ReportUnusedLocal: Ignore -> Warning
 - CompilerOptions.OPTION_ReportUnusedPrivateMember: Ignore -> Warning
 - CompilerOptions.OPTION_ReportFieldHiding: Ignore -> Warning
 - CompilerOptions.OPTION_ReportLocalVariableHiding: Ignore -> Warning

Comment 13 Frederic Fusier 2005-05-31 08:05:56 EDT

Created attachment 22022 [details]
New patch for this functionality

Previous patch didn't change default for
CompilerOptions.OPTION_ReportUnusedLocal.
Sorry for the confusion, this one is correct...

I'll start new performance tests this afternoon to verify that everything is
still OK...

Comment 14 Frederic Fusier 2005-06-01 05:32:36 EDT

I confirm that performance are not significantly impacted by these default
values changes (ie. less than 1% measured with our performance tests).

Dirk, please let me know when it would ok for me to release the fix in HEAD, thx

Comment 15 Dirk Baeumer 2005-06-01 12:52:23 EDT

Frederic, sorry for the delay but I spent most of the time today to sort out
bugs. I will have a look at it tomorrow.

Comment 16 Tobias Widmer 2005-06-03 10:42:36 EDT

Created attachment 22323 [details]
patches for JDT UI/Text

Attaching patches for JDT UI/JDT Text

Comment 17 Frederic Fusier 2005-06-03 10:46:41 EDT

Fixed and released in HEAD.

Batch compiler now warns byt default field hiding, local variable hiding, unused
local varaibles and unused private members.

[jdt-core internal]
Changes done in CompilerOptions: following bits:
		| UnusedLocalVariable
		| UnusedPrivateMember
		| FieldHiding
		| LocalVariableHiding
was added to warningThreshold.

No specific test cases added but failing existing ones modified to disable these
values...

Comment 18 Olivier Thomann 2005-06-06 15:52:46 EDT

Verified using N20050606-0010 + JDT/Core HEAD

Comment 19 Jerome Lanneluc 2005-06-10 06:44:10 EDT

Verified with I20050610-0010