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

Replace 'NULL' with 'null' #84842

Merged
merged 1 commit into from
May 3, 2021
Merged

Replace 'NULL' with 'null' #84842

merged 1 commit into from
May 3, 2021

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
Collaborator

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
Contributor

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
Contributor

bors commented May 3, 2021

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

@bors
Copy link
Contributor

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
@bors bors mentioned this pull request May 3, 2021
Merged
@ghost ghost mentioned this pull request May 4, 2021
Merged
RalfJung
RalfJung reviewed May 4, 2021
View reviewed changes
compiler/rustc_middle/src/mir/interpret/error.rs
@@ -309,7 +309,7 @@ impl fmt::Display for UndefinedBehaviorInfo<'_> {
allocation_size.bytes()
),
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
@bors
Auto merge of #1788 - hyd-dev:rustup, r=RalfJung
67c04af 
`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
@ghost ghost mentioned this pull request May 4, 2021
Merged
RalfJung added a commit to RalfJung/rust that referenced this pull request May 5, 2021
@RalfJung
Rollup merge of rust-lang#84903 - hyd-dev:dead-check-in-alloc-msg, r=…
1dbc2dc 
…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
Rollup merge of rust-lang#84903 - hyd-dev:dead-check-in-alloc-msg, r=…
c340bf9 
…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
Rollup merge of rust-lang#84903 - hyd-dev:dead-check-in-alloc-msg, r=…
2c7bf41 
…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.
@LeSeulArtichaut LeSeulArtichaut mentioned this pull request May 24, 2021
Merged
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request May 24, 2021
@GuillaumeGomez
Rollup merge of rust-lang#85628 - LeSeulArtichaut:thir-null-lowercase…
16fe40d 
…, 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).
@ChrisDenton ChrisDenton mentioned this pull request Jun 24, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RalfJung RalfJung RalfJung left review comments

Assignees

@dtolnay dtolnay

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
Milestone
1.54.0
Development

Successfully merging this pull request may close these issues.
7 participants
@blkerby @rust-highfive @joshtriplett @bors @RalfJung @dtolnay @rustbot