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.

Sign up for GitHub

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

Jump to bottom

remove const_raw_ptr_to_usize_cast feature #87020

Merged
merged 3 commits into from
Jul 10, 2021
Merged

remove const_raw_ptr_to_usize_cast feature #87020

merged 3 commits into from
Jul 10, 2021

Conversation

RalfJung
Copy link
Member

This feature currently has the strange status of "const-only unsafe", which was an experiment that we no longer think is a good idea. We need to find better ways to enable things like "messing with the low bits of a pointer" during CTFE.

r? @oli-obk

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 10, 2021
RalfJung
RalfJung commented Jul 10, 2021
View reviewed changes
compiler/rustc_mir/src/transform/check_consts/ops.rs
@@ -397,9 +397,13 @@ impl NonConstOp for PanicNonStr {
}
}

/// Comparing raw pointers for equality.
/// Not currently intended to ever be allowed, even behind a feature gate: operation depends on
/// allocation base addresses that are not known at compile-time.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any other "ops" that should get a comment like this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well... there are similarly unallowable things. thread local statics, static mut, inline asm, ... they aren't really different, just more obvious?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm yeah those are less subtle.

RalfJung
RalfJung commented Jul 10, 2021
View reviewed changes
src/test/ui/consts/issue-52432.rs
//~| ERROR: type annotations needed
[(); &(static || {}) as *const _ as usize];
//~^ ERROR: closures cannot be static
//~| ERROR evaluation of constant value failed
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this has long stopped testing the right thing... looks like someone adjusted the expected errors without realizing that this makes the test pointless.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, looks like this was actually about testing an ICE: #66331.
But with the feature gate gone I don't think keeping the test makes a lot of sense.

@RalfJung RalfJung force-pushed the const_raw_ptr_to_usize_cast branch from 368cf15 to 1a40c3d Compare July 10, 2021 09:50
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@RalfJung RalfJung force-pushed the const_raw_ptr_to_usize_cast branch from 1a40c3d to 5f0dd6d Compare July 10, 2021 10:09
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@RalfJung RalfJung force-pushed the const_raw_ptr_to_usize_cast branch from 8c70d09 to f5094aa Compare July 10, 2021 11:48
@oli-obk
Copy link
Contributor

oli-obk commented Jul 10, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Jul 10, 2021

📌 Commit f5094aa has been approved by oli-obk

@bors 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 Jul 10, 2021
@JohnTitor JohnTitor mentioned this pull request Jul 10, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 10, 2021
@bors
Auto merge of rust-lang#87029 - JohnTitor:rollup-0yapv7z, r=JohnTitor
a31431f 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#87006 (Revert the revert of renaming traits::VTable to ImplSource)
 - rust-lang#87011 (avoid reentrant lock acquire when ThreadIds run out)
 - rust-lang#87013 (Fix several ICEs related to malformed `#[repr(...)]` attributes)
 - rust-lang#87020 (remove const_raw_ptr_to_usize_cast feature)
 - rust-lang#87028 (Fix type: `'satic` -> `'static`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ad2a0fc into rust-lang:master Jul 10, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jul 10, 2021
@RalfJung RalfJung deleted the const_raw_ptr_to_usize_cast branch July 11, 2021 12:04
@timothee-haudebourg timothee-haudebourg mentioned this pull request Jul 28, 2021
Closed
@RalfJung RalfJung mentioned this pull request Sep 16, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk left review comments

Assignees

@oli-obk oli-obk

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.55.0
Development

Successfully merging this pull request may close these issues.
6 participants
@RalfJung @rust-log-analyzer @oli-obk @bors @rust-highfive @rustbot