Loading ...
Sorry, an error occurred while loading the content.

Re: Re: Error reporting - Complete patch for 2.7.1

Expand Messages
  • Michael Schmitt
    ... Well, for the moment, the output is fixed. But it should be fairly simple to make it customizable. You only have to replace the calls to
    Message 1 of 15 , Oct 2, 2000
    • 0 Attachment
      Robert Colquhoun wrote:

      > Sorry for the dumb question - but i was looking at your patch and you seem
      > to have removed customizable error reporting from the exceptions....What
      > happens if i don't want line + column number reporting in the exception output?

      Well, for the moment, the output is fixed. But it should be fairly simple to
      make it customizable.

      You only have to replace the calls to "getFileLineColumnString()" in
      'RecognitionException.java/cpp' by calls to another method. Then at the same
      time you can delete the method above which becomes superfluent.

      One thing I am not very happy about it the fact that there are too many
      different functions for reporting errors and warnings, as you can see when
      running ANTLR itself.

      I would appreciate it very much if you clean up the existing code (both Java
      and C++) in a way that there is only one method (or maybe two) for reporting
      errors and warnings. I would have done it by myself but my ressources are
      limited (ANTLR is just a meta problem for me).

      Nevertheless, I think my patch is a first small step in the right direction and
      I would like to see it taken into the source code repository because other
      motivated ANTLR developers start turning everyhting inside out :-)

      Kind regards,

      Michael

      --
      ======================================================================
      Michael Schmitt phone: +49 451 500 3725
      Institute for Telematics secretary: +49 451 500 3721
      Medical University of Luebeck fax: +49 451 500 3722
      Ratzeburger Allee 160 eMail: schmitt@...-luebeck.de
      D-23538 Luebeck, Germany WWW: http://www.itm.mu-luebeck.de
      ======================================================================
    • Ross Bencina
      ... At which point I jump in to reiterate my suggestion for a MessageLog strategy abstract class. Adding a class to support error/warning logging would allow
      Message 2 of 15 , Oct 2, 2000
      • 0 Attachment
        Michael Schmitt mailto:schmitt@...-luebeck.de> wrote:
        > One thing I am not very happy about it the fact that there are too many
        > different functions for reporting errors and warnings, as you can see when
        > running ANTLR itself.

        At which point I jump in to reiterate my suggestion for a MessageLog
        strategy abstract class.

        Adding a class to support error/warning logging would allow users to
        substitute their own message handling on a per-parser basis using something
        like:

        MyMessageLog ml;

        myParser->setMessageLog( &ml ); // override default message handling

        The standard implementation would output to the standard streams as is
        currently the case, but custom implementations could be written by users.
        The utility of this may be lost on command-line unix users, but when a
        parser is embedded in a GUI app it would be nice to have a clean OO
        interface to message handling: for example the custom MessageLog could store
        all received messages in a vector<> and then update a list view when the
        parser has finished (bad example, but you get the idea...)

        See my message 15th September "proposal: MessageLog Strategy refactoring"
        for further rationale and details.

        Best wishes,

        Ross.
      • Ric Klaren
        ... ACK! ... Hadn t forgotten about that one =) Ric -- ... -- Ric Klaren - klaren@cs.utwente.nl ------------------------------------------ ... And this
        Message 3 of 15 , Oct 3, 2000
        • 0 Attachment
          On Tue, Oct 03, 2000 at 06:25:35AM +1000, Ross Bencina wrote:
          > Michael Schmitt mailto:schmitt@...-luebeck.de> wrote:
          > > One thing I am not very happy about it the fact that there are too many
          > > different functions for reporting errors and warnings, as you can see when
          > > running ANTLR itself.
          >
          > Adding a class to support error/warning logging would allow users to
          > substitute their own message handling on a per-parser basis using something
          > like:

          ACK!

          > See my message 15th September "proposal: MessageLog Strategy refactoring"
          > for further rationale and details.

          Hadn't forgotten about that one =)

          Ric
          --
          -----+++++*****************************************************+++++++++-------
          -- Ric Klaren - klaren@... ------------------------------------------
          -----+++++*****************************************************+++++++++-------
          'And this 'rebooting' business? Give it a good kicking, do you?' 'Oh, no,
          of course, we ... that is ... well, yes, in fact,' said Ponder. 'Adrian
          goes round the back and ... er ... prods it with his foot. But in a
          technical way,' he added. --- From: Hogfather by Terry Pratchett.
          -------------------------------------------------------------------------------
        • Ric Klaren
          ... Seems I forgot to sync one time. :) ... Okie... ... Somewhere near the big unicode announcement (a except for C++ mode atm would be sufficient I think) :)
          Message 4 of 15 , Oct 3, 2000
          • 0 Attachment
            On Tue, Oct 03, 2000 at 10:20:00AM -0700, Terence Parr wrote:
            > I made this in response to label types for hetero trees from Jerry
            > James (last night) ;)

            Seems I forgot to sync one time. :)

            > Tweaks go in the mainline, but in general dev should happen in your
            > dev branch. mainline is essentially stuff that compiles, works, and
            > is to be included in next release. Be sure to do a

            Okie...

            > > BTW2 Unicode support is not working in C++ mode this could be stated in the
            > > 2.7.1 docs. If I recall right. This will need some changes to the support
            > > classes. The lexer and probably also Parser/TreeParser need to be
            > > templatized with the character type used (wchar_t for unicode) to make this
            > > work.
            >
            > Shoot...where should we stick it?

            Somewhere near the big unicode announcement (a except for C++ mode atm
            would be sufficient I think) :)

            Cheers,

            Ric
            --
            -----+++++*****************************************************+++++++++-------
            -- Ric Klaren - klaren@... ------------------------------------------
            -----+++++*****************************************************+++++++++-------
            'And this 'rebooting' business? Give it a good kicking, do you?' 'Oh, no,
            of course, we ... that is ... well, yes, in fact,' said Ponder. 'Adrian
            goes round the back and ... er ... prods it with his foot. But in a
            technical way,' he added. --- From: Hogfather by Terry Pratchett.
            -------------------------------------------------------------------------------
          • Ross Bencina
            ... refactoring ... Ok, I m off on a road trip until next monday, I ll implement it when I get back (unless someone else beats me to it)... presumably 2.7.2a1
            Message 5 of 15 , Oct 3, 2000
            • 0 Attachment
              Ric Klaren <klaren@...> wrote:
              > > See my message 15th September "proposal: MessageLog Strategy
              refactoring"
              > > for further rationale and details.
              >
              > Hadn't forgotten about that one =)

              Ok, I'm off on a road trip until next monday, I'll implement it when I get
              back (unless someone else beats me to it)... presumably 2.7.2a1 with the
              other error patches included will be available by then...

              Best wishes,

              Ross.
            • Terence Parr
              ... I made this in response to label types for hetero trees from Jerry James (last night) ;) ... Tweaks go in the mainline, but in general dev should happen in
              Message 6 of 15 , Oct 3, 2000
              • 0 Attachment
                Monday, October 02, 2000, Ric Klaren hath spoken:
                > schmitt@...-luebeck.de on Sat, Sep 30, 2000 at 06:28:31PM +0200

                > Hi,

                > On Sat, Sep 30, 2000 at 06:28:31PM +0200, Michael Schmitt wrote:
                > I finally managed to complete the patch for improved error reporting.
                > Parsers generated by ANTLR (based on Java and C++) will report errors with
                > filename, line and column information now. The output format is chosen in
                > a way that (x)emacs directly jumps to the erroneous file (at the right
                > line/column) if you click on the error message.

                > Great patch :) Allows me too strike exception class rework from the Todo
                > list :)

                > You can install the patches very easily:

                > When it comes to patches I (personally) prefer diffs it is a lot easier to
                > see what really changes with those. Preferably diff -uwbBpPr oldir/ newdir/.

                > Which leads up to my question:

                > @@ -1284,21 +1280,9 @@ public class JavaCodeGenerator extends C
                > // It is a token or literal reference. Generate the
                > // correct variable type for this grammar
                > println(labeledElementType + " " + a.getLabel() + " = "$
                > - // In addition, generate *_AST variables if
                > - // building ASTs
                > + // In addition, generate *_AST variables if building AS$
                > if (grammar.buildAST) {
                > - if (a instanceof GrammarAtom &&
                > - ((GrammarAtom)a).getASTNodeType()!=null ) {
                > - GrammarAtom ga = (GrammarAtom)a;
                > - println(ga.getASTNodeType()+" " +
                > - a.getLabel() +
                > - "_AST = null;");
                > - }
                > - else {
                > - println(labeledElementASTType+" " +
                > - a.getLabel() +
                > - "_AST = null;");
                > - }
                > + println(labeledElementASTType+" " + a.getLabel() + $
                > }
                > }
                > }

                > Is this a change you made to the JavaCodegenerator, Michael, or? ('-'

                I made this in response to label types for hetero trees from Jerry
                James (last night) ;)

                > BTW Terence. How is development going to continue? In the main branch?
                > Shall I include the C++ changes of this patch? Or you do all?

                Tweaks go in the mainline, but in general dev should happen in your
                dev branch. mainline is essentially stuff that compiles, works, and
                is to be included in next release. Be sure to do a

                p4 sync
                p4 integrate -b klaren.dev
                p4 resolve -at # accept all changes from main
                p4 submit

                from your branch to get a clean copy of main before continuing on.

                I would like to examine the exception hierarchy personally before we
                decide on a path there. I'm sure Michael has done most of the
                thinking for us though! ;):D

                > BTW2 Unicode support is not working in C++ mode this could be stated in the
                > 2.7.1 docs. If I recall right. This will need some changes to the support
                > classes. The lexer and probably also Parser/TreeParser need to be
                > templatized with the character type used (wchar_t for unicode) to make this
                > work.

                Shoot...where should we stick it?

                > BTW3 The website does not mention 2.7.1, yet...

                Ooops...yeah I'll get to that soon.

                Thanks,
                Ter
                --
                Chief Scientist & Co-founder, http://www.jguru.com
                Co-founder, http://www.NoWebPatents.org -- Stop Patent Stupidity
                parrt@...
              • Terence Parr
                ... First rule of releasing software: release it ;) Doing last minute patches is very dangerous and as you saw the one last patch stretched 2.7.1 for a few
                Message 7 of 15 , Oct 3, 2000
                • 0 Attachment
                  Monday, October 02, 2000, Michael Schmitt hath spoken:
                  >> Remember how often I said that =) (Not that this patch caused any trouble
                  >> of course (yet ;) ) just saying that I've grown a little suspicious of
                  >> small patches that really don't change anything even if I made them myself)

                  > That is probably the reason why the patch is not in 2.7.1. Of course, I am a
                  > little bit sad about that.

                  First rule of releasing software: release it ;) Doing last minute
                  patches is very dangerous and as you saw the "one last patch"
                  stretched 2.7.1 for a few months! ;)

                  > But if I remember correctly, Terence offered to
                  > release 2.7.2a1, didn't he!?

                  You bet! I need to actually turn on my brain though for the exception
                  hiearchy...

                  BTW, I actually prefer full files not changes. I have tkdiff, which
                  does a great job of telling me the changes whereas patch just says
                  "uh, i can't apply the patch" when my files change. Inevitably my
                  files will change before I can get to the patch ;)

                  >> Shall I include the C++ changes of this patch? Or you do all?

                  > I hope the patch will be put into the source repository as soon as possible
                  > because you can image I would not like to inspect it another times. Please
                  > note that there are also changes in the Java classes (this was said to be a
                  > prerequisite for code merging)

                  Ric, how about you freshen your dev branch and apply Michael's changes
                  to check it out. Then I can take a look at the Java part etc...

                  Ter
                  --
                  Chief Scientist & Co-founder, http://www.jguru.com
                  Co-founder, http://www.NoWebPatents.org -- Stop Patent Stupidity
                  parrt@...
                Your message has been successfully submitted and would be delivered to recipients shortly.