Rust: Rustdoc cannot infer type when using type_alias_impl_trait, while rustc works

[bjorn3]

#![feature(type_alias_impl_trait)]

pub trait Backend {}

impl Backend for () {}

pub struct Module<T>(T);

pub type BackendImpl = impl Backend;

pub fn make_module() -> Module<BackendImpl> {
    Module(())
}

$ rustc -V
rustc 1.38.0 (625451e37 2019-09-23)
$ cargo doc
error[E0282]: type annotations needed
  --> src/main.rs:13:1
   |
13 | pub type BackendImpl = impl Backend<Product: std::any::Any>;
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type

error: aborting due to previous error
$ cargo build

[bjorn3]

@rustbot modify labels: +requires-nightly +F-type_alias_impl_trait

[GuillaumeGomez]

That is... interesting.

[bjorn3]

Forgot to add T-rustdoc :)

[Aaron1011]

This looks like an issue in should_ignore_fn, which is used by Rustdoc to determine which functions should have their bodies replaced by loops.

This check is done on the AST. I think what's happening is that we detect BackendImpl (since it appears as a generic argument to Module), but are unable to determine that BackendImpl is an opaque type (since we haven't resolved paths yet).

I'm not familiar enough with the various phases of the compiler to know if it would be possible to do this check later, once we have ty::Tys available for the function signature (e.g. resolved types).

We may have to largely give up on this check, and actually compile the bodies of most functions. That could mean a fairly large regression in doc build times - but if we can't come up with a way to detect which functions might constrain opaque types, I don't think we have another option.

[Aaron1011]

Actually, we might be able to check for the presence any impl Trait definitions within a parent scope of the function. This would have a large number of false positives (any impl Traits at the crate root would cause every function to be flagged as 'may define an opaque type'), but it would be better than nothing.

[clarfonthey]

Also ran into this bug, came up with another example:

#![feature(type_alias_impl_trait)]
use core::iter::empty;
type Test = impl Iterator<Item = ()>;
fn test() -> Test {
    empty()
}

Rustdoc error:

error[E0277]: `()` is not an iterator
 --> src/lib.rs:3:1
  |
3 | type Test = impl Iterator<Item = ()>;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `()` is not an iterator
  |
  = help: the trait `std::iter::Iterator` is not implemented for `()`
  = note: the return type of a function must have a statically known size

[GuillaumeGomez]

@clarfon Nice one, thanks!

@ollie27 Didn't you fixed something close to this recently? (I might be wrong though...)

[ollie27]

@ollie27 Didn't you fixed something close to this recently? (I might be wrong though...)

I don't think so, no.

[jonhoo]

Here's another instance of this bug:

#![feature(type_alias_impl_trait)]
use std::future::Future;

trait Foo {
    type X: Future<Output = ()>;
    fn x() -> Self::X;
}

impl Foo for () {
    type X = impl Future<Output = ()>;
    fn x() -> Self::X {
        async {}
    }
}

[jonhoo]

As an interesting aside, when trying to work around this using Box<dyn Trait> under #[cfg(doc)], I kept running into this unimplemented! in librustdoc.

[jonhoo]

@Aaron1011 did you start the work of adding a check for the presence of impl Trait definitions within the parent scope, or were you just musing? As crates start adopting impl Trait existentials, this is probably going to become a pretty important use-case to support.

[Aaron1011]

@jonhoo: I haven't done any work on this issue as of yet.

[jonhoo]

A brief update — I asked the nice people over at docs.rs to run some queries over their failed build logs, and here are crates where this issue shows up:

cratesfyi=> select name, version, releases.id from builds inner join releases on releases.id = builds.rid inner join crates on crates.id = releases.crate_id where output like '%error[E0282]: type annotations needed%';
        name         | version |   id   
---------------------+---------+--------
 small-logger        | 0.2.1   |  17558
 diesel_infer_schema | 0.13.0  |  49451
 diesel_infer_schema | 0.13.0  |  49451
 diesel_infer_schema | 0.12.0  |  44145
 searchspot          | 0.15.2  |  62618
 diesel_infer_schema | 0.14.0  |  54552
 diesel_infer_schema | 0.16.0  |  59094
 searchspot          | 0.14.0  |  59745
 diesel_infer_schema | 0.15.0  |  56265
 searchspot          | 0.15.0  |  62500
 searchspot          | 0.15.1  |  62603
 ipfsapi             | 0.1.6   |  68582
 searchspot          | 0.15.4  |  63062
 ipfsapi             | 0.1.3   |  64146
 ipfsapi             | 0.1.5   |  64240
 ipfsapi             | 0.1.0   |  63153
 ipfsapi             | 0.1.1   |  63479
 ipfsapi             | 0.1.2   |  64075
 ipfsapi             | 0.1.4   |  64157
 pom                 | 2.0.0   |  64739
 ipfsapi             | 0.1.7   |  68726
 fantoccini          | 0.8.1   |  77851
 fantoccini          | 0.8.0   |  71985
 searchspot          | 0.16.0  |  72539
 semverver           | 0.1.0   |  74641
 semverver           | 0.1.1   |  75472
 swc_ecma_parser     | 0.8.0   | 123132
 infer_schema_macros | 1.4.0   | 124335
 infer_schema_macros | 1.4.0   | 124335
 amadeus-parquet     | 0.1.3   | 176821
 amadeus-parquet     | 0.1.4   | 180078
 hcs-rs              | 0.2.1   | 188689
 hcs-rs              | 0.2.0   | 188669
(33 rows)

cratesfyi=> select name, version, releases.id from builds inner join releases on releases.id = builds.rid inner join crates on crates.id = releases.crate_id where output like '%is not implemented for `()`%';
         name          |    version    |   id   
-----------------------+---------------+--------
 reducer               | 1.1.0         | 125897
 amadeus-parquet       | 0.1.1         | 169206
 amadeus-postgres      | 0.1.3         | 176820
 amadeus-parquet       | 0.1.2         | 169266
 yukikaze              | 1.0.0-alpha.5 | 169612
 amadeus-parquet       | 0.1.3         | 176821
 amadeus-commoncrawl   | 0.1.3         | 176822
 amadeus-serde         | 0.1.3         | 176824
 amadeus-aws           | 0.1.3         | 176826
 vicuna                | 0.1.2         | 189614
 amadeus-postgres      | 0.1.4         | 180077
 amadeus-parquet       | 0.1.4         | 180078
 amadeus-commoncrawl   | 0.1.4         | 180079
 amadeus-serde         | 0.1.4         | 180080
 amadeus-aws           | 0.1.4         | 180081
 vicuna                | 0.1.1         | 189190
 amadeus-aws           | 0.1.5         | 189240
 amadeus-parquet       | 0.1.5         | 189242
 nu                    | 0.6.1         | 186499
 vicuna                | 0.1.0         | 189185
 amadeus-serde         | 0.1.5         | 189233
 amadeus-postgres      | 0.1.5         | 189235
 amadeus-commoncrawl   | 0.1.5         | 189237
 vicuna                | 0.1.3         | 189615
 xtra                  | 0.2.1         | 199674
 wasm-reader           | 0.1.0         | 198978
 amadeus-parquet       | 0.1.6         | 193727
 wasm-reader           | 0.1.1         | 200658
 cpp_to_rust_generator | 0.0.0         |  38434
 noria                 | 0.4.0         | 206426
 noria-server          | 0.4.0         | 206463
 noria                 | 0.4.1         | 208596
 wasm-reader           | 0.2.0         | 207153
(33 rows)

So looks like this isn't _too_ widespread yet, but it's clearly something that "real" crates are running into :)

[jonhoo]

For those watching, when https://github.com/rust-lang/rust/pull/72080 (which fixes https://github.com/rust-lang/rust/issues/73061) lands, the workaround for this is basically:

#![feature(type_alias_impl_trait)]
use core::iter::empty;

#[cfg(not(doc))]
type Test = impl Iterator<Item = ()>;

#[cfg(doc)]
type Test = MockIterator<()>;
#[cfg(doc)]
struct MockIterator<T>(std::marker::PhantomType<T>);
#[cfg(doc)]
impl<T> Iterator for MockIterator<T> {
  type Item = T;
  fn next(&mut self) -> Self::Item { unreachable!() }
}

fn test() -> Test {
    empty()
}

It ain't pretty, but it'll make things work. You can even potentially re-use the mocker for multiple traits that you're using behind impl. See https://github.com/mit-pdos/noria/commit/062b880809c155e10bdb9641414b30137e012400 for a larger example of this change. H/T to @alecmocatta for suggesting this workaround in https://github.com/constellation-rs/amadeus/issues/54.

[jyn514]

I expect this to be fixed by https://github.com/rust-lang/rust/pull/73566, I'll add a test case there.