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

Aggregation of cosmetic changes made during work on REPL PRs: librustc_mir #64274

Closed
wants to merge 1 commit into from

Conversation

alexreg
Copy link
Contributor

@alexreg alexreg commented Sep 8, 2019

Factored out from hacking on rustc for work on the REPL.

r? @Centril

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 8, 2019
/// I'm not sure this is the right approach - @eddyb could you try and
/// figure this out?
//
// FIXME: I'm not sure this is the right approach -- @eddyb, could you try and
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep as doc-comment for same reason as the case in the other PR.

ProjectionElem::Index(..)
| ProjectionElem::ConstantIndex { .. }
| ProjectionElem::Subslice { .. } => {
ProjectionElem::Index(..) |
Copy link
Contributor

Choose a reason for hiding this comment

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

This is formatting away from the official rustfmt style; can you please remove the styling changes to match arms in the PR (and if there are other ones in the other PR) altogether? It's not clear to me that they are a win; they mostly just introduce more commits in blame and rustfmt will reformat them anyways later.

@@ -375,17 +376,17 @@ fn do_mir_borrowck<'a, 'tcx>(
mbcx.errors_buffer.sort_by_key(|diag| diag.span.primary_span());

if !mbcx.disable_error_downgrading && tcx.migrate_borrowck() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Aaw... this is going to conflict with my PR in #64221 unless git is clever... oh well; I guess I can rebase.

@@ -833,13 +832,13 @@ enum AccessDepth {
/// this action."
Deep,

/// Access is Deep only when there is a Drop implementation that
/// Access is deep only when there is a `Drop` implementation that
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Access is deep only when there is a `Drop` implementation that
/// Access is `Deep` only when there is a `Drop` implementation that

@@ -1580,9 +1578,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
) {
debug!("check_if_reassignment_to_immutable_state({:?})", local);

// Check if any of the initializiations of `local` have happened yet:
// Check if any of the initializiations of `local` have happened yet.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Check if any of the initializiations of `local` have happened yet.
// Check if any of the initializiations of `local` have happened yet...

let alloc_id = self.tcx.alloc_map.lock().create_static_alloc(cid.instance.def_id());
let ptr = self.tag_static_base_pointer(Pointer::from(alloc_id));
MPlaceTy::from_aligned_ptr(ptr, layout)
}
})
}

/// Computes a place. You should only use this if you intend to write into this
/// place; for reading, a more efficient alternative is `eval_place_for_read`.
Copy link
Contributor

Choose a reason for hiding this comment

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

The warning here is presumably there for a reason, revert.

StorageDead(local) => {
let old_val = self.storage_dead(local);
self.deallocate_local(old_val)?;
}

// No dynamic semantics attached to `FakeRead`; MIR
// No dynamic semantics attached to FakeRead; MIR
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert.

@@ -121,7 +121,7 @@ impl<'tcx> MirPass<'tcx> for AddRetag {
}
TerminatorKind::Drop { .. } |
TerminatorKind::DropAndReplace { .. } => {
// `Drop` is also a call, but it doesn't return anything so we are good.
// Drop is also a call, but it doesn't return anything so we are good.
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert.

@@ -269,7 +269,7 @@ impl MutVisitor<'tcx> for TransformVisitor<'tcx> {
fn visit_basic_block_data(&mut self,
block: BasicBlock,
data: &mut BasicBlockData<'tcx>) {
// Remove StorageLive and StorageDead statements for remapped locals
// Remove StorageLive and StorageDead statements for remapped locals.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Remove StorageLive and StorageDead statements for remapped locals.
// Remove `StorageLive` and `StorageDead` statements for remapped locals.

@@ -1702,9 +1702,9 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> {
Checker::new(tcx, def_id, body, mode).check_const().1
};

// In `const` and `static` everything without `StorageDead`
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert the removal of backticks here in this file.

@oli-obk
Copy link
Contributor

oli-obk commented Sep 8, 2019

Thanks for the PR. Unfortunately, as per our rules set up in #58619, I am closing this PR.

@oli-obk oli-obk closed this Sep 8, 2019
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.

4 participants