rustc_hir_analysis: add a helper to check function the signature mismatches

[eduardosm]

This function is now used to check #[panic_handler], start lang item, main, #[start] and intrinsic functions.

The diagnosis produced are now closer to the ones produced by trait/impl method signature mismatch.

This is the first time I do anything with rustc_hir_analysis/rustc_hir_typeck, so comments and suggestions about things I did wrong or that could be improved will be appreciated.

[rustbot]

r? @jackh726

(rustbot has picked a reviewer for you, use r? to override)

[compiler-errors]

This is pretty confusing. We probably want to actually replace the call to ocx.resolve_regions_and_report_errors to a manual call to ocx.infcx.resolve_regions and report any region errors manually from there with a more descriptive message.

[eduardosm]

What message would we want here?

In the case of a trait/impl:

use std::panic::PanicInfo;

trait Tr {
    fn panic(info: &PanicInfo<'_>);
}

impl Tr for () {
    fn panic(_info: &'static PanicInfo) {
        unimplemented!();
    }
}

It is reported similarly:

error[E0308]: method not compatible with trait
 --> lib.rs:8:5
  |
8 |     fn panic(_info: &'static PanicInfo) {
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ lifetime mismatch
  |
  = note: expected signature `fn(&PanicInfo<'_>)`
             found signature `fn(&'static PanicInfo<'_>)`
note: the anonymous lifetime as defined here...
 --> lib.rs:8:30
  |
8 |     fn panic(_info: &'static PanicInfo) {
  |                              ^^^^^^^^^
  = note: ...does not necessarily outlive the static lifetime

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.

The difference is that in the test, the note spans the whole signature instead of just PanicInfo:

note: the anonymous lifetime as defined here...
  --> $DIR/panic-handler-bad-signature-2.rs:9:1
   |
LL | fn panic(info: &'static PanicInfo) -> !
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = note: ...does not necessarily outlive the static lifetime

I'm not sure why.

EDIT: I think it depends on whether the region is BrAnon or BrNamed with kw::UnderscoreLifetime.

[eduardosm]

I opened #115903 regarding confusing lifetime diagnosis for trait impl functions. I think fixing that its outside the scope of this PR.

[compiler-errors]

This can be a DefId, and you can use CRATE_DEF_ID for the cases that it's required to be a LocalDefId.

[compiler-errors]

Can you make the caller pass in a full ObligationCause? That's usually more conventional than just passing the code.

[compiler-errors]

It concerns me that we're only checking the subtyping in one direction instead of both directions (with eq) here.

We should ideally be instantiating the binder in both directions -- this should cause us to accept more code, possibly inadvertently?

[eduardosm]

@rustbot ready

[bors]

☔ The latest upstream changes (presumably #115909) made this pull request unmergeable. Please resolve the merge conflicts.

[compiler-errors]

This is not how to compare two binders. This is not very much different than using sub, which I think is still wrong. You should probably wrap both binders in a FnPtr and compare them directly. The error reporting will kinda suck, but that can be dealt with differently.

[eduardosm]

The output changed from

   = note: expected signature `fn(&PanicInfo<'_>) -> _`
              found signature `fn(&'static PanicInfo<'_>) -> _`

to

   = note: expected fn pointer `for<'a, 'b> fn(&'a PanicInfo<'b>) -> _`
              found fn pointer `for<'a> fn(&'static PanicInfo<'a>) -> _`

But only for lifetime errors.

For non-lifetime errors, it stayed expected/found signature, I believe that thanks to passing the signatures in infer::ValuePairs here (I'm not sure that's entirely correct)

err_ctxt.note_type_err(
    &mut diag,
    &cause,
    None,
    Some(infer::ValuePairs::Sigs(ExpectedFound {
        expected: tcx.liberate_late_bound_regions(fn_id, expected_sig),
        found: tcx.liberate_late_bound_regions(fn_id, actual_sig),
    })),
    err,
    false,
    false,
);

[bors]

☔ The latest upstream changes (presumably #115952) made this pull request unmergeable. Please resolve the merge conflicts.

[compiler-errors]

@bors r+

[bors]

📌 Commit 85d61b0 has been approved by compiler-errors

It is now in the queue for this repository.

[bors]

⌛ Testing commit 85d61b0 with merge 99b63d0...

[bors]

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 99b63d0 to master...

[rust-timer]

Finished benchmarking commit (99b63d0): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.7%, 0.7%]	1
Regressions ❌ (secondary)	2.0%	[1.5%, 2.6%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.7%	[0.7%, 0.7%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 634.138s -> 633.396s (-0.12%)
Artifact size: 317.56 MiB -> 317.61 MiB (0.02%)