Documentation for hir::Repetition is misleading

[nwalfield]

Consider the following code:

use regex_syntax::hir;

fn main() {
    // Is this: xab+y or x(ab)+y?
    let xabplusy = hir::Hir::concat(vec![
        hir::Hir::literal(hir::Literal::Unicode('x')),
        hir::Hir::repetition(hir::Repetition {
            kind: hir::RepetitionKind::OneOrMore,
            greedy: true,
            hir: Box::new(
                hir::Hir::concat(vec![
                    hir::Hir::literal(hir::Literal::Unicode('a')),
                    hir::Hir::literal(hir::Literal::Unicode('b')),
                ])
            )
        }),
        hir::Hir::literal(hir::Literal::Unicode('x'))
    ]);

    let regex = xabplusy.to_string();
    eprintln!("{}", regex);
}

Running the above code yields: xab+y

The documentation for Repetition says:

hir: Box<Hir>

The expression being repeated.

This leads me to believe that the whole Hir will be repeated (in this case ab). But, at least in the case of a Hir::concat, it appears that only the last Hir in the vector (b) is repeated.

[mingyli]

Is this an issue with how Hir is rendered? Looking at Writer::visit_pre

regex/regex-syntax/src/hir/print.rs

Lines 86 to 91 in 04e025b

	fn visit_pre(&mut self, hir: &Hir) -> fmt::Result {
	match *hir.kind() {
	HirKind::Empty
	| HirKind::Repetition(_)
	| HirKind::Concat(_)
	| HirKind::Alternation(_) => {}

and Writer::visit_post

regex/regex-syntax/src/hir/print.rs

Lines 173 to 180 in 04e025b

	HirKind::Repetition(ref x) => {
	match x.kind {
	hir::RepetitionKind::ZeroOrOne => {
	self.wtr.write_str("?")?;
	}
	hir::RepetitionKind::ZeroOrMore => {
	self.wtr.write_str("*")?;
	}

repetitions should probably be wrapped in parentheses.

[mingyli]

It looks like alternations have the same issue:

use regex_syntax::hir;

fn main() {
    // Is this: xab+y or x(ab)+y?
    let xabplusy = hir::Hir::concat(vec![
        hir::Hir::literal(hir::Literal::Unicode('x')),
        hir::Hir::repetition(hir::Repetition {
            kind: hir::RepetitionKind::OneOrMore,
            greedy: true,
            hir: Box::new(hir::Hir::concat(vec![
                hir::Hir::literal(hir::Literal::Unicode('a')),
                hir::Hir::literal(hir::Literal::Unicode('b')),
            ])),
        }),
        hir::Hir::alternation(vec![
            hir::Hir::concat(vec![
                hir::Hir::literal(hir::Literal::Unicode('f')),
                hir::Hir::literal(hir::Literal::Unicode('g')),
            ]),
            hir::Hir::literal(hir::Literal::Unicode('h')),
        ]),
        hir::Hir::literal(hir::Literal::Unicode('x')),
    ]);

    let regex = xabplusy.to_string();
    eprintln!("{}", regex);
}

yields xab+fg|hx

[BurntSushi]

Yeah I think this is probably a bug in the printer. I don't think this kind of ambiguity is produced when roundtripping from the concrete syntax, because groups (even non-capturing groups) aren't flattened. Arguably they should be via the Hir's smart constructors, which would surface this bug more readily.

[BurntSushi]

This ended up being a duplicate of #516. And a fix is incoming. See #516 (comment) for more details.

[nwalfield]

Thanks for taking the time to work on this.