[m-rev.] [for review 1/4] Add mfilterjavac utility

Julien Fischer jfischer at opturion.com
Fri Apr 19 18:26:23 AEST 2013


Hi Paul,

On Thu, 18 Apr 2013, Paul Bone wrote:

> Add mfilterjavac utility
>
> Add the mfilterjavac utility to filter javac's output and re-write the error
> locations.
>
> mfilterjavac/mfilterjavac.m:
>    As above
>
> Mmakefile:
> configure.ac:
> mfilterjavac/Mmakefile
> mfilterjavac/MFILTERJAVAC_FLAGS.in:
>    Adjust the build system to support the new mfilterjavac directory.

You are missing .mgnuc_copts and .mgnuc_opts files in the new directory.
(See for example, the profiler and slice directories.)

...

> --- /dev/null
> +++ b/mfilterjavac/MFILTERJAVAC_FLAGS.in
> @@ -0,0 +1,25 @@
> + at BOOTSTRAP_MC_ARGS@
> +--no-infer-all
> +--halt-at-warn
> +--no-warn-inferred-erroneous
> +--no-mercury-stdlib-dir
> +-I../library
> +-I../browser
> +-I../ssdb
> +--c-include-directory ../boehm_gc
> +--c-include-directory ../boehm_gc/include
> +--c-include-directory ../runtime
> +--c-include-directory ../library
> +--c-include-directory ../library/Mercury/mihs
> +--c-include-directory ../browser
> +--c-include-directory ../browser/Mercury/mihs
> +--c-include-directory ../ssdb
> +--c-include-directory ../ssdb/Mercury/mihs
> +--c-include-directory ../trace
> +--csharp-flag -keyfile:../mercury.snk
> +--no-java-classpath
> +--java-classpath ../library/mer_rt.jar
> +--java-classpath ../library/mer_std.jar
> +--java-classpath ../browser/mer_browser.jar
> +--java-classpath ../mdbcomp/mer_mdbcomp.jar
> +--config-file ../scripts/Mercury.config.bootstrap

There should also be:

     --erlang-include-directory ../library/Mercury/hrls

otherwise, mfilterjavac won't compile in the erlang grade.

> diff --git a/mfilterjavac/Mmakefile b/mfilterjavac/Mmakefile
> new file mode 100644
> index 0000000..fc46535
> --- /dev/null
> +++ b/mfilterjavac/Mmakefile
> @@ -0,0 +1,152 @@
> +#-----------------------------------------------------------------------------#
> +# Copyright (C) 2013 The University of Melbourne.
> +# This file may only be copied under the terms of the GNU General
> +# Public Licence - see the file COPYING in the Mercury distribution.
> +#-----------------------------------------------------------------------------#
> +
> +# This is the Mmakefile for building the mfilterjavac tool.
> +
> +MERCURY_DIR=..
> +LINK_STATIC=yes
> +include $(MERCURY_DIR)/Mmake.common
> +
> +#----------------------------------------------------------------------------#
> +
> +-include Mmake.mfilterjavac.params
> +
> +# Override the default rule in `mmake --use-mmc-make' that asks `mmc' to
> +# create a missing optional params file.
> +Mmake.mfilterjavac.params:
> +
> +# Module-specific options should go in Mercury.options so they
> +# can be found by `mmc --make'.  But this hasn't been used in this directory
> +# so it's commented out.
> +# include Mercury.options
> +
> +MAIN_TARGET = all
> +
> +ALL_MODULES = mfilterjavac
> +
> +# Always compile the deep profiler, even if it is not enabled.
> +#

That looks like a cut-and-paste error; this directory has nothing to
do with the deep profiler.

