RFR: JDK-8243417 Clean up com.sun.tools.javac.main.CommandLine

[Jonathan Gibbons]

Pavel,

Performance of this method is not an issue, when you compare the amount 
of work done by this method compared to the total work done by javac and 
javadoc. That being said, the new code is better than the old since when 
using the array wrappers in the old code, internally, the array was 
converted to a list, a new list was created with the expanded contents, 
the list was converted to an array, and then the client converted that 
array back to a list, for a total of 3 conversions. The new code just 
has a single conversion. I think that's an acceptable cost for a 
simplified API.

-- Jon


On 4/24/20 1:44 PM, Pavel Rappo wrote:
> Hey Jon,
>
> My only comment is that while eliminating the need for conversion in one place, the change introduces it to another place, effectively discounting the benefits of the change.
>
> Maybe it's okay for that method to consume String[] and produce List<String>. Or at least to have that as an option. `public static void main(String[] args)` accepts array, while the code it calls accepts List. We can't change the signature of the `main` method. And the clients want to work with List. Someone has to do the job of conversion. Why not CommandLine.parse? Typically that method is the first method to consume the arguments from the `main` method anyway.
>
> -Pavel
>
>> On 24 Apr 2020, at 19:32, Jonathan Gibbons <jonathan.gibbons at oracle.com> wrote:
>>
>> javac folk,
>>
>> Please review a small cleanup to the code to handle @file expansion.
>>
>> Pavel recently noted that the consumers of CommandLine.parse all eventually wanted Lists, which is the primary internal form inside CommandLine, but that the clients were all using array-based wrappers, causing unnecessary conversions between the two forms.
>>
>> The fix is to have the clients use the List-based methods, and to remove the legacy array-based methods.  In some of the javac code, Iterable<String> is used to store the result, to avoid confusion with javac's internal List class.
>>
>> -- Jon
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8243417
>> Webrev: http://cr.openjdk.java.net/~jjg/8243417/webrev.00/
>>