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

_locals(), _params() - what are they for? #104

Closed
akrzemi1 opened this issue Feb 21, 2024 · 7 comments
Closed

_locals(), _params() - what are they for? #104

akrzemi1 opened this issue Feb 21, 2024 · 7 comments

Comments

@akrzemi1
Copy link
Contributor

https://tzlaine.github.io/parser/doc/html/boost_parser__proposed_/tutorial/the_parse_context.html

After reading it, I have no idea what _locals and _params are for. Maybe the page should give a purpose for these two, and a motivating example. Also, no clue why two are needed.

Also, in that page, the links to _locals() and _params() are dangling.

@tzlaine
Copy link
Owner

tzlaine commented Feb 22, 2024

Yeah, an example is definitely needed here. Thanks for the dangling link notice.

@tzlaine
Copy link
Owner

tzlaine commented Feb 24, 2024

Dangling links fixed. I just assumed that there were no examples when I first saw this issue, but after looking, they each have examples and explanations. Are you looking for "when would I use this"-type of info? At least for the _params(), I feel like the YAML example gives that. Sometimes you just want to write a parameterized parser.

@akrzemi1
Copy link
Contributor Author

akrzemi1 commented Feb 25, 2024

The first exposure of the of the reader to _locals() and _params() is in section The Parse Context. In that section the reader -- I at least -- has no clue neither how these are used, nor what the motivaiton for having them is. It only has a "disclaimer" sentence: "Rules with locals are something we haven't gotten to yet". No cross-reference.

I believe that the docs would benefit from a separate section "Rule State" that would separately cover rule locals, rule parameters, the way to access them via _locals(), _params() and _p, and better examples. Then you could link to it from The Parse Context.

From section More About Rules I still cannot figure out a couple of things:

  • Can I only put a single type for a rule-local or can I put more? Name _locals() suggests that I can have more than one local in the rule, but the signature at Struct template rule suggests that I can have only one.
  • BTW, the signature at Struct template rule fails to indicate that the third and the fourth parameters are optional: they probably have default values.
  • Are types used for rule-locals need to be default-initializable?
  • Are rule-locals and rule-params separate parts of rule state, and can I have both in one rule?
  • Can I modify the state of rule-params?
  • The YAML exaple tells me to assume a number of things, and I don't think it would compile. I still don't see a full example how I determine the parameter and then set it, and then read it.
  • Given that we have _p is there any use case for the naked _params()?

In a couple of places you use terms "the currently processed rule" and "the bottommost rule". I think they mean the same thing. I would recommend defining one term, putting it in the Definitions pge, and using it throughout the documentation.

@tzlaine
Copy link
Owner

tzlaine commented Feb 27, 2024

Ok, thanks for the explanations. I agree with most of the points above. I'll be updating this section soon.

@akrzemi1
Copy link
Contributor Author

akrzemi1 commented Mar 2, 2024

And BTW, as someone mentioned in the review, is there still a need to have these names start with an underscore? Boost.Phoenix and Boost.Lambda needed them for obvious reasons, but bacause you endorse lambdas, the need for those magic names disappears, I guess.

@tzlaine
Copy link
Owner

tzlaine commented Mar 3, 2024

I still think there's a need for a naming convention here. There's no magic, but since these functions are called without qualification, it can be hard to distinguish them from your own code. _foo() immediately jumps out as "that's my code".

tzlaine added a commit that referenced this issue Mar 3, 2024
tzlaine added a commit that referenced this issue Mar 3, 2024
tzlaine added a commit that referenced this issue Mar 3, 2024
tzlaine added a commit that referenced this issue Mar 3, 2024
tzlaine added a commit that referenced this issue Mar 3, 2024
@akrzemi1
Copy link
Contributor Author

akrzemi1 commented Mar 4, 2024

How about calling them with qualification: ctx.val() = ctx.attr()?

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

No branches or pull requests

2 participants