[m-rev.] for review: split getopt*.m's work into three phases
Zoltan Somogyi
zoltan.somogyi at runbox.com
Thu Jan 6 16:51:16 AEDT 2022
For review by anyone. Note: the diff to getopt.m
is the same as the diff to getopt_io.m; only the latter
needs review.
The ultimate objective of this work is to try to simplify
the option handling code of mmc --make, which, in some cases,
calls handle_options.m to process slight variations of the
same argument list three or four times in a row. I *think*
that what some of the current code is *trying* to do is to make
slight changes in one of the three phases that I have now
divided the functionality of getopt into: recognizing option values,
expanding file special options, and updating the option table,
but, not having access to the individual phases, it was forced
to let getopt do all three, and then undo (or ignore) the parts
it did not need.
One new thing we could do with the new functionality in getopt
would be to check that Mercury.options entries for modules
did not specify options that they shouldn't, such as grade or
op_mode options. Doing this with access to the option value list
generated by the first phase is trivial; doing it with the old getopt
would be infeasible. (One *could* imitate the first phase by using
option tracking, but this would just repeat getopt.m's work outside
getopt itself.)
This form of this diff exports from getopt_io.m (and its getopt.m clone)
predicates that do the first two phases. The third phase will
also need to be exported; the question is how. The current diff's
process_option_values predicate takes an argument of type option_ops_special,
which is currently private, and which has four alternatives, all of which
contain information whose type is public. We can either export this one type,
or we can export four variants of this predicate, each of which takes
an argument corresponding to one of the four alternatives (none,
notrack, track, and userdata). (Exporting the type would require
inventing better names for these alternatives.)
The rest of getopt_io.m follows the "export one predicate for each variant"
approach, but I believe this is mainly because this allowed us to maintain
backwards compatibility as we added new variants. That issue would not
arise here. I would be fine with either approach. What about you?
The new code in getopt_io.m uses cords to represent option values,
because using reversed lists would be too complex in this application
due to the fact they would have to mix with *unreversed* lists.
On the other hand, we want to encode the invariant that expanding out
file_special options leaves no file_special options in the option value list.
This diff does that using insts, which requires cord operations that preserve
insts. One consequence of this is that this diff exports the implementation
type of cords, though not in the documented part of cord.m's interface.
This is undesirable.
I see two possible alternatives. First, we getopt*.m need only a very small
part of cord.m's functionality, so we could bodily duplicate that part
in getopt*.m, leaving cord.m unchanged. Second, we could use a subtype
to record the absence of file_specials, instead of an inst. The latter would
require a decision that subtypes are mature enough that we are willing
to let the whole Mercury implementation break if they break. Opinions,
especially from Peter on the last heading?
There will need to be entries in NEWS, but I want to hold off on that
until we agree about yet another issue. The new predicates exported
by getopt*.m have an argument that returns an error indication
if an error was in fact seen, BUT, even if an error *was* seen, they
do return other output as well, such as the list of option values
seen before the error. Should this be the case? Should we instead return
the non-error output *only* in the absence of an error? Having the
non-error output up to the error may be useful to some callers,
but do we want to commit to preserving the exact same non-error
output in the presence of errors? This diff itself affects error behavior,
in that before this diff, if an argument list had both a reference to a
nonexistent option and a reference to an existing file_special option
that named a nonexistent file, we used to report whichever came first
on the command line, while after this diff, we will always report the
nonexistent option.
(The resolution of this issue affects the arity of the exported
predicates, and hence their NEWS entries.)
Zoltan.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Log.go
Type: application/octet-stream
Size: 2058 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20220106/93af0def/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: DIFF.go
Type: application/octet-stream
Size: 164268 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20220106/93af0def/attachment-0003.obj>
More information about the reviews
mailing list