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

fix: fix context shallow copy #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kiprey
Copy link

@Kiprey Kiprey commented Feb 23, 2023

Classic python object shallow-copy problem.

context = {
    'lastvar': last_var,
    'lines': [],
    'variables': {},
    'interesting_lines': [],
    'force_var_reuse': False
}
...
while len(context['lines']) < num_lines:
    tmp_context = context.copy()
    try:
        if (random.random() < self._interesting_line_prob) and (len(tmp_context['interesting_lines']) > 0):
            tmp_context['force_var_reuse'] = True
            lineno = random.choice(tmp_context['interesting_lines'])
        else:
            lineno = random.choice(self._all_nonhelper_lines)
        creator = self._creators['line'][lineno]
        self._expand_rule('line', creator, tmp_context, 0, False)
        context = tmp_context
    except RecursionError as e:
        print('Warning: ' + str(e))

When RecursionError is triggered, some of the generated lines are retained in the original context.
This can lead to some unintended behavior, such as variable redefinition.

@ifratric
Copy link
Collaborator

Thanks for looking into this. Unfortunately, it seems that doing a proper deepcopy is too slow in practice (you can try python3 generator.py --output_dir test --no_of_files 100 with and without the patch, an order of magnitude difference). So currently it seems that the current solution is more usable in practice, despite its flaws :( Unless some custom solution is found for this case that gives us the best of both worlds.

@Kiprey
Copy link
Author

Kiprey commented Mar 1, 2023

Yes, it's really slow, and probably requires a lot of code structure changes, to get the generated content into a temporary context each time the _expand_rule function is executed, so that it can later decide whether to discard the currently generated content.

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

Successfully merging this pull request may close these issues.

2 participants