Equivalent .class fields should share the same symbol

[Jan Lahoda]

Hi Bernard,

There are two things I wonder about:
-can the key to the map be TypeSymbol? Basically "site.tsym" instead of 
"new Types.UniqueType(site...)". To me that would feel simpler.
-should there be a method in Symtab managing the map? Something like:
Symtab.getClassField(TypeSymbol tsym) {/*create the VarSymbol is needed*/}

Thanks,
     Jan

On 15.4.2018 18:34, 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());
>