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

[Julien Fischer]

Hi Zoltan,

On Mon, 23 Nov 2015, Zoltan Somogyi wrote:

> Make the parsing of goals more direct.
> 
> compiler/prog_io_dcg.m:
> compiler/prog_io_goal.m:
>     We used to parse goals using a fallback strategy: first we tried to parse
>     each term as a language construct, and if that failed, as a call.
>     The code that tested whether a term was a language construct looked at
>     not just the top function symbol of the term, but also its arity,
>     and sometimes at some of the function symbols nested inside. If the
>     arity or the nested function symbols weren't what was expected, the term
>     was parsed as a call. This meant that if the term was a malformed
>     language construct (e.g. because it used the right keyword/functor with
>     the wrong arity), it could be eventually parsed as a call
>
>     This diff changes over to a direct parsing strategy: deciding what kind
>     of goal a term SHOULD BE given its top level functor, and committing
>     to that choice.
> 
> doc/reference_manual.texi:
>     Fix a typo in the example of try expressions, and make that example
>     more readable. (The code for parsing try expressions was complex enough
>     that I had to look up the manual.)

The test suite should also be updated to cover the error messages added or
modified below.

...

> diff --git a/compiler/prog_io_dcg.m b/compiler/prog_io_dcg.m
> index cbcb2e4..c020273 100644
> --- a/compiler/prog_io_dcg.m
> +++ b/compiler/prog_io_dcg.m

...

> @@ -204,203 +226,316 @@ parse_non_call_dcg_goal(Functor, Args, Context, ContextPieces, MaybeGoal,
>              Functor = "promise_impure",
>              PromisedPurity = purity_impure

...

>          Functor = "[|]",
> -        Args = [X, Xs],
> -        % Non-empty list of terminals. Append the DCG output arg as the
> -        % new tail of the list, and unify the result with the DCG input arg.
> -        InVar = !.DCGVar,
> -        new_dcg_var(!VarSet, !Counter, OutVar),
> -        !:DCGVar = OutVar,
> -        ConsTerm0 = term.functor(term.atom("[|]"), [X, Xs], Context),
> -        term.coerce(ConsTerm0, ConsTerm),
> -        term_list_append_term(ConsTerm, term.variable(OutVar, Context), Term),
> -        Goal = unify_expr(Context, variable(InVar, Context), Term,
> -            purity_pure),
> -        MaybeGoal = ok1(Goal)
> +        ( if Args = [X, Xs] then
> +            % Translate
> +            %   [A, B, C]
> +            % as
> +            %   DCGIn = [A, B, C | DCGOut].
> +            InVar = !.DCGVar,
> +            new_dcg_var(!VarSet, !Counter, OutVar),
> +            !:DCGVar = OutVar,
> +            ConsTerm0 = term.functor(term.atom("[|]"), [X, Xs], Context),
> +            term.coerce(ConsTerm0, ConsTerm),
> +            OutVarTerm = term.variable(OutVar, Context),
> +            ( if term_list_append_term(ConsTerm, OutVarTerm, Term) then
> +                Goal = unify_expr(Context, variable(InVar, Context), Term,
> +                    purity_pure),
> +                MaybeGoal = ok1(Goal)
> +            else
> +                Pieces = [words("Error: there no"),

s/there no/there is no/

> +                    quote("[]"), words("at the end of the list."), nl],
> +                Spec = error_spec(severity_error, phase_term_to_parse_tree,
> +                    [simple_msg(Context, [always(Pieces)])]),
> +                MaybeGoal = error1([Spec])
> +            )
> +        else
> +            Pieces = [words("Error: in DCG clauses,"),
> +                words("the"), quote("[|]"), words("operator"),
> +                words("may only be used to match the input"),
> +                words("against a list of one or more items,"),
> +                words("and must therefore be used with arity 2."), nl],
> +            Spec = error_spec(severity_error, phase_term_to_parse_tree,
> +                [simple_msg(Context, [always(Pieces)])]),
> +            MaybeGoal = error1([Spec])
> +        )

...

> diff --git a/compiler/prog_io_goal.m b/compiler/prog_io_goal.m
> index 3749522..2fac107 100644
> --- a/compiler/prog_io_goal.m
> +++ b/compiler/prog_io_goal.m
> @@ -104,6 +104,19 @@
>  :- pred apply_purity_marker_to_maybe_goal(term::in, purity::in,
>      maybe1(goal)::in, maybe1(goal)::out) is det.
> 
> +    % Functions to construct error messages. Exported to prog_io_cdg.m,

s/prog_io_cdg/prog_io_dcg/


> +    % to allow DCG and non-DCG clauses to generate identical error messages
> +    % in analogous situations.
> +    %
> +:- func should_have_one_goal_prefix(list(format_component),
> +    term.context, string) = error_spec.
> +:- func should_have_two_terms_infix(list(format_component),
> +    term.context, string) = error_spec.
> +:- func should_have_two_goals_infix(list(format_component),
> +    term.context, string) = error_spec.
> +:- func should_have_one_x_one_goal_prefix(list(format_component),
> +    term.context, string, string) = error_spec.
> +
>  %-----------------------------------------------------------------------------%
>  %-----------------------------------------------------------------------------%
>

...


> @@ -169,159 +182,255 @@ parse_goal(Term, ContextPieces, MaybeGoal, !VarSet) :-

...


>      ;
>          Functor = "then",
> -        Args = [TryTerm, ThenTerm],
> -        parse_then_try_term(
> -            term.functor(atom("then"), [TryTerm, ThenTerm], Context),
> -            no, [], no, Context, ContextPieces, MaybeGoal, !VarSet)
> +        ( if Args = [TryTerm, ThenTerm] then
> +            parse_then_try_term(
> +                term.functor(atom("then"), [TryTerm, ThenTerm], Context),
> +                no, [], no, Context, ContextPieces, MaybeGoal, !VarSet)
> +        else
> +            % Since there was no "else" wrapped around this use of "then",
> +            % it is quite likely that this may have been intended to be
> +            % a try goal.
> +            % XXX Should we list all the things that may follow
> +            % the initial part of a try goal?
> +            Pieces = [words("Error: the "), quote("then"), words("operator,"),
> +                words("should be used either in an expression of the form"),
> +                quote("( if goal then goal else goal )"), suffix(","),
> +                words("or in an expression of the form"),
> +                quote("try [try_params] main_goal then success_goal"),
> +                suffix(","), words("optionally followed by"),
> +                quote("else failue_goal"), suffix(","),

s/failue_goal/failure_goal/

> +                words("which in turn may be followed by zero or more"),
> +                quote("catch"), words("clauses, and optionally by a single"),
> +                quote("catch_any"), words("clause."), nl],
> +            Spec = error_spec(severity_error, phase_term_to_parse_tree,
> +                [simple_msg(Context, [always(Pieces)])]),
> +            MaybeGoal = error1([Spec])
> +        )
>      ;

...

>          Functor = "catch_any",
> -        Args = [TermA, ArrowTerm],
> -        parse_catch_any_term(ArrowTerm, Context, ContextPieces,
> -            MaybeCatchAnyExpr, !VarSet),
> -        (
> -            MaybeCatchAnyExpr = ok1(CatchAnyExpr),
> -            ( if TermA = term.functor(atom("catch"), _, _) then
> -                parse_catch_then_try_term(TermA, yes(CatchAnyExpr),
> -                    Context, ContextPieces, MaybeGoal, !VarSet)
> -            else
> -                parse_else_then_try_term(TermA, [], yes(CatchAnyExpr),
> -                    Context, ContextPieces, MaybeGoal, !VarSet)
> +        ( if Args = [TermA, ArrowTerm] then
> +            parse_catch_any_term(ArrowTerm, Context, ContextPieces,
> +                MaybeCatchAnyExpr, !VarSet),
> +            (
> +                MaybeCatchAnyExpr = ok1(CatchAnyExpr),
> +                % YYY

What's YYY in aid of?

The diff looks fine otherwise.

Julien.