Equivalent .class fields should share the same symbol

[Remi Forax]

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 

----- Mail original -----
> De: "B. Blaser" <bsrbnd at gmail.com>
> À: "compiler-dev" <compiler-dev at openjdk.java.net>
> Envoyé: Dimanche 15 Avril 2018 14:11:00
> Objet: Equivalent .class fields should share the same symbol

> Hi,
> 
> As mentioned in [1], equivalent .class fields should share the same symbol.
> The fix below caches .class symbols per unique type.
> 
> All javac tests are passing successfully.
> 
> Does this look reasonable?
> 
> Thanks,
> Bernard
> 
> [1] http://mail.openjdk.java.net/pipermail/amber-dev/2018-April/002897.html
> 
> 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 13:42:07 2018 +0200
> @@ -247,6 +247,8 @@
>      */
>     private final Map<Name, ModuleSymbol> modules = new LinkedHashMap<>();
> 
> +    public final Map<Types.UniqueType, VarSymbol> _class = 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 13:42:07 2018 +0200
> @@ -3734,8 +3734,11 @@
>                     Type t = syms.classType;
>                     List<Type> typeargs = List.of(types.erasure(site));
>                     t = new ClassType(t.getEnclosingType(), typeargs, t.tsym);
> -                    return new VarSymbol(
> +                    VarSymbol _class = new VarSymbol(
>                         STATIC | PUBLIC | FINAL, names._class, t, site.tsym);
> +                    VarSymbol existing = syms._class.putIfAbsent(
> +                        new Types.UniqueType(site, types), _class);
> +                    return existing != null ? existing : _class;
>                 } else {
>                     // We are seeing a plain identifier as selector.
>                     Symbol sym = rs.findIdentInType(env, site, name,
> resultInfo.pkind);
> @@ -3776,8 +3779,11 @@
>                     Type t = syms.classType;
>                     Type arg = types.boxedClass(site).type;
>                     t = new ClassType(t.getEnclosingType(),
> List.of(arg), t.tsym);
> -                    return new VarSymbol(
> +                    VarSymbol _class = new VarSymbol(
>                         STATIC | PUBLIC | FINAL, names._class, t, site.tsym);
> +                    VarSymbol existing = syms._class.putIfAbsent(
> +                        new Types.UniqueType(site, types), _class);
> +                    return existing != null ? existing : _class;
>                 } 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 13:42:07 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());