Fix opaque type ICE in late lints under the next-generation trait solver

[Gri-ffin]

fixes rust-lang/rust-clippy#17411

Under the next trait solver, late lints trigger an ICE due to having non-empty opaque storage when dropping InferCtxt.

Rather than forcing all current and future late lints to manually use ignoring_regions, use PostBorrowck in LateContext.

cc @adwinwhite

[rustbot]

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 74 candidates
• Random selection from 18 candidates

[Jarcho]

Is the clippy ICE not caused by the fallback to TypingMode::non_body_analysis() for bodies which don't define opaque types?

[adwinwhite]

Thanks! Could you add a next solver revision in the future_not_send clippy test?

Also the reason we trigger ICE is that we have non-empty opaque storage when dropping InferCtxt rather than we have unresolved hidden types. Please update the description :>

We should also update the comments in the Drop impl of InferCtxt to remove the mention of lints.

[Jarcho]

I definitely did not read the condition correctly. It wouldn't even matter anyway since it's in a function defining an opaque type.

The ICE is fixed by normalizing the return type first. Given that we have 158 instances of skip_norm_wip there's probably more issues to be found.

[adwinwhite]

Is the clippy ICE not caused by the fallback to TypingMode::non_body_analysis() for bodies which don't define opaque types?

No, we're not using TypingMode::non_body_analysis in the next solver. And using it is wrong as late lints are run after borrowck so we should have access to opaque types defined in the body.

[adwinwhite]

The ICE is fixed by normalizing the return type first.

care to elaborate more? normalizing it shouldn't fix this 🤔 normalization only registers/check this opaque.

Ah, I'm blind. you mean normalizing the return type. that will only fix cases that the opaques types are return types.
The skip_norm_wip is #155345.
Normalization has cost and it's unsure we can normalize everywhere.

[Jarcho]

The type is currently retrieved via:

let ret_ty = cx.tcx.normalize_erasing_regions(cx.typing_env(), cx.tcx.fn_sig(fn_def_id).instantiate_identity()).output();
cx.tcx.instantiate_bound_regions_with_erased(ret_ty)

Switching it to:

let ret_ty = cx.tcx.normalize_erasing_regions(cx.typing_env(), cx.tcx.fn_sig(fn_def_id).instantiate_identity()).output();
cx.tcx.instantiate_bound_regions_with_erased(ret_ty)

Gets rid of the ICE.

[Jarcho]

The crashing lint looks for functions returning impl Future and checks if they implement Send. Currently we pass the opaqued type alias into the trait solver.

[Jarcho]

Normalization has cost and it's unsure we can normalize everywhere.

I thought the new solver generally wanted things normalized first. There's quite a few times where we want to explicitly skip normalization since we want what was actually typed, but we also do quite a few trait queries.

[nnethercote]

r? @adwinwhite

...who clearly knows much more about this than I do :)

[adwinwhite]

Thanks for the clarification.

But actually the root cause of this ICE affects more than the future_not_send lint or return_ty.
short list:

• tests/ui/future_not_send.rs
• tests/ui/large_futures.rs
• tests/ui/missing_transmute_annotations_unfixable.rs
• tests/ui/new_ret_no_self_overflow.rs
• tests/ui/search_is_some_fixable_some.rs

Any use of defining opaque types under TypingMode::PostTypeckUntilBorrowck will register entry to the opaque storage.
And they'll trigger the drop bomb in InferCtxt::drop unless we explicitly consume/take the opaques type from the storage which we don't often do in late lints, or ever do?
Using the correct typing mode would eliminate this footgun entirely - With TypingMode::PostBorrowck we don't register any opaque type into the storage.

I thought the new solver generally wanted things normalized first.

Yeah, just that it's not always achievable 😔

[rustbot]

clippy is developed in its own repository. If possible, consider making this change to rust-lang/rust-clippy instead.

cc @rust-lang/clippy

[Jarcho]

Any use of defining opaque types under TypingMode::PostTypeckUntilBorrowck will register entry to the opaque storage.
And they'll trigger the drop bomb in InferCtxt::drop unless we explicitly consume/take the opaques type from the storage which we don't often do in late lints, or ever do?

I assume the point of what both the typeck and borrowck modes are doing is to gather up all the different types that flow into the opaque type. If that's the the case then lint passes are using the wrong mode since we don't ever do that. If a lint ever needed to it could still switch to a different mode.

[adwinwhite]

@bors r+ rollup

[rust-bors]

📌 Commit 964e8ee has been approved by adwinwhite

It is now in the queue for this repository.

[Mark-Simulacrum]

@rustbot build 919a1dc

cc #160003 (comment)