[m-rev.] for review: library grade set filters

[Peter Wang]

On 2007-11-05, Julien Fischer <juliensf at csse.unimelb.edu.au> wrote:
> 
>  Estimated hours taken: 4
>  Branches: main
> 
>  Add support for filtering the set of library grades to be installed
>  from the command line.  This is useful, for example, if you wish to
>  install a library in all the trailing grades that are installed with
>  a particular system.  (The new options are only useful with `--make',
>  as the library grade set is handled externally of the compiler with Mmake.)
...

>  +
>  +    % string_to_grade_component(OptionStr, Comp, !Comps, !Errors):
>  +    %
>  +    % If `Comp' is a string that represents a valid grade component
>  +    % then add it to !Comps.  If it is not then emit an error message.
>  +    % `OptionStr' should be the name of the command line option for
>  +    % which the error is to be reported.
>  +    %
>  +:- pred string_to_grade_component(string::in, string::in,
>  +    list(string)::in, list(string)::out,
>  +    list(string)::in, list(string)::out) is det.
>  +
>  +string_to_grade_component(OptionStr, Comp, !Comps, !Errors) :-
>  +    ( grade_component_table(Comp, _, _, _, _) ->
>  +        !:Comps = [Comp | !.Comps]
>  +    ;
>  +        add_error("`" ++ OptionStr ++ "' unknown grade component:  "
>  +            ++ Comp, !Errors)
>  +    ).

Wouldn't that introduce a (slight) double maintenance problem, since the
option names are listed here and in options.m?  And the user might have
written `--libgrades-include' instead of `--libgrades-include-component'.
So maybe it's better to leave out the option string from the error
message.

>  +
>  +    % filter_grade(FilterPred, Components, GradeString, !Grades, !Errors):
>  +    % +    % Convert `GradeString' into a list of grade component strings, 
>  and
>  +    % then check whether the given grade should be filtered from the
>  +    % library grade set by applying the closure `FilterPred(Components)',
>  +    % to that list.  The grade removed from the library grade set if
>  +    % the application fails.

is removed ... if that application fails.

>  +:- pred must_contain(list(string)::in, list(string)::in) is semidet.
>  +
>  +must_contain(IncludeComponents, GradeComponents) :-
>  +    all [Component] ( list.member(Component, IncludeComponents) =>
>  +        list.member(Component, GradeComponents)
>  +    ).
>  +
>  +:- pred must_not_contain(list(string)::in, list(string)::in) is semidet.
>  +
>  +must_not_contain(OmitComponents, GradeComponents) :-
>  +    all [Component] (list.member(Component, OmitComponents) =>
>  +        not list.member(Component, GradeComponents)
>  +    ).
>  +

I prefer

	all [X] (
	    p(X)
	=>
	    ...
	)

>  Index: compiler/options.m
>  ===================================================================
>  RCS file: /home/mercury/mercury1/repository/mercury/compiler/options.m,v
>  retrieving revision 1.598
>  diff -u -r1.598 options.m
>  --- compiler/options.m	31 Oct 2007 03:58:28 -0000	1.598
>  +++ compiler/options.m	5 Nov 2007 06:16:27 -0000
...
>  @@ -4989,6 +4997,18 @@
>           "--no-libgrade",
>           "\tClear the list of compilation grades in which a library",
>           "\tto be installed should be built.",
>  +        "--libgrades-include-component <component>",
>  +        "--libgrades-include <component>",
>  +        "\tFilter the list of compilation grades in which a library to",
>  +        "\tbe installed should be built so that it only contains grades",
>  +        "\tthat include the specified grade component.",
>  +        "\t(This option does not work with Mmake, only `--make'.)",

It's a bit hard to read.

    In the list of grades in which to install a library, keep only the
    grades which include the specified grade component.

>  +        "--libgrades-omit-component <component>",
>  +        "--libgrades-omit <component>",
>  +        "\tFilter the list of compilation grades in which a library to",
>  +        "\tbe installed should be built so that it does not contain any",
>  +        "\tgrades that include the specified grade component.",
>  +        "\t(This option does not work with Mmake, only `--make'.)",

    In the list of grades in which to install a library, remove any
    grade which includes the specified grade component.

>  Index: doc/user_guide.texi
>  ===================================================================
>  RCS file: /home/mercury/mercury1/repository/mercury/doc/user_guide.texi,v
>  retrieving revision 1.549
>  diff -u -r1.549 user_guide.texi
>  --- doc/user_guide.texi	25 Oct 2007 06:53:44 -0000	1.549
>  +++ doc/user_guide.texi	5 Nov 2007 06:16:27 -0000
>  @@ -7170,7 +7170,7 @@
>   @samp{spf} (the default is to use double-precision floats)
> 
>   @item Whether to allocate a new stack segment when the limit on the old one
>  -is reached: @samp{stseg} (the default is not to allocate a new stack 
>  segement)
>  +is reached: @samp{stseg} (the default is not to allocate a new stack 
>  segment)

The description is correct but doesn't explain the reasoning.  I
suggest:

    @item Whether to use dynamically-sized stacks, composed of small segments:
    @samp{stseg} (the default is to use fixed-sized stacks)

with a paragraph to be added later to `LLDS back-end compilation model
options' explaining that it increases safety and (almost certainly)
reduces memory consumption, at the expense of speed.

>   @item What debugging features to enable:
>   @samp{debug} and @samp{decldebug} (the default is no debugging features).
>  @@ -8731,6 +8731,26 @@
>   building and installing the default set of grades.
> 
>   @sp 1
>  + at item --libgrades-include-component @var{component}
>  + at itemx --libgrades-include @var{component}
>  + at findex --libgrade-include-component
>  + at findex --libgrades-include
>  +Filter the list of compilation grades in which a library to be
>  +installed should be built so that it only contains grades
>  +that include the specified grade component.
>  +(This option does not work with Mmake, only @samp{--make}.)

only @samp{mmc --make}

>  +
>  + at sp 1
>  + at item --libgrades-omit-component @var{component}
>  + at itemx --libgrade-omit @var{component}
>  + at findex --libgrades-omit-component
>  + at findex --libgrade-omit
>  +Filter the list of compilation grades in which a library to be
>  +installed should be built so that it does not contain any grades
>  +that include the specified grade component.
>  +(This option does not work with Mmake, only @samp{--make}.)

only @samp{mmc --make}

Peter

--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------