rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.47k stars 12.73k forks source link

Refactor improper_ctypes to separate "UCG questions" from linting decisions #72774

Open hanna-kruppe opened 4 years ago

hanna-kruppe commented 4 years ago

(Elaborating on https://github.com/rust-lang/rust/issues/66220#issuecomment-557537105)

I think that the current state of improper_ctypes is difficult to maintain and extend -- especially once we add more lints that are similar but distinct -- because it's a big pile of mud that interleaving several different concerns: UCG-ish questions about ABI and layout guarantees, plus value judgements about how these interact with "proper" FFI, plus diagnostics code and suggestions. I imagine (but have not worked out the details) that we could improve this by separating this code into two "layers":

I think such an organization would make it clearer what are the language-level guarantees are (including making it easier to audit and evolve) vs what's just a choice or limitation of some lint. More importantly, it would also enable us to implement (without lots of duplication and without making the visitor even more of a big ball of mud) a broader variety of lints that need similar information. For example:

workingjubilee commented 3 weeks ago

from @RalfJung in #130628

The logic here now does not quite match what has been FCP'd, so the lint will fire on some cases where it doesn't have to fire -- specifically if the "other" variant (the one that does not contain the non-null type) has more than one field but all fields are stably 1-ZST.

This PR can land without fixing that though, it just means the lint can be improved, but that's already well-established. ;)

(I also find is_niche_optimization_candidate a very confusing name for "test if this is a 1-ZST and will remain a 1-ZST under semver updates". This type doesn't even have any niches... at least the function has a doc comment clearly describing what it does.)