[m-rev.] for post-commit review: new options for querying the compiler

Julien Fischer jfischer at opturion.com
Thu Nov 13 16:16:00 AEDT 2014


Hi,

On Thu, 13 Nov 2014, Zoltan Somogyi wrote:

> On Thu, 13 Nov 2014 11:12:42 +1100 (AEDT), Julien Fischer <jfischer at opturion.com> wrote:
>> @@ -451,6 +455,14 @@ main_after_setup(DetectedGradeFlags, OptionVariables, OptionArgs, Args,
>>       ; OutputCInclDirFlags = yes ->
>>           io.stdout_stream(StdOut, !IO),
>>           output_c_include_directory_flags(Globals, StdOut, !IO)
>> +    ; OutputTargetArch = yes ->
>> +        io.stdout_stream(StdOut, !IO),
>> +        globals.lookup_string_option(Globals, target_arch, TargetArch),
>> +        io.write_string(StdOut, TargetArch ++ "\n", !IO)
>> +    ; OutputClassDir = yes ->
>> +        io.stdout_stream(StdOut, !IO),
>> +        get_class_dir_name(Globals, ClassName),
>> +        io.write_string(StdOut, ClassName ++ "\n", !IO)
>>       ; GenerateMapping = yes ->
>>           source_file_map.write_source_file_map(Globals, Args, !IO)
>
> I have always thought that this long nested if-then-else
> should be replaced with code that does not silently ignore
> situations in which more than one of these options is specified.

Agreed, that predicate is becoming increasingly unwieldly.  I think
a better approach would be to have a separate type that represents
the different modes of operation of the compiler, e.g. something
like:

       :- type mode_of_operation
 	--->	output_target_arch
 	;	output_class_dir
 	;	generate_file_mapping
 	;	compile_module
 	... etc etc

and first convert the all the relevant command line options (and any
other required information) into a value of the above type.  If the
options conflict with each other we can then print an error message it
at that point, if not we can then switch on the new type (which would
replace the big if-then-else).  Using a switch means that the bodies of
most of the --output-* cases could be merged.

>> --- a/compiler/options.m
>> +++ b/compiler/options.m
>> @@ -212,6 +212,8 @@
>>       ;       output_library_link_flags
>>       ;       output_grade_defines
>>       ;       output_c_include_directory_flags
>> +    ;       output_target_arch
>> +    ;       output_class_dir
>
> Are there any tabs here? The diff looks funny that way.

No.

>> +        "\tThe flags are printed to the standard output.",
>> +        "--output-target-arch",
>> +        "\tPrint the target architecture to the standard output.",
>> +        "--output-class-dir, --output-class-directory",
>> +        "\tPrint the name of the directory in which generated Java",
>> +        "\tclass files will be placed to the standard output."
>
> I would reword the latter to
>
> Print to standard output the name of ...
>
> both here and in the users' guide.

Done.

>> + at item --output-class-dir
>> + at item --output-class-directory
>> + at findex --output-class-dir
>> + at findex --output-class-directory
>> +Print the name of the direcoty in which generated Java class files
>> +will be placed to the standard output.
>
> direcoty

Fixed.

Thanks for that.

Cheers,
Julien.



More information about the reviews mailing list