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

Redefine dropck in terms of bound-like constructs #563

Closed
2 of 3 tasks
SoniEx2 opened this issue Oct 23, 2022 · 7 comments
Closed
2 of 3 tasks

Redefine dropck in terms of bound-like constructs #563

SoniEx2 opened this issue Oct 23, 2022 · 7 comments
Labels
I-types-nominated major-change A proposal to make a major change to rustc major-change-accepted A major change proposal that was accepted T-compiler Add this label so rfcbot knows to poll the compiler team

Comments

@SoniEx2
Copy link

SoniEx2 commented Oct 23, 2022

Proposal

The main goal of this proposal is to redefine dropck in terms of bound-like constructs. We believe this to be a good first step towards deprecating may_dangle (and hopefully generally avoiding issues like rust-lang/rust#99413 entirely) and possibly stabilizing something similar in the future, tho our main goal is to see if it's even possible in the first place.

We believe it is sufficient to have 3 kinds of dropck-related bounds: "drop" bounds, "borrow" bounds, with these being inferred by default for non-Drop types, and "access" bounds, the default for Drop types. A "drop" bound indicates values of the given type may be dropped, and a "borrow" bound indicates values of the given type are borrowed and will not be dropped.

To match existing rules, Copy types are generally composed exclusively of "borrow" bounds: this includes shared references and pointers. The exception to this is PhantomData, which has a "drop" bound instead. (N.B. This is overly restrictive, but matches existing rules, and is observable on stable. It might be worth reconsidering/changing this after a crater run and removal of may_dangle however.) On the other hand, the 2 special non-Copy types are mutable references and ManuallyDrop, both of which have a "borrow" bound. Everything else follows inference from these and Drop impls, which by default have only "access" bounds (the existing may_dangle mechanism should be deprecated, but before being removed and replaced with "something better", it can simply be redefined to not apply "access" bounds to the relevant parameter, and do the inference stuff as described here; we can figure out "something better" another time).

The inference rules for these dropck bounds are pretty simple: there's a hierarchy of dropck bounds ("access" is stronger than "drop" is stronger than "borrow"), and when a generic type contains generic fields, it gains the weakest possible bound that satisfies these. For example:

struct Foo<T> {
  drop_bound: PhantomData<T>,
  borrow_bound: *const T,
}

causes T to have a "drop" bound, because that's the weakest bound satisfying both PhantomData<T> and *const T, meanwhile:

struct Foo<T> {
  borrow_bound: *const T,
}

only gives T a "borrow" bound, and finally:

struct Foo<T: Display> {
  access_bound: PrintOnDrop<T>,
}

struct PrintOnDrop<T: Display>(T);
impl<T: Display> Drop for PrintOnDrop<T> {
  fn drop(&mut self) {
    println!("{}", self.0);
  }
}

gives T an "access" bound.

We believe these are sufficient to capture all necessary dropck semantics and provide a pathway for removal of may_dangle. This MCP does not propose any interactions between these bounds and function code at this point, in other words, it doesn't apply any restrictions to Drop impls or to drop_in_place, tho that would certainly be an obvious next step. The main question is whether or not this actually does capture all the necessary dropck semantics and whether it can be built upon or should be scrapped.

Mentors or Reviewers

Process

The main points of the Major Change Process are as follows:

  • File an issue describing the proposal.
  • A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
    • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
    • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
  • Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

@SoniEx2 SoniEx2 added major-change A proposal to make a major change to rustc T-compiler Add this label so rfcbot knows to poll the compiler team labels Oct 23, 2022
@rustbot
Copy link
Collaborator

rustbot commented Oct 23, 2022

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

cc @rust-lang/compiler @rust-lang/compiler-contributors

@rustbot rustbot added the to-announce Announce this issue on triage meeting label Oct 23, 2022
@jackh726
Copy link
Member

This is about the semantics of the language, and not the implementation of the compiler. This is very safely in T-lang territory. I'm not sure what the current "official" process for T-lang feature proposals right now, other than a formal RFC. You'd probably have the most luck opening a dedicated thread on zulip, making a post on internals, or opening a formal RFC.

Given this isn't compiler team territory, I'm going to close this.

@jackh726 jackh726 closed this as not planned Won't fix, can't repro, duplicate, stale Oct 23, 2022
@SoniEx2
Copy link
Author

SoniEx2 commented Oct 23, 2022

no, the semantics already exist. this behaviour is already observable on stable, it's just not formally defined nor implemented in this specific way.

@pnkfelix
Copy link
Member

I’m willing to consider whether the proposal falls under a compiler implementation detail or not.

(I suspect that there will inevitably be divergences between the operational behavior of the two forms, and then deciding whether those divergences are allowed according to the language definition is the thing that would be under the purview of the Lang team.)

@pnkfelix pnkfelix reopened this Oct 23, 2022
@rustbot
Copy link
Collaborator

rustbot commented Oct 23, 2022

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

cc @rust-lang/compiler @rust-lang/compiler-contributors

@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Nov 3, 2022
@pnkfelix
Copy link
Member

@rustbot second

I'm planning to carry on technical conversation in the zulip topic, but overall I think something like this seems like a good experiment to push on.

@rustbot rustbot added the final-comment-period The FCP has started, most (if not all) team members are in agreement label Nov 17, 2022
@apiraino
Copy link
Contributor

@rustbot label -final-comment-period +major-change-accepted

@rustbot rustbot added major-change-accepted A major change proposal that was accepted to-announce Announce this issue on triage meeting and removed final-comment-period The FCP has started, most (if not all) team members are in agreement labels Nov 29, 2022
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-types-nominated major-change A proposal to make a major change to rustc major-change-accepted A major change proposal that was accepted T-compiler Add this label so rfcbot knows to poll the compiler team
Projects
None yet
Development

No branches or pull requests

5 participants