[m-rev.] for review: auto detection of library grades

[Paul Bone]

On Wed, Aug 28, 2013 at 05:47:00PM +1000, Julien Fischer wrote:
>
> For review by anyone.
>
>
> compiler/options.m:
> 	Add a new option `--auto-detect-libgrades' that can be used to control
> 	the new feature.  For now, this is a developer-only option and is
> 	disabled by default -- I will document it properly when it is enabled
> 	by default.

Why disable this by default?  Is there any specific reason why it might
break things?  Or do you want to enable it after 13.05.x?


> diff --git a/compiler/mercury_compile.m b/compiler/mercury_compile.m
> index 9e690bb..8623595 100644
> --- a/compiler/mercury_compile.m
> +++ b/compiler/mercury_compile.m
> +%
> +% Library grade detection.
> +%
> +
> +:- pred auto_detect_libgrades(globals::in, maybe(list(string))::in,
> +    list(string)::out, io::di, io::uo) is det.
> +
> +auto_detect_libgrades(Globals, MaybeConfigMerStdLibDir, GradeOpts, !IO) :-
> +    globals.lookup_bool_option(Globals, auto_detect_libgrades, AutoDetect),
> +    (
> +        AutoDetect = yes,
> +        ( if
> +            % Was the standard library directory set on the command line?
> +            % +            globals.lookup_maybe_string_option(Globals,
> +                mercury_standard_library_directory, MaybeStdLibDir),
> +            MaybeStdLibDir = yes(MerStdLibDir)

Some strange formatting in your patch.

> +        then
> +            do_auto_detect_libgrades(MerStdLibDir, GradeOpts, !IO)
> +        else if
> +            % Was the standard library directory set using the
> +            % MERCURY_STDLIB_DIR variable?
> +            MaybeConfigMerStdLibDir = yes([MerStdLibDir])
> +        then
> +            do_auto_detect_libgrades(MerStdLibDir, GradeOpts, !IO)
> +        else
> +            GradeOpts = [] +        )

You can factor out common code by putting both ITE conditions in a
disjunction and then using a single then branch.

> +:- pred do_auto_detect_libgrades(string::in, list(string)::out,
> +    io::di, io::uo) is det.
> +
> +do_auto_detect_libgrades(StdLibDir, GradeOpts, !IO) :-
> +    ModulesDir = StdLibDir / "modules",
> +    dir.foldl2(do_auto_detect_libgrade, ModulesDir, [], MaybeGradeOpts, !IO),
> +    (
> +        MaybeGradeOpts = ok(GradeOpts0),
> +        (
> +            GradeOpts0 = [],
> +            GradeOpts = GradeOpts0
> +        ;
> +            GradeOpts0 = [_ | _],
> +            % Override any --libgrades settings from Mercury.config.
> +            GradeOpts = ["--no-libgrade" | GradeOpts0]
> +        )
> +    ;
> +        MaybeGradeOpts = error(_, _),
> +        GradeOpts = []
> +    ).
> +
> +:- pred do_auto_detect_libgrade(string::in, string::in, io.file_type::in,
> +    bool::out, list(string)::in, list(string)::out, io::di, io::uo) is det.
> +
> +do_auto_detect_libgrade(DirName, FileName, FileType, Continue, !GradeOpts, !IO) :-

The above line is over 80 charicters long.  I beleive our coding standards
specify something like 74 or 76.  There are some other overlong lines as
well.

> +    (
> +        FileType = directory,
> +        (
> +            % We do not generate .init files for the non-C grades so just check
> +            % for directories in StdLibDir / "modules" containing the name of
> +            % their base grade.
> +            %
> +            ( string.prefix(FileName, "csharp")
> +            ; string.prefix(FileName, "erlang")
> +            ; string.prefix(FileName, "java")
> +            )
> +        ->
> +            !:GradeOpts = ["--libgrade", FileName | !.GradeOpts]
> +        ;
> +            % For C grades, we check for the presence of the .init file for
> +            % mer_std to test whether the grade is present or not.
> +            %
> +            InitFile = DirName / FileName / "mer_std.init",
> +            io.check_file_accessibility(InitFile, [read], Result, !IO),
> +            (
> +                Result = ok,
> +                !:GradeOpts = ["--libgrade", FileName | !.GradeOpts]
> +            ;
> +                Result = error(_)
> +            )
> +        ),

Do we have to handle any of the experimental backends like the bytecode
backend?

> diff --git a/compiler/options.m b/compiler/options.m
> index 40121d8..755a994 100644
> --- a/compiler/options.m
> +++ b/compiler/options.m
> @@ -5786,6 +5789,11 @@ options_help_build_system -->
>          "\ta directory. The given command will be invoked as",
>          "\t`<command> <option> <source> <target>'",
>          "\tto install each directory. The default option is `-r'.",
> +
> +        % `--auto-detect-libgrades' is a developer-only that controls
> +        % whether the compiler should scan the installation directory
> +        % to determine what standard library grades are available.
> +

developer-only option that

s/what/which/

The rest is fine, sorry for the delay.


-- 
Paul Bone
http://www.bone.id.au