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

run rustfmt on librustc_typeck/check/method folder #36736

Merged
merged 1 commit into from
Oct 5, 2016
Merged

run rustfmt on librustc_typeck/check/method folder #36736

merged 1 commit into from
Oct 5, 2016

Conversation

srinivasreddy
Copy link
Contributor

No description provided.

@rust-highfive
Copy link
Collaborator

r? @Aatch

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

self.body_id,
poly_trait_ref.to_predicate());
let obligation =
traits::Obligation::misc(span, self.body_id, poly_trait_ref.to_predicate());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is interesting. Why did the function call take a separate line? Previous formatting looks good to me. @nrc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

&method_ty.fty.sig).0;
let fn_sig =
self.replace_late_bound_regions_with_fresh_var(span, infer::FnCall, &method_ty.fty.sig)
.0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is ugly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

steps,
opt_simplified_steps);
let mut probe_cx =
ProbeContext::new(self, span, mode, item_name, steps, opt_simplified_steps);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem too bad to me, arguably it is better since using less vertical space and the call all on one line is not too hard to read.

new_trait_ref.self_ty(),
new_trait_ref.substs);
let xform_self_ty =
this.xform_self_ty(&item, new_trait_ref.self_ty(), new_trait_ref.substs);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -1218,8 +1205,10 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
// are given do not include type/lifetime parameters for the
// method yet. So create fresh variables here for those too,
// if there are any.
assert_eq!(substs.types().count(), method.generics.parent_types as usize);
assert_eq!(substs.regions().count(), method.generics.parent_regions as usize);
assert_eq!(substs.types().count(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

why did rustfmt convert this? These expressions are happily fit on a single line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

.impl_trait_ref(impl_did)
.unwrap()
.def_id,

Copy link
Contributor Author

@srinivasreddy srinivasreddy Sep 26, 2016

Choose a reason for hiding this comment

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

rustfmt didnot remove empty line between parameters in a function call @nrc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nrc
Copy link
Member

nrc commented Sep 26, 2016

@srinivasreddy could you file rustfmt issues for these things please?

@@ -135,7 +136,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
self.tcx.used_trait_imports.borrow_mut().insert(import_id);
}

Ok(self.confirm_method(span, self_expr, call_expr, self_ty, pick, supplied_method_types))
Ok(self.confirm_method(span,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@srinivasreddy
Copy link
Contributor Author

@nrc Done. Thanks.

@nrc
Copy link
Member

nrc commented Sep 26, 2016

Awesome, thank you!

@srinivasreddy
Copy link
Contributor Author

@nrc Shall we roll-up?

///////////////////////////////////////////////////////////////////////////
// ADJUSTMENTS
/// ////////////////////////////////////////////////////////////////////////
/// ADJUSTMENTS
Copy link
Member

Choose a reason for hiding this comment

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

Could you manually fixup these changes to comments please?

//! *reborrow* it to a shorter lifetime. This allows us to
//! transparently pass `&mut` pointers, in particular, without
//! consuming them for their entire lifetime.
//!
Copy link
Member

Choose a reason for hiding this comment

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

Could you remove the dangling sigil please

"it"
} else {
"one of them"
});
Copy link
Member

Choose a reason for hiding this comment

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

These should stay on a line each I think

} else {
"one of them"
},
name = item_name);
Copy link
Member

Choose a reason for hiding this comment

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

same

@nrc
Copy link
Member

nrc commented Sep 29, 2016

Thanks for doing this! Could you manually fixup the areas where there are issues please?

@srinivasreddy
Copy link
Contributor Author

@nrc Done.

@srinivasreddy
Copy link
Contributor Author

r? @nrc

@rust-highfive rust-highfive assigned nrc and unassigned Aatch Sep 30, 2016
@bors
Copy link
Contributor

bors commented Oct 1, 2016

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

@nrc
Copy link
Member

nrc commented Oct 2, 2016

looks good, r=me, but needs a rebase

@srinivasreddy
Copy link
Contributor Author

@nrc Done

@nrc
Copy link
Member

nrc commented Oct 3, 2016

@bors: r+

@bors
Copy link
Contributor

bors commented Oct 3, 2016

📌 Commit 9e22f39 has been approved by nrc

@bors
Copy link
Contributor

bors commented Oct 3, 2016

⌛ Testing commit 9e22f39 with merge 0d77bd2...

@bors
Copy link
Contributor

bors commented Oct 3, 2016

💔 Test failed - auto-mac-64-opt

@nrc
Copy link
Member

nrc commented Oct 3, 2016

@srinivasreddy test fail looks real, perhaps a rebasing error?

@srinivasreddy
Copy link
Contributor Author

srinivasreddy commented Oct 4, 2016

@nrc Fixed it

@nrc
Copy link
Member

nrc commented Oct 5, 2016

@bors: r+

@bors
Copy link
Contributor

bors commented Oct 5, 2016

📌 Commit 5be0acc has been approved by nrc

@bors
Copy link
Contributor

bors commented Oct 5, 2016

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

@srinivasreddy
Copy link
Contributor Author

@nrc pls do it again.

@nrc
Copy link
Member

nrc commented Oct 5, 2016

@bors: r+

@bors
Copy link
Contributor

bors commented Oct 5, 2016

📌 Commit dcb6b15 has been approved by nrc

@bors
Copy link
Contributor

bors commented Oct 5, 2016

⌛ Testing commit dcb6b15 with merge fd1ea13...

bors added a commit that referenced this pull request Oct 5, 2016
run rustfmt on librustc_typeck/check/method  folder
@bors bors merged commit dcb6b15 into rust-lang:master Oct 5, 2016
@srinivasreddy srinivasreddy deleted the method branch October 5, 2016 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants