Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MutableState has non-mutable initializations resulting in errors on deepCopy calls #30

Open
EdwardRaff opened this issue Jun 7, 2023 · 2 comments
Labels

Comments

@EdwardRaff
Copy link

The initialization of these lists uses the emptyList collection that is hard-coded to be an always empty list, and so will throw an error if anything is added to these lists.

public MutableState(final boolean isFinal) {
this.isFinal = isFinal;
this.transitions = Collections.emptyList();
this.associations = Collections.emptyList(); // = new ArrayList<T>(0);
}

In most of the code base this results in several isEmpty() checks to create a new ArrayList as necessary to add to it.

However, this does not occur in the deepCopy method, resulting in an error when I attempt to build a rather large regex (it has 10000 matchers, not yet at a minimum viable example, but I think this bug is pretty easy/clear fix).

public MutableState<T> deepCopy(Map<DeepCopy, DeepCopy> oldToNewObjects) {
@SuppressWarnings("unchecked")
// if there is a copy of this in the map, it will be of the same type.
MutableState<T> stateCopy = (MutableState<T>) oldToNewObjects.get(this);
if (stateCopy == null) {
stateCopy = new MutableState<T>(this.isFinal);
oldToNewObjects.put(this, stateCopy);
for (Transition<T> transition : transitions) {
final Transition<T> transitionCopy = transition.deepCopy(oldToNewObjects);
stateCopy.transitions.add(transitionCopy);
}
}
return stateCopy;
}

This is blocking me from being able to create a new TrieMultiSequenceMatcher

@nishihatapalmer
Copy link
Owner

nishihatapalmer commented Jun 7, 2023

Hi Edward, thanks for the bug report. It is indeed a bug in the deepCopy implementation. The fix should be to use the normal addTransition() method of the stateCopy, instead of trying to add them directly to the collection.

public MutableState<T> deepCopy(Map<DeepCopy, DeepCopy> oldToNewObjects) {
		@SuppressWarnings("unchecked")
		// if there is a copy of this in the map, it will be of the same type.
		MutableState<T> stateCopy = (MutableState<T>) oldToNewObjects.get(this);
		if (stateCopy == null) {
			stateCopy = new MutableState<T>(this.isFinal);
			oldToNewObjects.put(this, stateCopy);
			for (Transition<T> transition : transitions) {
				final Transition<T> transitionCopy = transition.deepCopy(oldToNewObjects);
				stateCopy.addTransition(transitionCopy);
			}
		}
		return stateCopy;
	}

@nishihatapalmer
Copy link
Owner

To set expectations here, I am unlikely to issue a bug fix for the current version of byteseek. I have been working on byteseek 3 for some years now, and that will be the next release I make. Hopefully this year, but I have some important other priorities taking up my time at the moment.

To further set expectations, all of the multi-string matching and searching in byteseek 3 is being placed in an "incubator" status. This means that it is largely untested code, whereas the rest of byteseek will be fully tested. I am not getting rid of it, but being honest, I do not know how much time I will have to develop this aspect of byteseek going forward. I guess if there is actual interest in this aspect, that might motivate me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants