Skip to content
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

Add basic support for RPIT #1351

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add basic support for RPIT #1351

wants to merge 3 commits into from

Conversation

xldenis
Copy link
Collaborator

@xldenis xldenis commented Feb 8, 2025

A proof I was working on was using impl Iterator and I got annoyed at us not supporting it. Return position impl trait is a lot simpler than the argument position since we can just compile it to an opaque type outside the defining scope (now that we "monomorphize" everything).

@@ -436,7 +438,7 @@ fn resolve_function<'tcx>(

// Try to extract a function defid from an operand
fn func_defid<'tcx>(op: &Operand<'tcx>) -> Option<(DefId, GenericArgsRef<'tcx>)> {
let fun_ty = op.constant().unwrap().const_.ty();
let fun_ty = op.constant()?.const_.ty();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this just allows us to report a nicer error rather than panic.

@@ -444,7 +444,12 @@ impl<'tcx> TranslationCtx<'tcx> {
pub(crate) fn typing_env(&self, def_id: DefId) -> TypingEnv<'tcx> {
// FIXME: is it correct to pretend we are doing a non-body analysis?
let param_env = self.param_env(def_id);
TypingEnv { typing_mode: TypingMode::non_body_analysis(), param_env }
let mode = if self.is_mir_available(def_id) && def_id.is_local() {
TypingMode::post_borrowck_analysis(self.tcx, def_id.as_local().unwrap())
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to use [analysis_in_body](https://doc.rust-lang.org/beta/nightly-rustc/rustc_type_ir/infer_ctxt/enum.TypingMode.html#tymethod.analysis_in_body) but that panicked whereas this seems to give the correct results.

tcx.def_path(aty.def_id).data.last().unwrap().disambiguator
))),
TyKind::Alias(_, aty) => Some(Symbol::intern(&type_name(
tcx.opt_item_name(aty.def_id).unwrap_or(Symbol::intern("synthetic")).as_str(),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how but I'm getting these new "synthetic" types in the code I'm looking at which I don't yet know how to handle cleanly here.

@jhjourdan
Copy link
Collaborator

Thanks, Xavier, I'll have a look into that.

In particular I'd like to better understand the choices about the type checker mode. Ot does not seem like that post analysis mode is the right mode...

Also, I don't see why these types in parameter position are more difficult to handle : cannot we handle them as abstract types in the body, and ask the type checker what they at the call site ?

@xldenis
Copy link
Collaborator Author

xldenis commented Feb 8, 2025

Also, I don't see why these types in parameter position are more difficult to handle : cannot we handle them as abstract types in the body, and ask the type checker what they at the call site ?

It's not fundamentally more difficult but Rust compiles them to additional generic parameters literally called "impl TraitWhatever" with the expected bounds. It's just more plumbing to make it work properly that's all. For my purposes I only had a few impl Iterators returned by functions so I didn't care about that for the moment.

@xldenis
Copy link
Collaborator Author

xldenis commented Feb 8, 2025

In particular I'd like to better understand the choices about the type checker mode. Ot does not seem like that post analysis mode is the right mode...

I agree but normalization panics when in analysis_in_body, I couldn't find any clear explanation for why that would happen. Perhaps it has to do with the fact that we recover MIR after borrowck?

pub fn main() {
let x = returns_iterator().a();

proof_assert!(x);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I should move this to should_fail as I specifically want to test that we can't observe anything about x (other than what is specified by the trait). Would need to add a spec to the impl though for that to work.

@xldenis
Copy link
Collaborator Author

xldenis commented Feb 8, 2025

I forgot I actually did have some argument position impl traits: which generate invalid coma:

type impl Document'0

@xldenis
Copy link
Collaborator Author

xldenis commented Feb 8, 2025

hmm I may just have added support for APIT by replacing spaces with _ in the argument name.

@jhjourdan
Copy link
Collaborator

Please try to avoid name clashes...

@xldenis
Copy link
Collaborator Author

xldenis commented Feb 9, 2025

The whole thing goes through the clone map after so it should get a unique count.

@xldenis
Copy link
Collaborator Author

xldenis commented Feb 9, 2025

Specifying functions with RPIT is tricky because our encoding of ensures won't work, specifically we create a closure with result : ResultTy but you can't have impl Trait arguments to closures. I think this requires changing the generation of ensures to look the one we give to closures, but that's not without its own complications!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants