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