-
Notifications
You must be signed in to change notification settings - Fork 83
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
avoid relying on implicit promotion #41
Conversation
That's really anoying: the only usage of the one letter const function is to have an ergonomic way of defining a keyboard keymap. See for example this: https://github.com/TeXitoi/ortho60-keyberon/blob/master/src/layout.rs The users of keyberon highly rely on these functions to define their keymap. Is there another workaround? I'd prefer that these function always return a 'static that breaking all the existing keymap. I'll try to understand the RFC, a quick look didn't make the modification obvious. |
The modification is as such: your code relies on the fact that
Other possible work-arounds that come to my mind:
|
The macro could even be a generic one, e.g. {
const _HIDDEN: Action = expr;
_HIDDEN
} That would basically do promotion explicitly, and thus avoid relying on implicit promotion. |
How to do that explicitly? I need to declare a
That's a possibility. I try to avoid them when possible, that's why I was using const fn The better I can imagine is https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=2eaf47075e038b31d55e4f3e16bac214 But it looks quite hackish. I'd better define a
Well, pub const fn l<T>(layer: usize) -> Action<T> {
Action::Layer(layer)
}
Well, the idea is to have a static with the type |
A const for each array would also work (but then the const type needs to state the array length).
Hm... I wonder if there's another way to define an alias, but I can't think of one.
Yeah that looks pretty good as well. |
Funnily enough, that isn't an issue by repeating the array expression: macro_rules! const_array {(
const $NAME:tt : [$T:ty; _] = [ $($expr:expr),+ $(,)? ];
) => (
const $NAME : [$T; [$($expr),+].len()] = [$($expr),+];
)} |
Oh, nice trick. This will evaluate the expression twice, though. |
Indeed, although, afaik, that shouldn't matter for a |
Not sure what you mean by "shouldn't matter" -- the CTFE engine will evaluate this code twice, then. |
You're right, I should have clarified. I think that it affecting compile-time is indeed not to be disregarded; (I was thinking about some semantic change / something about the result of a Anyways, - const $NAME : [$T; [$($expr),+].len()] = [$($expr),+];
+ const $NAME : [$T; 0 $( + {ignore!($expr);1} )+] = [$($expr),+];
// or
+ const $NAME : [$T; [$( ignore!($expr) ),+].len()] = [$($expr),+]; can be a way to make sure the CTFE of the len is "simple enough" (where |
rust-lang/rust#80243 is closed, so let's also close this. |
OK, thanks. |
There are plans to change the rules for implicit promotion such that calls to
const fn
are not implicitly promoted to'static
lifetime any more. A crater experiment showed that only very few crates would be affected by this change, and this is one of them.This PR adjusts the code to no longer rely on implicit promotion, by explicitly putting the result of the function call into a
const
item. Long-term, there will be the possibility of using inline const expressions instead, which will be more ergonomic.