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

lub: don't bail out due to empty binders #97867

Merged
merged 2 commits into from
Jun 22, 2022
Merged

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Jun 8, 2022

allows for the following to compile. The equivalent code using struct Wrapper<'upper>(fn(&'upper ()); already compiles on stable.

let _: fn(&'upper ()) = match v {
    true => lt_in_fn::<'a>(),
    false => lt_in_fn::<'b>(),
};

see https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=7034a677190110941223cafac6632f70 for a complete example

r? @rust-lang/types

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 8, 2022
@rust-highfive
Copy link
Collaborator

r? @estebank

(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 Jun 8, 2022
@jackh726
Copy link
Member

jackh726 commented Jun 8, 2022

r? @jackh726

@rust-highfive rust-highfive assigned jackh726 and unassigned estebank Jun 8, 2022
@jackh726
Copy link
Member

jackh726 commented Jun 8, 2022

This does allow more behavior, but I don't think there's anything that we don't want from a more "complete" system. Right?

@lcnr
Copy link
Contributor Author

lcnr commented Jun 8, 2022

I think the only way this change could be a future compat concern is if we infer the
LUB of fn(fn(&'a ())) and fn(fn(&'b ())) to fn(for<'c> fn(&'c ())) in the future, at which point something like the following would break.

// if we infer `x` to be higher ranked in the future,
// this would cause a type error.
let x = match v {
    true => lt_in_fn_fn::<'a>(),
    false => lt_in_fn_fn::<'b>(),
};

