RFR 8156101: JShell SPI: Provide a pluggable execution control SPI

[Jan Lahoda]

Overall, seems reasonable to me. Would be good to get a confirmation 
from the clients, that it matches their needs. For the top-level 
changes, a review from build-dev would be good.

Comments:
-I am not sure of SPIConstants - is there a chance we could avoid this 
class? I see that:
--DOIT_METHOD_NAME is used in invoke - maybe invoke could take the 
method name as a parameter?
--PREFIX_PATTERN is used in implementation of ExecutionControl.varValue; 
but do we want everybody to re-implement the Object->String conversion? 
I was thinking if a method like:
ExecutionControl:
public static String toString(Object value) {...}
that every ExecutionControl implementation would call wouldn't work better.

-also not sure about SPIDebugControl - is there a chance we could 
replace that with a group of Loggers, or something similar?

-in KullaTesting I wonder if we could avoid the code duplication and let 
the setUp() method delegate to setUp(ExecutionControl)

A few nits:
-commented-out code in LocalExecutionControl.java
-javadoc of ClassTracker.ClassInfo:
Associates a class name class bytes and ReferenceType.
maybe a missing comma between "class name" and "class bytes"?

Jan

On 14.5.2016 05:55, Robert Field wrote:
> Updated langtools webrev with additional code comments only (to ease
> review).  No changes to code:
>
>      http://cr.openjdk.java.net/~rfield/8156101v2_lang.webrev/
>
> -Robert
>
>
>
> On 05/13/2016 12:29 AM, Robert Field wrote:
>> Updated langtools webrev:
>>     http://cr.openjdk.java.net/~rfield/8156101v1_lang.webrev/
>>
>> -Robert
>>
>>
>> On 05/12/2016 11:41 PM, Robert Field wrote:
>>> Attached is a test for this functionality utilizing a local
>>> implementation of ExecutionControl courtesy of Grigory Ptashko.
>>> Thank you Grigory!
>>>
>>> All files in test/jdk/jshell.
>>>
>>> This test needs one method added to KullaTesting --
>>>
>>> diff -r c51b40933e0c test/jdk/jshell/KullaTesting.java
>>> --- a/test/jdk/jshell/KullaTesting.java    Wed May 11 20:28:22 2016
>>> +0000
>>> +++ b/test/jdk/jshell/KullaTesting.java    Thu May 12 23:31:14 2016
>>> -0700
>>> @@ -72,6 +72,7 @@
>>>  import static jdk.jshell.Snippet.Status.*;
>>>  import static org.testng.Assert.*;
>>>  import static jdk.jshell.Snippet.SubKind.METHOD_SUBKIND;
>>> +import jdk.jshell.spi.ExecutionControl;
>>>
>>>  public class KullaTesting {
>>>
>>> @@ -166,6 +167,21 @@
>>>          classpath = new ArrayList<>();
>>>      }
>>>
>>> +    public void setUp(ExecutionControl ec) {
>>> +        inStream = new TestingInputStream();
>>> +        outStream = new ByteArrayOutputStream();
>>> +        errStream = new ByteArrayOutputStream();
>>> +        state = JShell.builder()
>>> +                .executionEngine(ec)
>>> +                .in(inStream)
>>> +                .out(new PrintStream(outStream))
>>> +                .err(new PrintStream(errStream))
>>> +                .build();
>>> +        allSnippets = new LinkedHashSet<>();
>>> +        idToSnippet = new LinkedHashMap<>();
>>> +        classpath = new ArrayList<>();
>>> +    }
>>> +
>>>      @AfterMethod
>>>      public void tearDown() {
>>>          if (state != null) state.close();
>>>
>>>
>>> On 05/11/2016 10:53 PM, Robert Field wrote:
>>>> Please review the new Service Provider Interface for using alternate
>>>> execution implements.
>>>>
>>>> Bug:
>>>>     https://bugs.openjdk.java.net/browse/JDK-8156101
>>>>
>>>> Webrevs:
>>>>     http://cr.openjdk.java.net/~rfield/8156101v0_lang.webrev/
>>>>     http://cr.openjdk.java.net/~rfield/8156101v0_ws.webrev/
>>>>
>>>> Javadoc:
>>>>     http://cr.openjdk.java.net/~rfield/jshell_spi/
>>>>
>>>> Note: The SPI is tested since it is used by the default JDI
>>>> implementation (which has no special access).  However, tests using
>>>> an alternative execution engine will be included.
>>>>
>>>> Thanks,
>>>> Robert
>>>>
>>>
>>
>