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

Parser silent exit on invalid input #63288

Closed
fkohlgrueber opened this issue Aug 5, 2019 · 4 comments
Closed

Parser silent exit on invalid input #63288

fkohlgrueber opened this issue Aug 5, 2019 · 4 comments
Labels
A-parser Area: The parsing of Rust source code to an AST

Comments

@fkohlgrueber
Copy link
Contributor

I'm using rustc's parser (parse_crate_mod more specifically) as part of a program I'm writing and for some inputs, the parser terminates my program.

Using the current nightly and fn test(a as the input, the following code terminates without printing the parsing result:

#![feature(rustc_private)]

extern crate syntax;
use std::rc::Rc;
use syntax::source_map::{FilePathMapping, SourceMap};
use syntax::parse::ParseSess;
use syntax::errors::emitter::ColorConfig;
use syntax::errors::Handler;

fn main() {
    let text = "fn test() ->".to_string();  // produces Err result
    let text = "fn test(a".to_string();     // exits silently

    syntax::with_globals(syntax::source_map::edition::Edition::Edition2018, || {
        let source_map = Rc::new(SourceMap::new(FilePathMapping::empty()));
        let parse_session = ParseSess::with_span_handler(
            Handler::with_tty_emitter(ColorConfig::Auto, true, None, Some(source_map.clone())),
            source_map
        );
        let parser = syntax::parse::maybe_new_parser_from_source_str(
            &parse_session,
            syntax::source_map::FileName::Custom("stdin".to_owned()),
            text,
        );

        println!("{:?}", parser.unwrap().parse_crate_mod().map_err(|mut x| {x.cancel(); x}));
    })
}

Full test repository

Output:

felix@the-machine:~/tmp/rust_parser_test$ cargo run
   Compiling rust_parser_test v0.1.0 (/home/felix/tmp/rust_parser_test)
    Finished dev [unoptimized + debuginfo] target(s) in 1.89s
     Running `target/debug/rust_parser_test`
error: this file contains an un-closed delimiter
 --> <stdin>:1:10
  |
1 | fn test(a
  |        - ^
  |        |
  |        un-closed delimiter

error: expected one of `:` or `@`, found `)`
 --> <stdin>:1:9
  |
1 | fn test(a
  |         ^ expected one of `:` or `@` here
  |
  = note: anonymous parameters are removed in the 2018 edition (see RFC 1685)
help: if this was a parameter name, give it a type
  |
1 | fn test(a: TypeName
  |         ^^^^^^^^^^^
help: if this is a type, explicitly ignore the parameter name
  |
1 | fn test(_: a
  |         ^^^^

felix@the-machine:~/tmp/rust_parser_test$ 

For other invalid code (e.g. fn test() ->), the parser correctly returns an Err result:

felix@the-machine:~/tmp/rust_parser_test$ cargo run
   Compiling rust_parser_test v0.1.0 (/home/felix/tmp/rust_parser_test)
    Finished dev [unoptimized + debuginfo] target(s) in 0.96s
     Running `target/debug/rust_parser_test`
Err(Diagnostic { level: Cancelled, message: [("expected type, found `<eof>`", NoStyle)], code: None, span: MultiSpan { primary_spans: [Span { lo: BytePos(10), hi: BytePos(12), ctxt: #0 }], span_labels: [(Span { lo: BytePos(10), hi: BytePos(12), ctxt: #0 }, "expected type")] }, children: [], suggestions: [] })
felix@the-machine:~/tmp/rust_parser_test$ 

Is this a bug or am I missing something?

Thanks in advance.

@estebank
Copy link
Contributor

estebank commented Aug 5, 2019

This is caused by the parser's error recovery mechanisms. When encountering these kind of situation, errors are always emitted (as shown above being printed out) if recoverable or returned. If you are trying to use the parser in a strict mode, that should be added as a config flag to it. There are also invocations to FailError.raise() which causes the process to abort, which might be what you're seeing.

@estebank estebank added the A-parser Area: The parsing of Rust source code to an AST label Aug 5, 2019
@fkohlgrueber
Copy link
Contributor Author

@estebank thanks for your reply. I took a closer look at the parser and it raises a FatalError (line 599) for my input. What I don't understand is why some parse errors are emitted, some are returned as an Err variant of the PResult struct and some panic. Can you explain the difference or can you provide links where I can read about this topic? Thanks in advance!

@estebank
Copy link
Contributor

estebank commented Aug 7, 2019

  • emitted: there are some errors that are recoverable, like if you have fn foo -> {} you can infer that the return type was supposed to be there but isn't, so the compiler would internally emit the error, accept it as correct and keep an internal representation close to Item::Fn { name: Ident(foo), return_type: Error }
  • returned: Err is returned when we can't reliably recover from a parse error, it is and should be the default, and I guess it doesn't need too much explanation :)
  • FatalError: this is raised when there's no way the Parser will be able to return a workable AST, things are beyond any possible solution other than stopping the compiler. This used to be pervasive throughout the compiler to stop it from going from one step to the next and avoid having support for malformed ASTs during MIR generation or typechecking. We have spend quite a bit of effort to remove these choke points from the codebase, but progress is slow because it's very easy to introduce problems when doing so carelessly. We should eventually (🤞) be able to remove these.

Part of the problem you're facing is that libsyntax is written with only one customer in mind really: rustc. As I mentioned in #63284, an option would be to bifurcate the Parser's behavior between the current "best-effort recovery" mode and "strict" mode, that would suit 3rd party apps better.

@Mark-Simulacrum
Copy link
Member

We should probably be clear that libsyntax is written and maintained essentially for just rustc; it is not really a general use Rust parser.

Also, discussions/questions like this should happen either on chat (e.g. rust-lang.zulipchat.com) or internals, the issue tracker is for bugs only. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parser Area: The parsing of Rust source code to an AST
Projects
None yet
Development

No branches or pull requests

3 participants