Fix a error suggestion of situation when using placeholder _ as return types on function signature. by surechen · Pull Request #126017 · rust-lang/rust (original) (raw)

I'm doubling checking a detail about the visitor itself; I'll circle back later after I finish with that.

Okay, so your code is pretty interesting. It works hard to try to figure out whether it can suggest the non-static lifetime.

Two main notes:

Your visitor approach does miss some cases, such as if I return the parameter only through a level of indirection -- in this situation it still suggests S<'static> when only S<'_> would work)

struct S<'a>(&'a ()); fn g(s: S<'_>) -> _ { id(s) }

fn id(t: T) -> T { t }

But I don't think there's a reasonable way for you to fix that via a visitor-based system. The "right" answer would involve getting help from the type inference system, and I don't think we can expect the type inference to infer a correct solution efficiently in all case. (I.e. I wouldn't be surprised if this is an NP-hard problem.)

So lets set that aside for a moment, and assume "its okay to fallback sometimes on suggesting 'static as a candidate lifetime even if it won't end up working in the long run."

Here's what i see as a slightly more problematic thing with your PR: I'm concerned you might end up with the compiler reporting distinct errors that, when put together, do not make sense because they come from inconsistent views of one piece of code.

Let me explain a little more carefully:

The compiler, upon encountering the -> _ in the signature, does three things:

  1. it notes an error, since it knows the _ is unacceptable in the signature's return type
  2. it provides a diagnostic suggestion to the user of a potentially correct type to use
  3. it continues the static analysis of the rest of the program, using what it inferred to be the corrected version of the function.

So, consider the following example:

#[allow(dead_code)] struct S<'a>(&'a mut ());

fn h(s: S<'_>) -> _ { s }

fn main() { h(S(&mut ())); }

with your PR in place, here is the diagnostic I get:

error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types
 --> ../../demo-126017.rs:4:19
  |
4 | fn h(s: S<'_>) -> _ {
  |                   ^
  |                   |
  |                   not allowed in type signatures
  |                   help: replace with the correct return type: `S<'_>`

error[E0716]: temporary value dropped while borrowed
 --> ../../demo-126017.rs:9:14
  |
9 |     h(S(&mut ()));
  |     ---------^^--- temporary value is freed at the end of this statement
  |     |        |
  |     |        creates a temporary value which is freed while still in use
  |     argument requires that borrow lasts for `'static`

error: aborting due to 2 previous errors

If you read those two errors, it is really confusing: "where is the 'static coming from in the second message?

(Note that if one puts in the correction suggested in the first diagnostic, both errors go away.)

My theory as to why this is happening: Under your PR, the compiler encounters erroneous fn signature h, and makes two corrected versions: h_1 is one that returns S<'_> and is reported by your diagnostic, while h_2 is one that returns S<'static> and is installed by the compiler as a the "corrected" h that should be used in the rest of the static analysis (which then leads to the error reported against fn main.

Overall, the latter situation seems like it will yield very subtle diagnostic problems that are hard for us (the compiler mainainers) to diagnose.

So I'd prefer we try to fix up your PR to avoid getting into such a situation, if possible.