[m-rev.] for post-commit review: from_ground_term_initial improvements

Paul Bone pbone at csse.unimelb.edu.au
Tue Apr 17 13:53:50 AEST 2012


On Mon, Apr 16, 2012 at 06:15:33PM +1000, Zoltan Somogyi wrote:
> I am committing this now because I am working on other changes that
> build on this diff. It passes bootcheck clean with several different
> options.
> 
> Index: from_ground_term_util.m
> ===================================================================
> RCS file: from_ground_term_util.m
> diff -N from_ground_term_util.m
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ from_ground_term_util.m	16 Apr 2012 05:35:09 -0000
> +
> +    % introduce_partial_fgt_scopes(GoalInfo0, SubGoalInfo0, RevMarkedSubGoals,
> +    %   Kind, SubGoal):
> +    %
> +    % Thes input to this predicate are:
> +    %
> +    % - The original goal infos for a fgt{i,c} goal and for its subgoal
> +    %   (GoalInfo0 and SubGoalInfo0).
> +    %
> +    % - A list (MarkedSubGoals) of the transformed versions of the conjuncts
> +    %   that were originally in that scope, in bottom up (construct) order,
> +    %   marked up in the way required by the comment on the definition of the
> +    %   fgt_marked_goal type). This predicate assumes that this list contains
> +    %   some violations of the fgt{i,c} invariants (other than the order
> +    %   invariant for initial scopes); if it doesn't, then the the WHOLE
> +    %   conjunction can have a fgt{i,c} scope wrapped around it, using code
> +    %   much simpler than what is in this predicate.
> +    %
> +    % - The order into which the goals should be put (Order).
> +    %

I don't see an argument named Order earlier in this comment.
It appears to be called Kind.

> +    % The SubGoal returned by this predicate will contain the code in the
> +    % MarkedSubGoals in the desired order, but with any sequence of goals that
> +    % does obey the fgt{i,c} invariants will be wrapped in a fgt scope;
> +    % initial if Order = deconstruct_top_down, construct if Order =
> +    % construct_bottom_up.
> +    %
> +    % This allows us to preserve the effect of the compiler optimizations of
> +    % fgt{i,c} scopes for as large a portion of the original scope as possible.
> +    %
> +:- pred introduce_partial_fgt_scopes(hlds_goal_info::in, hlds_goal_info::in,
> +    list(fgt_marked_goal)::in, goal_order::in, hlds_goal::out)
> +    is det.
> +


> +
> +    % Has a goal or sequence of goals broken the fgt{i,c} invariants?
> +    %
> +:- type maybe_kept
> +    --->    kept
> +    ;       broken.
> +

There was a type similar to this above.  If they have different semantics
please explain the semantics in the comment.

The rest looks okay, but I'm not that familiar with these parts of the
compiler, so my confidence in my own reviewing is low.

Thanks.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20120417/1dabfe5c/attachment.sig>


More information about the reviews mailing list