RFR: 7903156: remove jopt-simple dependency from jextract [v2]

Maurizio Cimadamore mcimadamore at openjdk.java.net
Tue Apr 12 13:07:00 UTC 2022


On Tue, 12 Apr 2022 12:48:34 GMT, Athijegannathan Sundararajan <sundar at openjdk.org> wrote:

>> removed jopt-simple dependency as well as moditect grade plugin dependency. Piggybacking build.gradle change to enforce 'jtreg_home' check in jtreg task's doFirst block.
>
> Athijegannathan Sundararajan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   removed unused java.prefs module

Looks a good simplification build-wise.
I think the code has some issue. The handling of -C forces a bit too much magic in the code that I'm not sure is correct.
The truth is that -C doesn't integrate well with the OptionSpec support. IMHO it could be better to have a class for OptionSpec which you can, for normal option, instantiate simply - but that you can also subclass (for -C) so that you can define custom match logic.

src/main/java/org/openjdk/jextract/JextractTool.java line 218:

> 216:     }
> 217: 
> 218:     private static final class OptionParser {

I suggest putting this (and related classes) in a separate file

src/main/java/org/openjdk/jextract/JextractTool.java line 245:

> 243:                    if (spec == null) {
> 244:                        // check if we have usage like -C<clang_option> such as -C-xc++
> 245:                        int idx = arg.indexOf('-', 1);

This logic seems flawed for `--` arguments. E.g. `--source-I /usr/local/include`.

src/main/java/org/openjdk/jextract/JextractTool.java line 255:

> 253:                    }
> 254:                    // handle argument associated with the current option, if any
> 255:                    List<String> values;

Why a list? Isn't there only one possible value?

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

PR: https://git.openjdk.java.net/jextract/pull/19


More information about the jextract-dev mailing list