[m-rev.] for review: disallow absolute-pathname targets

[Julien Fischer]

On Fri, 23 Feb 2007, Ondrej Bojar wrote:

> For review by anyone (Julien?)
>
> Estimated hours taken: 0.5
>
> Emit an error message is 'mmc --make' get a target specified using an

s/is/if/

> absolute
> pathname.

I suggest:

 	Emit an error message if `mmc --make' is invoked with a target
 	that is specified using an absolute pathname.

I've just had a play around with the compiler, the problems are not
restricted to targets with absolute pathnames, relative pathnames cause
it problems as well, e.g.

 	mmc --make ../hello

I suggest extending this diff to disallow any form of directory
qualified targets.  (If files are in different directories users always
have the option of using -f.)

> Proceeding further led to the following uncaught exception:
> Software Error: dir./: second argument is absolute

I would just say:

 	Previously this lead to an assertion failure in the compiler.

(I think the log message should also mention something that you have
included in the comments below, namely that the rest of the code for
mmc --make doesn't targets specified via absolute pathnames.)

> compiler/make.m:
>    Check for absolute pathnames.


 	Check for targets specified with absolute pathnames and
 	emit an error message.
...

> Index: compiler/make.m
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/compiler/make.m,v
> retrieving revision 1.43
> diff -u -r1.43 make.m
> --- compiler/make.m	1 Dec 2006 15:04:04 -0000	1.43
> +++ compiler/make.m	22 Feb 2007 23:08:54 -0000
> @@ -73,6 +73,7 @@
> :- import_module top_level.mercury_compile. % XXX unwanted dependency
>
> :- import_module bool.
> +:- import_module dir.
> :- import_module int.
> :- import_module map.
> :- import_module maybe.
> @@ -234,23 +235,49 @@
>             MaybeMAIN_TARGET = yes(Targets),
>             (
>                 Targets = [_ | _],
> -                Continue = yes
> +                Continue0 = yes
>             ;
>                 Targets = [],
> -                Continue = no,
> +                Continue0 = no,
>                 io.write_string("** Error: no targets specified " ++
>                     "and `MAIN_TARGET' not defined.\n", !IO)
>             )
>         ;
>             MaybeMAIN_TARGET = no,
>             Targets = [],
> -            Continue = no
> +            Continue0 = no
>         )
>     ;
>         Targets0 = [_ | _],
> -        Continue = yes,
> +        Continue0 = yes,
>         Targets = Targets0
>     ),
> +    %
> +    % Ensure none of the targets is an absolute pathname. This is not
> +    % supported by the rest of the code.
> +    %
> +    list.filter(dir.path_name_is_absolute, Targets, AbsTargets),
> +    (
> +        AbsTargets = [],
> +        Continue = Continue0
> +        ;

The semicolon is indented by one level too much.

> +        AbsTargets = [_ | AbsTargetsTail],
> +        Continue = no,
> +        (
> +            AbsTargetsTail = [],
> +                Plural = "target is an absolute pathname"
> +            ;

Likewise.


> +            AbsTargetsTail = [_ | _],
> +                Plural = "targets are absolute pathnames"
> +        ),
> +        io.write_string("** Error: The following " ++ Plural ++ "\n", !IO),
> +        list.foldl(
> +            (pred(Target::in, !.I::di, !:I::uo) is det :-
> +                io.write_string("    ", !I),
> +                io.write_string(Target, !I),
> +                io.nl(!I)
> +            ), AbsTargets, !IO)
> +    ),

The code in compiler/error_util is intended for formatting error
messages.  (For historical reasons it isn't used everywhere but it
certainly should be for new ones.)

For the Plural variable you should have a look at
error_util.choose_number.

Julien.






--------------------------------------------------------------------------
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
--------------------------------------------------------------------------