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

Fix some clippy warnings in libsyntax #41957

Merged
merged 3 commits into from
May 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ impl Stmt {
StmtKind::Mac(mac) => StmtKind::Mac(mac.map(|(mac, _style, attrs)| {
(mac, MacStmtStyle::Semicolon, attrs)
})),
node @ _ => node,
node => node,
};
self
}
Expand Down Expand Up @@ -1076,16 +1076,16 @@ impl LitKind {
pub fn is_unsuffixed(&self) -> bool {
match *self {
// unsuffixed variants
LitKind::Str(..) => true,
LitKind::ByteStr(..) => true,
LitKind::Byte(..) => true,
LitKind::Char(..) => true,
LitKind::Int(_, LitIntType::Unsuffixed) => true,
LitKind::FloatUnsuffixed(..) => true,
LitKind::Str(..) |
LitKind::ByteStr(..) |
LitKind::Byte(..) |
LitKind::Char(..) |
LitKind::Int(_, LitIntType::Unsuffixed) |
LitKind::FloatUnsuffixed(..) |
LitKind::Bool(..) => true,
// suffixed variants
LitKind::Int(_, LitIntType::Signed(..)) => false,
LitKind::Int(_, LitIntType::Unsigned(..)) => false,
LitKind::Int(_, LitIntType::Signed(..)) |
LitKind::Int(_, LitIntType::Unsigned(..)) |
LitKind::Float(..) => false,
}
}
Expand Down
17 changes: 9 additions & 8 deletions src/libsyntax/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,15 @@ impl NestedMetaItem {
/// Returns the MetaItem if self is a NestedMetaItemKind::MetaItem.
pub fn meta_item(&self) -> Option<&MetaItem> {
match self.node {
NestedMetaItemKind::MetaItem(ref item) => Some(&item),
NestedMetaItemKind::MetaItem(ref item) => Some(item),
_ => None
}
}

/// Returns the Lit if self is a NestedMetaItemKind::Literal.
pub fn literal(&self) -> Option<&Lit> {
match self.node {
NestedMetaItemKind::Literal(ref lit) => Some(&lit),
NestedMetaItemKind::Literal(ref lit) => Some(lit),
_ => None
}
}
Expand Down Expand Up @@ -259,7 +259,7 @@ impl MetaItem {
match self.node {
MetaItemKind::NameValue(ref v) => {
match v.node {
LitKind::Str(ref s, _) => Some((*s).clone()),
LitKind::Str(ref s, _) => Some(*s),
_ => None,
}
},
Expand Down Expand Up @@ -1217,9 +1217,10 @@ impl LitKind {
Token::Literal(token::Lit::Float(symbol), Some(Symbol::intern(ty.ty_to_string())))
}
LitKind::FloatUnsuffixed(symbol) => Token::Literal(token::Lit::Float(symbol), None),
LitKind::Bool(value) => Token::Ident(Ident::with_empty_ctxt(Symbol::intern(match value {
true => "true",
false => "false",
LitKind::Bool(value) => Token::Ident(Ident::with_empty_ctxt(Symbol::intern(if value {
"true"
} else {
"false"
Copy link
Contributor

Choose a reason for hiding this comment

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

Style bikeshed: unless if cond { x } else { y } fits into one line, match is terser and more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Terser, yes. More readable, I guess we'll have to agree to disagree here.

}))),
}
}
Expand Down Expand Up @@ -1261,7 +1262,7 @@ impl<T: HasAttrs> HasAttrs for Spanned<T> {

impl HasAttrs for Vec<Attribute> {
fn attrs(&self) -> &[Attribute] {
&self
self
}
fn map_attrs<F: FnOnce(Vec<Attribute>) -> Vec<Attribute>>(self, f: F) -> Self {
f(self)
Expand All @@ -1270,7 +1271,7 @@ impl HasAttrs for Vec<Attribute> {

impl HasAttrs for ThinVec<Attribute> {
fn attrs(&self) -> &[Attribute] {
&self
self
}
fn map_attrs<F: FnOnce(Vec<Attribute>) -> Vec<Attribute>>(self, f: F) -> Self {
f(self.into()).into()
Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax/codemap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ impl CodeMap {
match self.span_to_snippet(sp) {
Ok(snippet) => {
let snippet = snippet.split(c).nth(0).unwrap_or("").trim_right();
if snippet.len() > 0 && !snippet.contains('\n') {
if !snippet.is_empty() && !snippet.contains('\n') {
Span { hi: BytePos(sp.lo.0 + snippet.len() as u32), ..sp }
} else {
sp
Expand All @@ -502,7 +502,7 @@ impl CodeMap {
pub fn get_filemap(&self, filename: &str) -> Option<Rc<FileMap>> {
for fm in self.files.borrow().iter() {
if filename == fm.name {
(self.dep_tracking_callback.borrow())(&fm);
(self.dep_tracking_callback.borrow())(fm);
return Some(fm.clone());
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/libsyntax/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ impl<'a> StripUnconfigured<'a> {
return false;
}

let mis = if !is_cfg(&attr) {
let mis = if !is_cfg(attr) {
return true;
} else if let Some(mis) = attr.meta_item_list() {
mis
Expand All @@ -150,7 +150,7 @@ impl<'a> StripUnconfigured<'a> {
// flag the offending attributes
for attr in attrs.iter() {
if !self.features.map(|features| features.stmt_expr_attributes).unwrap_or(true) {
let mut err = feature_err(&self.sess,
let mut err = feature_err(self.sess,
"stmt_expr_attributes",
attr.span,
GateIssue::Language,
Expand Down Expand Up @@ -258,7 +258,7 @@ impl<'a> StripUnconfigured<'a> {
pub fn configure_struct_expr_field(&mut self, field: ast::Field) -> Option<ast::Field> {
if !self.features.map(|features| features.struct_field_attributes).unwrap_or(true) {
if !field.attrs.is_empty() {
let mut err = feature_err(&self.sess,
let mut err = feature_err(self.sess,
"struct_field_attributes",
field.span,
GateIssue::Language,
Expand Down Expand Up @@ -290,7 +290,7 @@ impl<'a> StripUnconfigured<'a> {
for attr in attrs.iter() {
if !self.features.map(|features| features.struct_field_attributes).unwrap_or(true) {
let mut err = feature_err(
&self.sess,
self.sess,
"struct_field_attributes",
attr.span,
GateIssue::Language,
Expand Down
6 changes: 3 additions & 3 deletions src/libsyntax/diagnostics/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ pub fn expand_register_diagnostic<'cx>(ecx: &'cx mut ExtCtxt,

// URLs can be unavoidably longer than the line limit, so we allow them.
// Allowed format is: `[name]: https://www.rust-lang.org/`
let is_url = |l: &str| l.starts_with('[') && l.contains("]:") && l.contains("http");
let is_url = |l: &str| l.starts_with("[") && l.contains("]:") && l.contains("http");

if msg.lines().any(|line| line.len() > MAX_DESCRIPTION_WIDTH && !is_url(line)) {
ecx.span_err(span, &format!(
Expand Down Expand Up @@ -177,7 +177,7 @@ pub fn expand_build_diagnostic_array<'cx>(ecx: &'cx mut ExtCtxt,
if let Err(e) = output_metadata(ecx,
&target_triple,
&crate_name.name.as_str(),
&diagnostics) {
diagnostics) {
ecx.span_bug(span, &format!(
"error writing metadata for triple `{}` and crate `{}`, error: {}, \
cause: {:?}",
Expand Down Expand Up @@ -227,7 +227,7 @@ pub fn expand_build_diagnostic_array<'cx>(ecx: &'cx mut ExtCtxt,

MacEager::items(SmallVector::many(vec![
P(ast::Item {
ident: name.clone(),
ident: *name,
attrs: Vec::new(),
id: ast::DUMMY_NODE_ID,
node: ast::ItemKind::Const(
Expand Down
12 changes: 6 additions & 6 deletions src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -635,8 +635,8 @@ pub struct ExpansionData {
}

/// One of these is made during expansion and incrementally updated as we go;
/// when a macro expansion occurs, the resulting nodes have the backtrace()
/// -> expn_info of their expansion context stored into their span.
/// when a macro expansion occurs, the resulting nodes have the `backtrace()
/// -> expn_info` of their expansion context stored into their span.
pub struct ExtCtxt<'a> {
pub parse_sess: &'a parse::ParseSess,
pub ecfg: expand::ExpansionConfig<'a>,
Expand Down Expand Up @@ -709,7 +709,7 @@ impl<'a> ExtCtxt<'a> {
}
ctxt = info.call_site.ctxt;
last_macro = Some(info.call_site);
return Some(());
Some(())
}).is_none() {
break
}
Expand Down Expand Up @@ -770,9 +770,9 @@ impl<'a> ExtCtxt<'a> {
}
pub fn trace_macros_diag(&self) {
for (sp, notes) in self.expansions.iter() {
let mut db = self.parse_sess.span_diagnostic.span_note_diag(*sp, &"trace_macro");
let mut db = self.parse_sess.span_diagnostic.span_note_diag(*sp, "trace_macro");
for note in notes {
db.note(&note);
db.note(note);
}
db.emit();
}
Expand All @@ -795,7 +795,7 @@ impl<'a> ExtCtxt<'a> {
v.push(self.ident_of(s));
}
v.extend(components.iter().map(|s| self.ident_of(s)));
return v
v
}
pub fn name_of(&self, st: &str) -> ast::Name {
Symbol::intern(st)
Expand Down
26 changes: 11 additions & 15 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,19 +415,19 @@ impl<'a, 'b> MacroExpander<'a, 'b> {

match *ext {
MultiModifier(ref mac) => {
let meta = panictry!(attr.parse_meta(&self.cx.parse_sess));
let meta = panictry!(attr.parse_meta(self.cx.parse_sess));
let item = mac.expand(self.cx, attr.span, &meta, item);
kind.expect_from_annotatables(item)
}
MultiDecorator(ref mac) => {
let mut items = Vec::new();
let meta = panictry!(attr.parse_meta(&self.cx.parse_sess));
let meta = panictry!(attr.parse_meta(self.cx.parse_sess));
mac.expand(self.cx, attr.span, &meta, &item, &mut |item| items.push(item));
items.push(item);
kind.expect_from_annotatables(items)
}
SyntaxExtension::AttrProcMacro(ref mac) => {
let item_toks = stream_for_item(&item, &self.cx.parse_sess);
let item_toks = stream_for_item(&item, self.cx.parse_sess);

let span = Span { ctxt: self.cx.backtrace(), ..attr.span };
let tok_result = mac.expand(self.cx, attr.span, attr.tokens, item_toks);
Expand All @@ -439,7 +439,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
}
_ => {
let msg = &format!("macro `{}` may not be used in attributes", attr.path);
self.cx.span_err(attr.span, &msg);
self.cx.span_err(attr.span, msg);
kind.dummy(attr.span)
}
}
Expand All @@ -454,7 +454,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
};
let path = &mac.node.path;

let ident = ident.unwrap_or(keywords::Invalid.ident());
let ident = ident.unwrap_or_else(|| keywords::Invalid.ident());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does clippy recommend this replacement?
It thinks keywords::Invalid.ident() may be "heavy"? (It's not actually, and unwrap_or looked better.)

Copy link
Member

Choose a reason for hiding this comment

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

@petrochenkov I don't think clippy is able to determine whether a function is heavy or not. It recommends unwrap_or_else whenever the inner expression is not a variable/primitive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@petrochenkov I thought so, too, until I benchmarked a very simple expression (_.unwrap_or..(vec![])) with / without the else. Interestingly, though there's no allocation, and the result should be basically static, the closure version was a wee bit faster. Not that it matters in this place, so if you like, I can roll it back.

let marked_tts = noop_fold_tts(mac.node.stream(), &mut Marker(mark));
let opt_expanded = match *ext {
NormalTT(ref expandfun, exp_span, allow_internal_unstable) => {
Expand Down Expand Up @@ -591,7 +591,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
}
_ => {
let msg = &format!("macro `{}` may not be used for derive attributes", attr.path);
self.cx.span_err(span, &msg);
self.cx.span_err(span, msg);
kind.dummy(span)
}
}
Expand Down Expand Up @@ -749,19 +749,15 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
fn check_attributes(&mut self, attrs: &[ast::Attribute]) {
let features = self.cx.ecfg.features.unwrap();
for attr in attrs.iter() {
feature_gate::check_attribute(&attr, &self.cx.parse_sess, features);
feature_gate::check_attribute(attr, self.cx.parse_sess, features);
}
}
}

pub fn find_attr_invoc(attrs: &mut Vec<ast::Attribute>) -> Option<ast::Attribute> {
for i in 0 .. attrs.len() {
if !attr::is_known(&attrs[i]) && !is_builtin_attr(&attrs[i]) {
return Some(attrs.remove(i));
}
}

None
attrs.iter()
.position(|a| !attr::is_known(a) && !is_builtin_attr(a))
.map(|i| attrs.remove(i))
Copy link
Contributor

Choose a reason for hiding this comment

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

Style bikeshed: Closures, closures... Nobody likes good old obvious for loops anymore :(

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 do think that the code better conveys the meaning now.

}

// These are pretty nasty. Ideally, we would keep the tokens around, linked from
Expand Down Expand Up @@ -923,7 +919,7 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
let result = noop_fold_item(item, self);
self.cx.current_expansion.module = orig_module;
self.cx.current_expansion.directory_ownership = orig_directory_ownership;
return result;
result
}
// Ensure that test functions are accessible from the test harness.
ast::ItemKind::Fn(..) if self.cx.ecfg.should_test => {
Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax/ext/quote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use tokenstream::{TokenStream, TokenTree};
///
/// This is registered as a set of expression syntax extension called quote!
/// that lifts its argument token-tree to an AST representing the
/// construction of the same token tree, with token::SubstNt interpreted
/// construction of the same token tree, with `token::SubstNt` interpreted
/// as antiquotes (splices).

pub mod rt {
Expand Down Expand Up @@ -389,7 +389,7 @@ pub fn unflatten(tts: Vec<TokenTree>) -> Vec<TokenTree> {
result = results.pop().unwrap();
result.push(tree);
}
tree @ _ => result.push(tree),
tree => result.push(tree),
}
}
result
Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax/ext/source_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ pub fn expand_include_str(cx: &mut ExtCtxt, sp: Span, tts: &[tokenstream::TokenT
cx.span_err(sp,
&format!("{} wasn't a utf-8 file",
file.display()));
return DummyResult::expr(sp);
DummyResult::expr(sp)
}
}
}
Expand All @@ -167,7 +167,7 @@ pub fn expand_include_bytes(cx: &mut ExtCtxt, sp: Span, tts: &[tokenstream::Toke
Err(e) => {
cx.span_err(sp,
&format!("couldn't read {}: {}", file.display(), e));
return DummyResult::expr(sp);
DummyResult::expr(sp)
}
Ok(..) => {
// Add this input file to the code map to make it available as
Expand Down
Loading