Thanks, Christian.  Here's the updated webrev:

http://cr.openjdk.java.net/~jrose/7001424/webrev.01/

There are three code changes among the javadoc changes.  Unit tests continue to 
pass.

Responses below.

-- John

On Dec 16, 2010, at 7:39 AM, Christian Thalinger wrote:

> On Dec 16, 2010, at 1:04 PM, John Rose wrote:
>> This is a JDK-only change request, in response to the last several weeks of 
>> JSR 292 EG work.
>> 
>> http://cr.openjdk.java.net/~jrose/7001424/webrev.00/
>> 
>> May I have an eyeball or two, please?
>> 
>> Thanks,
> 
> src/share/classes/java/dyn/ClassValue.java:
> 
>  93      * If no value has yet been computed, it is obtained by
>  94      * by an invocation of the {...@link #computeValue computeValue} 
> method.
> 
> Superfluous "by".

Fixed.

> src/share/classes/java/dyn/MethodHandle.java:
> 
> 241  * This is true even if the method handle is published through a shared
> 242  * variables in a data race.
> 
>        ^variable

Thanks.

> 651      * If the implementation is faced is able to prove that an equivalent
> 652      * type handler call has already occurred (on the same two arguments),
> 
> I'm not sure this is correct: "is faced is able"

Deleted "is faced".

> 331          * <li>If no access is allowed, the suffix is "/noaccess".
> 332          * <li>If only public access is allowed, the suffix is "/public".
> 333          * <li>If only public and package access are allowed, the suffix 
> is "/package".
> 334          * <li>If only public, package, and private access are allowed, 
> the suffix is "/private".
> 
> I don't understand this.  Why is the suffix "/private" if public access is 
> allowed?

If there are several permission, the highest is the one mentioned (private) not 
the lowest (public).

It turns out that "protected" is the most restrictive.  You lose that as soon 
as you shift views from the original lookup object, leaving only "private" at 
most.  I'll add some clarification to the javadoc, and to the code.

> src/share/classes/java/dyn/MethodType.java:
> 
>  73  * in a class file's constant pool as constants. The may be loaded by an 
> {...@code ldc}
> 
> "The may be" seems wrong.  "They" or "The entry"?

Improved:

 * Bytecode in the JVM can directly obtain a method handle
 * for any accessible method from a {...@code ldc} instruction
 * which refers to a {...@code CONSTANT_MethodHandle} constant pool entry.
 * (Each such entry refers directly to a {...@code CONSTANT_Methodref},
 * {...@code CONSTANT_InterfaceMethodref}, or {...@code CONSTANT_Fieldref}
 * constant pool entry.

> 316      * @param ptypesToInsert zero or more a new parameter types to insert 
> after the end of the parameter list
> 
> "more a new", is this correct?

Oops; deleted "a", four times.  Thanks.
> 
> 533      * If a type name name is array, it the base type followed
> 
> There is obviously something wrong.

Deleted those two lines:

    /**
     * The string representation of a method type is a
     * parenthesis enclosed, comma separated list of type names,
     * followed immediately by the return type.
     * <p>
     * Each type is represented by its
     * {...@link java.lang.Class#getSimpleName simple name}.
     */
    @Override
    public String toString() {

> src/share/classes/java/dyn/package-info.java:
> 
> 189  * If the resoultion succeeds, the same object reference is produced
> 
> Typo.

Fixed.

> src/share/classes/java/dyn/Switcher.java:
> 
>  43  * The invalidation is also permanent, which means the switcher
>  44  * 
> 
> What about the switcher?  It just became interesting... ;-)

As Remi said.  Here's what I put:

 * The invalidation is also permanent, which means the switcher
 * can change state only once.
 * The switcher will always delegate to {...@code F} after being invalidated.
 * At that point {...@code guardWithTest} may ignore {...@code T} and return 
{...@code F}.

_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Reply via email to