Skip to content

stop special-casing 'static in evaluation#102472

Merged
bors merged 1 commit intorust-lang:masterfrom
lcnr:static-in-eval
Mar 28, 2023
Merged

stop special-casing 'static in evaluation#102472
bors merged 1 commit intorust-lang:masterfrom
lcnr:static-in-eval

Conversation

@lcnr
Copy link
Contributor

@lcnr lcnr commented Sep 29, 2022

fixes #102360

I have no idea whether this actually removed all places where 'static matters. Without canonicalization it's very easy to accidentally rely on 'static again. Blocked on changing the order_dependent_trait_objects future-compat lint to a hard error

r? @nikomatsakis

@lcnr lcnr added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Sep 29, 2022
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 29, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 29, 2022
@rust-log-analyzer

This comment has been minimized.

notriddle added a commit to notriddle/rust that referenced this pull request Oct 21, 2022
…, r=jackh726

make `order_dependent_trait_objects` show up in future-breakage reports

tried to change it to a hard error in rust-lang#102474 but breaking the more than 1000 dependents of `traitobject` doesn't feel great 😅

This lint has existed since more than 3 years now and the way this is currently implemented is buggy and will break with rust-lang#102472. imo we should upgrade it to also report for dependencies and maybe also backport this to beta. Then after maybe 2-3 stable versions I would like to finally convert this lint to a hard error.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 21, 2022
…, r=jackh726

make `order_dependent_trait_objects` show up in future-breakage reports

tried to change it to a hard error in rust-lang#102474 but breaking the more than 1000 dependents of `traitobject` doesn't feel great 😅

This lint has existed since more than 3 years now and the way this is currently implemented is buggy and will break with rust-lang#102472. imo we should upgrade it to also report for dependencies and maybe also backport this to beta. Then after maybe 2-3 stable versions I would like to finally convert this lint to a hard error.
@bors
Copy link
Collaborator

bors commented Oct 22, 2022

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

@lcnr lcnr marked this pull request as ready for review November 8, 2022 14:24
@bors
Copy link
Collaborator

bors commented Nov 23, 2022

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

@pnkfelix
Copy link
Contributor

@rustbot label: -T-compiler +T-types

@rustbot rustbot 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 Dec 15, 2022
@pnkfelix
Copy link
Contributor

pnkfelix commented Dec 15, 2022

@rustbot assign @jackh726

(whoops I guess that should have been a r?)

@rustbot rustbot assigned jackh726 and unassigned nikomatsakis Dec 15, 2022
@jackh726
Copy link
Member

@lcnr what's the status of this?

@jackh726 jackh726 removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 18, 2023
@jackh726
Copy link
Member

jackh726 commented Mar 8, 2023

Given this only changes unstable marker trait behavior, fine to land with open issue referenced in known bug

@lcnr
Copy link
Contributor Author

lcnr commented Mar 22, 2023

opened #109481 for overlapping-impl-1-modulo-regions.rs and we already have #89515 for the other test.

r? @jackh726

@rustbot ready

@rustbot
Copy link
Collaborator

rustbot commented Mar 22, 2023

Could not assign reviewer from: jackh726.
User(s) jackh726 are either the PR author or are already assigned, and there are no other candidates.
Use r? to specify someone else to assign.

@rustbot rustbot removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Mar 22, 2023
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 22, 2023
@jackh726
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 28, 2023

📌 Commit 73c79cd has been approved by jackh726

It is now in the queue for this repository.

@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 Mar 28, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 28, 2023
Rollup of 8 pull requests

Successful merges:

 - rust-lang#91793 (socket ancillary data implementation for FreeBSD (from 13 and above).)
 - rust-lang#92284 (Change advance(_back)_by to return the remainder instead of the number of processed elements)
 - rust-lang#102472 (stop special-casing `'static` in evaluation)
 - rust-lang#108480 (Use Rayon's TLV directly)
 - rust-lang#109321 (Erase impl regions when checking for impossible to eagerly monomorphize items)
 - rust-lang#109470 (Correctly substitute GAT's type used in `normalize_param_env` in `check_type_bounds`)
 - rust-lang#109562 (Update ar_archive_writer to 0.1.3)
 - rust-lang#109629 (remove obsolete `givens` from regionck)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4bd33fd into rust-lang:master Mar 28, 2023
@rustbot rustbot added this to the 1.70.0 milestone Mar 28, 2023
@lcnr lcnr deleted the static-in-eval branch March 28, 2023 19:35
@lqd
Copy link
Member

lqd commented Mar 31, 2023

For triage purposes: from the results in the rollup where this PR landed, and the results in a revert.

@rustbot label: +perf-regression

@rustbot rustbot added the perf-regression Performance regression. label Mar 31, 2023
traviscross added a commit to traviscross/rust that referenced this pull request Mar 14, 2026
The `TypeOutlives` handler in `evaluate_predicate_recursively` checks
whether a type in a `T: 'a` predicate has free regions, bound regions,
or inference variables -- and if so, returns
`EvaluatedToOkModuloRegions` rather than `EvaluatedToOk`.  The comment
says "no free lifetimes or generic parameters", but the code checked
`has_non_region_infer()` twice instead of checking
`has_non_region_param()` for the second condition.

This meant that `TypeOutlives(T, 'static)` where `T` is a type
parameter returned `EvaluatedToOk` -- claiming the result holds
unconditionally -- when the correct answer is
`EvaluatedToOkModuloRegions`.

The distinction matters during marker trait winnowing in
`prefer_lhs_over_victim`, which uses
`must_apply_considering_regions()` (true only for `EvaluatedToOk`) to
decide whether one overlapping impl beats another.  With the bug, a
`T: 'static`-bounded impl appeared equally strong as an unrestricted
impl, making the winner depend on source order.  This caused spurious
E0310 errors when the more-constrained impl happened to appear after
the less-constrained one.

Fixes rust-lang#109481.

This same symptom was originally filed as rust-lang#84917 and
fixed in PR rust-lang#88139.  Then PR rust-lang#102472
rewrote the `TypeOutlives` handler, introducing the duplicate
`has_non_region_infer()` and losing the param check, regressing this.
Shortly after, rust-lang#109481 was filed.  It noted the
connection to rust-lang#102472 -- that it was only relevant after
it -- but apparently no one noticed the duplicate condition.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

perf-regression Performance regression. 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.

broken interaction between marker traits, lifetimes, and caching

9 participants