Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error handling during process!-matchers #51

Closed
steffengy opened this issue Jun 10, 2016 · 12 comments
Closed

Error handling during process!-matchers #51

steffengy opened this issue Jun 10, 2016 · 12 comments

Comments

@steffengy
Copy link
Contributor

Are there any plans to introduce some syntax-sugar for error handling
during process!-matchers, to make error handling easier?

The only way I currently see is if for example every matcher returns a Result<A, B>,
that you'd always have to do something like:

_xyz(&self) -> Result<A, B> {
    (&x: y, z_result: _matcher_z()) => {
        let z = try!(z_result);
        Ok(...)
   }
}

Some possible syntax sugar (without checking if it can be implemented):

_xyz(&self) -> Result<A, B> {
    (&x: y, z?: _matcher_z()) => {
        Ok(...)
   }
}

Is there any better method to do it and what are your plans in this area as well as
for error handling generally?

@dragostis
Copy link
Contributor

I was thinking about exactly the same way of implementing error handling, but I'm not completely sold on the idea of adding extra syntax sugar for this particular case.

Passing the Err everywhere might not be so great, though, so I was thinking about some sort lf design that works with Result<A, B> under the hood. The problem with this idea is that I can't currently think of a way for the macro to understand whether you're trying to return an A or B.

So, right now, the try! approach seems to be the best solution and probably the one that sticks closest to the Rust philosophy. Adding the ? syntax sugar helps, but it also has the precondition of calling matcher to also be a Result<_, some sort of B>, which might make it harder for people new to Rust to understand.

@steffengy
Copy link
Contributor Author

steffengy commented Jun 11, 2016

When I try to use the try!-approach similar to how I described it,
I currently get loads of

src\lib.rs:42:33: 42:43 note: in this expansion of try! (defined in <std macros>)
<pest macros>:85:1: 85:65 note: in this expansion of process! (defined in <pest macros>)
<pest macros>:85:1: 85:65 note: in this expansion of process! (defined in <pest macros>)
<pest macros>:113:26: 114:56 note: in this expansion of process! (defined in <pest macros>)
<pest macros>:124:1: 124:50 note: in this expansion of process! (defined in <pest macros>)
src\lib.rs:39:12: 67:27 note: in this expansion of process! (defined in <pest macros>)
src\lib.rs:17:1: 103:2 note: in this expansion of impl_rdp! (defined in <pest macros>)
<std macros>:5:8: 6:45 help: run `rustc --explain E0308` to see a detailed explanation
<std macros>:5:8: 6:45 note: expected type `(std::result::Result<T, ()>, usize)`
<std macros>:5:8: 6:45 note:    found type `std::result::Result<_, _>`
<std macros>:5:8: 6:45 error: mismatched types [E0308]
<std macros>:5 return $ crate :: result :: Result :: Err (

Any idea what might be going wrong there?

Example:

        main(&self) -> Result<Expr, ParseError> {
            (z: main_z()) => {
                let g = try!(z);
                Ok(g)
            }
        }

        main_z(&self) -> Result<Expr, ParseError> {
            () => Ok(Expr::None)
        }

@dragostis
Copy link
Contributor

Yes. I've just realized that the usual kind of try! won't work here because of the way process! is defined.

One solution would be to replace try! with an internal process! call behind the scenes, or to add some syntax sugar for it. What do you think?

@steffengy
Copy link
Contributor Author

I cannot think of an appropriate syntax suggar right now,
so how would that process!-call solution work exactly?

(Maybe creating an alias like process_try!( would make sense to make it more explicit on why it's used in a specific grammar)

Is that something that's doable at the moment or would both solutions require some work?

@dragostis
Copy link
Contributor

dragostis commented Jun 11, 2016

Syntax sugar would be easy to implement. I was thinking maybe adding the try! inside of the match?

_xyz(&self) -> Result<A, B> {
    (&x: y, z_result: try!(_matcher_z())) => {
        Ok(...)
   }
}

@steffengy
Copy link
Contributor Author

steffengy commented Jun 11, 2016

That would be cool, but would that work for "internal expressions" for example

if you wanted to do: try!(u32::from_str_radix(...)) within the matcher?
From my understanding this would fail, since that would require a return,
which then mismatches in terms of data types?

@dragostis
Copy link
Contributor

dragostis commented Jun 11, 2016

I was thinking about this, but it would definitely make everything really complicated behind the scenes, apart from overcomplicating the patterns. I think simple calls are enough.

EDIT: we can discuss more on Gitter.

@steffengy
Copy link
Contributor Author

steffengy commented Jun 11, 2016

so how would you then convert a hex-number in string from into an integer for e.g. AST-building (<T>.from_str_radix)?
you'll also need some kind of processing if you want to collect multiple chars (as e.g. bytes)
and then build a string using String::from_utf8.

Using .unwrap in all these cases seems really not ideal, or is there another pattern you suggest for such cases?

@dragostis
Copy link
Contributor

Good point. I'll open up a separate issue. try! needs to work.

@dragostis
Copy link
Contributor

@steffengy I've fixed the issue. Here's an example of how it works now.

@steffengy
Copy link
Contributor Author

works great so far 👍

@dragostis
Copy link
Contributor

I'm closing this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants