Equivalent .class fields should share the same symbol

Jan Lahoda jan.lahoda at oracle.com
Mon Apr 23 12:43:01 UTC 2018


I think the patch itself is OK, but I concur with Vicente that we may 
want focused testing. I've put the patch here:
http://cr.openjdk.java.net/~jlahoda/8202141/webrev.00/

And added a test that does not depend on the lambda deduplication.

Vicente, what do you think?

Thanks,
    Jan

On 23.4.2018 10:31, B. Blaser wrote:
> Is there something left to do with this?
> Tier1 seems OK (jdk & langtools are successful and hotspot looks quite
> good modulo some unrelated issues on my system [1]).
>
> Thanks,
> Bernard
>
> [1] http://mail.openjdk.java.net/pipermail/amber-dev/2018-April/003031.html
>
> On 17 April 2018 at 00:28, B. Blaser <bsrbnd at gmail.com> wrote:
>> On 16 April 2018 at 19:40, Jan Lahoda <jan.lahoda at oracle.com> wrote:
>>> On 16.4.2018 19:31, B. Blaser wrote:
>>>>
>>>> Hi Jan,
>>>>
>>>> On 16 April 2018 at 17:02, Jan Lahoda <jan.lahoda at oracle.com> wrote:
>>>>>
>>>>> Hi Bernard,
>>>>>
>>>>> [...]
>>>>>
>>>>> -should there be a method in Symtab managing the map? Something like:
>>>>> Symtab.getClassField(TypeSymbol tsym) {/*create the VarSymbol is
>>>>> needed*/}
>>>>
>>>>
>>>> I'll take a look if you think it's worth adding a method for this.
>>>
>>>
>>> I think it would be nicer than maintaining the map from another class, if
>>> possible/doable.
>>>
>>> Thanks,
>>>      Jan
>>
>> Here it is, javac tests are OK.
>> Cheers,
>> Bernard
>>
>> diff -r 8c85a1855e10
>> src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symtab.java
>> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symtab.java
>> Fri Apr 13 11:14:49 2018 -0700
>> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symtab.java
>> Mon Apr 16 23:33:02 2018 +0200
>> @@ -1,5 +1,5 @@
>>   /*
>> - * Copyright (c) 1999, 2017, Oracle and/or its affiliates. All rights reserved.
>> + * Copyright (c) 1999, 2018, Oracle and/or its affiliates. All rights reserved.
>>    * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>    *
>>    * This code is free software; you can redistribute it and/or modify it
>> @@ -51,6 +51,7 @@
>>   import com.sun.tools.javac.code.Type.JCVoidType;
>>   import com.sun.tools.javac.code.Type.MethodType;
>>   import com.sun.tools.javac.code.Type.UnknownType;
>> +import com.sun.tools.javac.code.Types.UniqueType;
>>   import com.sun.tools.javac.comp.Modules;
>>   import com.sun.tools.javac.util.Assert;
>>   import com.sun.tools.javac.util.Context;
>> @@ -247,6 +248,26 @@
>>        */
>>       private final Map<Name, ModuleSymbol> modules = new LinkedHashMap<>();
>>
>> +    private final Map<Types.UniqueType, VarSymbol> classFields = new
>> HashMap<>();
>> +
>> +    public VarSymbol getClassField(Type type, Types types) {
>> +        return classFields.computeIfAbsent(
>> +            new UniqueType(type, types), k -> {
>> +                Type arg = null;
>> +                if (type.getTag() == ARRAY || type.getTag() == CLASS)
>> +                    arg = types.erasure(type);
>> +                else if (type.isPrimitiveOrVoid())
>> +                    arg = types.boxedClass(type).type;
>> +                else
>> +                    throw new AssertionError(type);
>> +
>> +                Type t = new ClassType(
>> +                    classType.getEnclosingType(), List.of(arg),
>> classType.tsym);
>> +                return new VarSymbol(
>> +                    STATIC | PUBLIC | FINAL, names._class, t, type.tsym);
>> +            });
>> +    }
>> +
>>       public void initType(Type type, ClassSymbol c) {
>>           type.tsym = c;
>>           typeOfTag[type.getTag().ordinal()] = type;
>> diff -r 8c85a1855e10
>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
>> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
>> Fri Apr 13 11:14:49 2018 -0700
>> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
>> Mon Apr 16 23:33:02 2018 +0200
>> @@ -3731,11 +3731,7 @@
>>                   } else if (name == names._class) {
>>                       // In this case, we have already made sure in
>>                       // visitSelect that qualifier expression is a type.
>> -                    Type t = syms.classType;
>> -                    List<Type> typeargs = List.of(types.erasure(site));
>> -                    t = new ClassType(t.getEnclosingType(), typeargs, t.tsym);
>> -                    return new VarSymbol(
>> -                        STATIC | PUBLIC | FINAL, names._class, t, site.tsym);
>> +                    return syms.getClassField(site, types);
>>                   } else {
>>                       // We are seeing a plain identifier as selector.
>>                       Symbol sym = rs.findIdentInType(env, site, name,
>> resultInfo.pkind);
>> @@ -3773,11 +3769,7 @@
>>                   if (name == names._class) {
>>                       // In this case, we have already made sure in Select that
>>                       // qualifier expression is a type.
>> -                    Type t = syms.classType;
>> -                    Type arg = types.boxedClass(site).type;
>> -                    t = new ClassType(t.getEnclosingType(),
>> List.of(arg), t.tsym);
>> -                    return new VarSymbol(
>> -                        STATIC | PUBLIC | FINAL, names._class, t, site.tsym);
>> +                    return syms.getClassField(site, types);
>>                   } else {
>>                       log.error(pos, Errors.CantDeref(site));
>>                       return syms.errSymbol;
>> diff -r 8c85a1855e10
>> test/langtools/tools/javac/lambda/deduplication/Deduplication.java
>> --- a/test/langtools/tools/javac/lambda/deduplication/Deduplication.java
>> Fri Apr 13 11:14:49 2018 -0700
>> +++ b/test/langtools/tools/javac/lambda/deduplication/Deduplication.java
>> Mon Apr 16 23:33:02 2018 +0200
>> @@ -38,6 +38,31 @@
>>                   (Runnable) () -> { ( (Runnable) () -> {} ).run(); }
>>           );
>>
>> +        group(
>> +                (Runnable) () -> { Deduplication.class.toString(); },
>> +                (Runnable) () -> { Deduplication.class.toString(); }
>> +        );
>> +
>> +        group(
>> +                (Runnable) () -> { Integer[].class.toString(); },
>> +                (Runnable) () -> { Integer[].class.toString(); }
>> +        );
>> +
>> +        group(
>> +                (Runnable) () -> { char.class.toString(); },
>> +                (Runnable) () -> { char.class.toString(); }
>> +        );
>> +
>> +        group(
>> +                (Runnable) () -> { Void.class.toString(); },
>> +                (Runnable) () -> { Void.class.toString(); }
>> +        );
>> +
>> +        group(
>> +                (Runnable) () -> { void.class.toString(); },
>> +                (Runnable) () -> { void.class.toString(); }
>> +        );
>> +
>>           group((Function<String, Integer>) x -> x.hashCode());
>>           group((Function<Object, Integer>) x -> x.hashCode());


More information about the compiler-dev mailing list