Constify more of alloc::Layout #67494

lukaslueg · 2019-12-21T18:04:19Z

Making more methods of alloc::Layout const would allow us to compute alignment/size information for arbitrary (sized) types at compile-time, including placing the information in associated constants. While mem::size_of and mem::align_of are already const and Layout is solely based on those, there is no guarantee in the implementation that a const derived from these functions will be exactly the same as what Layout uses and is subsequently used in a call to alloc::alloc. Constifying Layout makes this possible.

First contribution to core, excuse my ignorance.

rust-highfive · 2019-12-21T18:04:22Z

r? @withoutboats

(rust_highfive has picked a reviewer for you, use r? to override)

Centril · 2019-12-21T19:09:31Z

r? @oli-obk

oli-obk

Please file a tracking issue for this constification and use the id instead of "0"

src/libcore/alloc.rs

lukaslueg · 2019-12-22T13:15:21Z

I split the consttransmogrification of Result into a separate feature, I hope this was ok.

oli-obk · 2019-12-22T13:28:09Z

src/libcore/alloc.rs

-    pub fn align_to(&self, align: usize) -> Result<Self, LayoutErr> {
-        Layout::from_size_align(self.size(), cmp::max(self.align(), align))
+    pub const fn align_to(&self, align: usize) -> Result<Self, LayoutErr> {
+        let align = if self.align() >= align { self.align() } else { align };


Please make cmp::max const fn, too :)

Should I file a separate tracking issue?

sorry I missed this in the first pass

the PR lgtm after this

Oh I just realized: std::cmp::{max; min} are defined as pub fn max<T>(v1: T, v2: T) -> T where T: Ord. There is no way to require an impl of Ord to be const?!

Oh, no that's not possible. So leave align_to as not const fn for now, or is it super necessary?

Not to me, no. I actually don't see this one-liner as a problem: The type is always going to be usize, so even if it never gets changed back to cmp::max(), it's totally obvious what it does.

it's totally obvious what it does.

I agree, but

the line of what is obvious and what not is very subjective, if at some point we have to start arguing about it, it seems good to start out conservative if there's no strong desire for the main change

the original author must have thought that cmp::max is even more obvious (and I think this, too). So making code less obvious just to make it const without a strong reason to do so seems not like a good strategy.

Thanks for changing it back

Maybe a const fn align_max(align1: usize, align2: usize) -> usize helper function to replace cmpp::max()?

(Not in this PR, it's already merged)

oli-obk · 2019-12-22T20:49:10Z

@bors r+

bors · 2019-12-22T20:49:11Z

📌 Commit b56e0e2126eaee3675805485169338d7736dd5bd has been approved by oli-obk

bors · 2019-12-22T20:49:12Z

🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened

lukaslueg · 2019-12-22T21:21:37Z

I've just seen this will conflict with #66254 which does constification for just new. You may want to r- ?

oli-obk · 2019-12-22T21:28:48Z

@bors r-

bors · 2019-12-23T05:53:12Z

☔ The latest upstream changes (presumably #67540) made this pull request unmergeable. Please resolve the merge conflicts.

@oli-obk

Constify Result r? @oli-obk This is just the `Result`-part of rust-lang#67494 which I'll resubmit once rust-lang#66254 has landed.

@oli-obk

Constify Result r? @oli-obk This is just the `Result`-part of rust-lang#67494 which I'll resubmit once rust-lang#66254 has landed.

lukaslueg · 2019-12-31T23:29:09Z

Force-pushed a commit that wont conflict with #66254

CAD97 · 2020-01-09T18:45:20Z

wont conflict with #66254

Actually, since we both (necessarily) touch size_align to make it const. You could rebase on top of #66254 or just wait for it to get merged.

lukaslueg · 2020-01-10T16:48:42Z

rebased again

bors · 2020-01-11T14:47:16Z

☔ The latest upstream changes (presumably #68126) made this pull request unmergeable. Please resolve the merge conflicts.

Tracking issue rust-lang#67521, Layout::new in rust-lang#66254

lukaslueg · 2020-01-11T16:02:16Z

rebased again

oli-obk · 2020-01-11T23:22:32Z

@bors r+

bors · 2020-01-11T23:22:33Z

📌 Commit c5a9a14 has been approved by oli-obk

bors · 2020-01-12T01:53:07Z

⌛ Testing commit c5a9a14 with merge eb9af48328fc6dee800ed3fe2f55551322a09cd6...

Constify more of alloc::Layout Making more methods of `alloc::Layout` const would allow us to compute alignment/size information for arbitrary (sized) types at compile-time, including placing the information in associated constants. While `mem::size_of` and `mem::align_of` are already const and `Layout` is solely based on those, there is no guarantee in the implementation that a const derived from these functions will be exactly the same as what `Layout` uses and is subsequently used in a call to `alloc::alloc`. Constifying `Layout` makes this possible. First contribution to core, excuse my ignorance.

Centril · 2020-01-12T02:28:41Z

@bors retry rolled up.

@ghost

Rollup of 6 pull requests Successful merges: - #67494 (Constify more of alloc::Layout) - #67867 (Correctly check for opaque types in `assoc_ty_def`) - #67948 (Galloping search for binary_search_util) - #68045 (Move more of `rustc::lint` into `rustc_lint`) - #68089 (Unstabilize `Vec::remove_item`) - #68108 (Add suggestions when encountering chained comparisons) Failed merges: r? @ghost

rust-highfive assigned withoutboats Dec 21, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 21, 2019

rust-highfive assigned oli-obk and unassigned withoutboats Dec 21, 2019

oli-obk requested changes Dec 21, 2019

View reviewed changes

src/libcore/alloc.rs Outdated Show resolved Hide resolved

src/libcore/alloc.rs Outdated Show resolved Hide resolved

src/libcore/alloc.rs Outdated Show resolved Hide resolved

This was referenced Dec 22, 2019

Tracking issue for const Result #67520

Closed

Tracking issue for const alloc::Layout #67521

Closed

oli-obk reviewed Dec 22, 2019

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 22, 2019

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 22, 2019

This was referenced Dec 25, 2019

Make Layout::new const #66254

Merged

Constify Result #67685

Merged

Centril added a commit to Centril/rust that referenced this pull request Dec 31, 2019

Rollup merge of rust-lang#67685 - lukaslueg:const_result, r=oli-obk

89fbed9

Constify Result r? @oli-obk This is just the `Result`-part of rust-lang#67494 which I'll resubmit once rust-lang#66254 has landed.

lukaslueg force-pushed the const_alloc branch from b56e0e2 to cf89869 Compare December 31, 2019 23:27

lukaslueg force-pushed the const_alloc branch from cf89869 to c5ac45e Compare January 10, 2020 16:47

Constify alloc::Layout

c5a9a14

Tracking issue rust-lang#67521, Layout::new in rust-lang#66254

lukaslueg force-pushed the const_alloc branch from c5ac45e to c5a9a14 Compare January 11, 2020 16:01

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 11, 2020

Centril mentioned this pull request Jan 12, 2020

Rollup of 6 pull requests #68142

Merged

bors merged commit c5a9a14 into rust-lang:master Jan 12, 2020

lukaslueg deleted the const_alloc branch January 12, 2020 08:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Constify more of alloc::Layout #67494

Constify more of alloc::Layout #67494

lukaslueg commented Dec 21, 2019

rust-highfive commented Dec 21, 2019

Centril commented Dec 21, 2019

oli-obk left a comment

lukaslueg commented Dec 22, 2019

oli-obk Dec 22, 2019

lukaslueg Dec 22, 2019

oli-obk Dec 22, 2019

lukaslueg Dec 22, 2019

oli-obk Dec 22, 2019

lukaslueg Dec 22, 2019

oli-obk Dec 22, 2019

rustonaut Sep 21, 2020 •

edited

Loading

oli-obk commented Dec 22, 2019

bors commented Dec 22, 2019

bors commented Dec 22, 2019

lukaslueg commented Dec 22, 2019

oli-obk commented Dec 22, 2019

bors commented Dec 23, 2019

lukaslueg commented Dec 31, 2019

CAD97 commented Jan 9, 2020

lukaslueg commented Jan 10, 2020

bors commented Jan 11, 2020

lukaslueg commented Jan 11, 2020

oli-obk commented Jan 11, 2020

bors commented Jan 11, 2020

bors commented Jan 12, 2020

Centril commented Jan 12, 2020

Constify more of alloc::Layout #67494

Constify more of alloc::Layout #67494

Conversation

lukaslueg commented Dec 21, 2019

rust-highfive commented Dec 21, 2019

Centril commented Dec 21, 2019

oli-obk left a comment

Choose a reason for hiding this comment

lukaslueg commented Dec 22, 2019

oli-obk Dec 22, 2019

Choose a reason for hiding this comment

lukaslueg Dec 22, 2019

Choose a reason for hiding this comment

oli-obk Dec 22, 2019

Choose a reason for hiding this comment

lukaslueg Dec 22, 2019

Choose a reason for hiding this comment

oli-obk Dec 22, 2019

Choose a reason for hiding this comment

lukaslueg Dec 22, 2019

Choose a reason for hiding this comment

oli-obk Dec 22, 2019

Choose a reason for hiding this comment

rustonaut Sep 21, 2020 • edited Loading

Choose a reason for hiding this comment

oli-obk commented Dec 22, 2019

bors commented Dec 22, 2019

bors commented Dec 22, 2019

lukaslueg commented Dec 22, 2019

oli-obk commented Dec 22, 2019

bors commented Dec 23, 2019

lukaslueg commented Dec 31, 2019

CAD97 commented Jan 9, 2020

lukaslueg commented Jan 10, 2020

bors commented Jan 11, 2020

lukaslueg commented Jan 11, 2020

oli-obk commented Jan 11, 2020

bors commented Jan 11, 2020

bors commented Jan 12, 2020

Centril commented Jan 12, 2020

rustonaut Sep 21, 2020 •

edited

Loading