Skip to content

resolve: Always resolve visibilities on impl items - #67236

Merged
bors merged 1 commit into
rust-lang:masterfrom
petrochenkov:docerr2
Dec 12, 2019
Merged

resolve: Always resolve visibilities on impl items#67236
bors merged 1 commit into
rust-lang:masterfrom
petrochenkov:docerr2

Conversation

@petrochenkov

Copy link
Copy Markdown
Contributor

Fixes#64705.

Similarly to #67106 this was an issue with visitor discipline.
Impl items were visited as a part of visiting ast::ItemKind::Impl, but they should be visit-able in isolation from their parents as well, because that's how they are visited when they are expanded from macros.

I've checked that all the remaining resolve_visibility calls are used correctly.

r? @matthewjasper

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 11, 2019
@petrochenkovpetrochenkov added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Dec 11, 2019
@petrochenkov

Copy link
Copy Markdown
ContributorAuthor

I'm going to stable-nominate this as well in case there's going to be a point release for other reasons, because this PR fixes a relatively recent stable-to-stable regression that allows erroneous code to be silently accepted.

@petrochenkovpetrochenkov added the stable-nominated Nominated for backporting to the compiler in the stable channel. label Dec 11, 2019
@jonas-schievinkjonas-schievink added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 11, 2019
@matthewjasper

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Dec 11, 2019

Copy link
Copy Markdown
Collaborator

📌 Commit 914c9aa has been approved by matthewjasper

@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 11, 2019
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Dec 12, 2019
resolve: Always resolve visibilities on impl items
Fixesrust-lang#64705.
Similarly to rust-lang#67106 this was an issue with visitor discipline.
Impl items were visited as a part of visiting `ast::ItemKind::Impl`, but they should be visit-able in isolation from their parents as well, because that's how they are visited when they are expanded from macros.
I've checked that all the remaining `resolve_visibility` calls are used correctly.
r? @matthewjasper
@JohnTitorJohnTitor mentioned this pull request Dec 12, 2019
bors added a commit that referenced this pull request Dec 12, 2019
Rollup of 8 pull requests
Successful merges:
- #62514 (Clarify `Box<T>` representation and its use in FFI)
- #66983 (Fix `unused_parens` triggers on macro by example code)
- #67215 (Fix `-Z print-type-sizes`'s handling of zero-sized fields.)
- #67230 (Remove irelevant comment on `register_dtor`)
- #67236 (resolve: Always resolve visibilities on impl items)
- #67237 (Some small readability improvements)
- #67238 (Small std::borrow::Cow improvements)
- #67239 (Make TinyList::remove iterate instead of recurse)
Failed merges:
r? @ghost
@bors
bors merged commit 914c9aa into rust-lang:masterDec 12, 2019
@pnkfelix

Copy link
Copy Markdown
Contributor

discussed at T-compiler meeting. accepted for beta backport.

@pnkfelixpnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Dec 12, 2019
@Mark-SimulacrumMark-Simulacrum removed beta-nominated Nominated for backporting to the compiler in the beta channel. stable-nominated Nominated for backporting to the compiler in the stable channel. labels Dec 14, 2019
bors added a commit that referenced this pull request Dec 14, 2019
[beta] Beta backports
Backporting the following pull requests:
* resolve: Always resolve visibilities on impl items #67236
* resolve: Resolve visibilities on fields with non-builtin attributes #67106
* E0023: handle expected != tuple pattern type #67044
* Fix `unused_parens` triggers on macro by example code #66983
* Fix some issues with attributes on unnamed fields #66669
* Ensure that we get a hard error on generic ZST constants if their bodies #67134 (via #67297)
Some of these conflicted on merge, I resolved where possible, sometimes by cherry-picking a commit or two more from the relevant PRs. Since those changes are necessary though for backport to proceed (otherwise not even std/core compile), seems fine -- they're fairly minor cleanups anyway.
@petrochenkov
petrochenkov deleted the docerr2 branch February 22, 2025 18:44
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

beta-acceptedAccepted for backporting to the compiler in the beta channel.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ICE when cargo doc on lexical-core: attempted .def_id() on invalid res: Err

7 participants

@petrochenkov@matthewjasper@bors@pnkfelix@jonas-schievink@Mark-Simulacrum@rust-highfive