maetl / calyx

A Ruby library for generating text with recursive template grammars.
MIT License
61 stars 5 forks source link

"RuntimeError: Rule already declared in grammar" if trying to dynamically construct rules again #5

Closed tra38 closed 8 years ago

tra38 commented 8 years ago

After some struggle, I did manage to initialize a context hash successfully, thereby being able to dynamically define rules. But when I tried to pass in a different context hash again in IRB (with different definitions of the rules), the following error occurs.

Even if I create a new instance of the class, the previously defined rules gets carried over, and so I still get "Rule already declared in grammar" error.

This might not be an issue in production if the script runs to completion and then shuts down. Then when you start the script again, the whole memory is cleared, meaning the program has forgotten the old context hash. You can then pass in the new context hash without any problem.

Way too hacky to be relied on.

maetl commented 8 years ago

I’ll write a failing test for this. It’s probably worth making a decision on. I think your expectation of how the API should work is the correct one—the dynamic/context map rules shouldn’t affect the shared state.

tra38 commented 8 years ago

I am able to fix the issue by using this one weird trick in the Registry class:

      def evaluate(context={})
        duplicate_registry = Marshal.load(Marshal.dump(self))
        duplicate_rules = duplicate_registry.rules
        context.each do |key, value|
          if duplicate_rules.key?(key.to_sym)
            raise "Rule already declared in grammar: #{key}"
          end

          duplicate_rules[key.to_sym] = if value.is_a?(Array)
            Production::Choices.parse(value, duplicate_registry)
          else
            Production::Concat.parse(value.to_s, duplicate_registry)
          end
        end

        duplicate_rules[:start].evaluate
      end

"Marshal.load(Marshal.dump(self))" creates a deep copy of our original registry. We can then modify this new deep copy as much as we want, while never impacting the shared state at all. (This StackOverflow post showcase several ways of creating "deep copies", and "Marshal Dump, then Marshal Load" seems to be both pretty fast and easiest to understand.)

I have already written an automated test that tests if this approach is working...I just need your approval before I submit a pull request (and clean up my commits so that your history doesn't get littered with my previous attempts at memoization).

maetl commented 8 years ago

This is fixed by #6 (thanks @tra38), and is released in 0.7.1.