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

What are the guarantees around which constants (and callees) in a function get monomorphized? #122301

Open
the8472 opened this issue Mar 10, 2024 · 16 comments
Labels
A-const-generics Area: const generics (parameters and arguments) C-discussion Category: Discussion or questions that doesn't represent real issues. I-lang-nominated The issue / PR has been nominated for discussion during a lang team meeting. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@the8472
Copy link
Member

the8472 commented Mar 10, 2024

Currently this compiles in stable.

struct Foo<T, const N: usize>(T);

impl<T, const N: usize> Foo<T, N> {
    const BAR: () = if N == 0 {
        panic!()
    };
}

struct Invoke<T, const N: usize>(T);

impl<T, const N: usize> Invoke<T, N> {
    const FUN: fn() = if N != 0 {
        || Foo::<T, N>::BAR
    } else {
        || {}
    };
}

fn main() {
    Invoke::<(), 0>::FUN();
}

This is a useful property¹ but if the panic path were not completely excluded this could lead to monomorphization-time errors which are undesirable since they only show up in cargo build and not in cargo check. But not as undesirable as #107503 where the errors are optimization-dependent.

Still, @RalfJung indicated that the current behavior might not be intentional so I'm filing this issue for clarification.

(Also see rust-lang/rfcs#3582 for more discussion of the same question.)


¹actual use could look like this:

// library-provided function:

/// Fails to compile if N == 0
fn chunks<const N: usize>(foo: Foo) -> &[[u8; N]] {
   const {
       assert!(N > 0)!
   }

   todo!()
}

// user-code, generic over all N
fn library_user_code<const N: usize>(foo: Foo) ->  {

   // this could be generated by a `const_if!()` macro or use inline const blocks
   struct ConstIf< const N: usize>;

   impl<const N: usize> ConstIf<N> {
     const FUN: fn() = if N != 0 {
         |foo| chunks::<N>(foo)
     } else {
         |foo| { /* fallback code here */ }
     };
   }
   
   ConstIf::<N>::FUN(foo)
}
@the8472 the8472 added T-lang Relevant to the language team, which will review and decide on the PR/issue. A-const-generics Area: const generics (parameters and arguments) C-discussion Category: Discussion or questions that doesn't represent real issues. labels Mar 10, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 10, 2024
@RalfJung
Copy link
Member

Cc @rust-lang/wg-const-eval

Ultimately this is a t-lang question though so might need a t-lang design meeting or some such thing.

@RalfJung
Copy link
Member

RalfJung commented Mar 10, 2024

In fact you don't need closures, this builds as well:

struct S<T>(T);
impl<T> S<T> {
    const C: () = panic!();
}

const fn bar<T>() { S::<T>::C }

struct Invoke<T, const N: usize>(T);

impl<T, const N: usize> Invoke<T, N> {
    const FUN: () = if N != 0 {
        bar::<T>() // not called for N == 0, and hence not monomorphized
    } else {
        ()
    };
}

fn main() {
    let _val = Invoke::<(), 0>::FUN;
}

@RalfJung
Copy link
Member

RalfJung commented Mar 10, 2024

This is a useful property

Your example doesn't quite seem to have the right shape for that use-case; could you flesh out the example a bit more to make this more clear?

@the8472
Copy link
Member Author

the8472 commented Mar 10, 2024

could you flesh out the example a bit more

Added it to the top post.

@RalfJung
Copy link
Member

Makes sense, thanks!

Note that without generic_const_exprs such a const_if! macro will be very limited.

@clubby789 clubby789 removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 10, 2024
the8472 added a commit to the8472/rust that referenced this issue Mar 15, 2024
if this ought to be broken it should at least happen intentionally
the8472 added a commit to the8472/rust that referenced this issue Mar 17, 2024
if this ought to be broken it should at least happen intentionally
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 17, 2024
…fJung

add test for rust-lang#122301 to cover behavior that's on stable

If this ought to be broken it should at least happen intentionally

See rust-lang#122301
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 17, 2024
…fJung

add test for rust-lang#122301 to cover behavior that's on stable

If this ought to be broken it should at least happen intentionally

See rust-lang#122301
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 17, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#120640 (Mark UEFI std support as WIP)
 - rust-lang#121862 (Add release notes for 1.77.0)
 - rust-lang#122572 (add test for rust-lang#122301 to cover behavior that's on stable)
 - rust-lang#122578 (Only invoke `decorate` if the diag can eventually be emitted)
 - rust-lang#122615 (Mention Zalathar for coverage changes)
 - rust-lang#122636 (some minor code simplifications)

r? `@ghost`
`@rustbot` modify labels: rollup
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Mar 17, 2024
Rollup merge of rust-lang#122572 - the8472:test-const-deadness, r=RalfJung

add test for rust-lang#122301 to cover behavior that's on stable

If this ought to be broken it should at least happen intentionally

See rust-lang#122301
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 20, 2024
…l, r=<try>

select Vec::from_iter impls in a const block to optimize compile times

This relies on the trick from rust-lang#122301
Split out from rust-lang#120682
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 21, 2024
…l, r=Amanieu

select Vec::from_iter impls in a const block to optimize compile times

Ignoring whitespace diffs should make this easier to review.

This relies on the trick from rust-lang#122301
Split out from rust-lang#120682
@the8472 the8472 added the I-lang-nominated The issue / PR has been nominated for discussion during a lang team meeting. label Mar 22, 2024
@the8472
Copy link
Member Author

the8472 commented Mar 22, 2024

Nominated to get clarification from T-lang on whether this pattern - and the inline const block variant - can be relied on. I have already found this useful for compilation performance (#122785) and it could also provide a (partial?) solution to rust-lang/rfcs#3582 until more ergonomic and more powerful language features are available.

Edit: And a followup question if we can rely on this, can we also expect this to be forward-compatible with pattern types on const generics or const generic exprs?
I.e. if a fn foo<const N: usize>() caller uses this approach to call a fn bar<const N: usize> { const { assert!(N != 0); } } today would it continue to compile when we convert the callee to fn bar<const N: usize in 1..>() or fn bar<const N>() where {N != 0}?

@RalfJung
Copy link
Member

Note that we are currently also exploring various ways to evaluate more constants, which requires some amount of monomorphization in dead code. This is done to combat the problem of optimization-level-dependent build failures:

@the8472
Copy link
Member Author

the8472 commented Mar 22, 2024

This one shouldn't be optimization-dependent though, so I hope there's no conflict between them.

@RalfJung
Copy link
Member

It's not opt-dependent, but it's also unclear how we want to resolve the opt-dependent issue. Some proposals involve also walking all items "mentioned" in a const. That would be in direct conflict with your goal here I think. To be clear I think that's a weakness of those proposals. But if that turns out to be the only viable strategy then we'll have to decide what we want more: using const tricks to control what gets monomorphized, or not having optimization-dependent errors.

@RalfJung
Copy link
Member

One crucial part of this construction is that everything involved is generic. If somewhere in the two "branches" you end up calling a monomorphic function, then that may have its constants evaluated even if it is in the "dead" branch -- or it may not, it depends on which functions are deemed cross-crate-inlinable. That's basically what #122814 is about.

@scottmcm
Copy link
Member

scottmcm commented Apr 3, 2024

I would really like to have a const if language feature that people can use to communicate to us that they're relying on it, and thus that we can ensure actually does keep doing what they want.

(Just speaking for me, not for Lang as a whole.)

@RalfJung
Copy link
Member

RalfJung commented Apr 3, 2024

Yeah it would be nice to have that. Currently I don't see how to implement it though.

@RalfJung RalfJung changed the title Should dead code in const be monomorphized? Provide better control of which constants (and callees) in a function get monomorphized May 8, 2024
@RalfJung
Copy link
Member

RalfJung commented May 8, 2024

I would really like to have a const if language feature that people can use to communicate to us that they're relying on it, and thus that we can ensure actually does keep doing what they want.

That's rust-lang/rfcs#3582.

This issue here is about whether we want to stably guarantee something about the current syntax that can stand-in for the desired feature, if somewhat crudely.

I think what we'd need is that in a const item, only the functions that are actually called get monomorphized. Which seems like a reasonable promise to make? But the reachable root computation is extremely involved, and it does statically traverse const items (see the updated comments in #122769), and we have bugs like #119214, so -- @the8472 I don't know if your guarantee actually holds today, but I suspect not, though it's kind of a bug.

That said, even if we had const if, I am not sure what #119214 would do to that. Nothing good, probably. reachable can't evaluate constants, it has to go entirely based on syntactic appearance, so everything in both arms of the if would be "reachable". And once #119214 is resolved we can maybe make the guarantee without new syntax (though new syntax may still be nice).

@RalfJung
Copy link
Member

RalfJung commented May 8, 2024

#119214 can only affect monomorphic functions.

I don't think we can provide any guarantees that a monomorphic function won't be codegen'd and thus have its consts evaluated. Even if there is a const if or with the trick from this issue, monomorphic functions can always end up being collected under some odd conditions. The best we can do is encode their MIR instead of codegen'ing them so that if we are wrong and they are needed, a downstream crate can do the codegen. But with the current strategy of not encoding the MIR for (sufficiently big, i.e. non-cross-crate-inlineable) monomorphic functions but always encoding their compiled form, fundamentally we have to conservatively compile them if they may be needed, and then fundamentally we have to evaluate their consts and stop compilation if any of them panics. (That's why I suggested in #122814 that we just always evaluate all consts in monomorphic functions, and in functions they "mention".)

But... I think if we disregard monomorphic functions, then I think it is true that in a const item we'll only monomorphize what actually gets called. We may very much overapproximate what is "reachable", and hence encode MIR for things, but the only way we start eval'ing the consts in a generic function is if it gets codegen'd (which can only happen if it gets mentioned in a codegen'd function, which is not the case here), or const-eval'd.

Well, and there's #122828. But that can only happen when the function in question is called from somewhere that we compute optimized_mir on. And in your example @the8472 I assume chunks only ever gets called via that FUN constant. We don't run the inliner on const item bodies so this can't happen either.

So... yeah I think your pattern is safe. But there's so many moving parts here it's really hard to say. That said, guaranteeing that your pattern is safe is exactly as hard as guaranteeing that an const if is safe, the difficulty is the same in both cases. const if is just syntactically nicer, but no easier to implement.

@RalfJung RalfJung changed the title Provide better control of which constants (and callees) in a function get monomorphized What are the guarantees around which constants (and callees) in a function get monomorphized? May 8, 2024
@RalfJung
Copy link
Member

RalfJung commented May 8, 2024

So concretely, I think that if a function is

  • generic (in a type or a const -- lifetimes are not enough)
  • and only ever mentioned in a const

(both of which should be true for your pattern)
then we will not ever consider it in the monomorphization collector, and therefore the only way it can ever be considered is by const eval, which will only consider function items that actually occur in the given execution.

That said, there is this thing called polymorphization, and I have absolutely no idea how it works. If a function gets polymorphized, does it behave basically like a monomorphic function despite being generic? In that case you'd have to further make sure that your function does not get polymorphized. I don't even know which functions we can polymorphize. There does not seem to be a tracking issue for polymorphization, and it does seem to be nightly-only.

@lcnr
Copy link
Contributor

lcnr commented May 9, 2024

That said, there is this thing called polymorphization, and I have absolutely no idea how it works. If a function gets polymorphized, does it behave basically like a monomorphic function despite being generic? In that case you'd have to further make sure that your function does not get polymorphized. I don't even know which functions we can polymorphize. There does not seem to be a tracking issue for polymorphization, and it does seem to be nightly-only.

I think it's fine to ignore polymorphization in the decision here. It should not change what gets monomorphized but instead only merges different monomorphic instances in one polymorphic one. It's the same as the linker/llvm deduplicating equivalent functions, which is something we already have to support

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-generics Area: const generics (parameters and arguments) C-discussion Category: Discussion or questions that doesn't represent real issues. I-lang-nominated The issue / PR has been nominated for discussion during a lang team meeting. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants