[EXPERIMENT] Ban generics in variadic args #127401
Conversation
rustbot
added
S-waiting-on-review
There was a problem hiding this comment.
These comments are likely premature given that this is a draft but I'm gonna comment them anyways so I don't forget lmao
What exactly are the rules for what types are allowed in variadic args? If that set of rules doesn't exist, we may want to figure those out first. If they do, do they apply structurally?
It seems like we should be walking through types using something like a type folder rather than just matching on the outermost type constructor here.
Similarly, there are a lot of types that probably should be banned if we're banning ty::Param, things like ty::Alias and then structural types that contain ty::Param recursively.
| @@ -428,6 +428,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |||
| let ptr_ty = self.resolve_vars_if_possible(ptr_ty); | |||
| variadic_error(tcx.sess, arg.span, arg_ty, &ptr_ty.to_string()); | |||
| } | |||
| // generics? in C variadic? naaah | |||
| ty::Adt(_def, gen_args) if gen_args.len() > 0 => { | |||
There was a problem hiding this comment.
Seems weird to ban Adt<i32> (fully monomorphic type) here.
There was a problem hiding this comment.
I'm not sure what that looks like in Rust source?
There was a problem hiding this comment.
fn variadic_fn(...) {}
struct Adt<T>(T);
fn call() {
let x: Adt<i32> = Adt(0);
variadic_fn(x);
}
There was a problem hiding this comment.
oh yeah, I am in fact kinda hoping no one's relying on that lmao.
There was a problem hiding this comment.
well, was at the time. clearly some people have made exciting choices.
There was a problem hiding this comment.
hmm how does one tell apart Box<T> then?
There was a problem hiding this comment.
Well I don't know yet if we care to tell apart T from Box<T>, you still haven't told me what the rules for a valid c_variadic arg is 😆
There was a problem hiding this comment.
Oh, it should probably have a trivial destructor (by which I mean does not contain Drop) for one, but setting that aside, it must be:
c_intc_uint- and their longer friends
c_double- a thin pointer
- something
repr(transparent)with the above - a
repr(C) structthat recursively contains only "repr(C)or FFI compatible" types, but it is fine if this result is smaller than an integer or contains only types that don't necessarily fit the above qualifications except for beingrepr(C), for instance it is acceptable for it to be
#[repr(C)]
struct TwoBytes {
a_byte: c_char,
byte_byte: c_char,
}There was a problem hiding this comment.
- Oh, and function pointers yes (pretending they're thin pointers, I guess).
repr(C) unionwith the same qualifications asrepr(C) struct- a "unit"
repr(C) enumshould also be fine. - a "unit"
repr(u8) enumis right out.
| ty::Adt(_def, gen_args) if gen_args.len() > 0 => { | ||
| bug!("what the ferris are you doing") | ||
| } | ||
| ty::Param(_) => bug!("what the ferris are you doing"), |
There was a problem hiding this comment.
Why are we ok w/ [T; N] 🤔
There was a problem hiding this comment.
Let me be clear here that I mean this for example. This also applies to all the other built-in types that allow other types inside of them, like tuples, refs, slices, etc.
There was a problem hiding this comment.
we probably aren't!
|
|
Note that this implies that a |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
that's an evil omen. |
|
(if you're curious where the ICE is coming from: https://docs.rs/perf-event-open-sys/latest/src/perf_event_open_sys/functions.rs.html#99-105) |
|
yeah, I spotted it. |
|
@compiler-errors btw am I understanding the delegation test failure as being because delegation syntax for fn self::whatever(a: Type1, b: Type2) {
from_original_path::whatever(a, b)
}because that's not correct for varargs. |
|
I don't actually know if delegation turns into a fn call or if it does some other kind of forwarding of the associated item. @petrochenkov would know. |
|
The |
|
moving to instantiation time it is, then. |
|
Not sure what delegation test failure you are talking about, I don't see anything delegation-related in the thread above or in CI logs. Delegation is supposed to copy the wrapper function signature from the target function, and then just pass all the parameters to the target function in the body. |
That said, Rust should make passing |
|
I will open an issue for the delegation bug and in it explain why that is likely impossible. |
Dylan-DPC
added
S-experimental
pnkfelix mentioned doing this but it looks like I'm beating him to it.
r? @ghost