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.

Sign up for GitHub

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

Jump to bottom

Apply adjustments for field expression even if inaccessible #90508

Merged
merged 1 commit into from
Nov 7, 2021
Merged

Apply adjustments for field expression even if inaccessible #90508

merged 1 commit into from
Nov 7, 2021

Conversation

nbdd0121
Copy link
Contributor

@nbdd0121 nbdd0121 commented Nov 2, 2021

The adjustments are used later by ExprUseVisitor to build Place projections and without adjustments it can produce invalid result.

Fix #90483

@rustbot label: T-compiler

@nbdd0121
Apply adjustments for field expression even if inaccessible
f556075 
The adjustments are used later by ExprUseVisitor to build Place projections
and without adjustments it can produce invalid result.
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 2, 2021
@rust-highfive
Copy link
Collaborator

r? @davidtwco

(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 Nov 2, 2021
davidtwco
davidtwco approved these changes Nov 3, 2021
View reviewed changes
Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

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

LGTM, r=me unless you want to address the nit.

compiler/rustc_typeck/src/check/expr.rs
@@ -1729,7 +1729,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
self.structurally_resolved_type(autoderef.span(), autoderef.final_ty(false));

if let Some((did, field_ty)) = private_candidate {
if let Some((adjustments, did, field_ty)) = private_candidate {
// (#90483) apply adjustments to avoid ExprUseVisitor from
Copy link
Member

Choose a reason for hiding this comment

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

nit: I'm not sure that this comment is necessary.

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 think it's better to have it because usually we don't care much once an error is emitted, but for this one adjustment is needed to avoid ICEing despite emitted error.

@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 6, 2021

📌 Commit f556075 has been approved by davidtwco

@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 Nov 6, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 6, 2021
@matthiaskrgr
Rollup merge of rust-lang#90508 - nbdd0121:issue-90483, r=davidtwco
1e4e823 
Apply adjustments for field expression even if inaccessible

The adjustments are used later by ExprUseVisitor to build Place projections and without adjustments it can produce invalid result.

Fix rust-lang#90483

`@rustbot` label: T-compiler
This was referenced Nov 6, 2021
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 6, 2021
@bors
Auto merge of rust-lang#90661 - matthiaskrgr:rollup-1umbdlx, r=matthi…
089a016 
…askrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#90487 (Add a chapter on reading Rustdoc output)
 - rust-lang#90508 (Apply adjustments for field expression even if inaccessible)
 - rust-lang#90627 (Suggest dereference of `Box` when inner type is expected)
 - rust-lang#90642 (use matches!() macro in more places)
 - rust-lang#90646 (type error go brrrrrrrr)
 - rust-lang#90649 (Run reveal_all on MIR when inlining is activated.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4c49db3 into rust-lang:master Nov 7, 2021
@rustbot rustbot added this to the 1.58.0 milestone Nov 7, 2021
@nbdd0121
Copy link
Contributor Author

nbdd0121 commented Nov 7, 2021

Needs backport as this is a stable-to-stable regression

@nbdd0121 nbdd0121 deleted the issue-90483 branch November 7, 2021 03:48
@davidtwco davidtwco added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Nov 7, 2021
@apiraino apiraino added stable-nominated Nominated for backporting to the compiler in the stable channel. and removed stable-nominated Nominated for backporting to the compiler in the stable channel. labels Nov 11, 2021
@apiraino
Copy link
Contributor

Beta backport approved as per compiler team on Zulip

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Nov 11, 2021
@apiraino
Copy link
Contributor

@nbdd0121 @davidtwco would this patch warrant also a backport to stable?

@nbdd0121
Copy link
Contributor Author

This is an error to ICE regression, so not worth a dot release.

@nbdd0121
Copy link
Contributor Author

(But if there is going to be a point release then it could be included)

@cuviper cuviper mentioned this pull request Nov 16, 2021
Merged
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Nov 16, 2021
@cuviper cuviper modified the milestones: 1.58.0, 1.57.0 Nov 16, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 19, 2021
@bors
Auto merge of rust-lang#90938 - cuviper:beta-next, r=Mark-Simulacrum
7611e64 
[beta] backports

-  Fix assertion failures in OwnedHandle with windows_subsystem. rust-lang#88798
-  Ensure that pushing empty path works as before on verbatim paths rust-lang#89665
-  Feature gate + make must_not_suspend allow-by-default rust-lang#89826
-  Only use clone3 when needed for pidfd rust-lang#89930
-  Fix documentation header sizes rust-lang#90186
-  Fixes incorrect handling of ADT's drop requirements rust-lang#90218
-  Fix ICE when forgetting to Box a parameter to a Self::func call rust-lang#90221
-  Prevent duplicate caller bounds candidates by exposing default substs in Unevaluated rust-lang#90266
-  Update odht crate to 0.3.1 (big-endian bugfix) rust-lang#90403
-  rustdoc: Go back to loading all external crates unconditionally rust-lang#90489
-  Split doc_cfg and doc_auto_cfg features rust-lang#90502
-  Apply adjustments for field expression even if inaccessible rust-lang#90508
-  Warn for variables that are no longer captured rust-lang#90597
-  Properly register text_direction_codepoint_in_comment lint. rust-lang#90626
-  CI: Use ubuntu image to download openssl, curl sources, cacert.pem for x86 dist builds rust-lang#90457
-  Android is not GNU rust-lang#90834
-  Update llvm submodule rust-lang#90954

Additionally, this bumps the stage 0 compiler from beta to stable 1.56.1.

r? `@Mark-Simulacrum`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidtwco davidtwco davidtwco approved these changes

Assignees

@davidtwco davidtwco

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.57.0
Development

Successfully merging this pull request may close these issues.

Typecheck panics feat. a closure and a struct with a private field and a public method
7 participants
@nbdd0121 @rust-highfive @davidtwco @bors @apiraino @cuviper @rustbot