[antlr-interest] Altering static FOLLOW bitsets! fixed

[Terence Parr]

Hi Guys, yes looks like a problem. I do a remove in 1 place that's  
incorrect:

	public boolean mismatchIsMissingToken(IntStream input, BitSet follow) {

		if ( follow.member(Token.EOR_TOKEN_TYPE) ) {
			if ( state._fsp>=0 ) { // remove EOR if we're not the start symbol
 >>>>>			follow.remove(Token.EOR_TOKEN_TYPE);
			}
			BitSet viableTokensFollowingThisRule =  
computeContextSensitiveRuleFOLLOW();
			follow = follow.or(viableTokensFollowingThisRule);
		}

Easiest thing is to just move the IF below the  
computeContextSensitiveRuleFOLLOW which calls combineFollows which does:

		BitSet followSet = new BitSet();

so it's a set we can modify.

So now it looks like:

		if ( follow.member(Token.EOR_TOKEN_TYPE) ) {
			BitSet viableTokensFollowingThisRule =  
computeContextSensitiveRuleFOLLOW();
			follow = follow.or(viableTokensFollowingThisRule);
             if ( state._fsp>=0 ) { // remove EOR if we're not the  
start symbol
                 follow.remove(Token.EOR_TOKEN_TYPE);
             }
		}

edit //depot/code/antlr/main/CHANGES.txt#82
edit //depot/code/antlr/main/runtime/Java/src/org/antlr/runtime/ 
BaseRecognizer.java#17
Change 5156 submitted.
Ter
On Jul 17, 2008, at 5:51 AM, Johannes Luber wrote:

> Cameron Skinner schrieb:
>> Hello,
>> I’m using the 3.1 beta release, CSharp2 target grammar.
>> I’m seeing different behavior running my parser with a syntax error  
>> the second time run. I’ve tracked it down to where I believe the  
>> problem is, located in BaseRecognizer.cs. The  
>> “follow.Remove( Token.EOR_TOKEN_TYPE )” logic is being called,  
>> which modifies the state of the static readonly BitSet object found  
>> on the parser. The problem with that is that even though I am  
>> creating a completely different parser instance, the static  
>> variable has been altered for as long as the assembly is in memory.  
>> I’ve copied a simple program to illustrate the problem at the  
>> bottom of this mail.
>> Is this a known problem?
>
> I haven't heard of it yet, but it's a Java target problem, too, so  
> Ter has to say, if removing the static modifier for the BitSet  
> variables in the parser is enough and doesn't affect anything else.
>
> Johannes
>
>> Regards,
>> Cameron
>> public bool MismatchIsMissingToken(IIntStream input, BitSet follow) {
>>                  if (follow == null) {
>>                        // we have no information about the follow;  
>> we can only consume
>>                        // a single token and hope for the best
>>                        return false;
>>                  }
>>                   // compute what can follow this grammar element  
>> reference
>>                  if (follow.Member(Token.EOR_TOKEN_TYPE)) {
>>                        if (state.followingStackPointer >= 0) { //  
>> remove EOR if we're not the start symbol
>>                              follow.Remove(Token.EOR_TOKEN_TYPE);  
>> *<<<<<<<<< THIS IS THE PROBLEM, modifies static variable*
>>                        }
>>                        BitSet viableTokensFollowingThisRule =  
>> ComputeContextSensitiveRuleFOLLOW();
>>                        follow =  
>> follow.Or(viableTokensFollowingThisRule);
>>                  }
>>                   // if current token is consistent with what could  
>> come after set
>>                  // then we know we're missing a token; error  
>> recovery is free to
>>                  // "insert" the missing token
>>                   // BitSet cannot handle negative numbers like -1  
>> (EOF) so I leave EOR
>>                  // in follow set to indicate that the fall of the  
>> start symbol is
>>                  // in the set (EOF can follow).
>>                  if ( follow.Member(input.LA(1)) ||  
>> follow.Member(Token.EOR_TOKEN_TYPE) ) {
>>                        return true;
>>                  }
>>                  return false;
>>            }
>> Simple program that exhibits the fundamental problem of trying to  
>> use a static  readonly member. The state of that member can change,  
>> just not the instance itself:
>> using System;
>> using System.Collections.Generic;
>> using System.Linq;
>> using System.Text;
>> namespace ConsoleApplication2
>> {
>>   class Program
>>   {
>>      static void Main( string[] args )
>>      {
>>         Testing t = new Testing();
>>         Console.WriteLine( Testing.test.isSet );
>>          Testing.test.isSet = true;
>>         Testing x = new Testing();
>>          Console.WriteLine( Testing.test.isSet );
>>      }
>>   }
>>    public class Testing
>>   {
>>      public static readonly Test test = new Test();
>>   }
>>    public class Test
>>   {
>>      public bool isSet = false;
>>   }
>> }
>>
>