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

compiler panic impl Trait #43021

Closed
m4b opened this Issue Jul 3, 2017 · 15 comments

Comments

Projects
None yet
6 participants
@m4b
Copy link
Contributor

m4b commented Jul 3, 2017

Trying to Compile

fn derp (cfg: &ControlFlowGraph) -> impl Iterator<Item = &ControlFlowTarget> {
    cfg.vertices().filter_map(|vx| cfg.vertex_label(vx))
}

Panic

-*- mode: compilation; default-directory: "~/projects/panopticon/core/src/" -*-
Compilation started at Sun Jul  2 21:45:03

cargo check
   Compiling panopticon-core v0.16.0 (file:///home/m4b/projects/panopticon/core)
error: internal compiler error: /checkout/src/librustc_typeck/check/mod.rs:614: escaping regions in predicate Obligation(predicate=Binder(ProjectionPredicate(ProjectionTy { trait_ref: <_ as std::iter::Iterator>, item_def_id: DefId { krate: CrateNum(2), node: DefIndex(1612) => core/640b678::iter[0]::iterator[0]::Iterator[0]::Item[0] } }, &function::ControlFlowTarget)),depth=0)
  --> function.rs:93:37
   |
93 | fn derp (cfg: &ControlFlowGraph) -> impl Iterator<Item = &ControlFlowTarget> {
   |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.20.0-nightly (69c65d296 2017-06-28) running on x86_64-unknown-linux-gnu

thread 'rustc' panicked at 'Box<Any>', /checkout/src/librustc_errors/lib.rs:426
note: Run with `RUST_BACKTRACE=1` for a backtrace.

error: Could not compile `panopticon-core`.

To learn more, run the command again with --verbose.

Compilation exited abnormally with code 101 at Sun Jul  2 21:45:05
@PlasmaPower

This comment has been minimized.

Copy link
Contributor

PlasmaPower commented Jul 3, 2017

Minimal code to reproduce (that should compile and run fine edit: maybe it shouldn't, since implicit lifetimes in impl Trait appear to be not yet allowed):

struct Example {}

fn example(input: &Example) -> impl Iterator<Item = &Example> {
    ::std::iter::once(input)
}

Edit: debug log and backtrace: https://gist.github.com/PlasmaPower/7f1ffb636aa5d17119e639aaca7dc9eb

It appears to be a bug with lifetime inference, as this works:

struct Example {}

fn example<'a>(input: &'a Example) -> impl Iterator<Item = &'a Example> {
    ::std::iter::once(input)
}

hawkw added a commit to BuoyantIO/flossy that referenced this issue Jul 3, 2017

Start on test running (this causes an ICE)
This code is currently causing the compiler to panic
(rust-lang/rust#43021); I'm commiting the full snapshot in the hopes
that it'll aid in debugging the issue.
@hawkw

This comment has been minimized.

Copy link

hawkw commented Jul 3, 2017

I'm having the same issue, trying to compile the following code using the futures library:

fn run<T>(socket: TcpStreamNew, test: &T)
         -> impl Future<Item=Status, Error=Error>
where T: Test {
    let request = socket.and_then(move |socket|
        io::write_all(socket, T::request().as_bytes()));

    let response = request
        .and_then(|(socket, _req)| io::read_to_end(socket, Vec::new()) )
        .and_then(|(_socket, bytes)| {
            let mut headers = [EMPTY_HEADER; MAX_HEADERS];
            let mut response = Response::new(&mut headers);
            future::result(
                response.parse(&bytes)
                    .map(|_| response)
                    .map_err(|e| Error::new(ErrorKind::Other, e)))
        });
    let status = response.map(T::check);
    status
}

Backtrace: https://gist.github.com/hawkw/f69c536918266225c70402458ad867f2

@PlasmaPower

This comment has been minimized.

Copy link
Contributor

PlasmaPower commented Jul 3, 2017

@hawkw I'm assuming either Status or Error is a typedef to a borrowed type? If you explicitly specify the borrow lifetimes, then it shouldn't ICE (as a temporary fix).

@m4b

This comment has been minimized.

Copy link
Contributor Author

m4b commented Jul 3, 2017

@PlasmaPower Interesting, I tried your example, and it doesn't ICE, but it won't compile with:

fn derp<'a> (cfg: &'a ControlFlowGraph) -> impl Iterator<Item = &'a ControlFlowTarget> {
    cfg.vertices().filter_map(|vx| cfg.vertex_label(vx))
}
cargo check
   Compiling panopticon-core v0.16.0 (file:///home/m4b/projects/panopticon/core)
error[E0564]: only named lifetimes are allowed in `impl Trait`, but `` was found in the type `std::iter::FilterMap<std::iter::Map<std::collections::hash_map::Keys<'_, panopticon_graph_algos::adjacency_list::AdjacencyListVertexDescriptor, function::ControlFlowTarget>, fn(&panopticon_graph_algos::adjacency_list::AdjacencyListVertexDescriptor) -> panopticon_graph_algos::adjacency_list::AdjacencyListVertexDescriptor>, [closure@function.rs:448:31: 448:56 cfg:&&'a panopticon_graph_algos::AdjacencyList<function::ControlFlowTarget, il::Guard>]>`
   --> function.rs:447:44
    |
447 | fn derp<'a> (cfg: &'a ControlFlowGraph) -> impl Iterator<Item = &'a ControlFlowTarget> {
    |                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0564]: only named lifetimes are allowed in `impl Trait`, but `` was found in the type `std::iter::FilterMap<std::iter::Map<std::collections::hash_map::Keys<'_, panopticon_graph_algos::adjacency_list::AdjacencyListVertexDescriptor, function::ControlFlowTarget>, fn(&panopticon_graph_algos::adjacency_list::AdjacencyListVertexDescriptor) -> panopticon_graph_algos::adjacency_list::AdjacencyListVertexDescriptor>, [closure@function.rs:448:31: 448:56 cfg:&&'a panopticon_graph_algos::AdjacencyList<function::ControlFlowTarget, il::Guard>]>`
   --> function.rs:447:44
    |
447 | fn derp<'a> (cfg: &'a ControlFlowGraph) -> impl Iterator<Item = &'a ControlFlowTarget> {
    |                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

but this may be an unrelated error, haven't thought too much about it, though the error message doesn't really make sense to me

@m4b

This comment has been minimized.

Copy link
Contributor Author

m4b commented Jul 3, 2017

Like this just seems still symptomatic of the bug:

only named lifetimes are allowed in `impl Trait`, but ``

the double tick after but has nothing in it...

Note: ControlFlowGraph is a typedef, and ControlFlowtarget is a regular enum (with no typedefs)

@PlasmaPower

This comment has been minimized.

Copy link
Contributor

PlasmaPower commented Jul 3, 2017

@m4b Do ControlFlow or ControlFlowtarget include borrowing or references in their definitions? There may be additional implicit borrowing.

@m4b

This comment has been minimized.

Copy link
Contributor Author

m4b commented Jul 3, 2017

ControlFlowTarget has a Cow.

/// Node of the function graph.
#[derive(Serialize,Deserialize,Debug,Clone)]
pub enum ControlFlowTarget {
    /// A basic block
    Resolved(BasicBlock),
    /// An unresolved indirect jump
    Unresolved(Rvalue),
    /// An error occured while disassembling
    Failed(u64, Cow<'static, str>),
}

/// Graph of basic blocks and jumps
pub type ControlFlowGraph = AdjacencyList<ControlFlowTarget, Guard>;
#[derive(Clone, Debug, Serialize,Deserialize)]
pub struct AdjacencyList<N, E> {
    vertex_labels: HashMap<AdjacencyListVertexDescriptor, N>,
    edge_labels: HashMap<AdjacencyListEdgeDescriptor, E>,
    out_edges: HashMap<AdjacencyListVertexDescriptor, Vec<AdjacencyListEdgeDescriptor>>,
    in_edges: HashMap<AdjacencyListVertexDescriptor, Vec<AdjacencyListEdgeDescriptor>>,
    edges: HashMap<AdjacencyListEdgeDescriptor, (AdjacencyListVertexDescriptor, AdjacencyListVertexDescriptor)>,
    next_edge: AdjacencyListEdgeDescriptor,
    next_vertex: AdjacencyListVertexDescriptor,
}
@PlasmaPower

This comment has been minimized.

Copy link
Contributor

PlasmaPower commented Jul 3, 2017

Interesting, I think that should compile (but it fails, probably due to the 'static). It would appear that lifetime handling with impl Trait has a number of problems.

@PlasmaPower

This comment has been minimized.

Copy link
Contributor

PlasmaPower commented Jul 3, 2017

Oh wait, nevermind. The error message is deceiving, but here's the solution:

fn derp<'a> (cfg: &'a ControlFlowGraph) -> impl Iterator<Item = &'a ControlFlowTarget> {
    cfg.vertices().filter_map(move |vx| cfg.vertex_label(vx))
}

It needs to be a move closure to work.

Edit: to clarify, it's not moving ControlFlowGraph, it's moving &'a ControlFlowGraph (or at least I think so).

@m4b

This comment has been minimized.

Copy link
Contributor Author

m4b commented Jul 3, 2017

Ah yea of course! The error message definitely is totally wrong though, hehe :)

There's still some work to do for lifetimes and this feature I think

EDIT:

@PlasmaPower actually does not even work with the (correct) move annotation, same empty `` message:

