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

syntax: Rewrite parsing of impls #46455

Merged
merged 2 commits into from
Jan 14, 2018
Merged

syntax: Rewrite parsing of impls #46455

merged 2 commits into from
Jan 14, 2018

Conversation

petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented Dec 2, 2017

Properly parse impls for the never type !
Recover from missing for in impl Trait for Type
Prohibit inherent default impls and default impls of auto traits (#37653 (comment), #37653 (comment))
Change wording in more diagnostics to use "auto traits"
Fix some spans in diagnostics
Some other minor code cleanups in the parser
Disambiguate generics and qualified paths in impls (parse impl <Type as Trait>::AssocTy { ... })
Replace the future-compatibility hack from #38268 with actually parsing generic parameters
Add a test for #46438

@rust-highfive
Copy link
Collaborator

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

}
if !impl_items.is_empty() {
self.span_err(impl_items[0].span, "auto impls cannot have impl items")
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't add new tests for all this stuff because impls for .. are going to be removed pretty soon.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can even do it now, if we do some more #[cfg(stage0)] stuff.

@petrochenkov
Copy link
Contributor Author

r? @nikomatsakis

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 3, 2017
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

r=me with a delay-span-bug call

fn error_380(tcx: TyCtxt, span: Span) {
span_err!(tcx.sess, span, E0380,
"traits with default impls (`e.g. impl \
"traits with auto impls (`e.g. impl \
Copy link
Contributor

Choose a reason for hiding this comment

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

aren't these just called auto trait now or something? I thought we landed some such PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We added auto trait (#45247), but not removed impl Trait for .. yet (#46480).

@@ -69,6 +62,9 @@ impl<'cx, 'tcx, 'v> UnsafetyChecker<'cx, 'tcx> {
g.attr_name());
}

(_, _, Unsafety::Unsafe, hir::ImplPolarity::Negative) => {
// Reported in AST validation
Copy link
Contributor

Choose a reason for hiding this comment

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

delay_span_bug please

}
if !impl_items.is_empty() {
self.span_err(impl_items[0].span, "auto impls cannot have impl items")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can even do it now, if we do some more #[cfg(stage0)] stuff.

@petrochenkov
Copy link
Contributor Author

This PR is not super urgent, I'll merge it after #46480 lands.

@petrochenkov petrochenkov added S-blocked Status: Blocked on something else such as an RFC or other implementation work. 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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 6, 2017
@kennytm kennytm added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Dec 7, 2017
@kennytm kennytm added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Dec 20, 2017
@bors
Copy link
Contributor

bors commented Dec 21, 2017

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

@shepmaster shepmaster added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Jan 6, 2018
@kennytm kennytm added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Jan 10, 2018
Properly parse impls for the never type `!`
Recover from missing `for` in `impl Trait for Type`
Prohibit inherent default impls and default impls of auto traits
Change wording in more diagnostics to use "auto traits"
Some minor code cleanups in the parser
@petrochenkov
Copy link
Contributor Author

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Jan 14, 2018

📌 Commit 60c48dd has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Jan 14, 2018

⌛ Testing commit 60c48dd with merge 3f92e8d...

bors added a commit that referenced this pull request Jan 14, 2018
syntax: Rewrite parsing of impls

Properly parse impls for the never type `!`
Recover from missing `for` in `impl Trait for Type`
Prohibit inherent default impls and default impls of auto traits (#37653 (comment), #37653 (comment))
Change wording in more diagnostics to use "auto traits"
Fix some spans in diagnostics
Some other minor code cleanups in the parser
Disambiguate generics and qualified paths in impls (parse `impl <Type as Trait>::AssocTy { ... }`)
Replace the future-compatibility hack from #38268 with actually parsing generic parameters
Add a test for #46438

This will compile:

```ignore (ignore auto_trait future compatibility warning)
Copy link
Contributor

Choose a reason for hiding this comment

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

this ignore can be removed

@bors
Copy link
Contributor

bors commented Jan 14, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 3f92e8d to master...

@bors bors merged commit 60c48dd into rust-lang:master Jan 14, 2018
@petrochenkov petrochenkov deleted the pimpl branch June 5, 2019 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked Status: Blocked on something else such as an RFC or other implementation work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants