Skip to content

Make IMPLIED_BOUNDS_ENTAILMENT into a hard error from a lint - #117984

Merged
bors merged 2 commits into
rust-lang:masterfrom
compiler-errors:implied-bounds-entailment
Dec 16, 2023
Merged

Make IMPLIED_BOUNDS_ENTAILMENT into a hard error from a lint#117984
bors merged 2 commits into
rust-lang:masterfrom
compiler-errors:implied-bounds-entailment

Conversation

@compiler-errors

@compiler-errorscompiler-errors commented Nov 16, 2023•
edited by lcnr
Loading

Copy link
Copy Markdown
Contributor

closes#105572

Removes the IMPLIED_BOUNDS_ENTAILMENT and makes the compare_method_predicate_entailment logic just run once.

r? lcnr

@compiler-errorscompiler-errors added needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. T-types Relevant to the types team, which will review and decide on the PR/issue. labels Nov 16, 2023
@rustbotrustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 16, 2023
@lqd

lqd commented Nov 16, 2023•
edited
Loading

Copy link
Copy Markdown
Member

I also looked into this and did it the same way you did, but then I also wondered whether we actually wanted to keep the same diagnostics, and emit a "impl method assumes more implied bounds than the corresponding trait method" error instead of a region error (i.e. make a new error code and all that...)?

@Urgau

Urgau commented Nov 16, 2023

Copy link
Copy Markdown
Member

I actually have no idea what to do with the existing lint. We don't want to remove it, since it'll cause code that has allow(implied_bounds_entailment) to fail, making it annoying to compile between versions or whatever.

In think you can just remove it. unknown_lints is just warn-by-default and lints can be added, renamed and removed without prior notice anyway.

@compiler-errors

compiler-errors commented Nov 16, 2023•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

@lqd: I have no idea how to make that possible, unless we literally just keep the same "try running compare_method_predicate_entailment twice" strategy, which is what this PR aims to remove. The problem here is that we don't really have a good way of emitting special error messages for region obligations that come from different sources, unlike ObligationCause.

@Urgau: Thanks for the clarification. Makes sense. Totally missed that unknown_lints was just a warning.

@rust-log-analyzer

This comment was marked as outdated.

Sign in to view
@lqd

lqd commented Nov 16, 2023

Copy link
Copy Markdown
Member

@compiler-errors I came to the exact same conclusion, then closed my editor and deleted my branch 😂

lcnr
lcnr reviewed Nov 17, 2023
View reviewed changes
Comment threadcompiler/rustc_hir_analysis/src/check/compare_impl_item.rs
}
}
return Err(infcx
.tainted_by_errors()

@lcnrlcnrNov 17, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't tainted_by_errors always be None, given that we return in case select_all_or_error failed?

@compiler-errorscompiler-errorsNov 17, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idk, astconv might taint the infcx or something. I didn't want to unwrap here, avoids an unnecessary ICE for no cost.

@lcnr

lcnr commented Nov 17, 2023

Copy link
Copy Markdown
Contributor

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

Sign in to view
@rustbotrustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Nov 17, 2023
@bors

bors commented Nov 17, 2023

Copy link
Copy Markdown
Collaborator

⌛ Trying commit 16f7de6 with merge aa7c6ba...

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 17, 2023
@bors
Auto merge ofrust-lang#117984- compiler-errors:implied-bounds-entai…
aa7c6ba
…lment, r=<try>
Make `IMPLIED_BOUNDS_ENTAILMENT` into a hard error from a lint
closesrust-lang#105572
Removes the `IMPLIED_BOUNDS_ENTAILMENT` and makes the `compare_method_predicate_entailment` logic just run once.
r? lcnr
@lcnr

lcnr commented Nov 17, 2023

Copy link
Copy Markdown
Contributor

want to run crater, looking at discussions referencing #105572 there are still some projects depending on rustc-serialize, would start the FCP afterwards 👍

@bors

bors commented Nov 17, 2023

Copy link
Copy Markdown
Collaborator

☀️ Try build successful - checks-actions
Build commit: aa7c6ba (aa7c6ba69acce8b10a275eabfe6157ea72c8b599)

1 similar comment
@bors

bors commented Nov 17, 2023

Copy link
Copy Markdown
Collaborator

☀️ Try build successful - checks-actions
Build commit: aa7c6ba (aa7c6ba69acce8b10a275eabfe6157ea72c8b599)

@rust-timer

This comment has been minimized.

Sign in to view
@lcnr

lcnr commented Nov 17, 2023

Copy link
Copy Markdown
Contributor

@craterbot check

@craterbot

craterbot commented Nov 17, 2023

Copy link
Copy Markdown
Collaborator

👌 Experiment pr-117984 created and queued.
🤖 Automatically detected try build aa7c6ba
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbotcraterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Nov 17, 2023
@rust-timer

rust-timer commented Nov 17, 2023

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (aa7c6ba): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

meanrangecount
Regressions ❌
(primary)		--0
Regressions ❌
(secondary)		--0
Improvements ✅
(primary)		--0
Improvements ✅
(secondary)		-2.0%[-2.0%, -2.0%]1
All ❌✅ (primary)--0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

meanrangecount
Regressions ❌
(primary)		0.6%[0.4%, 0.9%]2
Regressions ❌
(secondary)		2.1%[1.4%, 2.7%]2
Improvements ✅
(primary)		-1.8%[-1.8%, -1.8%]1
Improvements ✅
(secondary)		--0
All ❌✅ (primary)-0.2%[-1.8%, 0.9%]3

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

meanrangecount
Regressions ❌
(primary)		--0
Regressions ❌
(secondary)		--0
Improvements ✅
(primary)		-0.7%[-0.7%, -0.6%]2
Improvements ✅
(secondary)		--0
All ❌✅ (primary)-0.7%[-0.7%, -0.6%]2

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 675.609s -> 676.495s (0.13%)
Artifact size: 313.59 MiB -> 313.62 MiB (0.01%)

lcnr
lcnr reviewed Nov 21, 2023
View reviewed changes

@lcnrlcnr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the lint should be registered as a removed lint in

rust/compiler/rustc_lint/src/lib.rs

Line 509 in e24e5af

store.register_removed(

@compiler-errors
compiler-errorsforce-pushed the implied-bounds-entailment branch 2 times, most recently from 890caa1 to 5b088a4CompareNovember 21, 2023 20:21
@craterbot

craterbot commented Nov 24, 2023

Copy link
Copy Markdown
Collaborator

🚧 Experiment pr-117984 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@bors

bors commented Nov 26, 2023

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #118256) made this pull request unmergeable. Please resolve the merge conflicts.

@rust-cloud-vms
rust-cloud-vmsBotforce-pushed the implied-bounds-entailment branch from ea0e059 to 1cb4017CompareDecember 15, 2023 17:05
@lcnr

lcnr commented Dec 15, 2023

Copy link
Copy Markdown
Contributor

@bors r+ rollup=never

@bors

bors commented Dec 15, 2023

Copy link
Copy Markdown
Collaborator

📌 Commit ea0e059 has been approved by lcnr

It is now in the queue for this repository.

@borsbors 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 Dec 15, 2023
@bors

bors commented Dec 16, 2023

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #119002) made this pull request unmergeable. Please resolve the merge conflicts.

@borsbors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 16, 2023
@rust-cloud-vms
rust-cloud-vmsBotforce-pushed the implied-bounds-entailment branch from 1cb4017 to 32907c7CompareDecember 16, 2023 01:31
@compiler-errors

compiler-errors commented Dec 16, 2023

Copy link
Copy Markdown
ContributorAuthor

@bors r=lcnr

@bors

bors commented Dec 16, 2023

Copy link
Copy Markdown
Collaborator

📌 Commit 32907c7 has been approved by lcnr

It is now in the queue for this repository.

@borsbors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 16, 2023
@bors

bors commented Dec 16, 2023

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 32907c7 with merge 5c927ab...

@bors

bors commented Dec 16, 2023

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing 5c927ab to master...

@borsbors added the merged-by-bors This PR was explicitly merged by bors. label Dec 16, 2023
@bors
bors merged commit 5c927ab into rust-lang:masterDec 16, 2023
@rustbotrustbot added this to the 1.76.0 milestone Dec 16, 2023
@rust-timer

rust-timer commented Dec 16, 2023

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (5c927ab): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

meanrangecount
Regressions ❌
(primary)		0.4%[0.4%, 0.4%]1
Regressions ❌
(secondary)		--0
Improvements ✅
(primary)		--0
Improvements ✅
(secondary)		--0
All ❌✅ (primary)0.4%[0.4%, 0.4%]1

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

meanrangecount
Regressions ❌
(primary)		0.5%[0.5%, 0.5%]1
Regressions ❌
(secondary)		--0
Improvements ✅
(primary)		-1.8%[-3.8%, -0.5%]3
Improvements ✅
(secondary)		-1.2%[-1.2%, -1.2%]1
All ❌✅ (primary)-1.2%[-3.8%, 0.5%]4

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

meanrangecount
Regressions ❌
(primary)		0.5%[0.4%, 0.7%]2
Regressions ❌
(secondary)		--0
Improvements ✅
(primary)		--0
Improvements ✅
(secondary)		--0
All ❌✅ (primary)0.5%[0.4%, 0.7%]2

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 671.592s -> 672.112s (0.08%)
Artifact size: 312.57 MiB -> 312.58 MiB (0.00%)

heinrich5991 added a commit to heinrich5991/libtw2 that referenced this pull request Jan 4, 2024
@heinrich5991
Fix compilation with newest Rust
a445cf2
rust-lang-deprecated/rustc-serialize#200rust-lang-deprecated/rustc-serialize@75ce924rust-lang/rust#117984
heinrich5991 added a commit to heinrich5991/libtw2 that referenced this pull request Jan 4, 2024
@heinrich5991
Fix compilation with Rust 1.76.0
8246fba
rust-lang-deprecated/rustc-serialize#200rust-lang-deprecated/rustc-serialize@75ce924rust-lang/rust#117984
@Mark-SimulacrumMark-Simulacrum mentioned this pull request Jan 12, 2024
Closed
This was referenced Jan 12, 2024
Closed
Closed
@Mark-SimulacrumMark-Simulacrum added the relnotes Marks issues that should be documented in the release notes of the next release. label Jan 12, 2024
wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Feb 18, 2024
@he32
Add a package for rust version 1.76.0.
0e9a2a1
Pkgsrc changes:
* Adapt checksums and patches.
Upstream chnages:
Version 1.76.0 (2024-02-08)
==========================
Language
--------
- [Document Rust ABI compatibility between various types]
(rust-lang/rust#115476)
- [Also: guarantee that char and u32 are ABI-compatible]
(rust-lang/rust#118032)
- [Warn against ambiguous wide pointer comparisons]
(rust-lang/rust#117758)
Compiler
--------
- [Lint pinned `#[must_use]` pointers (in particular, `Box<T>`
where `T` is `#[must_use]`) in `unused_must_use`.]
(rust-lang/rust#118054)
- [Soundness fix: fix computing the offset of an unsized field in
a packed struct]
(rust-lang/rust#118540)
- [Soundness fix: fix dynamic size/align computation logic for
packed types with dyn Trait tail]
(rust-lang/rust#118538)
- [Add `$message_type` field to distinguish json diagnostic outputs]
(rust-lang/rust#115691)
- [Enable Rust to use the EHCont security feature of Windows]
(rust-lang/rust#118013)
- [Add tier 3 {x86_64,i686}-win7-windows-msvc targets]
(rust-lang/rust#118150)
- [Add tier 3 aarch64-apple-watchos target]
(rust-lang/rust#119074)
- [Add tier 3 arm64e-apple-ios & arm64e-apple-darwin targets]
(rust-lang/rust#115526)
Refer to Rust's [platform support page][platform-support-doc]
for more information on Rust's tiered platform support.
Libraries
---------
- [Add a column number to `dbg!()`]
(rust-lang/rust#114962)
- [Add `std::hash::{DefaultHasher, RandomState}` exports]
(rust-lang/rust#115694)
- [Fix rounding issue with exponents in fmt]
(rust-lang/rust#116301)
- [Add T: ?Sized to `RwLockReadGuard` and `RwLockWriteGuard`'s Debug impls.]
(rust-lang/rust#117138)
- [Windows: Allow `File::create` to work on hidden files]
(rust-lang/rust#116438)
Stabilized APIs
---------------
- [`Arc::unwrap_or_clone`]
(https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#method.unwrap_or_clone)
- [`Rc::unwrap_or_clone`]
(https://doc.rust-lang.org/stable/std/rc/struct.Rc.html#method.unwrap_or_clone)
- [`Result::inspect`]
(https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.inspect)
- [`Result::inspect_err`]
(https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.inspect_err)
- [`Option::inspect`]
(https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.inspect)
- [`type_name_of_val`]
(https://doc.rust-lang.org/stable/std/any/fn.type_name_of_val.html)
- [`std::hash::{DefaultHasher, RandomState}`]
(https://doc.rust-lang.org/stable/std/hash/index.html#structs)
These were previously available only through `std::collections::hash_map`.
- [`ptr::{from_ref, from_mut}`]
(https://doc.rust-lang.org/stable/std/ptr/fn.from_ref.html)
- [`ptr::addr_eq`](https://doc.rust-lang.org/stable/std/ptr/fn.addr_eq.html)
Cargo
-----
See [Cargo release notes]
(https://github.com/rust-lang/cargo/blob/master/CHANGELOG.md#cargo-176-2024-02-08).
Rustdoc
-------
- [Don't merge cfg and doc(cfg) attributes for re-exports]
(rust-lang/rust#113091)
- [rustdoc: allow resizing the sidebar / hiding the top bar]
(rust-lang/rust#115660)
- [rustdoc-search: add support for traits and associated types]
(rust-lang/rust#116085)
- [rustdoc: Add highlighting for comments in items declaration]
(rust-lang/rust#117869)
Compatibility Notes
-------------------
- [Add allow-by-default lint for unit bindings]
(rust-lang/rust#112380)
This is expected to be upgraded to a warning by default in a future Rust
release. Some macros emit bindings with type `()` with user-provided spans,
which means that this lint will warn for user code.
- [Remove x86_64-sun-solaris target.]
(rust-lang/rust#118091)
- [Remove asmjs-unknown-emscripten target]
(rust-lang/rust#117338)
- [Report errors in jobserver inherited through environment variables]
(rust-lang/rust#113730)
This [may warn](rust-lang/rust#120515)
on benign problems too.
- [Update the minimum external LLVM to 16.]
(rust-lang/rust#117947)
- [Improve `print_tts`](rust-lang/rust#114571)
This change can break some naive manual parsing of token trees
in proc macro code which expect a particular structure after
`.to_string()`, rather than just arbitrary Rust code.
- [Make `IMPLIED_BOUNDS_ENTAILMENT` into a hard error from a lint]
(rust-lang/rust#117984)
- [Vec's allocation behavior was changed when collecting some iterators]
(rust-lang/rust#110353)
Allocation behavior is currently not specified, nevertheless
changes can be surprising.
See [`impl FromIterator for Vec`]
(https://doc.rust-lang.org/nightly/std/vec/struct.Vec.html#impl-FromIterator%3CT%3E-for-Vec%3CT%3E)
for more details.
- [Properly reject `default` on free const items]
(rust-lang/rust#117818)
@lcnrlcnr mentioned this pull request Feb 29, 2024
Closed
@github-actionsgithub-actionsBot mentioned this pull request Apr 9, 2024
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lcnrlcnrlcnr left review comments

Assignees

@lcnrlcnr

Labels

disposition-mergeThis issue / PR is in PFCP or FCP with a disposition to merge it.finished-final-comment-periodThe final comment period is finished for this PR / Issue.merged-by-borsThis PR was explicitly merged by bors.needs-fcpThis change is insta-stable, or significant enough to need a team FCP to proceed.relnotesMarks issues that should be documented in the release notes of the next release.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-typesRelevant to the types team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.76.0

Development

Successfully merging this pull request may close these issues.

Tracking issue for IMPLIED_BOUNDS_ENTAILMENT lint

12 participants

@compiler-errors@lqd@Urgau@rust-log-analyzer@lcnr@rust-timer@bors@craterbot@rfcbot@Mark-Simulacrum@apiraino@rustbot