[antlr-interest] C# Error Recovery

[Terence Parr]

On Apr 16, 2008, at 1:55 PM, Johannes Luber wrote:

> Gavin Lambert schrieb:
>> I have a rule like this in my grammar (C# target, latest interim  
>> build):
>> tag[String name]
>>    :    WS? t=TAG { $t.text.Equals("<" + $name + ">") }? NL
>>    ;
>> catch [FailedPredicateException ex] { throw new  
>> RecognitionException("Expected <" + $name + "> but found " +  
>> $t.text, ex, Input); }
>> The intent should be clear -- I want to validate that I'm receiving  
>> the tag I'm expecting to get at that point in the input, but I want  
>> to give it a custom error message.
>> I want all the normal ANTLR backtracking and recovery mechanisms to  
>> take effect, which basically seems to mean that I need to be  
>> throwing a RecognitionException or derived class thereof.
>> The problem lies in BaseRecognizer.GetErrorMessage, which  
>> completely ignores the Message of the incoming exception and uses a  
>> series of type-specific tests to construct a new message; leaving  
>> it blank if a base RecognitionException is thrown.  I can work  
>> around this by deriving a new exception from RecognitionException  
>> and overriding GetErrorMessage to deal with that appropriately, but  
>> I shouldn't need to.
>> Proposed fixes:
>> 1. At minimum, the first line of GetErrorMessage should be changed  
>> to assign "e.Message" as the default message, not null.
>> 2. Preferably, *all* the type specific code in GetErrorMessage  
>> should be removed, and the exception Message should be set to the  
>> equivalent when the exception is thrown in the first place (in  
>> their constructors).  It just seems silly to try to "fix it up"  
>> after the fact.
> I'm not opposed against a fix, but the last paragraph isn't quite  
> clear to me. Equivalent of what? Furthermore, Ter, why is that code,  
> as it is? After all, it is like this in the Java target, too.

Here were my thoughts at the time:

	/** What error message should be generated for the various
	 *  exception types?
	 *
	 *  Not very object-oriented code, but I like having all error message
	 *  generation within one method rather than spread among all of the
	 *  exception classes. This also makes it much easier for the exception
	 *  handling because the exception classes do not have to have  
pointers back
	 *  to this object to access utility routines and so on. Also, changing
	 *  the message for an exception type would be difficult because you
	 *  would have to subclassing exception, but then somehow get ANTLR
	 *  to make those kinds of exception objects instead of the default.
	 *  This looks weird, but trust me--it makes the most sense in terms
	 *  of flexibility.
	 *
	 *  For grammar debugging, you will want to override this to add
	 *  more information such as the stack frame with
	 *  getRuleInvocationStack(e, this.getClass().getName()) and,
	 *  for no viable alts, the decision description and state etc...
	 *
	 *  Override this to change the message generated for one or more
	 *  exception types.
	 */

Still seems valid I guess.

Ter