RFR: JDK-8187950: javax.lang.model APIs throws CompletionFailure or a subtype of CompletionFailure.

[Jonathan Gibbons]

Looks OK to me,

-- Jon


On 2/19/18 10:57 AM, Jan Lahoda wrote:
> Hi,
>
> I was testing the webrev.01-ext, and so far it seems OK, so I added 
> that to the patch.
>
> One thing I've noticed is that 
> com.sun.source.tree.Scope.getLocalElements() may still cause the 
> CompletionFailure to be thrown to the client code, so I've fixed that.
>
> Updated webrev:
> http://cr.openjdk.java.net/~jlahoda/8187950/webrev.02/
>
> There are two bugs related to this (JDK-8177068 and JDK-8198378, both 
> already existing) on which I plan to work separately.
>
> Are there any comments/feedback on the patch?
>
> Thanks,
>     Jan
>
> On 14.2.2018 18:58, Jan Lahoda wrote:
>> On 14.2.2018 18:04, Liam Miller-Cushon wrote:
>>> On Wed, Feb 14, 2018 at 7:33 AM, Jan Lahoda <jan.lahoda at oracle.com
>>> <mailto:jan.lahoda at oracle.com>> wrote:
>>>
>>>     Today, I got an idea that might solve this issue without too much
>>>     hassle, sketched here:
>>>     http://cr.openjdk.java.net/~jlahoda/8187950/webrev.01-ext/
>>> <http://cr.openjdk.java.net/~jlahoda/8187950/webrev.01-ext/>
>>>
>>>     Will need more testing, but possibly could work. Any 
>>> opinions/ideas?
>>>
>>>
>>> Thanks - that fixed the first two regressions I saw. I'm not sure
>>> whether getKind() was the only instance of this problem, but I'm 
>>> running
>>> more tests and will report back.
>>
>> Cool, thanks!
>>
>>>
>>>     One thing I'd like to point out is that this can (AFAIK) happen 
>>> even
>>>     now - if the interface B is completed in a way that will throw away
>>>     the CompletionFailure (e.g. Elements.getTypeElement), then the
>>>     subsequent code will AFAIK see the same state as this AP sees. 
>>> So it
>>>     seemed somewhat acceptable to let the behavior be similar in case
>>>     where the CompletionFailure would be thrown, esp. since I didn't 
>>> see
>>>     a good way to do better.
>>>
>>>
>>> Good point. I guess I've seen more issues around processors catching
>>> completion failures and leaving the model in a bad state for other
>>> processors (or javac), than around the completion failure itself 
>>> causing
>>> problems. It's important that processors can detect when they're seeing
>>> invalid input. If they end up needing to be more vigilant about 
>>> checking
>>
>> FWIW, in the current state, it is possible to check
>> DeclaredType.asElement().asType().getKind() == TypeKind.ERROR to see if
>> the type's symbol is broken. (I agree it is quite cumbersome.)
>>
>>> for error types that's a slightly incompatible change.
>>
>> AFAIK, doing TypeElement.getSuperclass() may currently lead to several
>> results depending on the exact circumstances:
>> 1) a DECLARED type, that will throw a CompletionFailure (once) at some
>> point if used
>> 2) a DECLARED type that will not throw a CompletionFailure (because it
>> was already thrown)
>> 3) an ERROR type (if the TypeElement originates in the source code, and
>> its superclass is missing)
>>
>> So eliminating any of these is probably a slightly incompatible change
>> (which will need a CSR), but I personally think it is better than having
>> different behaviors. OTOH, I suspect many APs already deal with these in
>> some ways, so eliminating some shouldn't hopefully be that disruptive.
>>
>>>
>>> There was some discussion in JDK-8187950 about declaring an 
>>> exception in
>>> the API contract for methods that currently throw completion failures.
>>> Did you consider taking that approach, and having the completion 
>>> failure
>>> be rethrown to ensure other processors and javac see it if they try to
>>> complete the same symbol? Maybe JDK-8190054 answers that question - it
>>> sounds like there's a preference for returning error objects instead of
>>> throwing?
>>
>> I was thinking of that, but I personally:
>> -think the model is cleaner without the exceptions
>> -am not convinced that it would be much simpler if we tried to change
>> the implementation to more consistently throw the exception
>>
>> Thanks for your feedback,
>>      Jan