Skip to content

marker_trait_attr: discard eval modulo regions in favor of eval to ok #84922

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

Closed
wants to merge 1 commit into from

Conversation

@rust-highfive
Copy link
Contributor

r? @petrochenkov

(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 May 4, 2021
@lcnr lcnr force-pushed the marker-trait-attr-overlap branch from 72bf374 to 3675894 Compare May 4, 2021 18:57
@lcnr lcnr force-pushed the marker-trait-attr-overlap branch from 3675894 to 40c714c Compare May 4, 2021 19:07
@petrochenkov
Copy link
Contributor

r? @nikomatsakis

@@ -1494,7 +1494,18 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// will then correctly report an inference error, since the
// existence of multiple marker trait impls tells us nothing
// about which one should actually apply.
Copy link
Member

Choose a reason for hiding this comment

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

Can this cause inference variables to be arbitrarily constrained, as described in the comments above?

e.g. if we had

#[marker]
pub trait F {}
impl F for u32 {}
impl<'a> F for &'a str where 'a: 'static {}

Would this change cause the <_#0t as F> obligation to unify _#0t with u32?

(contrived example because I don't know enough about trait selection/winnowing to provide an uncontrived one)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, because the !needs_infer condition still applies.

We drop strictly fewer candidates with this change.

Copy link
Member

Choose a reason for hiding this comment

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

derp. boolean logic, it's no joke

@lcnr
Copy link
Contributor Author

lcnr commented May 11, 2021

both the current state and this PR are unsound. Going to close this until we come up with an actually sound approach (cc #84955)

@lcnr lcnr closed this May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

marker_trait_attr: overlapping impls can cause compilation failures
5 participants