rust-peg icon indicating copy to clipboard operation
rust-peg copied to clipboard

Allow for mutable parameters in rules

Open mxgordon opened this issue 9 months ago • 3 comments

Currently (from my experimentation) rust-peg will not parse mutable parameters in the header for a rule it seems to mistake mut for the parameter name, thus expecting a colon to follow it instead of the actual parameter name. This would allow for more powerful use cases where parameters can be used to create some amount of context or side effects for rules.

The following code won't compile, but I believe it should:

    parser!(
        grammar space_counter() for str {
            rule run(mut space_count: i32) -> i32 =  // mut parameter here is what fails
               needs_mutable_borrw(&mut space_count) {space_count}

            rule needs_mutable_borrw(n_spaces: &mut i32) -> String = v:" " {
                n_spaces += 1;
                return v;
            }
        }
    );

This might be outside the scope of rust-peg, but I believe it would be a helpful feature especially given that this is default rust behavior.

mxgordon avatar Mar 22 '25 21:03 mxgordon

Allowing side effects may not be a good choice, considering attempting failure after side effects occur. You can use Cell<Option<T>> or RefCell<T> If mut is allowed, it is also easy for users who are not familiar with peg principles to write unexpected code

A4-Tacks avatar Mar 23 '25 01:03 A4-Tacks

Consider this code with logical errors due to side effects:

    peg::parser!(
        grammar space_counter() for str {
            pub rule run(space_count: &std::cell::RefCell<i32>) -> i32 =
                (needs_mutable_borrw(&mut space_count.borrow_mut()) "."
                /needs_mutable_borrw(&mut space_count.borrow_mut()) ";"
                )+
                {space_count.take()}

            rule needs_mutable_borrw(n_spaces: &mut i32) -> String = v:$(" ") {
                *n_spaces += 1;
                v.to_owned()
            }
        }
    );
    assert_eq!(space_counter::run(" . .", &0.into()), Ok(2));
    assert_eq!(space_counter::run(" . ;", &0.into()), Ok(2)); // failed!

Change to:

    peg::parser!(
        grammar space_counter() for str {
            pub rule run() -> usize = counter:" "+ { counter.len() }
        }
    );
    assert_eq!(space_counter::run(" . ."), Ok(2));
    assert_eq!(space_counter::run(" . ;"), Ok(2));

performance: counter: Vec<()>, () is ZST, no allocate

A4-Tacks avatar Mar 23 '25 02:03 A4-Tacks

I mean, I see your point about potential logical errors. However, as you pointed out, its already possible using RefCell. So why not just make it a bit simpler and more readable by allowing mutable parameters?

Additionally, I know the example I made can be solved using standard PEG, I just wanted to come up with something simple to illustrate my point.

mxgordon avatar Mar 23 '25 22:03 mxgordon