Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

[Stuart Marks]

On 8/29/12 4:56 PM, Joe Darcy wrote:
> On 8/29/2012 4:20 PM, Stuart Marks wrote:
>> The original code was like this:
>>
>>  427     private static int getMask(String actions) {
>>              ...
>>  435         // Check against use of constants (used heavily within the JDK)
>>  436         if (actions == SecurityConstants.FILE_READ_ACTION) {
>>  437             return READ;
>>  438         } else if (actions == SecurityConstants.FILE_WRITE_ACTION) {
>>
>> The various SecurityConstants being used here are Strings.
>>
>> Note that this is doing String comparisons using == which is usually a bug.
>> But this code is relying on String interning to provide object identity for
>> equal string literals, in order to provide a fast path optimization for these
>> common cases. Changing this code to use switch-over-strings would be
>> semantically equivalent, but it would probably slow things down, since switch
>> provides equals() semantics instead of == semantics.
>
> This would be a fine point to highlight in the comments around this check!

The comment at line 435 did serve to tip me off that something unusual was 
going on with this code, so it served its purpose. But I'll admit that it's not 
as explicit as it could be. How about this:

     // Use object identity comparisons with String constants for performance
     // reasons (these values are used heavily within the JDK).

s'marks