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

Remove rustc_middle::mir::interpret::CheckInAllocMsg::NullPointerTest #84903

Merged
merged 1 commit into from May 5, 2021
Merged

Remove rustc_middle::mir::interpret::CheckInAllocMsg::NullPointerTest #84903

merged 1 commit into from May 5, 2021

Conversation

ghost
Copy link

@ghost ghost commented May 4, 2021

Removing it per #84842 (comment): it's a dead enum variant.

Note that PointerArithmeticTest also seems dead:

$ rg -F PointerArithmeticTest -C5
compiler/rustc_middle/src/mir/interpret/error.rs
169-
170-/// Details of why a pointer had to be in-bounds.
171-#[derive(Debug, Copy, Clone, TyEncodable, TyDecodable, HashStable)]
172-pub enum CheckInAllocMsg {
173-    MemoryAccessTest,
174:    PointerArithmeticTest,
175-    InboundsTest,
176-}
177-
178-impl fmt::Display for CheckInAllocMsg {
179-    /// When this is printed as an error the context looks like this
--
182-        write!(
183-            f,
184-            "{}",
185-            match *self {
186-                CheckInAllocMsg::MemoryAccessTest => "memory access",
187:                CheckInAllocMsg::PointerArithmeticTest => "pointer arithmetic",
188-                CheckInAllocMsg::InboundsTest => "inbounds test",
189-            }
190-        )
191-    }
192-}

Not sure if that is also desirable to be removed, however.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 4, 2021
@ghost ghost mentioned this pull request May 4, 2021
@RalfJung
Copy link
Member

RalfJung commented May 4, 2021

r? @RalfJung

@RalfJung
Copy link
Member

RalfJung commented May 4, 2021

Note that PointerArithmeticTest also seems dead:

Hm, this is strange... it should be used by this code:

CheckInAllocMsg::InboundsTest,

but it looks like at some point we switched to the wrong variant...

@RalfJung
Copy link
Member

RalfJung commented May 4, 2021

Ah wait... we still have "inbounds" vs "memory access". So we still have that distinction.
Now I wonder what "pointer arithmetic" was for.

Arguably, "InboundsTest" is a bad name -- "Details of why a pointer had to be in-bounds" should not be answered with "because it has to be inbounds". I will need to check what InboundsTest is used for though before we make further changes here.

@RalfJung
Copy link
Member

RalfJung commented May 4, 2021

Meanwhile, this seems like a reasonable step.
@bors r+ rollup

@bors
Copy link
Contributor

bors commented May 4, 2021

📌 Commit ee7a6c6 has been approved by RalfJung

@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 May 4, 2021
RalfJung added a commit to RalfJung/rust that referenced this pull request May 5, 2021
…RalfJung

Remove `rustc_middle::mir::interpret::CheckInAllocMsg::NullPointerTest`

Removing it per rust-lang#84842 (comment): it's a dead enum variant.

Note that `PointerArithmeticTest` also seems dead:
```
$ rg -F PointerArithmeticTest -C5
compiler/rustc_middle/src/mir/interpret/error.rs
169-
170-/// Details of why a pointer had to be in-bounds.
171-#[derive(Debug, Copy, Clone, TyEncodable, TyDecodable, HashStable)]
172-pub enum CheckInAllocMsg {
173-    MemoryAccessTest,
174:    PointerArithmeticTest,
175-    InboundsTest,
176-}
177-
178-impl fmt::Display for CheckInAllocMsg {
179-    /// When this is printed as an error the context looks like this
--
182-        write!(
183-            f,
184-            "{}",
185-            match *self {
186-                CheckInAllocMsg::MemoryAccessTest => "memory access",
187:                CheckInAllocMsg::PointerArithmeticTest => "pointer arithmetic",
188-                CheckInAllocMsg::InboundsTest => "inbounds test",
189-            }
190-        )
191-    }
192-}
```
Not sure if that is also desirable to be removed, however.
RalfJung added a commit to RalfJung/rust that referenced this pull request May 5, 2021
…RalfJung

Remove `rustc_middle::mir::interpret::CheckInAllocMsg::NullPointerTest`

Removing it per rust-lang#84842 (comment): it's a dead enum variant.

Note that `PointerArithmeticTest` also seems dead:
```
$ rg -F PointerArithmeticTest -C5
compiler/rustc_middle/src/mir/interpret/error.rs
169-
170-/// Details of why a pointer had to be in-bounds.
171-#[derive(Debug, Copy, Clone, TyEncodable, TyDecodable, HashStable)]
172-pub enum CheckInAllocMsg {
173-    MemoryAccessTest,
174:    PointerArithmeticTest,
175-    InboundsTest,
176-}
177-
178-impl fmt::Display for CheckInAllocMsg {
179-    /// When this is printed as an error the context looks like this
--
182-        write!(
183-            f,
184-            "{}",
185-            match *self {
186-                CheckInAllocMsg::MemoryAccessTest => "memory access",
187:                CheckInAllocMsg::PointerArithmeticTest => "pointer arithmetic",
188-                CheckInAllocMsg::InboundsTest => "inbounds test",
189-            }
190-        )
191-    }
192-}
```
Not sure if that is also desirable to be removed, however.
bors added a commit to rust-lang-ci/rust that referenced this pull request May 5, 2021
Rollup of 11 pull requests

Successful merges:

 - rust-lang#83553 (Update `ptr` docs with regards to `ptr::addr_of!`)
 - rust-lang#84183 (Update RELEASES.md for 1.52.0)
 - rust-lang#84709 (Add doc alias for `chdir` to `std::env::set_current_dir`)
 - rust-lang#84803 (Reduce duplication in `impl_dep_tracking_hash` macros)
 - rust-lang#84808 (Account for unsatisfied bounds in E0599)
 - rust-lang#84843 (use else if in std library )
 - rust-lang#84865 (rustbuild: Pass a `threads` flag that works to windows-gnu lld)
 - rust-lang#84878 (Clarify documentation for `[T]::contains`)
 - rust-lang#84882 (platform-support: Center the contents of the `std` and `host` columns)
 - rust-lang#84903 (Remove `rustc_middle::mir::interpret::CheckInAllocMsg::NullPointerTest`)
 - rust-lang#84913 (Do not ICE on invalid const param)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2c7bf41 into rust-lang:master May 5, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 5, 2021
@ghost ghost deleted the dead-check-in-alloc-msg branch May 18, 2021 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants