-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
const if
to supress instantiation/codegen of unreachable branches
#3582
Comments
I would note pattern types as an alternative; you could constrain an input #![feature(inline_const)]
fn panic_on_zero<const N: usize>() {
const { assert!(N > 0) };
}
trait FooImpl<const N: usize> {
const FN: fn();
}
impl<const N: usize in 1..> FooImpl<N> for () {
const FN: fn() = panic_on_zero::<N>;
}
impl FooImpl<0> for () {
const FN: fn() = || {};
}
fn foo<const N: usize>() -> fn() {
<() as FooImpl<N>>::FN
}
fn main() {
let _fun = foo::<0>();
} |
That is significantly harder to understand/review. |
That looks like the same kind of unification as the const expr issue that I linked to. |
I'm a big proponent of pattern types, but I have to agree this is not a good use case for it. On the topic at hand, do you want a |
Oh, having the issue written like this (returning a function pointer) I now realize that anonymous consts + function pointers might help, if the optimizer plays along. I.e. fn foo<const N: usize>() -> fn() {
const {
if N > 0 {
panic_on_zero::<N>
} else {
|| {} // fallback
}
}
} But this is not as ergonomic as a real const-if because it means callers needs to hide the dead branches inside functions that get selected by a const block. At least the playground doesn't complain. Edit: #3582 (comment) says that this will stop working in the future. |
I fully agree, but if pattern types are added to the language for other reasons anyway, then perhaps the ergonomics improvement alone wouldn't justify a separate dedicated I would also note, that with pattern types as const generic parameters, much of what is now implemented as post-mono assertions could become pre-mono checks, eg: // Now checked pre-mono
fn require_nonzero<const N: usize in 1..>() {}
trait FooImpl<const N: usize> {
const FN: fn();
}
impl<const N: usize in 1..> FooImpl<N> for () {
const FN: fn() = require_nonzero::<N>;
}
impl FooImpl<0> for () {
const FN: fn() = || {};
}
fn foo<const N: usize>() -> fn() {
<() as FooImpl<N>>::FN
}
fn main() {
let _fun = foo::<0>();
} A hypothetical // Now checked pre-mono
fn require_nonzero<const N: usize in 1..>() {}
fn foo<const N: usize>() -> fn() {
const match N {
N_1_PLUS @ 1.. => require_nonzero::<N_1_PLUS>,
0 => || {},
}
}
fn main() {
let _fun = foo::<0>();
} |
Well, once there's const match then there's not a strong argument against const if, since it would seem that const if is a strict subset of const match. |
Imo that's just restating the problem. In this context pattern types are ~isomorphic to |
The advantage of the pattern types approach specifically is that the compiler can do exhaustiveness, disjointness etc checking, so rust-lang/project-const-generics#26 is potentially tractable even for impls involving them. With arbitrary |
I think even if we come up with a perfect It may be easier on the compiler implementation side to require a #[rustc_arg_required_const(0)]
#[rustc_intrinsic]
fn const_if<A, R>(cond: bool, on_true: fn(A) -> R, on_false: fn(A) -> R) -> fn(A) -> R; where, due to the magical nature of |
Would that mean less work for the compiler compared to #3582 (comment) |
that only works because rust-lang/rust#112879 hasn't landed in forever because it's such a massive perf regression |
I'll cite that as another reason to have const-if then. |
This seems like a component of something more generally usable, along the lines of Zig's Pattern types do indeed make selecting behavior more complicated, but something like a pattern type in the where bounds does seem more consistent with generic types if the intent is simply that compiling fails in certain conditions. fn foo<const N: usize>() -> fn()
where N == 0
{
|| {} // fallback
} |
I think we should not guarantee that (Rather like how we're trying to guarantee less about |
Yeah I already wasn't expecting |
This is basically the RFC issue equivalent of rust-lang/rust#122301, right? They're asking the same question, it seems to me -- providing a way to control which constants (and callee functions) in a function body are monomorphized. I agree this should have explicit syntax, something like if const { ... } {
const { panic!() }
} Currently we basically guarantee that the Sounds like we should land a change to the docs about guaranteed evaluation that says that inside a EDIT: the proposal above actually was I think I can see how we can implement this, but it's not trivial and may be expensive, too. Basically, currently we have struct MonoCondition {
must_be_true: List<Const>,
must_be_false: List<Const>,
}
required_consts: List<(MonoCondition, Const)>,
// similar for mentioned_items and then instead of evaluating all required_consts, we only evaluate those were all "must be true" consts are true and all "must be false" consts are false. When computing During inlining, we have to carefully take into account the And of course the mono collector can't just traverse all basic blocks, it has to ensure to only walk one side of an |
Yeah. I filed this RFC issue because I thought it's not possible at all in current rust. Shortly after I discovered that the function pointer approach does work but was then informed that it perhaps is not guaranteed behavior, so I filed the rust issue to get clarification on whether it can be relied on. rust issue: can we rely on the hack/trick?
correct
If it's prohibitive then the alternatives mentioned upthread such as unifying multiple function impls with non-overlapping where bounds or pattern types or something like |
Agreed. It seems worth at least exploring the |
That is entirely independent of whether we evaluate the consts, so if we emit IR for the dead arm of an You mention a bunch of tracking issues, but I am not entirely sure what to look for in them. I guess it's related to "Should this be a type error for N != 0? (See also this same question in [T]::as_chunks.)" in rust-lang/rust#100450? But I dont' know where I can find that "same question" (I guess I could go hunt for it, but there's no link), and there's background discussion here as well, so -- I understand the feature request in the abstract, but the issue description currently does not help to understand the concrete pain t-libs (or t-libs-api?) is running into. |
We used to emit IR at least. I can put it more generally to say that it's also a performance benefit because the compiler does not have to visit the items mentioned in the dead branch. This can be a significant benefit when choosing between two different implementations where each has a deep, generic call-tree that would have to be visited. I have already used rust-lang/rust#122301 to optimized compile performance and would use
They all follow a similar pattern that I already outlined in the code example above the linked tracking issues.
|
Used to, yeah. @saethlin did some work on that recently though in the context of the library UB checks, that should mean we no longer emit this IR.
Right, if the bottleneck is not codegen but actually just recursively traversing these functions then we still do that under
Do you have links to those PRs?
Okay, makes sense, thanks. As I mentioned in #122301, I think if we implement |
rust-lang/rust#122785 to prove that it's useful. I have not yet added more because I wasn't sure how shaky that foundation is.
Would that also be true for crate-private methods? |
Lol that landed a day after rust-lang/rust#122568. ^^ @saethlin's rust-lang/rust#123272 landed later. I can't tell how much of an effect rust-lang/rust#121421 had without that later fix. It may be worth trying to revert rust-lang/rust#122785 to see if it still makes a difference -- it should not make a difference in terms of how much we codegen, and I would be somewhat surprised if just the traversal of "mentioned" items makes such a difference.
Yes. The initial reachability analysis is necessarily conservative and there's no good way to exclude any monomorphic function from being considered. We can do our best to fix issues like rust-lang/rust#119214 but it's not possible to fix them all (without completely re-thinking how we handle monomorphic items that "may or may not" be needed by downstream crates). As I said, I think we should actually do item collection starting with all monomorphic items as roots, and
... until polymorphization optimizes away the pram and codegen's the function anyway. ;) |
rust-lang/rust#124907 the difference is tiny now |
Thanks for checking! |
I think we'd prefer to avoid a situation where The second is that there's a desire for a construct which guards not just post-mono asserts but also pre-mono bounds. In an ideal future, I'd like to be able to have a function declare a pre-mono bound of e.g. fn require_nonzero<const N: usize>()
where
const { N > 0 },
{
}
fn foo<const N: usize>() {
static if N > 0 {
// this scope checked with the added bound of
// where const { N > 0 }
// (purely syntactic resolution, no SMT solving)
require_nonzero::<N>();
} else {
// ideally only instantiated when bound is false
// allowing even compile_error! as a fallback
// but also ideally still otherwise typechecked
fallback();
}
} Further overloading the meaning of
This is wishful thinking, to be sure, but despite the details being hairy, the base concept is straightforward (and basically just a port of the C++ constexpr if semantics to Rust generics). |
Yes, this is critical for the libs usecase. Without it we can't promote a const assert to a bound later on. |
I don't think we need to burden either We already have fn foo<const N: usize>() {
where N > 0 {
require_nonzero::<N>();
} else {
fallback();
}
} This seems like a natural and straightforward extension that doesn't conflict with the existing semantics of |
Currently there's a suboptimal interaction between const asserts and const generics.
A more general function that is valid for all N cannot call a function that asserts at compile time that N fulfills some conditions even if the call is on a branch that's dead for the invalid N.
I.e. this does not compile:
This blocks / requires unpleasant choices in several T-libs features that want to check for
N != 0
or similar constraints:Iterator::array_chunks
rust#100450Iterator::map_windows
(featureiter_map_windows
) rust#87155If const-eval in the dead branches is expensive it might also help compile perf.
There are efforts (rust-lang/rust#99682) to move monomorphization-time errors to check time and also apply those checks to dead code (rust-lang/rust#112879), which will make it even more difficult to discharge compile-time obligations imposed by callees.
Currently the language does not seem to have any way to select different code paths in generic contexts without also instantiating those paths. Even
generic_const_exprs
does not offer this unless it gets extended rust-lang/project-const-generics#26Therefore it would be useful if we could write the case above as
so that
panic_on_zero::<0>
will never be instantiated.The text was updated successfully, but these errors were encountered: