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 reporting #47

Closed
dtolnay opened this issue Oct 18, 2016 · 7 comments
Closed

Error reporting #47

dtolnay opened this issue Oct 18, 2016 · 7 comments
Milestone

Comments

@dtolnay
Copy link
Owner

dtolnay commented Oct 18, 2016

Doesn't matter for Macros 1.1, but for parse_crate it would be helpful to be more specific about where parsing failed. Possibly behind a feature gate if it affects compile time.

Some ideas about error management: nom/docs/error_management.md

@dtolnay dtolnay added this to the 0.13 milestone Jan 2, 2018
@dtolnay
Copy link
Owner Author

dtolnay commented Jan 2, 2018

Triaging as out of scope for 0.12 but a major focus for 0.13.

@dtolnay
Copy link
Owner Author

dtolnay commented Jan 5, 2018

@Geal @SergioBenitez would either of you be interested in prototyping an error story for syn, in the style of nom parser combinators or in the style of SergioBenitez@b49a4b4 Parser struct?

I intend to start with a simple subset of Rust like enums + braced structs where all types are one ident and all enum variants are unit and there are no generics.

struct S {
    a: A,
    b: B,
}

enum E {
    A,
    B,
}

I would like to see how close we can get to error messages that are comparable with rustc's without blowing up compile time.

error: expected identifier, found `{`
 --> src/main.rs:1:8
  |
1 | struct {
  |        ^

@dtolnay
Copy link
Owner Author

dtolnay commented Aug 24, 2018

I did a first pass at an error API.

@SergioBenitez and others, let me know what you think. As a bonus, I dropped nom-style parser combinator macros in favor of plain Rust code so parsers get to use natural Rust control flow. Here is an excerpt:

pub enum Item {
    Struct(ItemStruct),
    Enum(ItemEnum),
}

pub struct ItemStruct {
    pub struct_token: Token![struct],
    pub ident: Ident,
    pub brace_token: token::Brace,
    pub fields: Vec<Field>,
}

impl Parse for Item {
    fn parse(input: ParseStream) -> Result<Self> {
        let lookahead = input.lookahead1();
        if lookahead.peek(Token![struct]) {
            input.parse().map(Item::Struct)
        } else if lookahead.peek(Token![enum]) {
            input.parse().map(Item::Enum)
        } else {
            Err(lookahead.error())
        }
    }
}

impl Parse for ItemStruct {
    fn parse(input: ParseStream) -> Result<Self> {
        let content;
        Ok(ItemStruct {
            struct_token: input.parse()?,
            ident: input.parse()?,
            brace_token: braced!(content in input),
            fields: content.parse()?,
        })
    }
}

Here is the macro I have been using for testing error messages.

cargo-features = ["edition"]

[package]
name = "demo"
version = "0.0.0"
authors = ["David Tolnay <dtolnay@gmail.com>"]
edition = "2018"
publish = false

[lib]
proc-macro = true

[dependencies]
proc-macro2 = { version = "0.4", features = ["nightly"] }
syn-error-experiment = "*"
use proc_macro::TokenStream;
use syn_error_experiment::{parse_macro_input, Item};

#[proc_macro]
pub fn demo(input: TokenStream) -> TokenStream {
    let _item = parse_macro_input!(input as Item);
    TokenStream::new()
}

The error messages will get more love but are already decent.

error: expected one of ["`struct`", "`enum`"]
 --> src/main.rs:3:7
  |
3 | demo!(what S {});
  |       ^^^^
error: expected identifier
 --> src/main.rs:4:14
  |
4 | demo!(struct 3.0f32 {});
  |              ^^^^^^
error: expected `,`
 --> src/main.rs:5:23
  |
5 | demo!(struct S { a: A b: B });
  |                       ^
error: unexpected end of input, expected identifier
 --> src/main.rs:6:16
  |
6 | demo!(struct S { a: });
  |                ^^^^^^

@dtolnay dtolnay modified the milestones: 0.14, 0.15 Aug 24, 2018
@SergioBenitez
Copy link
Contributor

@dtolnay That looks fantastic! It looks like the error is being reported on the span of the last TokenTree, is that right? For instance, instead of:

error: unexpected end of input, expected identifier
 --> src/main.rs:6:16
  |
6 | demo!(struct S { a: });
  |                ^^^^^^

I would expect:

error: unexpected end of input, expected identifier
 --> src/main.rs:6:16
  |
6 | demo!(struct S { a: });
  |                   ^

But this would imply that the error reporting mechanism can reach into a TokenTree.

@dtolnay
Copy link
Owner Author

dtolnay commented Aug 24, 2018

In the third of the four error messages in my previous comment the error does point inside of a set of braces. In the fourth error message the error is triggered on the close curly brace i.e. the compiler would display this one as:

error: expected type, found `}`
 --> src/main.rs:1:15
  |
1 | struct S { a: }
  |               ^

but proc macros do not have access to spans of closing delimiters because we only get one span associated with the balanced TokenTree::Group.

We are tracking this limitation in rust-lang/rust#48187.

@mystor
Copy link
Collaborator

mystor commented Aug 27, 2018

This is really awesome, nice work!

I had a small WIP patch which just hacked error reporting on top of the existing nom-style APIs, but this is definitely a nicer & more complete solution :-)

@dtolnay
Copy link
Owner Author

dtolnay commented Sep 2, 2018

I converted all of our parsing from nom to my new parsing API. The code is in https://github.com/dtolnay/syn/tree/next and a release candidate available in https://docs.rs/syn-next/0.15.0-rc1/syn/ documented and ready to play with. I am closing this issue but let's continue the discussion in #476 if anyone has suggestions, or file them as separate issues. I will plan to release 0.15 with this stuff mid-week and would love to hear about experiences people have if anyone tries porting some other crates before then.

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

No branches or pull requests

3 participants