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
Document using dynamic variables for state in grammars #2106
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although I personally don't like the use of global variables, or routines with side effects, I guess this is basically OK.
I have created an example that uses hidden class variables for state, instead of dynamic variables. I think it's a better alternative, since dynamic variables can be accessed from outside. Can you please check if this covers your intended behavior? |
@JJ Did you test that example alternative? I'm pretty sure it won't work, since every rule that is called gets a fresh instance of the grammar class (which serves as its cursor object). By contrast, dynamic variables are visible to subrule calls. When the parse is recursive, then the most recent declaration wins, which is also desirable. Furthermore, calling them "global variables" is wrong; they are dynamically scoped variables. Were they globals, parsing the same grammar in two threads at once would break horribly. But it doesn't, because each has its own call stack, and thus its own dynamic variables. |
@jnthn It's tested as written. It does not cover all the three examples in the original code, and it produces the self same output. I mean, just download it. Maybe using a class variable is not the best, but are you sure an instance variable wouldn't be much better in this case? Point is, dynamic variables can also be changed from outside, and it would be desirable to avoid side effects as a good practice. |
@JJ D'oh, I somehow read I'm not sure what "outside" you're worrying about with dynamic variables. It's visible in the dynamic scope of the parse, which means it can be seen by the grammar's own rules (which we want) and action methods (which is sometimes useful too). After the parse is done, it's no longer in scope. |
El lun., 18 jun. 2018 a las 11:40, Jonathan Worthington (<
notifications@github.com>) escribió:
@JJ <https://github.com/JJ> D'oh, I somehow read has instead of my,
because using my for this is so totally wrong that I didn't even imagine
somebody doing it. :-) Yes, it will "work", until you use the grammar from
multiple threads and then they will trample on the same state and do the
totally wrong thing. So in trying to avoid something that isn't global
state, your proposed alternative introduces real global state, and so isn't
thread safe. Please don't advise this.
:-) That was me, doing the unimaginable. My rationale for doing that I
wasn't sure "parse", which is the usual way of doing this, actually
instantiates a grammar. You are, of course, right, but I didn't think of
that. And then the problem with class variables is quite general, it's not
reduced to grammars, right?
I'm not sure what "outside" you're worrying about with dynamic variables.
It's visible in the dynamic scope of the parse, which means it can be seen
by the grammar's own rules (which we want) and action methods (which is
sometimes useful too). After the parse is done, it's no longer in scope.
It can be seen by sub-rules, which you might or might not want; you could
imagine them doing someting bad along the class hierarchy. I don't know.
There should be a thread-safe way of encapsulating state, other than
dynamic variables. At any rate, if you think this PR is OK such as it is
now, we can merge right away.
|
The problem
How to embed code in grammars is largely undocumented.
Solution provided
Add documentation on how to do that, starting with using dynamic variables for state.