Make the parse module public

[andrew-d]

Ref: #22

EDIT: Apparently my second commit has a wonky commit message - it's supposed to read: "Implement Copy for Repeater"

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see CONTRIBUTING.md for more information.

[BurntSushi]

So I guess the primary concern I have with this is that the AST is really an implementation detail of regular expressions, and as such, it could change in the future.

I am personally satisfied if the module documentation contains a clear warning that clients cannot rely on the interface to be stable, but that seems kind of wonky given 1) semantic versioning and 2) Rust's current approach to stability, which is, "if it isn't stable, don't let people use it."

On the other hand, having a regex parser can be immensely useful. The parser itself is at least as complicated as the VM, so it's a good piece of code to have available for general use.

[andrew-d]

Yeah, I agree that this is not something we want to commit to stability - but, like you said, the parser is super-useful to have available for common use. I considered also marking it as #[doc(hidden)], but all that really does is make it a bit more irritating to read about.

I'm happy to either put a big warning in the module-level documentation, or just outright hide it. I'd be interested in the input of @alexcrichton or any other core-team members here.

[BurntSushi]

I'm not a fan of hiding it. I see #[doc(hidden)] as a tool to expose something that shouldn't be exposed but must be exposed. For example, a whole bunch of internals are actually public so that regex_macros can use it: https://github.com/rust-lang/regex/blob/master/src/lib.rs#L391-L421

But yes, more input is welcome!

[alexcrichton]

In terms of long-term stability I believe I'd expect an implementation detail such as this to remain private, but we're not currently providing a whole bunch of guarantees about the stability of external crates beyond "we'll follow semver". I would personally err on the side of being conservative and leaving it private for now, although I don't have a great grasp on how it might change in the future.

Basically my consideration would be that any modification to the parser which broke an API would require a new major version for the crate, which is just something to balance :)

[andrew-d]

Basically my consideration would be that any modification to the parser which broke an API would require a new major version for the crate, which is just something to balance :)

That's a fair point, yeah. I wonder if there's a sensible way to say "this module is completely free from any stability guarantees" - essentially, omit it entirely from semver. I think that for internals like the parser, that's an entirely reasonable stance to take.

[alexcrichton]

Unfortunately we don't currently have a method of enforcing that sort of guarantee, so this may end up being too much of an overcommitment for now.

[BurntSushi]

That's fair.

@andrew-d For now, I'd recommend doing a copy & paste of the parser. We can re-examine opening it up later when we're more confident that it's actually stable.