RFR: JDK-8226585: Improve javac messages for using a preview API

[Jan Lahoda]

18. října 2019 14:05:45 SELČ, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> napsal:
>Looks good - but I suggest garbage collecting the isEssentialAPI() from
>
>the @PreviewFeature annotation, as the new behavior effectively removes
>
>any distinctions between the two (unless I miss something).

I am afraid there is still a distinction between them - when --enable-preview is not specified, use of essential APIs will lead to a compile time warning, while use of non essential API (reflection, typically) only produces warnings.

Jan

>
>Maurizio
>
>On 16/10/2019 13:50, Jan Lahoda wrote:
>> Hi,
>>
>> An updated patch is here:
>> http://cr.openjdk.java.net/~jlahoda/8226585/webrev.02/
>>
>> Changes in the update:
>> -added the dependency into the makefiles
>> -loosened the handling of essential preview APIs when
>--enable-preview 
>> and @SuppressWarnings is applied - there is no warning for the 
>> essential APIs (as there is no warning in such a case for 
>> non-essential APIs). This is per the discussion in the CSR:
>> https://bugs.openjdk.java.net/browse/JDK-8231411
>>
>> Any comments/feedback on this?
>>
>> Thanks!
>>     Jan
>>
>> On 09. 10. 19 17:41, Erik Joelsson wrote:
>>> Oh, you are absolutely correct, the dependency is missing.
>>>
>>> We need something like this inside "define SetupInterimModule":
>>>
>>> $$(BUILD_$1.interim): $(COPY_PREVIEW_FEATURES)
>>>
>>> /Erik
>>>
>>> On 2019-10-09 01:42, Magnus Ihse Bursie wrote:
>>>> I can’t see how the compilation is dependent on the copy being 
>>>> finished. Since Erik contributed this it will probably be correct
>:) 
>>>> but I’d appreciate an explanation on how this dependency is
>guaranteed.
>>>>
>>>> Or maybe I’m misunderstanding what this is supposed to do?
>>>>
>>>> /Magnus
>>>>
>>>>> 8 okt. 2019 kl. 17:27 skrev Jan Lahoda <jan.lahoda at oracle.com>:
>>>>>
>>>>> Thanks for the new code Erik!
>>>>>
>>>>> A new webrev/patch that includes this better way of copying is
>here:
>>>>> http://cr.openjdk.java.net/~jlahoda/8226585/webrev.01/
>>>>>
>>>>> Any feedback is welcome!
>>>>>
>>>>> Thanks,
>>>>>     Jan
>>>>>
>>>>>> On 03. 10. 19 18:06, Erik Joelsson wrote:
>>>>>> Hello Jan,
>>>>>> The build change looks ok, but I would recommend this construct 
>>>>>> for copying the file instead:
>>>>>> $(eval $(call SetupCopyFiles, COPY_PREVIEW_FEATURES, \
>>>>>>      FILES := 
>>>>>>
>$(TOPDIR)/src/java.base/share/classes/jdk/internal/PreviewFeature.java,
>
>>>>>> \
>>>>>>      DEST := 
>>>>>>
>$(BUILDTOOLS_OUTPUTDIR)/gensrc/java.base.interim/jdk/internal/PreviewFeature.java,
>
>>>>>> \
>>>>>> ))
>>>>>> TARGETS += $(COPY_PREVIEW_FEATURES)
>>>>>> Then you automatically get all the corner case handling we have 
>>>>>> implemented over the years for logging, making directories and 
>>>>>> copying files. Your version is still correct for this case
>though.
>>>>>> /Erik
>>>>>>> On 2019-10-03 02:57, Jan Lahoda wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> This is a continuation of Joe's patch from here:
>>>>>>>
>https://mail.openjdk.java.net/pipermail/compiler-dev/2019-June/013498.html
>
>>>>>>>
>>>>>>>
>>>>>>> APIs associated with preview features are split into two groups:
>
>>>>>>> essential and non-essential. These are marked with an 
>>>>>>> JDK-internal annotation, PreviewFeature, and a tag in the 
>>>>>>> javadoc, @preview. The javac follows the PreviewFeature 
>>>>>>> annotation, and produces either warnings or errors for the
>usages 
>>>>>>> of such APIs. For the @preview tag, there is a taglet in the JDK
>
>>>>>>> build that adds the content of the tag into the documentation. 
>>>>>>> The first part of the @preview's text goes into the summary, the
>
>>>>>>> second part goes into the detailed description.
>>>>>>>
>>>>>>> For build, a tricky problem is that the jdk.compiler module uses
>
>>>>>>> the PreviewFeature annotation as well, but that is not in the 
>>>>>>> bootstrap JDK. So, for the intermediate langtools build, the 
>>>>>>> PreviewFeature annotation is copied from java.base.
>>>>>>>
>>>>>>> Proposed webrev:
>>>>>>> http://cr.openjdk.java.net/~jlahoda/8226585/webrev.00/
>>>>>>>
>>>>>>> Javadoc with the change:
>>>>>>>
>http://cr.openjdk.java.net/~jlahoda/8226585/docs.00/api/index.html
>>>>>>>
>>>>>>> See for example:
>>>>>>>
>http://cr.openjdk.java.net/~jlahoda/8226585/docs.00/api/java.base/java/lang/String.html
>
>>>>>>>
>>>>>>>
>http://cr.openjdk.java.net/~jlahoda/8226585/docs.00/api/jdk.compiler/com/sun/source/tree/CaseTree.html
>
>>>>>>>
>>>>>>>
>>>>>>> JBS:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8226585
>>>>>>>
>>>>>>> CSR:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8231411
>>>>>>>
>>>>>>> Feedback is welcome!
>>>>>>>
>>>>>>> Thanks,
>>>>>>>      Jan