[m-rev.] for review: auto detection of library grades
Paul Bone
paul at bone.id.au
Tue Sep 3 16:12:48 AEST 2013
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
More information about the reviews
mailing list