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

rustc --pretty=expanded outputs invalid syntax for #[derive(RustcDecodable)] #42213

Closed
eqrion opened this issue May 25, 2017 · 2 comments
Closed
Labels
A-pretty Area: Pretty printing (including `-Z unpretty`) C-bug Category: This is a bug.

Comments

@eqrion
Copy link
Contributor

eqrion commented May 25, 2017

Run

  1. cargo rustc -- -Z unstable-options --pretty=expanded > lib-expanded.rs
  2. rustc -Z parse-only lib-expanded.rs

With

// src/lib.rs
extern crate rustc_serialize;

#[derive(RustcDecodable, RustcEncodable)]
struct Foo;
$ rustc --version
rustc 1.19.0-nightly (5b13bff52 2017-05-23)

Results

error: expected `{`, found `::`
  --> src/lib-expanded.rs:16:39
   |
16 |                             |_d| -> _ ::std::result::Result::Ok(Foo))
   |                                       ^^

error: expected one of `.`, `;`, `?`, `}`, or an operator, found `::`
  --> src/lib-expanded.rs:16:39
   |
16 |                             |_d| -> _ ::std::result::Result::Ok(Foo))
   |                                      -^^ unexpected token
   |                                      |
   |                                      expected one of `.`, `;`, `?`, `}`, or an operator here

error: aborting due to 2 previous errors

lib-expanded.rs:

#![feature(prelude_import)]
#![no_std]
#[prelude_import]
use std::prelude::v1::*;
#[macro_use]
extern crate std as std;
extern crate rustc_serialize;

struct Foo;
#[automatically_derived]
#[allow(unused_qualifications)]
impl ::rustc_serialize::Decodable for Foo {
    fn decode<__D: ::rustc_serialize::Decoder>(__arg_0: &mut __D)
     -> ::std::result::Result<Foo, __D::Error> {
        __arg_0.read_struct("Foo", 0usize,
                            |_d| -> _ ::std::result::Result::Ok(Foo))
    }
}
#[automatically_derived]
#[allow(unused_qualifications)]
impl ::rustc_serialize::Encodable for Foo {
    fn encode<__S: ::rustc_serialize::Encoder>(&self, __arg_0: &mut __S)
     -> ::std::result::Result<(), __S::Error> {
        match *self {
            Foo =>
            __arg_0.emit_struct("Foo", 0usize,
                                |_e| -> _
                                    { return ::std::result::Result::Ok(()) }),
        }
    }
}

Expected

rustc lib-expanded.rs to succeed. Pretty printing should output valid rust syntax.

Not sure if this is specific to rustc_serialize, I haven't tried to create a simpler testcase.

@Mark-Simulacrum Mark-Simulacrum added the A-pretty Area: Pretty printing (including `-Z unpretty`) label Jun 23, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 27, 2017
@varkor
Copy link
Member

varkor commented Mar 22, 2018

The problem here, as far as I can tell, is a result of a combination of factors:

  • The generated implementation of decode() generates an expression for the body of the lambda (rather than a block, unlike encode():
    let result =
    cx.expr_ok(trait_span,
    cx.expr_match(trait_span, cx.expr_ident(trait_span, variant), arms));
  • The closure-building method, lambda, generates the correct signature for the closure, but any closures that are printed will have full return type annotations regardless of whether they are _ or not.
    let fn_decl = self.fn_decl(
    ids.iter().map(|id| self.arg(span, *id, self.ty_infer(span))).collect(),
    self.ty_infer(span));
  • pprust prints the closure signature, including return type, then the closure body immediately after. This is because it expects valid Rust syntax as input (in which this construction is impossible), but the syntax tree generated by decodable is not necessarily syntactically valid:
    self.print_fn_block_args(decl)?;
    self.s.space()?;
    self.print_expr(body)?;
    self.end()?; // need to close a box

So the solutions are:

  1. Modify decodable_substructure (or optionally lambda) to always wrap the expression in a block.
  2. Modify the lambda printing that always prints the closure return type regardless, to special case _.
  3. Make pprust handle invalid inputs better.

I'm not sure which is best here: fixing just 1 or 2 means other issues could slip by; fixing 3 seems like covering up the issue. 1 might be best for now, and if this issue re-emerges, then we can re-evaluate.

@varkor
Copy link
Member

varkor commented Mar 22, 2018

On second thoughts, it seems reasonable for pprust to assume correct input. In that case, the correct fix would be: modify the closure printing code to omit return types of TyKind::Infer (which the functions generated by lambda always return). I could imagine this having other side effects, but I can't immediately think if that would cause any other problems.

bors added a commit that referenced this issue Mar 27, 2018
…ichton

Fix implicit closure return type generation for libsyntax

The `lambda` function for constructing closures in libsyntax was explicitly setting the return type to `_`, which resulted in incorrect corresponding syntax (as `|| -> _ x` is not valid, without the enclosing brackets). This meant the generated code, when printed, was invalid.

I also took the opportunity to slightly improve the generated code for the `RustcEncodable::encode` method for unit structs.

Fixes #42213.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-pretty Area: Pretty printing (including `-Z unpretty`) C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

3 participants