Add support for comments in lrlex files

[FranklinChen]

I started using lrlex and didn't find a formal definition of its file format but it seems there is no way to write comments in .l files. I think that would be a useful feature to support.

[ratmice]

Generally lrlex has focused on following the posix lex specification (but hasn't implemented comments yet if I recall),
my reading I didn't see any specific documentation of the comment format in lex, it is a little awkward, but generally mimics c-style comments but must be preceded by whitespace to avoid ambiguity with regexes. Except in cases where it is just a block of c code that gets copied verbatim.

Here are some examples, the latter showing some of the cases where initial whitespace is not required:
https://cs.gmu.edu/~henryh/330/Lex/comments.html
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/lex.html#tag_20_65_17

[ltratt]

Agreed, it would be good if lrlex supported comments, and I'd happily take a PR to that effect! If flex supports them, I might also be inclined to support // comments, but I don't feel strongly about it.

[ratmice]

So, I had noticed the text in #325 the following text in the posix lex spec:

Any such input (beginning with a or within "%{" and "%}" delimiter lines) appearing at the beginning of the Rules section before any rules are specified shall be written to lex.yy.c

Which is relevant to this bug, it would be easy enough to currently just ignore any line which starts with a space.
It seems like if we tried to emit these verbatim into generated sources, they would be in the middle of a vec![ Rule::new(), ..].
In order to actually emit them we'd need to change uses of struct Rule to something like enum RuleOrVerbatim<StorageT>{ Verbatim(String), Rule(Rule<StorageT>)}, as currently there isn't anywhere for them in our AST of the lex source format.

I believe Rule is public but #[doc(hidden)] and otherwise documented as unstable, so perhaps changing it to an enum is acceptable. But let me know if there are preferences here between emitting these verbatim or ignoring them entirely.

[ratmice]

Wanted to update my thoughts on this one,

Regarding my previous comment

The hack that lex uses to support comments by prefixing them with whitespace so they get written to the generated output source. We had decided we didn't want to support that in #441. Trying to use whitespace at the beginning of a rule now triggers to an error, and does not cause the subsequent text to be written to the generated source.

Current thoughts

The difficulty with trying to add comments is that they are currently valid regexes,
e.g. one might have a rule like

%%
// 'DoubleSlashes'
/* 'LeadingSlashes'
.*/ 'TrailingSlashes'`

Thus at a minimum it requires changing the escaping for regexes for the / character to start a comment.
We support the below, equivalently to the above

%%
\/\/ 'DoubleSlashes'
\/* 'LeadingSlashes'
.*\/ 'TrailingSlashes'`

For #490 i'll be adding a %grmtools section like the one we added for yacc in #492 my thought is to add a setting there,
something to the effect of

%%
%grmtools {
   allow_cxx_comments true
}
// Now treated as a cxx style comment
\/\/ 'Requires_Escaping_On_Double_Slashes'

I'm guessing this requires us to add a flag to the RegexOptions struct

I'd prefer to just leave it at that and not deal with C style comments?
Does that sound like, or even vaguely resemble a reasonable/actionable plan towards getting us comments?

[ltratt]

Thinking out loud: perhaps we can allow people to specify their own comments character? So something like:

%grmtools {
  comment_prefix "//"
}

It wouldn't go in RegexOptions, IMHO, but somewhere else... where... I'm not sure :)

[ratmice]

I'm not sure how feasible allowing arbitrary comment prefix would be, with how it affects regex escape sequences.
In the sense that most of the code paths around escaping are currently relying on static's and thus I'm not sure how we would set them so late at %grmtools section parsing time. It can't hurt to try though.

I guess I would prefer eol_comment_prefix "//", if that is the behavior/type of comment being implemented.
I suppose we should also have some documentation specifying explicitly that comment_prefix cannot contain the \ character since it is used for escaping, and the first character of the comment_prefix must be escaped for it to appear in any subsequent regex.

True, the relationship to RegexOptions is pretty limited to affecting just escape sequences, I haven't been sure either I was wondering if we should consider having a change to RegexOptions migrating to some kind of HashMap renaming it LexOptions instead. Because currently as a struct RegexOptions it requires a semver incompatible change to add fields?
Technically adding/removing fields isn't currently much of an issue because of Regex::new being a reserved function (private in the public scope). -- I guess i'm not sure where else to put it either, but these are thoughts I've had.

[ltratt]

I'm also not totally sure how this would best work, though we could even be brutally simple and say "comments can only appear at the start of a line" which would be very easy to implement and for users to understand.

[ratmice]

Yeah, I'm not too terribly worried about that, I think it shouldn't be too difficult to get that right in one shape or form or another.

I guess the thing I worry about with arbitrary comment prefixes is this comment in the unescape code.
Where when we see an escaped character in a regex, we have to decide whether to pass the escaped character through unmodified to regex, e.g. in the case of \* 'asterisk'. Otherwise it could be the case where a character is meaniningful to lex itself, and lex then removes the escape, before passing it to regex.

That latter case, is something like < 'Angle_brackets_not_a_start_state', that is the escaping of \< is required to avoid the character appearing as the beginning of a start state as in <INITIAL>[a-z] 'lowercase'

I haven't thought about it too much, but the impression I get is that we should exclude characters like < from being a valid comment prefix, but we must also exclude characters like * otherwise it becomes ambiguous whether the intent to pass the escape through to regex or unescape it.

That is I think making it arbitrary does come with some complexity that is avoided by hard coding, but I think the set of the < character and is_meta_character as not valid within comment prefix avoids gives us enough information to just always remove escapes from the first character of the comment_prefix before passing to regex.

[ltratt]

Point taken. So maybe we do just want to allow an option allow_wholeline_comments (I don't think we need true there) and that means "you can now use // ... at the beginning of a line as a comment marker"?

[ratmice]

Yeah, that is I think that would be the lowest complexity approach to take, and I'm fine with omitting the true, I had hesitated whether to include it or not in my earlier comment.

Edit: It may/may not be reasonable to extend it after rules like a 'a' // the a character where the comment comes at the end of a rule, I can't remember exactly how the rule parsing code works off the top of my head. I'd hope to allow it if it's reasonably implementable.