softdevteam / ykrustc

Yorick Meta-tracer
Other
6 stars 4 forks source link

Reject closures marked #[interp_step]. #174

Closed vext01 closed 3 years ago

vext01 commented 3 years ago

I was investigating ways of parametrising tests. One way involved passing variants of the interpreter step as closures. I found out the hard way that this won't work.

We can't accept closures, as these have a hidden argument prepended for the upvars and this breaks the invariant that Local(1) is the interpreter context.

This change causes ykrustc to reject a program that marks a closure #[interp_step].

ltratt commented 3 years ago

Instead of rejecting closures, should we only let through things that we know for sure work? Or is the only meaningful difference "closures or not closures"?

vext01 commented 3 years ago

I did try annotating a struct, and the compiler accepted it. It seems only function-like things make it into this particular section of code.

If we want to reject the others I'd have to dig deeper -- not something I'd like to do right now.

ltratt commented 3 years ago

What I'm suggesting is not that we list everything to reject but rather only list what we want to include and reject everything else.

vext01 commented 3 years ago

That's what we are doing.

Note the ! before matches!.

vext01 commented 3 years ago

Try the second hunk of 97392dd. I bodged the commit, the first hunk needs to be removed at squash time.

bjorn3 commented 3 years ago

If we want to reject the others I'd have to dig deeper -- not something I'd like to do right now.

All other checks of this kind are ad-hoc at https://github.com/rust-lang/rust/blob/446d4533e89db04f9568be4199e56b5fce0d176d/compiler/rustc_passes/src/check_attr.rs

vext01 commented 3 years ago

I see. So we'd have to add a check in there for our stuff.

One for another day I think.

vext01 commented 3 years ago

282632d is copy-paste-format your wording :)

ltratt commented 3 years ago

That new commit displays exceptional levels of wisdom. I couldn't have written it better myself 📦:p

Please squash.

vext01 commented 3 years ago

splat.

ltratt commented 3 years ago

bors r+

bors[bot] commented 3 years ago

Build failed:

vext01 commented 3 years ago

There's a typo in the test file.

[interp_step] can only be appled to regular function definitions.

Ok to fix and force push?

ltratt commented 3 years ago

Please go ahead.

vext01 commented 3 years ago

splat

ltratt commented 3 years ago

bors r+

bors[bot] commented 3 years ago

Build succeeded: