On 08/10/2011 10:43 PM, Alexandre Boulgakov wrote:
Hello Remi,


Hello,

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).

If the generics is an unbound wildcard, there is no problem.
To reuse the JLS words, a type parametrized by an unbound wildcard is reified.

Additionally, if I'm not mistaken, wildcards are limited to variable declarations, since they wouldn't make sense on a constructor or method call.

wildcards can be used only inside a parameterized type, whenever you use that type to define
a local variable or the type used by new.
Also you can't use a wildcard as an argument type, i.e. you can't substitute a T to a ?.

So here, there is no problem.


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.

but here you fall into a corner case of the spec,
a cast to TypeVariable<?> is safe (again it's a reified type) but
a cast to TypeVariable<? extends GenericDeclaration> is not safe
even if TypeVariable is declared like this:
  class TypeVariable<D extends GenericDeclaration> { ... }


-Sasha

Rémi



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


Reply via email to