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

Use std::ptr::null_mut::<T>() instead of 0 as *mut T #202

Open
samuela opened this issue Nov 18, 2019 · 7 comments
Open

Use std::ptr::null_mut::<T>() instead of 0 as *mut T #202

samuela opened this issue Nov 18, 2019 · 7 comments

Comments

@samuela
Copy link
Contributor

samuela commented Nov 18, 2019

c2rust current transpiles NULL pointers as 0 as *mut T, but it would be a bit cleaner and more rust-y to instead produce std::ptr::null_mut::<T>().

@ahomescu
Copy link
Contributor

ahomescu commented Nov 18, 2019

This keeps coming up in our internal discussion occasionally, but there are some corner cases where they're not equivalent (I think in static initializers, I have to look up the original discussion for details). Meanwhile, you can do the following with c2rust-refactor:

rewrite_expr '0 as *const $t:Ty' 'std::ptr::null()' ;
rewrite_expr '0 as *mut $t:Ty' 'std::ptr::null_mut()' ;

@TheDan64
Copy link
Contributor

I also recall the two not being equivalent. I wonder if it was just because it wasn't a const fn at the time? Maybe it'd be viable now, but I'm not certain

@ahomescu
Copy link
Contributor

I checked last night, null and null_mut have been const fns since 2015.

@TheDan64
Copy link
Contributor

Hmm. Guess it wasn't that, lol

@afetisov
Copy link
Contributor

One reason why they wouldn't be equivalent is static promotion. 0 as *mut T can be directly promoted to consts and statics. On the other hand, ptr::null_mut is a const fn, and const fn calls in general cannot be safely promoted. The specific functions ptr::null and ptr::null_mut are rustc_promotable, but this may change in the future and is harder to track for the refactoring tools. I think it's unlikely that the transpiled code will depend on static promotion (it's more of a syntactic sugar), but avoiding such code is still reasonable.

Also, being function calls they may be harder in general to analyze. Knowing that they are really null requires either interprocedural analysis (which may be intractable in general) or special-casing.

Finally, these function calls shouldn't even normally appear in idiomatic Rust code. They are generally an artifact of partial initialization, redundant default-initialization (which should be folded away) or nullable pointers (which are better expressed as Option<&T> or Option<NotNull<T>>). For this reason optimizing for their readability seems counterproductive, they should be optimized for easy elimination.

@kkysen kkysen added enhancement New feature or request wontfix This will not be worked on and removed enhancement New feature or request labels Jun 29, 2022
@kkysen kkysen closed this as completed Jun 29, 2022
@kkysen kkysen closed this as not planned Won't fix, can't repro, duplicate, stale Jun 29, 2022
@kkysen
Copy link
Contributor

kkysen commented Aug 26, 2022

I think this should be re-considered, as 0 as *const T and null() are no longer equivalent under strict provenance. We should prefer null() and null_mut() where possible to avoid the int-to-ptr casts. For example, miri warns about this, and can't catch all UB under permissive provenance. Where we do need an int-to-ptr cast, we should use the ptr::from_exposed_addr API to make this explicit, not just an as cast.

@kkysen kkysen reopened this Aug 26, 2022
@kkysen kkysen removed the wontfix This will not be worked on label Oct 10, 2022
@LegNeato
Copy link
Contributor

LegNeato commented Nov 2, 2022

FYI, this also makes clippy complain when run on a transpiled codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants