Equivalent .class fields should share the same symbol

[Vicente Romero]

On 04/16/2018 01:06 PM, B. Blaser wrote:
> 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?

I guess I was thinking about a dedicated test, probably a combo test, 
but yes I guess that reusing the deduplication test could be a good start

>
> Thanks,
> Bernard

Vicente

>
>
>> 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());
>>