Most specific method in diagnostic generation for overload resolution
Vicente Romero
vicente.romero at oracle.com
Tue Sep 26 13:50:42 UTC 2017
On 09/26/2017 07:17 AM, Maurizio Cimadamore wrote:
>
> 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.
>
question: shouldn't overridden methods be filtered out for both diagnostics?
> 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/2f229970/attachment-0001.html>
More information about the compiler-dev
mailing list