let y: fn(fn(&'lower ())) = x;

Adding that as a test.

@jackh726 jackh726 added T-types Relevant to the types team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 8, 2022
@jackh726

This comment was marked as outdated.

@jackh726

This comment was marked as outdated.

@compiler-errors

This comment was marked as outdated.

@jackh726

This comment was marked as outdated.

@compiler-errors
Copy link
Member

rust-lang/team#778

@compiler-errors
Copy link
Member

Let's try this again:

@rfcbot merge

Do we want to commit to not inferring the LUB of fn(fn(&'a ())) and fn(fn(&'b ())) to fn(for<'c> fn(&'c ())) in the future?

@rfcbot
Copy link

rfcbot commented Jun 9, 2022

Team member @compiler-errors has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jun 9, 2022
@lcnr
Copy link
Contributor Author

lcnr commented Jun 9, 2022

(as a side note, I don't expect this to be frequent enough that it would ever actually block us from inferring the lub to be higher ranked. People don't tend to have nested function pointers)

@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Jun 10, 2022
@rfcbot
Copy link

rfcbot commented Jun 10, 2022

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Jun 10, 2022
@nikomatsakis
Copy link
Contributor

@rfcbot reviewed

For future references, we had a detailed discussion in the types team meeting, where we went over what was being proposed here, how it fit historically, and how it relates to the new subtyping algorithm that we are developing in a-mir-formality.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jun 20, 2022
@rfcbot
Copy link

rfcbot commented Jun 20, 2022

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label Jun 20, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Jun 21, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Jun 21, 2022

📌 Commit 0667b00 has been approved by oli-obk

@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 Jun 21, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jun 21, 2022
lub: don't bail out due to empty binders

allows for the following to compile. The equivalent code using `struct Wrapper<'upper>(fn(&'upper ());` already compiles on stable.
```rust
let _: fn(&'upper ()) = match v {
    true => lt_in_fn::<'a>(),
    false => lt_in_fn::<'b>(),
};
```
see https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=7034a677190110941223cafac6632f70 for a complete example

r? `@rust-lang/types`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jun 21, 2022
lub: don't bail out due to empty binders

allows for the following to compile. The equivalent code using `struct Wrapper<'upper>(fn(&'upper ());` already compiles on stable.
```rust
let _: fn(&'upper ()) = match v {
    true => lt_in_fn::<'a>(),
    false => lt_in_fn::<'b>(),
};
```
see https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=7034a677190110941223cafac6632f70 for a complete example

r? ``@rust-lang/types``
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 22, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#97867 (lub: don't bail out due to empty binders)
 - rust-lang#98099 (interpret: convert_tag_add_extra: allow tagger to raise errors)
 - rust-lang#98199 (Move some tests to more reasonable directories)
 - rust-lang#98334 (Add a full regression test for rust-lang#73727)
 - rust-lang#98336 (Remove the unused-`#[doc(hidden)]` logic from the `unused_attributes` lint)
 - rust-lang#98344 (This comment is out dated and misleading, the arm is about TAITs)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fdeecb2 into rust-lang:master Jun 22, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 22, 2022
@lcnr lcnr deleted the lub-binder branch June 22, 2022 15:09
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Sep 8, 2022
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Oct 10, 2022
Pkgsrc changes:
 * Adjust patches as needed & checksum updates.

Upstream changes:

Version 1.63.0 (2022-08-11)
==========================

Language
--------
- [Remove migrate borrowck mode for pre-NLL errors.][95565]
- [Modify MIR building to drop repeat expressions with length zero.][95953]
- [Remove label/lifetime shadowing warnings.][96296]
- [Allow explicit generic arguments in the presence of `impl Trait` args.]
  [96868]
- [Make `cenum_impl_drop_cast` warnings deny-by-default.][97652]
- [Prevent unwinding when `-C panic=abort` is used regardless of
  declared ABI.][96959]
- [lub: don't bail out due to empty binders.][97867]

Compiler
--------
- [Stabilize the `bundle` native library modifier,][95818] also removing the
  deprecated `static-nobundle` linking kind.
- [Add Apple WatchOS compile targets\*.][95243]
- [Add a Windows application manifest to rustc-main.][96737]

\* Refer to Rust's [platform support page][platform-support-doc] for more
   information on Rust's tiered platform support.

Libraries
---------
- [Implement `Copy`, `Clone`, `PartialEq` and `Eq` for
  `core::fmt::Alignment`.][94530]
- [Extend `ptr::null` and `null_mut` to all thin (including extern)
  types.][94954]
- [`impl Read and Write for VecDeque<u8>`.][95632]
- [STD support for the Nintendo 3DS.][95897]
- [Make write/print macros eagerly drop temporaries.][96455]
- [Implement internal traits that enable `[OsStr]::join`.][96881]
- [Implement `Hash` for `core::alloc::Layout`.][97034]
- [Add capacity documentation for `OsString`.][97202]
- [Put a bound on collection misbehavior.][97316]
- [Make `std::mem::needs_drop` accept `?Sized`.][97675]
- [`impl Termination for Infallible` and then make the `Result` impls
  of `Termination` more generic.][97803]
- [Document Rust's stance on `/proc/self/mem`.][97837]

Stabilized APIs
---------------

- [`array::from_fn`]
- [`Box::into_pin`]
- [`BinaryHeap::try_reserve`]
- [`BinaryHeap::try_reserve_exact`]
- [`OsString::try_reserve`]
- [`OsString::try_reserve_exact`]
- [`PathBuf::try_reserve`]
- [`PathBuf::try_reserve_exact`]
- [`Path::try_exists`]
- [`Ref::filter_map`]
- [`RefMut::filter_map`]
- [`NonNull::<[T]>::len`][`NonNull::<slice>::len`]
- [`ToOwned::clone_into`]
- [`Ipv6Addr::to_ipv4_mapped`]
- [`unix::io::AsFd`]
- [`unix::io::BorrowedFd<'fd>`]
- [`unix::io::OwnedFd`]
- [`windows::io::AsHandle`]
- [`windows::io::BorrowedHandle<'handle>`]
- [`windows::io::OwnedHandle`]
- [`windows::io::HandleOrInvalid`]
- [`windows::io::HandleOrNull`]
- [`windows::io::InvalidHandleError`]
- [`windows::io::NullHandleError`]
- [`windows::io::AsSocket`]
- [`windows::io::BorrowedSocket<'handle>`]
- [`windows::io::OwnedSocket`]
- [`thread::scope`]
- [`thread::Scope`]
- [`thread::ScopedJoinHandle`]

These APIs are now usable in const contexts:

- [`array::from_ref`]
- [`slice::from_ref`]
- [`intrinsics::copy`]
- [`intrinsics::copy_nonoverlapping`]
- [`<*const T>::copy_to`]
- [`<*const T>::copy_to_nonoverlapping`]
- [`<*mut T>::copy_to`]
- [`<*mut T>::copy_to_nonoverlapping`]
- [`<*mut T>::copy_from`]
- [`<*mut T>::copy_from_nonoverlapping`]
- [`str::from_utf8`]
- [`Utf8Error::error_len`]
- [`Utf8Error::valid_up_to`]
- [`Condvar::new`]
- [`Mutex::new`]
- [`RwLock::new`]

Cargo
-----
- [Stabilize the `--config path` command-line argument.][cargo/10755]
- [Expose rust-version in the environment as
  `CARGO_PKG_RUST_VERSION`.][cargo/10713]

Compatibility Notes
-------------------

- [`#[link]` attributes are now checked more strictly,][96885]
  which may introduce errors for invalid attribute arguments that
  were previously ignored.

Internal Changes
----------------

These changes provide no direct user facing benefits, but represent
significant improvements to the internals and overall performance
of rustc and related tools.

- [Prepare Rust for LLVM opaque pointers.][94214]

[94214]: rust-lang/rust#94214
[94530]: rust-lang/rust#94530
[94954]: rust-lang/rust#94954
[95243]: rust-lang/rust#95243
[95565]: rust-lang/rust#95565
[95632]: rust-lang/rust#95632
[95818]: rust-lang/rust#95818
[95897]: rust-lang/rust#95897
[95953]: rust-lang/rust#95953
[96296]: rust-lang/rust#96296
[96455]: rust-lang/rust#96455
[96737]: rust-lang/rust#96737
[96868]: rust-lang/rust#96868
[96881]: rust-lang/rust#96881
[96885]: rust-lang/rust#96885
[96959]: rust-lang/rust#96959
[97034]: rust-lang/rust#97034
[97202]: rust-lang/rust#97202
[97316]: rust-lang/rust#97316
[97652]: rust-lang/rust#97652
[97675]: rust-lang/rust#97675
[97803]: rust-lang/rust#97803
[97837]: rust-lang/rust#97837
[97867]: rust-lang/rust#97867
[cargo/10713]: rust-lang/cargo#10713
[cargo/10755]: rust-lang/cargo#10755

[`array::from_fn`]: https://doc.rust-lang.org/stable/std/array/fn.from_fn.html
[`Box::into_pin`]: https://doc.rust-lang.org/stable/std/boxed/struct.Box.html#method.into_pin
[`BinaryHeap::try_reserve_exact`]: https://doc.rust-lang.org/stable/alloc/collections/binary_heap/struct.BinaryHeap.html#method.try_reserve_exact
[`BinaryHeap::try_reserve`]: https://doc.rust-lang.org/stable/std/collections/struct.BinaryHeap.html#method.try_reserve
[`OsString::try_reserve`]: https://doc.rust-lang.org/stable/std/ffi/struct.OsString.html#method.try_reserve
[`OsString::try_reserve_exact`]: https://doc.rust-lang.org/stable/std/ffi/struct.OsString.html#method.try_reserve_exact
[`PathBuf::try_reserve`]: https://doc.rust-lang.org/stable/std/path/struct.PathBuf.html#method.try_reserve
[`PathBuf::try_reserve_exact`]: https://doc.rust-lang.org/stable/std/path/struct.PathBuf.html#method.try_reserve_exact
[`Path::try_exists`]: https://doc.rust-lang.org/stable/std/path/struct.Path.html#method.try_exists
[`Ref::filter_map`]: https://doc.rust-lang.org/stable/std/cell/struct.Ref.html#method.filter_map
[`RefMut::filter_map`]: https://doc.rust-lang.org/stable/std/cell/struct.RefMut.html#method.filter_map
[`NonNull::<slice>::len`]: https://doc.rust-lang.org/stable/std/ptr/struct.NonNull.html#method.len
[`ToOwned::clone_into`]: https://doc.rust-lang.org/stable/std/borrow/trait.ToOwned.html#method.clone_into
[`Ipv6Addr::to_ipv4_mapped`]: https://doc.rust-lang.org/stable/std/net/struct.Ipv6Addr.html#method.to_ipv4_mapped
[`unix::io::AsFd`]: https://doc.rust-lang.org/stable/std/os/unix/io/trait.AsFd.html
[`unix::io::BorrowedFd<'fd>`]: https://doc.rust-lang.org/stable/std/os/unix/io/struct.BorrowedFd.html
[`unix::io::OwnedFd`]: https://doc.rust-lang.org/stable/std/os/unix/io/struct.OwnedFd.html
[`windows::io::AsHandle`]: https://doc.rust-lang.org/stable/std/os/windows/io/trait.AsHandle.html
[`windows::io::BorrowedHandle<'handle>`]: https://doc.rust-lang.org/stable/std/os/windows/io/struct.BorrowedHandle.html
[`windows::io::OwnedHandle`]: https://doc.rust-lang.org/stable/std/os/windows/io/struct.OwnedHandle.html
[`windows::io::HandleOrInvalid`]: https://doc.rust-lang.org/stable/std/os/windows/io/struct.HandleOrInvalid.html
[`windows::io::HandleOrNull`]: https://doc.rust-lang.org/stable/std/os/windows/io/struct.HandleOrNull.html
[`windows::io::InvalidHandleError`]: https://doc.rust-lang.org/stable/std/os/windows/io/struct.InvalidHandleError.html
[`windows::io::NullHandleError`]: https://doc.rust-lang.org/stable/std/os/windows/io/struct.NullHandleError.html
[`windows::io::AsSocket`]: https://doc.rust-lang.org/stable/std/os/windows/io/trait.AsSocket.html
[`windows::io::BorrowedSocket<'handle>`]: https://doc.rust-lang.org/stable/std/os/windows/io/struct.BorrowedSocket.html
[`windows::io::OwnedSocket`]: https://doc.rust-lang.org/stable/std/os/windows/io/struct.OwnedSocket.html
[`thread::scope`]: https://doc.rust-lang.org/stable/std/thread/fn.scope.html
[`thread::Scope`]: https://doc.rust-lang.org/stable/std/thread/struct.Scope.html
[`thread::ScopedJoinHandle`]: https://doc.rust-lang.org/stable/std/thread/struct.ScopedJoinHandle.html

[`array::from_ref`]: https://doc.rust-lang.org/stable/std/array/fn.from_ref.html
[`slice::from_ref`]: https://doc.rust-lang.org/stable/std/slice/fn.from_ref.html
[`intrinsics::copy`]: https://doc.rust-lang.org/stable/std/intrinsics/fn.copy.html
[`intrinsics::copy_nonoverlapping`]: https://doc.rust-lang.org/stable/std/intrinsics/fn.copy_nonoverlapping.html
[`<*const T>::copy_to`]: https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.copy_to
[`<*const T>::copy_to_nonoverlapping`]: https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.copy_to_nonoverlapping
[`<*mut T>::copy_to`]: https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.copy_to-1
[`<*mut T>::copy_to_nonoverlapping`]: https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.copy_to_nonoverlapping-1
[`<*mut T>::copy_from`]: https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.copy_from
[`<*mut T>::copy_from_nonoverlapping`]: https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.copy_from_nonoverlapping
[`str::from_utf8`]: https://doc.rust-lang.org/stable/std/str/fn.from_utf8.html
[`Utf8Error::error_len`]: https://doc.rust-lang.org/stable/std/str/struct.Utf8Error.html#method.error_len
[`Utf8Error::valid_up_to`]: https://doc.rust-lang.org/stable/std/str/struct.Utf8Error.html#method.valid_up_to
[`Condvar::new`]: https://doc.rust-lang.org/stable/std/sync/struct.Condvar.html#method.new
[`Mutex::new`]: https://doc.rust-lang.org/stable/std/sync/struct.Mutex.html#method.new
[`RwLock::new`]: https://doc.rust-lang.org/stable/std/sync/struct.RwLock.html#method.new
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.