Equivalent .class fields should share the same symbol

[B. Blaser]

Hi Vicente,

On 16 April 2018 at 16:07, Vicente Romero <vicente.romero at oracle.com> wrote:
> Hi Bernard,
>
> The change looks good. Suggestion: it would be nice to include some
> regression tests with it,
>
> Thanks,
> Vicente

I added this use case to the de-duplication test (at the end of the
patch) since the problem appears in such situations.
Did you miss it or do you have any other idea in mind?

Thanks,
Bernard


> On 04/15/2018 12:34 PM, B. Blaser wrote:
>>
>> Hi Rémi,
>>
>> On 15 April 2018 at 14:31, Remi Forax <forax at univ-mlv.fr> wrote:
>>>
>>> Hi,
>>> You can use Map.computeIfAbsent instead of putIfAbsent to avoid to create
>>> the Symbol before checking if one exist or not.
>>>
>>> Also you using _class for several purpose, the Map should be named
>>> classes and the local variable classSymbol or something like that.
>>>
>>> cheers,
>>> Rémi
>>
>> Thanks for your feedback, nice suggestions.
>> Here is the updated version.
>>
>> 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
>> Sun Apr 15 17:15:36 2018 +0200
>> @@ -247,6 +247,8 @@
>>        */
>>       private final Map<Name, ModuleSymbol> modules = new
>> LinkedHashMap<>();
>>
>> +    public final Map<Types.UniqueType, VarSymbol> classFields = new
>> HashMap<>();
>> +
>>       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
>> Sun Apr 15 17:15:36 2018 +0200
>> @@ -3731,11 +3731,14 @@
>>                   } 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.classFields.computeIfAbsent(
>> +                        new Types.UniqueType(site, types), k -> {
>> +                            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);
>> +                        });
>>                   } else {
>>                       // We are seeing a plain identifier as selector.
>>                       Symbol sym = rs.findIdentInType(env, site, name,
>> resultInfo.pkind);
>> @@ -3773,11 +3776,14 @@
>>                   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.classFields.computeIfAbsent(
>> +                        new Types.UniqueType(site, types), k -> {
>> +                            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);
>> +                         });
>>                   } 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
>> Sun Apr 15 17:15:36 2018 +0200
>> @@ -38,6 +38,21 @@
>>                   (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((Function<String, Integer>) x -> x.hashCode());
>>           group((Function<Object, Integer>) x -> x.hashCode());
>
>