fn derp<'a> (cfg: &'a ControlFlowGraph) -> impl Iterator<Item = &'a ControlFlowTarget> {
    cfg.vertices().filter_map(move |vx| cfg.vertex_label(vx))
}
   Compiling panopticon-core v0.16.0 (file:///home/m4b/projects/panopticon/core)
error[E0564]: only named lifetimes are allowed in `impl Trait`, but `` was found in the type `std::iter::FilterMap<std::iter::Map<std::collections::hash_map::Keys<'_, panopticon_graph_algos::adjacency_list::AdjacencyListVertexDescriptor, function::ControlFlowTarget>, fn(&panopticon_graph_algos::adjacency_list::AdjacencyListVertexDescriptor) -> panopticon_graph_algos::adjacency_list::AdjacencyListVertexDescriptor>, [closure@function.rs:448:31: 448:61 cfg:&'a panopticon_graph_algos::AdjacencyList<function::ControlFlowTarget, il::Guard>]>`
   --> function.rs:447:44
    |
447 | fn derp<'a> (cfg: &'a ControlFlowGraph) -> impl Iterator<Item = &'a ControlFlowTarget> {
    |                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@PlasmaPower

This comment has been minimized.

Copy link
Contributor

PlasmaPower commented Jul 3, 2017

We could name the implicit lifetime as the closure, instead of just ``. Past that, we couldn't make the error message better without resolving implicit lifetimes for impl Trait, which would ironically obsolete the error in the first place.

We could add in a help: note specifically for closures, though.

@m4b

This comment has been minimized.

Copy link
Contributor Author

m4b commented Jul 3, 2017

Yea the error message needs to be improved. Ftr, this will compile:

fn derp<'a> (cfg: &'a ControlFlowGraph) -> impl Iterator<Item = &'a ControlFlowTarget> + 'a {
    cfg.vertices().filter_map(move |vx| cfg.vertex_label(vx))
}

(note the additional 'a requirement).

This is exactly the same definition as the Boxed version I wrote, because I didn't want to write the type, so this is a perfect usecase for impl Trait.

Consequently, I think this is as @PlasmaPower you suggest, a problem with lifetime inference.

However, @hawkw example is a bit puzzling, because, afaik, you can't typedef a ref unless the lifetime is also exposed in the typedef, or it's 'static - so this could be combination of lifetimes + typedefs + static?

EDIT

removing the 'a on the boxed version, I think the error message for impl Trait should be mutatis mutandis basically the same:

   Compiling panopticon-core v0.16.0 (file:///home/m4b/projects/panopticon/core)
error[E0495]: cannot infer an appropriate lifetime for autoref due to conflicting requirements
  --> function.rs:92:43
   |
92 |         FunctionIter { iter: Box::new(cfg.vertices().filter_map(move |vx| cfg.vertex_label(vx))) }
   |                                           ^^^^^^^^
   |
note: first, the lifetime cannot outlive the lifetime 'a as defined on the impl at 89:1...
  --> function.rs:89:1
   |
89 | / impl<'a> FunctionIter<'a> {
90 | |     /// 
91 | |     pub fn new(cfg: &'a ControlFlowGraph) -> Self {
92 | |         FunctionIter { iter: Box::new(cfg.vertices().filter_map(move |vx| cfg.vertex_label(vx))) }
93 | |     }
94 | | }
   | |_^
note: ...so that reference does not outlive borrowed content
  --> function.rs:92:39
   |
92 |         FunctionIter { iter: Box::new(cfg.vertices().filter_map(move |vx| cfg.vertex_label(vx))) }
   |                                       ^^^
   = note: but, the lifetime must be valid for the static lifetime...
note: ...so that expression is assignable (expected std::boxed::Box<std::iter::Iterator<Item=&function::ControlFlowTarget> + 'static>, found std::boxed::Box<std::iter::Iterator<Item=&function::ControlFlowTarget>>)
@PlasmaPower

This comment has been minimized.

Copy link
Contributor

PlasmaPower commented Jul 3, 2017

I think the error message for impl Trait should be mutatis mutandis basically the same

As I said earlier,

Past that, we couldn't make the error message better without resolving implicit lifetimes for impl Trait, which would ironically obsolete the error in the first place

The best we can do now is to identify the currently unnamed lifetime. The compiler doesn't currently know the constraints on the lifetime, so it can't output a message similar to the one above (generated when using a box).

@PlasmaPower

This comment has been minimized.

Copy link
Contributor

PlasmaPower commented Jul 4, 2017

Interesting. If run with RUST_LOG=debug, the compiler gives a different error:

error[E0391]: unsupported cyclic reference between types/traits detected
 --> impl-trait-lifetime-inference.rs:8:32
  |
8 | fn example(input: &Example) -> impl Iterator<Item = &Example> {
  |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cyclic reference
  |
note: the cycle begins when processing `example::{{impl-Trait}}`...
 --> impl-trait-lifetime-inference.rs:8:32
  |
8 | fn example(input: &Example) -> impl Iterator<Item = &Example> {
  |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  = note: ...which then again requires processing `example::{{impl-Trait}}`, completing the cycle.

Edit: requires rustc to be built with debugging information.

@pietroalbini

This comment has been minimized.

Copy link
Member

pietroalbini commented Apr 3, 2018

This seems to be fixed now (the snippet in #43021 (comment) works fine on the latest nightly). Closing.

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