Skip to content

Conversation

@blkerby
Copy link
Contributor

@blkerby blkerby commented May 2, 2021

This replaces occurrences of "NULL" with "null" in docs, comments, and compiler error/lint messages. This is for the sake of consistency, as the lowercase "null" is already the dominant form in Rust. The all-caps NULL looks like the C macro (or SQL keyword), which seems out of place in a Rust context, given that NULL does not exist in the Rust language or standard library (instead having ptr::null()).

@rust-highfive
Copy link
Contributor

r? @dtolnay

(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 2, 2021
@joshtriplett
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented May 3, 2021

📌 Commit 6679f5c has been approved by joshtriplett

@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 3, 2021
@bors
Copy link
Collaborator

bors commented May 3, 2021

⌛ Testing commit 6679f5c with merge 2428cc4...

@bors
Copy link
Collaborator

bors commented May 3, 2021

☀️ Test successful - checks-actions
Approved by: joshtriplett
Pushing 2428cc4 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 3, 2021
@bors bors merged commit 2428cc4 into rust-lang:master May 3, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 3, 2021
),
DanglingIntPointer(_, CheckInAllocMsg::NullPointerTest) => {
write!(f, "NULL pointer is not allowed for this operation")
write!(f, "null pointer is not allowed for this operation")
Copy link
Member

Choose a reason for hiding this comment

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

Hm, looks like we don't have any test covering this output... I think CheckInAllocMsg::NullPointerTest might just be a dead enum variant.
@hyd-dev any chance you could prepare a PR to remove it?

Copy link

Choose a reason for hiding this comment

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

Sure, I'll have a try!

Copy link

Choose a reason for hiding this comment

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

Created #84903 to remove it.

bors added a commit to rust-lang/miri that referenced this pull request May 4, 2021
`encountered a NULL reference` -> `encountered a null reference`

It's changed from "NULL" to "null" (probably by rust-lang/rust#84842) in `rustc`, and causing some test failures:
https://github.com/rust-lang/miri/runs/2498333632#step:8:640
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.
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.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request May 24, 2021
…, r=estebank

Replace more "NULL" with "null"

Error messages in THIR unsafeck still contain "NULL", make them lowercase to be consistent with MIR unsafeck (cc rust-lang#84842).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-by-bors This PR was explicitly merged by bors. 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.

7 participants