Most specific method in diagnostic generation for overload resolution

Maurizio Cimadamore maurizio.cimadamore at oracle.com
Tue Sep 26 11:17:21 UTC 2017


One comment - we already have a method for 'filtering candidates' - see 
InapplicableSymbolsError::filterCandidates.

Currently, that method is simply discarding methods with wrong arity if 
the compact diagnostics are enabled (default since JDK 8).

I wonder if we could add to this method so that it would also filter out 
overridden methods? That way you could use the compiler flag to 
enable/disable the filtering.

Maurizio


On 26/09/17 01:14, Vicente Romero wrote:
> Hi Bernard,
>
> Thanks for the patch. I was playing a bit with it and made some 
> modifications to it, my changes are just in the put method of 
> MostSpecificMap.
>
> diff -r 19293ea3999f -r 102d38c4f033 
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java
> --- 
> a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java 
> Fri Sep 08 18:24:15 2017 +0000
> +++ 
> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java 
> Mon Sep 25 19:57:03 2017 -0400
> @@ -59,6 +59,7 @@
>  import java.util.HashSet;
>  import java.util.Iterator;
>  import java.util.LinkedHashMap;
> +import java.util.LinkedList;
>  import java.util.Map;
>  import java.util.Set;
>  import java.util.function.BiFunction;
> @@ -3995,14 +3996,30 @@
>          }
>          //where
>              private Map<Symbol, JCDiagnostic> mapCandidates() {
> -                Map<Symbol, JCDiagnostic> candidates = new 
> LinkedHashMap<>();
> +                MostSpecificMap candidates = new MostSpecificMap();
>                  for (Candidate c : resolveContext.candidates) {
>                      if (c.isApplicable()) continue;
> -                    candidates.put(c.sym, c.details);
> +                    candidates.put(c);
>                  }
>                  return candidates;
>              }
>
> +            @SuppressWarnings("serial")
> +            private class MostSpecificMap extends 
> LinkedHashMap<Symbol, JCDiagnostic> {
> +                private void put(Candidate c) {
> +                    keySet().stream()
> +                            .filter(s -> s != c.sym)
> +                            .filter(s -> c.sym.overrides(s, 
> (TypeSymbol)s.owner, types, false) )
> +                            .forEach(s -> remove(s));
> +
> +                    if (!keySet().stream()
> +                            .filter(s -> s == c.sym)
> +                            .anyMatch(s -> s.overrides(c.sym, 
> (TypeSymbol)c.sym.owner, types, false))) {
> +                        put(c.sym, c.details);
> +                    }
> +                }
> +            }
> +
>              Map<Symbol, JCDiagnostic> filterCandidates(Map<Symbol, 
> JCDiagnostic> candidatesMap) {
>                  Map<Symbol, JCDiagnostic> candidates = new 
> LinkedHashMap<>();
>                  for (Map.Entry<Symbol, JCDiagnostic> _entry : 
> candidatesMap.entrySet()) {
>
>
> in your previous version you were doing:
>
> +                      new LinkedList<>(keySet()).stream()
> +                            .filter( s -> !s.equals(c.sym) )
> +                            .filter( s -> c.sym.overrides(s, (TypeSymbol)s.owner, types, false) )
> +                            .forEach( s -> remove(s) );
> by creating a brand new LinkedList my impression is that any removal 
> of a key would happen in the linked list, not in the original map so 
> the first statement was a no-op. Also as symbols are unique in javac 
> you can safely use the "==" operator for comparison. Does my version 
> captures what you intended?
>
> In addition to all this I think that we should consider if List is the 
> best data structure to store the candidates. It sounds like it could 
> be a map to avoid duplicates, probably this won't be a great gain but 
> there is no point on storing duplicates when we can check for them in 
> O(1) time,
>
> Thanks,
> Vicente
>
> On 09/20/2017 10:56 AM, B. Blaser wrote:
>> Hi,
>>
>> Related to [1], I wrote a specialized map to store only the most
>> specific candidates during the diagnostic generation for overload
>> resolution, here under.
>>
>> It's then used in "Resolve.InapplicableSymbolsError.mapCandidates()"
>> when non-applicable methods are collected to generate the diagnostic.
>>
>> What do you think?
>> Bernard
>>
>> [1]http://mail.openjdk.java.net/pipermail/compiler-dev/2017-September/011062.html
>>
>> diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java
>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java
>> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java
>> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java
>> @@ -59,6 +59,7 @@
>>   import java.util.HashSet;
>>   import java.util.Iterator;
>>   import java.util.LinkedHashMap;
>> +import java.util.LinkedList;
>>   import java.util.Map;
>>   import java.util.Set;
>>   import java.util.function.BiFunction;
>> @@ -3995,14 +3996,29 @@
>>           }
>>           //where
>>               private Map<Symbol, JCDiagnostic> mapCandidates() {
>> -                Map<Symbol, JCDiagnostic> candidates = new LinkedHashMap<>();
>> +                MostSpecificMap candidates = new MostSpecificMap();
>>                   for (Candidate c : resolveContext.candidates) {
>>                       if (c.isApplicable()) continue;
>> -                    candidates.put(c.sym, c.details);
>> +                    candidates.put(c);
>>                   }
>>                   return candidates;
>>               }
>>
>> +            @SuppressWarnings("serial")
>> +            private class MostSpecificMap extends
>> LinkedHashMap<Symbol, JCDiagnostic> {
>> +                private void put(Candidate c) {
>> +                    new LinkedList<>(keySet()).stream()
>> +                            .filter( s -> !s.equals(c.sym) )
>> +                            .filter( s -> c.sym.overrides(s,
>> (TypeSymbol)s.owner, types, false) )
>> +                            .forEach( s -> remove(s) );
>> +
>> +                    if (!keySet().stream()
>> +                            .filter( s -> !s.equals(c.sym) )
>> +                            .anyMatch( s -> s.overrides(c.sym,
>> (TypeSymbol)c.sym.owner, types, false) ))
>> +                        put(c.sym, c.details);
>> +                }
>> +            }
>> +
>>               Map<Symbol, JCDiagnostic> filterCandidates(Map<Symbol,
>> JCDiagnostic> candidatesMap) {
>>                   Map<Symbol, JCDiagnostic> candidates = new LinkedHashMap<>();
>>                   for (Map.Entry<Symbol, JCDiagnostic> _entry :
>> candidatesMap.entrySet()) {
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20170926/5b7bf8c8/attachment-0001.html>


More information about the compiler-dev mailing list