Summary: | Private enum constant incorrectly marked as never read locally | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Matthew Hall <qualidafial> | ||||||
Component: | Core | Assignee: | Srikanth Sankaran <srikanth_sankaran> | ||||||
Status: | VERIFIED FIXED | QA Contact: | |||||||
Severity: | normal | ||||||||
Priority: | P3 | CC: | david_audel, kent_johnson, Olivier_Thomann | ||||||
Version: | 3.5 | ||||||||
Target Milestone: | 3.5 M7 | ||||||||
Hardware: | PC | ||||||||
OS: | Windows XP | ||||||||
Whiteboard: | |||||||||
Attachments: |
|
Description
Matthew Hall
2009-03-09 12:30:55 EDT
Created attachment 129830 [details]
Proposed patch & tests
This patch inhibits the emission of unused warning for enumerator fields.
As the submitted test case points out, enumeration constants can be
accessed in non-obvious ways that makes it difficult to reliably track
their usage. In addition to the the synthesized values() method,
valueOf(String) method and Enum.valueOf(Class<T>, String) method could
also be used to materialize enumerators indirectly.
Initially I attempted tracking the usage of these methods and flagging the
symbolic constants as being referenced, should these indirect means be seen
to employed - this is not again reliable as the valueOf() methods materialize
enumerators out of thin air (from Strings which could be variables which
the compiler has no means of peeking into at compile time).
Since this unused local reporting could be configured into an ERROR, this
has the potential to break builds, hence this patch simply inhibits the
warning/error for private enums.
I'd have to do a test, but I don't think that Enum.valueOf will work for a private enum. So in theory the usage test could depend on invocations of PrivateEnum.valueOf or PrivateEnum.values() (if you want to go to that much trouble) (In reply to comment #2) > I'd have to do a test, but I don't think that Enum.valueOf will work for a > private enum. So in theory the usage test could depend on invocations of > PrivateEnum.valueOf or PrivateEnum.values() (if you want to go to that much > trouble) The attached patch does have a contrived test that tests the use of Enum.valueOf(Class<T>, String). PrivateEnum.valueOf method and the Enum.valueOf(Class<T>, String) are no different really from in general using reflection to materialize handles to methods and fields. So this problem can appear in non-enum scenarios too (see bug #266132) So, one plausible approach is to ignore the valueOf method pair and track the usage of only values() method and flag all enumerator constants as being used if values() method was invoked. Kent, what is your opinion on this ? Do you think it is worthwhile tracking use of values() and flagging the fields as being in use ? If so, is it worthwhile to dedicate a bit at the enum *type* level e.g say: ExtraCompilerModifiers.AccIndirectlyUsed so the whole enum type could be flagged at one stroke ? I am just worried about iterating over the potentially numerous fields for every usage of values(). Much ado ? > Do you think it is worthwhile tracking use of values() and flagging the
> fields as being in use ?
No I don't.
I like the patch as is: Warn against the unused private enum type, but skip the constants.
Maybe mark it as unused if the enum class itself is never referenced? (In reply to comment #4) > I like the patch as is: Warn against the unused private enum type, but skip the > constants. (In reply to comment #5) > Maybe mark it as unused if the enum class itself is never referenced? Erm, I didn't read comment #4 too carefully before posting, I think we're on the same page. Created attachment 130205 [details]
Revised patch
Changed patch to make the "Enum.valueOf(Class<T>, String)"
test scenario more direct.
Released in HEAD for 3.5M7 Verified for 3.5M7 using I20090426-2000 |