RFR JDK-8080641: JEP-JDK-8042880 : Implement new tests on Project Coin

[Sergei Pikalev]

Hi Alex and compiler team,

New webrev is here: 
http://cr.openjdk.java.net/~shurailine/8080641/webrev.02/

Illegal expressions moved to TwrForVariable3.
Array components as variables moved to TwrVarKinds.
WeirdTwr_WithVar case moved to TwrForVariable1.

Sergi

On 20.10.2015 04:08, Alex Buckley wrote:
> Thanks for the pointers.
>
> I remember now: array components can't be used as resources because, 
> as variables, they are never final or effectively final. (Only an 
> array variable itself may be final or effectively final, not its 
> components.) That's why the grammar only allows ExpressionName and 
> FieldAccess, not ArrayAccess.
>
> But, since array components are variables, I would have expected them 
> to be tested in TwrVarKinds rather than TwrForVariable2. And, I would 
> have expected the negative tests in TwrForVariable2 for a conditional 
> expression and a null-with-cast to be located in TwrForVariable3 where 
> other (illegal) expressions are tried. Tests should be written in the 
> right place if we want people to read them.
>
> WeirdTwr_WithVar is misnamed and unnecessary. The single t-w-r 
> statement that it contains could live perfectly well in 
> TwrForVariable1, near the nice new cases for assertCloseCount(7) and  
> assertCloseCount(10).
>
> Alex
>
> On 10/19/2015 5:35 PM, Sergei Pikalev wrote:
>>
>> Hi Alex,
>>
>> These test are additional to existing test, and developers 
>> recommended me
>> not to create new files but enrich existing as far as possible.
>>
>> Array components assertions are located at TwrForVariable2.
>>
>> Other discussed issues are in TwrForVariable1. Null cases 'prove' that
>> null does
>> not generate exceptions beyond the t-w-r block. Positive part checks
>> initialized
>> resources.
>>
>> Some of this assertions came from developers' unit test. Some was added
>> by me.
>>
>> Sergi
>>
>> On 20.10.2015 02:57, Alex Buckley wrote:
>>> I don't see any testing of array components as resources, or testing
>>> that a resource declared outside the t-w-r statement can either be
>>> null or is guaranteed to initialize (as discussed in a separate mail).
>>>
>>> Alex
>>>
>>> On 10/19/2015 8:05 AM, Sergei Pikalev wrote:
>>>>
>>>> Hi All,
>>>>
>>>> http://cr.openjdk.java.net/~shurailine/8080641/webrev.01/
>>>>
>>>> There are updated tests with respect to Alex's remarks.
>>>>
>>>> Thanks,
>>>>      Sergi
>>>>
>>>> On 16.10.2015 02:20, Alex Buckley wrote:
>>>>> Hi Sergei,
>>>>>
>>>>> Most of the new regression tests are about conformance to the spec,
>>>>> rather than probing some javac-internal treatment of the t-w-r
>>>>> statement. I hope someone from JCK-Compiler can comment on their
>>>>> suppor for the expanded t-w-r statement.
>>>>>
>>>>> That said, here are some comments on the new tests:
>>>>>
>>>>> - ResourceVariableOutsideTry - @summary should be "Resource variable
>>>>> should be in scope throughout try, catch, and finally blocks".
>>>>> Accessibility is not relevant to a local variable like c.
>>>>>
>>>>> - TwrAndAnonymousClass - this is a positive test when an anonymous
>>>>> class implements AutoCloseable, but what about a negative test 
>>>>> when an
>>>>> anonymous class doesn't implement AutoCloseable?
>>>>>
>>>>> - TwrAndGenerics - should be called TwrAndTypeVariables - plus same
>>>>> comment as above (type variables S and T _not_ extending
>>>>> AutoCloseable).
>>>>>
>>>>> - TwrAndLambda - same comment as above (lambda expressions and method
>>>>> references _not_ implementing AutoCloseable).
>>>>>
>>>>> - TwrVarKinds - good test, but if you're doing to test four kinds of
>>>>> variables then you may as well test all seven (JLS 4.12.3). In
>>>>> particular, array components should be tested. You should test using
>>>>> components from an array of AutoCloseables, and components of
>>>>> components from an array of an array of AutoCloseables. (The second
>>>>> situation in particular will expose any mistreatment of array
>>>>> components by javac -- if javac accidentally erases the array-ness of
>>>>> a resource variable, then in the first situation it will end up with
>>>>> an AutoCloseable and that's ordinary enough that everything might
>>>>> still work, but in the second situation it ends up with an array of
>>>>> AutoCloseables and that might blow up some logic.)
>>>>>
>>>>> - WeirdTwr_WithVar - there is nothing weird or strange here. It's 
>>>>> just
>>>>> one resource variable aliasing another, which was possible already.
>>>>> You're right to extend it so that a resource variable declared by
>>>>> 'try' aliases a resource variable declared outside 'try' -- also 
>>>>> do it
>>>>> the other way around. Also, need a test where the resource variable
>>>>> declared outside 'try' is null.
>>>>>
>>>>> - What's truly new when a try (...) header uses a resource variable
>>>>> declared outside the 'try'? Answer: no exception can be thrown,
>>>>> whereas the declaration of a resource variable by 'try' includes an
>>>>> initializer expression that can throw an exception. There should be a
>>>>> test to "prove" that try (r1) {...} doesn't throw exceptions from
>>>>> resource initialization, and that try (r1; AutoCloseable r2 = ...)
>>>>> {...} only throws exceptions due to r2's initialization.
>>>>>
>>>>> Alex
>>>>>
>>>>> On 10/14/2015 12:12 AM, Sergei Pikalev wrote:
>>>>>>
>>>>>> Hello,
>>>>>>
>>>>>> This is the code with new Project Coin tests from SQE.
>>>>>>
>>>>>> A webrev with the tests is here:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~shurailine/webrev.8080641/
>>>>>>
>>>>>> Please review.
>>>>>>
>>>>>> Thanks,
>>>>>>      Sergei
>>>>>>
>>>>
>>