Code review request: 7077389 Reflection classes do not build with javac -Xlint:all -Werror
alexandre.boulgakov at oracle.com
Thu Aug 11 14:25:16 PDT 2011
Hello, Joe and Rémi:
Please review this updated webrev that incorporates your suggestions.
* removed the 4 serialVersionUIDs I added;
* used "new TypeName<?>[...]" in place of "new TypeName[...]" and
removed associated SuppressWarnings annotations; and
* changed TypeVariable<? extends GenericDeclaration> to
TypeVariable<?> since the conversion would otherwise be unchecked.
On 8/11/2011 9:19 AM, Joe Darcy wrote:
> Alexandre Boulgakov wrote:
>> Hello Joe,
>> Thanks for reviewing and responding so quickly!
>> On 8/10/2011 12: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/
>>>> 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.
>>> Hi Sasha.
>>> I approve the code changes with the following exceptions:
>>> Offhand, I'm not sure of the semantics of this change.
>> Do you mean changing the type of fieldOffset or the change in the
>> constructor? I changed fieldOffset from an int to a long per the
>> specification change mentioned in Unsafe.staticFieldOffset(Field) and
>> Unsafe.objectFieldOffset(Field). The change in the constructor is
>> related and stems from the fact that Unsafe.fieldOffset(Field) is now
>> deprecated. Since sun.reflect.UnsafeFieldAccessorImpl is
>> package-private, this change should not have any external effects.
> Okay; I wasn't aware of the reasoning for this change and I noted the
> int -> long field change; I think that change is fine then.
>>> 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.
>> Since I only have one week left in my internship, I would like to get
>> the changes in as quickly as possible. Should I remove the
>> serialVersionUIDs but keep the remaining changes?
More information about the core-libs-dev