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

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

Open
hanna-kruppe opened this issue May 30, 2020 · 1 comment
Assignees
Labels
A-FFI Area: Foreign function interface (FFI) A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. L-improper_ctypes Lint: improper_ctypes T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented May 30, 2020

(Elaborating on #66220 (comment))

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":

  • A module (some queries?) answering clean-cut spec questions such as: does this type have a defined memory layout? Defined ABI? If not, why not? etc.
  • Several lints that use the facts from that module as basis for emitting warnings, depending on what the lint is targeted at.

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:

@jonas-schievink jonas-schievink added A-FFI Area: Foreign function interface (FFI) A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 30, 2020
@workingjubilee workingjubilee added the L-improper_ctypes Lint: improper_ctypes label Sep 20, 2024
@workingjubilee workingjubilee self-assigned this Oct 19, 2024
@workingjubilee
Copy link
Member

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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-FFI Area: Foreign function interface (FFI) A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. L-improper_ctypes Lint: improper_ctypes T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants