Code review request: 7077389 Reflection classes do not build with javac -Xlint:all -Werror

[Alexandre Boulgakov]

Hello Remi,

Thanks for the feedback!

On 8/10/2011 1:22 PM, Rémi Forax wrote:
> On 08/10/2011 09:39 PM, Joe Darcy wrote:
>> On 8/10/2011 11:36 AM, Alexandre Boulgakov wrote:
>>> Hello Joe,
>>>
>>> Could you please review these small changes to remove javac build 
>>> warnings from the reflection classes?
>>>
>>> Bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7077389
>>> webrev: http://cr.openjdk.java.net/~mduigou/7077389/0/webrev/ 
>>> <http://cr.openjdk.java.net/%7Emduigou/7077389/0/webrev/>
>>>
>>> I haven't updated the copyrights to avoid polluting the webrev.
>>> I did not add JAVAC_WARNINGS_FATAL=true to the makefile because it 
>>> is responsible for other packages which still have build warnings.
>>>
>>> Thanks,
>>> Sasha
>>
>> Hi Sasha.
>>
>> I approve the code changes with the following exceptions:
>>
>>     src/share/classes/sun/reflect/UnsafeFieldAccessorImpl.java
>>
>> Offhand, I'm not sure of the semantics of this change.
>>
>> While adding serialVersionUIDs in sun/reflect/annotation/* is a good 
>> change, I believe this interacts with another bug I'm working on so 
>> please hold off on those changes for now.
>>
>> Thanks,
>>
>> -Joe
>
> In AccessorGenerator:
>
>   protected static final Class<?>[] primitiveTypes = new Class[] {
>
> should be
>
>   protected static final Class<?>[] primitiveTypes = new Class<?>[] {
Generic array creation is not currently supported (here and for your 
other suggestions). Additionally, if I'm not mistaken, wildcards are 
limited to variable declarations, since they wouldn't make sense on a 
constructor or method call.
>
> in AnnotationParser:
>
>   type.getClassLoader(), new Class[] { type },
>
> should be
>
>   type.getClassLoader(), new Class<?>[] { type },
>
>
> in TypeVariableImpl:
>
>   TypeVariable<? extends GenericDeclaration>  that =
>                     (TypeVariable<? extends GenericDeclaration>) o;
>
> should be
>      TypeVariable<?>  that =  (TypeVariable<?>) o;
I believe that it is more clear why we can assign the result of 
that.getGenericDeclaration() to a GenericDeclaration if we include the " 
extends GenericDeclaration" constraint on the wildcard.

-Sasha
>
>
> In GenericDeclRepository:
>
>   @SuppressWarnings("rawtypes")
>             TypeVariable<?>[] tps = new TypeVariable[ftps.length];
>
> should be
>             TypeVariable<?>[] tps = new TypeVariable<?>[ftps.length];
>
>
> It seems that the compiler is not able to find a raw type
> if it's the component type of an array.
> (CC to compiler-dev)
> cheers,
> Rémi
>