RFR: 8343540: Report preview error for inherited effectively-preview methods

[Jan Lahoda]

On Thu, 7 Nov 2024 13:10:34 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

> Consider a case where a new preview interface is introduced to JDK, and an existing non-preview class is retrofitted to use it. I.e. a case like:
> 
> 
> //JDK types:
> @PreviewFeature(...)
> public interface NewAPI {
>     public default void test() {}
> }
> public class ExistingRetrofittedClass implements NewAPI {}
> 
> //user type:
> class Test {
>     void t(ExistingRetrofittedClass c) {
>         c.test();
>     }
> }
> 
> 
> There is currently no error or warning about the invocation of the test method, as the method itself is not marked as preview, and the receiver is not preview either.
> 
> The proposal herein is that invoking a method on a receiver that is not of preview type itself, but the method is declared in a preview class or interface, the method will be considered to be a preview method, and the appropriate error or warning will be printed. Similar behavior is implementing for dereferencing fields.
> 
> 
> Similarly when implementing or overriding such "effectively" preview methods, like:
> 
> 
> //user types:
> class Test1 extends ExistingRetrofittedClass {
>     public void test() {}
> }
> 
> 
> java also does not produce any error or warning. If the method itself would be marked as preview, an error or warning would be printed.
> 
> The proposal herein is to produce an error or warning for method declared inside a non-preview class or interface, which is overridden by a method declared inside a preview class or interface.

> _Mailing list message from [Alex Buckley](mailto:alex.buckley at oracle.com) on [compiler-dev](mailto:compiler-dev at mail.openjdk.org):_
> Suppose ExistingRetrofittedClass does indeed override the test method. I would expect both of the examples in your original mail to give a warning/error. Both the invocation of `c.test()`, and the overriding of test in Test1, refer to an "effectively" preview method.

If `ExistingRetrofittedClass` would override the effectively preview method "`test`", without marking the method as preview, I am not sure it is desirable for the uses of the overriding method in `ExistingRetrofittedClass` to report errors/warnings.

There are two reasons for that:
- what if the `test` method existed before, and now is it is just being included in the superinterface? The method existed, and should still be generally usable, but if it would automatically get the preview handling, there would be no way to "un-subscribe" from the preview handling. But if the overriding method is by default an ordinary, non-preview, method, and the library developer desires the override to also be handled as a preview method, the solution is simple: use `@PreviewFeature` of the overriding method.
- for each method invocation, javac would need to go through all methods that the invoked method overrides, to see if some of them are (effectively) preview methods. This would be fairly costly, but only very few (if any) methods are overriding a preview method. So, overall, it seems it would be a considerable cost in the compilation time, for a very small gain. (Where the gain is only avoiding the addition of `@PreviewFeature` on a very limited number of methods.)

-------------

PR Comment: https://git.openjdk.org/jdk/pull/21953#issuecomment-2462815311