[m-rev.] for review: direct parsing of items

[Julien Fischer]

Hi Zoltan,

On Thu, 19 Nov 2015, Zoltan Somogyi wrote:

> ZZZ compiler/prog_io_iom.m:

Delete that.

> 
> Make the parsing of items more direct.
> 
> We used to parse items (and markers) using a fallback strategy: first we
> tried to parse a term as this kind of declaration, then as that kind of marker,
> etc, and finally as a clause. This meant that if the term was a malformed
> declaration or marker (e.g. because it used the right keyword/functor with
> the wrong arity), it could be eventually parsed as a clause.
> 
> This diff changes over to a direct parsing strategy: deciding what kind
> of item or marker a term SHOULD BE given its top level functor, and
> committing to that choice.
> 
> There are only two exceptions. The first is that when we see ":- mode",
> we keep our mind open about whether this is the definition of a new named mode,
> or the declaration of a mode of a predicate or function until we see
> a functor one level down.
> 
> The second exception is that when we see a purity annotation, a variable
> quantification or a constraint at the top level. We used to handle these,
> and the "solver" annotation, as "declaration attributes". We started the
> processing of every declaration by looking for these attributes, and switching
> on the declaration's top functor only if we didn't find them. We now
> process their functors in main switch on declarations' functors ("type",
> "inst", "pred" etc). Only if one of these switch arms is taken do we go
> over into a mode where we gather atributes, and once we have processed

s/atributes/attributes/

> all the attributes in the term, we switch between only the limited set
> of declaration types that allow attributes. This avoids having to include
> a check for "there were no attributes" in the code that processes all
> the *other* kinds of declarations.
> 
> Handle the purity annotations separately from the variable quantifications
> and the constraints, since while the latter are allowed on both item_pred_decls
> and item_mode_decls, the former are allowed only on item_pred_decls.
> 
> Handle the solver annotation outside the annotation system altogether,
> since it applies only type item_type_defns, and is simple enough
> not to need e.g. loops for its processing.
> 
> In situations where the top level functor of the term being parsed tells us
> that the term SHOULD be an item or marker of a particular kind, but it isn't,
> our new approach requuires us to generate new error messages.

s/requuires/requires/

> 
> compiler/prog_io_item.m:
>     Make the changes described above. By testing the top level of the term

...

> diff --git a/compiler/prog_io_item.m b/compiler/prog_io_item.m
> index ebac133..07e1d09 100644
> --- a/compiler/prog_io_item.m
> +++ b/compiler/prog_io_item.m

...

> @@ -1342,91 +1598,118 @@ get_class_context_and_inst_constraints(ModuleName, VarSet, RevAttributes0,
>      % practice it seems there would be little benefit in allowing that
>      % flexibility, so we don't.
>      %
> +    % NOTE We do NOT check that the order above is actually followed.
> +    %
>      % Universal quantification is the default, so we just ignore
> -    % universal quantifiers.  (XXX It might be a good idea to check
> -    % that any universally quantified type variables do actually
> -    % occur somewhere in the type declaration, and are not also
> -    % existentially quantified, and if not, issue a warning or
> -    % error message.)
> -
> -    list.reverse(RevAttributes0, Attributes0),
> -    get_quant_vars(quant_type_univ, ModuleName, Attributes0, Attributes1,
> -        [], _UnivQVars),
> -    get_quant_vars(quant_type_exist, ModuleName, Attributes1, Attributes2,
> -        [], ExistQVars0),
> +    % universal quantifiers. (XXX It might be a good idea to check that
> +    % any universally quantified type variables do actually occur SOMEWHERE
> +    % in the type declaration, and are not also existentially quantified,
> +    % and if not, issue a warning or error message.)
> +
> +    get_class_context_and_inst_constraints_loop(ModuleName, VarSet,
> +        QuantConstrAttrs, [], Specs,
> +        cord.init, _UnivQVarsCord, cord.init, ExistQVarsCord,
> +        cord.init, UnivClassConstraints, map.init, UnivInstConstraints,
> +        cord.init, ExistClassConstraints, map.init, ExistInstConstraints),
> +
> +    ExistQVars0 = cord.list(ExistQVarsCord),
>      list.map(term.coerce_var, ExistQVars0, ExistQVars),
> -    get_constraints(quant_type_univ, ModuleName, VarSet, Attributes2,
> -        Attributes3, MaybeUnivConstraints),
> -    get_constraints(quant_type_exist, ModuleName, VarSet, Attributes3,
> -        Attributes, MaybeExistConstraints),
> -    list.reverse(Attributes, RevAttributes),
> -
> -    ( if
> -        MaybeUnivConstraints = ok2(UnivConstraints, UnivInstConstraints),
> -        MaybeExistConstraints = ok2(ExistConstraints, ExistInstConstraints)
> -    then
> -        ClassConstraints = constraints(UnivConstraints, ExistConstraints),
> +    (
> +        Specs = [],
> +        ClassConstraints = constraints(
> +            cord.list(UnivClassConstraints),
> +            cord.list(ExistClassConstraints)),
>          InstConstraints =
>              map.old_merge(UnivInstConstraints, ExistInstConstraints),
>          MaybeExistClassInstContext = ok3(ExistQVars, ClassConstraints,
>              InstConstraints)
> -    else
> -        Specs = get_any_errors2(MaybeUnivConstraints) ++
> -            get_any_errors2(MaybeExistConstraints),
> +    ;
> +        Specs = [_ | _],
>          MaybeExistClassInstContext = error3(Specs)
>      ).
> 
> -:- pred get_constraints(quantifier_type::in, module_name::in, varset::in,
> -    decl_attrs::in, decl_attrs::out, maybe_class_and_inst_constraints::out)
> -    is det.
> -
> -get_constraints(QuantType, ModuleName, VarSet, !Attributes,
> -        MaybeClassInstConstraints) :-
> -    ( if
> -        !.Attributes = [
> -            decl_attr_constraints(QuantType, ConstraintsTerm) - _Term
> -            | !:Attributes]
> -    then
> +:- pred get_class_context_and_inst_constraints_loop(module_name::in,
> +    varset::in, list(quant_constr_attr)::in,
> +    list(error_spec)::in, list(error_spec)::out,
> +    cord(var)::in, cord(var)::out, cord(var)::in, cord(var)::out,
> +    cord(prog_constraint)::in, cord(prog_constraint)::out,
> +    inst_var_sub::in, inst_var_sub::out,
> +    cord(prog_constraint)::in, cord(prog_constraint)::out,
> +    inst_var_sub::in, inst_var_sub::out) is det.
> +
> +get_class_context_and_inst_constraints_loop(_ModuleName, _VarSet,
> +        [], !Specs, !UnivQVars, !ExistQVars,
> +        !UnivClassConstraints, !UnivInstConstraints,
> +        !ExistvClassConstraints, !ExistvInstConstraints).
> +get_class_context_and_inst_constraints_loop(ModuleName, VarSet,
> +        [QuantConstrAttr | QuantConstrAttrs], !Specs, !UnivQVars, !ExistQVars,
> +        !UnivClassConstraints, !UnivInstConstraints,
> +        !ExistClassConstraints, !ExistInstConstraints) :-
> +    (
> +        QuantConstrAttr = qca_quant_vars(QuantType, VarsTerm),
> +        (
> +            QuantType = quant_type_exist,
> +            ContextPieces = [words("in first argument of `some':")]

Don't hardcode the quoting style in the error message, use quote/1 instead.

> +        ;
> +            QuantType = quant_type_univ,
> +            ContextPieces = [words("in first argument of `all':")]

Ditto.

...

> diff --git a/compiler/prog_io_type_defn.m b/compiler/prog_io_type_defn.m
> index 43b983b..ba520dd 100644
> --- a/compiler/prog_io_type_defn.m
> +++ b/compiler/prog_io_type_defn.m

...

> @@ -241,32 +286,33 @@ parse_constructors_loop(ModuleName, VarSet, Head, Tail, MaybeConstructors) :-
>          )
>      ).
> 
> -:- func parse_constructor(module_name, varset, term) = maybe1(constructor).
> +:- pred parse_maybe_exist_quant_constructor(module_name::in, varset::in,
> +    term::in, maybe1(constructor)::out) is det.
> 
> -parse_constructor(ModuleName, VarSet, Term) = MaybeConstructor :-
> +parse_maybe_exist_quant_constructor(ModuleName, VarSet, Term,
> +        MaybeConstructor) :-
>      ( if Term = term.functor(term.atom("some"), [VarsTerm, SubTerm], _) then
> -        ( if parse_list_of_vars(VarsTerm, ExistQVars) then
> +        ContextPieces = [words("in first argument of `some':")],

Ditto.

The diff looks fine otherwise.

Julien.