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

[Jan Lahoda]

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