FromPest targetted API

[CAD97]

Currently:

/// Potentially borrowing conversion from a pest parse tree.
pub trait FromPest<'pest>: Sized {
    /// The rule type for the parse tree this type corresponds to.
    type Rule: RuleType;
    /// A fatal error during conversion.
    type FatalError;
    /// Convert from a Pest parse tree.
    ///
    /// # Return type semantics
    ///
    /// - `Err(ConversionError::NoMatch)` => node not at head of the cursor, cursor unchanged
    /// - `Err(ConversionError::Malformed)` => fatal error; node at head of the cursor but malformed
    /// - `Ok` => success; the cursor has been updated past this node
    fn from_pest(
        pest: &mut Pairs<'pest, Self::Rule>,
    ) -> Result<Self, ConversionError<Self::FatalError>>;
}

Potential changes we might want to make:

• Make Rule a parameter to the type rather than an associated type. This enables impl<'pest> FromPest<'pest, Rule> for ForeignType (in theory at least; didn't check).
• fn from_pest(..) -> FromPestResult<Self, Self::FatalError>. Either by type alias or making a new Try type. Either way, we should reconsider what we want to be propagated with ?; fatal errors for sure, but what about failed conversions?
• The more I work with it, the more I feel like I want the possibility to transfer state through the FromPest plumbing, because I want to have pest-ast derived FromPest implementations to be able to panic more eagerly on bad trees. (Or even have a FatalError mode.) This also makes encoding of precedence climbing more obvious.
• Having the default "no conversion" match store context may help as well, but may preclude being able to meaningfully ?-propogate it?

Draft "improved" FromPest (using cheat syntax):

pub trait FromPest<'pest, Rule>: Sized
where Rule: pest::RuleType {
    type FatalError;
    type ConversionState: Default;
    fn from_pest(pest: &mut Pairs<'pest, Rule>) =>
        Self::from_pest_with_state(pest, &mut Default::default());
    fn from_pest_with_state(
        pest: &mut pest::Pairs<'pest, Rule>,
        state: &mut Self::ConversionState,
    ) -> FromPestResult<'pest, Self, Self::FatalError, Rule>;
}

pub enum FromPestResult<'pest, T, E, Rule> {
    Success(T),
    Failure {
        expected: &'static [Rule],
        found: pest::Pair<'pest>,
        breadcrumbs: Vec<pest::Pair<'pest>>,
    }
    Error(E),
}

Please point out any flaws and over-engineering of the approach. Though this definitely gives up on sizeof(return) == sizeof(Option<Self>) where FatalError = !, which is a nice property, in order to get useful failure breadcrumbs. Maybe we can do like nom does and have a feature to enable verbose failure modes?

The complexity of this points towards leaving from-pest out of tree from pest for a good deal longer. I don't expect we should try to upstream any of this in the near future; 3.x maybe, 2.y probably not.

[CAD97]

Barring any further suggestions, I'm probably going to do a large refactoring of everything in the second half of December. That will set up for this change, though doesn't require it, and will definitely benefit us either way.

Large chunks of codegen are hard to manage super well.

[CAD97]

For context, nom4's error structure:

type IResult<I, O, E = u32> = Result<(I, O), Err<I, E>>;
pub enum Err<I, E = u32> {
    Incomplete(Needed),
    Error(Context<I, E>),
    Failure(Context<I, E>),
}
pub enum Context<I, E = u32> {
    Code(I, ErrorKind<E>),
    #[cfg(verbose_errors)]
    List(Vec<(I, ErrorKind<E>)>),
}

Translating into FromPest types, that'd look something like:

type Result<'pest, Type, Fatal, Rule> =
    std::Result<Type, Error<'pest, Fatal, Rule>>;
pub enum Error<'pest, Fatal, Rule> {
    Fatal(Fatal),
    Recoverable(Recoverable<'pest, Rule>),
}
pub enum Recoverable<'pest, Rule> {
    Succinct,
    #[cfg_attr(not(verbose-errors), doc(hidden))]
    #[doc(cfg(verbose-errors))]
    Verbose(Context<'pest, Rule>, OnlyWhenVerboseErrors)
}
#[cfg(verbose-errors)]
type OnlyWhenVerboseErrors = ();
#[cfg(not(verbose-errors))]
type OnlyWhenVerboseErrors = !;
struct Context<'pest, Rule> {
    expected: &'static [Rule],
    found: pest::Pair<'pest, Rule>,
    breadcrumbs: Vec<pest::Pair<'pest, Rule>>,
}

(Generic argument names/ordering is deliberately unrelated to the OP's, as it's just arbitrary bikeshed that I don't care about at the moment.)

[drahnr]

I like the state being passed around, that can potentially allow the builder pattern to be used for rather complex structs