> +MAIN_TARGET=all
> +MERCURY_MAIN_MODULES=$(ALL_MODULES)
> +DEPEND=$(patsubst %,%.depend,$(ALL_MODULES))
> +
> +VPATH = $(LIBRARY_DIR) $(SSDB_DIR)
> +
> +#-----------------------------------------------------------------------------#
> +
> +MLFLAGS += --shared
> +MCFLAGS += --flags MFILTERJAVAC_FLAGS $(CONFIG_OVERRIDE)
> +
> +#-----------------------------------------------------------------------------#
> +
> +# Tell the C# compiler where the stdlib assembly is.
> +#
> +ifneq ("$(filter csharp%,$(GRADE))","")
> +CSCFLAGS=-lib:../library -r:mer_std.dll
> +endif
> +
> +#-----------------------------------------------------------------------------#
> +
> +ifneq ("$(filter il% csharp% java% erlang%,$(GRADE))","")
> +MLOBJS =
> +endif
> +
> +#-----------------------------------------------------------------------------#
> +
> +# The deep profiler contains quite a lot of C code for which there are
> +# currently not C#, IL, Java or Erlang implementations.  We need to pass
> +# `--allow-stubs' in order to compile it.
> +#
> +ifneq ("$(filter il% csharp% java% erlang%,$(GRADE))","")
> +MCFLAGS += --allow-stubs --no-warn-stubs
> +endif

Again, what has this directory to do with the deep profiler?
You can just delete that entirely, mfilterjavac doesn't contain any
foreign code.

> +
> +#-----------------------------------------------------------------------------#
> +
> +.PHONY: nothing
> +nothing:
> +
> +.PHONY: depend
> +depend:	$(DEPEND)
> +
> +$(DEPEND): MFILTERJAVAC_FLAGS
> +
> +.PHONY: all
> +all:	$(ALL_MODULES) $(TAGS_FILE_EXISTS)
> +
> +#-----------------------------------------------------------------------------#
> +
> +# Add some additional dependencies, so that Mmake knows to remake the
> +# profiler if one of the libraries changes.
> +
> +ifeq ("$(filter il% csharp% java% erlang%,$(GRADE))","")
> +mfilterjavac:		$(RUNTIME_DIR)/lib$(RT_LIB_NAME).$A
> +mfilterjavac:		$(LIBRARY_DIR)/lib$(STD_LIB_NAME).$A
> +endif
> +
> +$(cs_subdir)mfilterjavac.c:			$(UTIL_DIR)/mkinit$(EXT_FOR_EXE)
> +
> +#-----------------------------------------------------------------------------#
> +
> +.PHONY: check
> +check:	DEPEND=$(patsubst %,%.check,$(ALL_MODULES))
> +
> +.PHONY: ints
> +ints:	DEPEND=$(patsubst %,%.ints,$(ALL_MODULES))
> +
> +#-----------------------------------------------------------------------------#
> +
> +# We need the shenanigans with .deep_tags to avoid situations in which an
> +# "mmake tags" in this directory does nothing even in the absence of a tags
> +# file in this directory, because mmake uses VPATH to find ../library/tags
> +# and believes it to be the tags file we are asking for.

Cut-and-paste error above.  .deep_tags won't be created in this
directory.

...

> new file mode 100644
> index 0000000..6aecc07
> --- /dev/null
> +++ b/mfilterjavac/mfilterjavac.m
> @@ -0,0 +1,301 @@
> +%----------------------------------------------------------------------------%
> +% vim: ft=mercury ts=4 sw=4 et
> +%----------------------------------------------------------------------------%
> +% Copyright (C) 2013 The University of Melbourne.
> +% This file may only be copied under the terms of the GNU General
> +% Public License - see the file COPYING in the Mercury distribution.
> +%----------------------------------------------------------------------------%
> +%
> +% File: mfilterjavac.m
> +% Author: pbone
> +%
> +% Java does not support pragmas that effect it's tokenisation such as the

s/effect/affect/

> +% #line macro that can be used with C compilers to affect their error
> +% output.  This means that foreign java code that is written in Mercury is

Delete "is".

> +% that causes compilation errors has the wrong filename and line number
> +% reported for it.  Making use of Mercury's Java backend more tedious.

> +% This program post processes the output of javac, translating file and line
> +% number references to references within the original Mercury file.  It
> +% recognizes comments placed in the Java code by the Mercury compiler,
> +% specifically compiler/mlds_to_java.m

However, I would just put this:

     This program processes the output of the Java compiler when compiling
     Java code generated by the Mercury compiler.  It translates the error
     contexts reported by the Java compiler into the corresponding error
     contexts in the Mercury source file.  This is done by looking for special
     comments inserted into the generated Java code by the Mercury compiler.
     (See compiler/mlds_to_java.m for details.)

If you feel as though it worth waffling on about the limitations of Java
compilers viz lack of #line directives then you can then include the first paragraph.
(I would leave it out though.)

> +%-----------------------------------------------------------------------------%
> +
> +:- module mfilterjavac.
> +:- interface.
> +
> +:- import_module io.
> +
> +%-----------------------------------------------------------------------------%
> +
> +:- pred main(io::di, io::uo) is det.
> +
> +%-----------------------------------------------------------------------------%
> +%-----------------------------------------------------------------------------%
> +
> +:- implementation.
> +
> +:- import_module char.
> +:- import_module int.
> +:- import_module list.
> +:- import_module maybe.
> +:- import_module require.
> +:- import_module string.
> +
> +%-----------------------------------------------------------------------------%
> +
> +main(!IO) :-
> +    filter_lines(MaybeError, !IO),
> +    (
> +        MaybeError = ok
> +    ;
> +        MaybeError = error(Error),
> +        io.write_string(io.stderr_stream, Error, !IO),
> +        io.set_exit_status(1, !IO)
> +    ).
> +
> +:- pred filter_lines(maybe_error::out, io::di, io::uo) is det.
> +
> +filter_lines(MaybeError, !IO) :-
> +    io.read_line_as_string(Result, !IO),
> +    (
> +        Result = ok(Line),
> +        filter_line(Line, MaybeOutLine, !IO),
> +        (
> +            MaybeOutLine = ok(OutLine),
> +            io.write_string(OutLine, !IO),
> +            filter_lines(MaybeError, !IO)
> +        ;
> +            MaybeOutLine = error(Error),
> +            MaybeError = error(Error)
> +        )
> +    ;
> +        Result = eof,
> +        MaybeError = ok
> +    ;
> +        Result = error(Error),
> +        ErrorStr = format("stdin: %s\n", [s(error_message(Error))]),
> +        MaybeError = error(ErrorStr)
> +    ).

Should this program be reporting errors and failing?  I don't think
compilation in the java grade should fail simply because mfilterajvac
comes across some javac output that it doesn't know how to handle.

> +:- pred filter_line(string::in, maybe_error(string)::out, io::di, io::uo)
> +    is det.
> +
> +filter_line(Line, MaybeOutLine, !IO) :-
> +    (
> +        PartsA = split_at_separator(char.is_whitespace, Line),
> +        PartsA = [PartAA | OtherPartsA],
> +        PartsAA = split_at_char(':', PartAA),
> +        PartsAA = [Filename, LineStr, Empty],
> +        string.to_int(LineStr, LineNo)
> +    ->
> +        maybe_get_line_info(Filename, MaybeLineInfo, !IO),
> +        (
> +            MaybeLineInfo = ok(LineInfo),
> +            line_info_translate(LineInfo, Filename, LineNo,
> +                MerFileName, MerLineNo),
> +            Rest = string.join_list(" ", OtherPartsA),
> +            OutLine = string.format("%s:%d:%s %s\n",
> +                [s(MerFileName), i(MerLineNo), s(Empty), s(Rest)]),
> +            MaybeOutLine = ok(OutLine)
> +        ;
> +            MaybeLineInfo = error(Error),
> +            MaybeOutLine = error(Error)
> +        )
> +    ;
> +        MaybeOutLine = ok(Line)
> +    ).
> +
> +%-----------------------------------------------------------------------------%
> +
> +:- type line_info
> +    --->    line_info(
> +                li_start        :: int, % inclusive
> +                li_end          :: int, % not inclusive
> +                li_delta        :: int,
> +                li_orig_file    :: string
> +            ).
> +
> +:- type line_info_error
> +    --->    line_info_error(
> +                li_filename     :: string,
> +                li_lineno       :: int,
> +                li_error        :: line_info_error_type
> +            ).
> +
> +:- type line_info_error_type
> +    --->    lie_end_without_beginning
> +    ;       lie_beginning_without_end
> +    ;       lie_duplicate_beginning.
> +
> +:- pred line_info_translate(list(line_info)::in, string::in, int::in,
> +    string::out, int::out) is det.
> +
> +line_info_translate([], Name, Line, Name, Line).
> +line_info_translate([Info | Infos], Name0, Line0, Name, Line) :-
> +    Info = line_info(Start, End, Delta, File),
> +    (
> +        Line0 < Start
> +    ->
> +        % No translation.
> +        Name = Name0,
> +        Line = Line0
> +    ;
> +        Line0 < End
> +    ->
> +        Line = Line0 + Delta,
> +        Name = File
> +    ;
> +        line_info_translate(Infos, Name0, Line0, Name, Line)
> +    ).
> +
> +:- func error_type_string(line_info_error_type) = string.
> +
> +error_type_string(lie_end_without_beginning) =
> +    "END token without BEGIN token".
> +error_type_string(lie_beginning_without_end) =
> +    "BEGIN token without END token".
> +error_type_string(lie_duplicate_beginning) =
> +    "BEGIN token followed by another BEGIN token".
> +
> +%----------------------------------------------------------------------------%
> +
> +:- pred maybe_get_line_info(string::in, maybe_error(list(line_info))::out,
> +    io::di, io::uo) is det.
> +
> +maybe_get_line_info(Filename, MaybeInfo, !IO) :-
> +    io.open_input(Filename, Res, !IO),
> +    (
> +        Res = ok(Stream),
> +        read_line_marks(Stream, 1, [], MaybeMarksRev, !IO),
> +        io.close_input(Stream, !IO),
> +        (
> +            MaybeMarksRev = ok(MarksRev),
> +            reverse(MarksRev, Marks),
> +            create_line_info(Marks, Filename, [], MaybeInfo0),
> +            (
> +                MaybeInfo0 = ok(Infos),
> +                MaybeInfo = ok(Infos)
> +            ;
> +                MaybeInfo0 = error(LineInfoError),
> +                LineInfoError = line_info_error(ErrFilename, ErrLine, Error),
> +                StringError = format(
> +                    "%s:%d: Error understanding line number declration: %s",
> +                    [s(ErrFilename), i(ErrLine), s(error_type_string(Error))]),
> +                MaybeInfo = error(StringError)
> +            )
> +        ;
> +            MaybeMarksRev = error(Msg),
> +            MaybeInfo = error(format("%s: %s", [s(Filename), s(Msg)]))
> +        )
> +    ;
> +        Res = error(_Error),
> +        % We ignore errors here as our parsing of javac's output could cause
> +        % false errors.
> +        MaybeInfo = ok([])
> +    ).
> +
> +:- type line_mark
> +    --->    line_mark(
> +                lm_type             :: begin_or_end_block,
> +                lm_mer_file         :: string,
> +                lm_java_line_no     :: int,
> +                lm_mer_line_no      :: int
> +            ).
> +
> +:- type begin_or_end_block
> +    --->    begin_block
> +    ;       end_block.
> +
> +:- pred read_line_marks(input_stream::in, int::in, list(line_mark)::in,
> +    maybe_error(list(line_mark))::out, io::di, io::uo) is det.
> +
> +read_line_marks(Stream, JavaLineNo, Marks0, MaybeMarks, !IO) :-
> +    read_line_as_string(Stream, Result, !IO),
> +    (
> +        Result = ok(Line),
> +        % The format string in mlds_to_java specificaly uses spaces

s/specificaly/specifically/

Cheers,
Julien.



More information about the reviews mailing list