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

Refactor/fix clippy lints #15615

Merged
merged 4 commits into from
Sep 22, 2023
+50 −55
Merged

Refactor/fix clippy lints #15615

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ebbbaaa
refactor: fix clippy lints
shogo-nakano-desu Sep 15, 2023
0bb2298
refactor: remove TODO with no explanation
shogo-nakano-desu Sep 15, 2023
96c3332
refactor: fix clippy lint
shogo-nakano-desu Sep 15, 2023
dd84306
refactor: remove boxing
shogo-nakano-desu Sep 20, 2023
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
6 changes: 4 additions & 2 deletions crates/flycheck/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,9 @@ impl CargoActor {
// Skip certain kinds of messages to only spend time on what's useful
JsonMessage::Cargo(message) => match message {
cargo_metadata::Message::CompilerArtifact(artifact) if !artifact.fresh => {
self.sender.send(CargoMessage::CompilerArtifact(artifact)).unwrap();
self.sender
.send(CargoMessage::CompilerArtifact(Box::new(artifact)))
.unwrap();
}
cargo_metadata::Message::CompilerMessage(msg) => {
self.sender.send(CargoMessage::Diagnostic(msg.message)).unwrap();
Expand Down Expand Up @@ -533,7 +535,7 @@ impl CargoActor {
}

enum CargoMessage {
CompilerArtifact(cargo_metadata::Artifact),
CompilerArtifact(Box<cargo_metadata::Artifact>),
Copy link
Member

Choose a reason for hiding this comment

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

Last time this came up (can't find the PR, though), it was pointed out that this is the most common variant, so boxing the contents will cause almost every instance to incur an allocation.

I don't know if this overhead will be larger than the speed-up obtained from the faster moves. Either way, I suspect it doesn't matter too much compared to the whole cargo check invocation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see. I will keep it in my mind. Thank you for your review.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lnicola I have removed Box in
dd84306 . Thanks

Diagnostic(Diagnostic),
}

Expand Down
14 changes: 4 additions & 10 deletions crates/intern/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,10 @@ impl<T: Internable> Interned<T> {
// - if not, box it up, insert it, and return a clone
// This needs to be atomic (locking the shard) to avoid races with other thread, which could
// insert the same object between us looking it up and inserting it.
match shard.raw_entry_mut().from_key_hashed_nocheck(hash as u64, &obj) {
match shard.raw_entry_mut().from_key_hashed_nocheck(hash, &obj) {
RawEntryMut::Occupied(occ) => Self { arc: occ.key().clone() },
RawEntryMut::Vacant(vac) => Self {
arc: vac
.insert_hashed_nocheck(hash as u64, Arc::new(obj), SharedValue::new(()))
.0
.clone(),
arc: vac.insert_hashed_nocheck(hash, Arc::new(obj), SharedValue::new(())).0.clone(),
},
}
}
Expand All @@ -54,13 +51,10 @@ impl Interned<str> {
// - if not, box it up, insert it, and return a clone
// This needs to be atomic (locking the shard) to avoid races with other thread, which could
// insert the same object between us looking it up and inserting it.
match shard.raw_entry_mut().from_key_hashed_nocheck(hash as u64, s) {
match shard.raw_entry_mut().from_key_hashed_nocheck(hash, s) {
RawEntryMut::Occupied(occ) => Self { arc: occ.key().clone() },
RawEntryMut::Vacant(vac) => Self {
arc: vac
.insert_hashed_nocheck(hash as u64, Arc::from(s), SharedValue::new(()))
.0
.clone(),
arc: vac.insert_hashed_nocheck(hash, Arc::from(s), SharedValue::new(())).0.clone(),
},
}
}
Expand Down
36 changes: 17 additions & 19 deletions crates/parser/src/shortcuts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,29 +32,27 @@ impl LexedStr<'_> {
let kind = self.kind(i);
if kind.is_trivia() {
was_joint = false
} else if kind == SyntaxKind::IDENT {
let token_text = self.text(i);
let contextual_kw =
SyntaxKind::from_contextual_keyword(token_text).unwrap_or(SyntaxKind::IDENT);
res.push_ident(contextual_kw);
} else {
if kind == SyntaxKind::IDENT {
let token_text = self.text(i);
let contextual_kw = SyntaxKind::from_contextual_keyword(token_text)
.unwrap_or(SyntaxKind::IDENT);
res.push_ident(contextual_kw);
} else {
if was_joint {
if was_joint {
res.was_joint();
}
res.push(kind);
// Tag the token as joint if it is float with a fractional part
// we use this jointness to inform the parser about what token split
// event to emit when we encounter a float literal in a field access
if kind == SyntaxKind::FLOAT_NUMBER {
if !self.text(i).ends_with('.') {
res.was_joint();
}
res.push(kind);
// Tag the token as joint if it is float with a fractional part
// we use this jointness to inform the parser about what token split
// event to emit when we encounter a float literal in a field access
if kind == SyntaxKind::FLOAT_NUMBER {
if !self.text(i).ends_with('.') {
res.was_joint();
} else {
was_joint = false;
}
} else {
was_joint = true;
was_joint = false;
}
} else {
was_joint = true;
}
}
}
Expand Down
14 changes: 10 additions & 4 deletions crates/syntax/src/ast/edit_in_place.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ pub trait AttrsOwnerEdit: ast::HasAttrs {
let after_attrs_and_comments = node
.children_with_tokens()
.find(|it| !matches!(it.kind(), WHITESPACE | COMMENT | ATTR))
.map_or(Position::first_child_of(node), |it| Position::before(it));
.map_or(Position::first_child_of(node), Position::before);

ted::insert_all(
after_attrs_and_comments,
Expand Down Expand Up @@ -433,7 +433,9 @@ impl ast::UseTree {
if &path == prefix && self.use_tree_list().is_none() {
if self.star_token().is_some() {
// path$0::* -> *
self.coloncolon_token().map(ted::remove);
if let Some(a) = self.coloncolon_token() {
ted::remove(a)
}
ted::remove(prefix.syntax());
} else {
// path$0 -> self
Expand All @@ -460,7 +462,9 @@ impl ast::UseTree {
for p in successors(parent.parent_path(), |it| it.parent_path()) {
p.segment()?;
}
prefix.parent_path().and_then(|p| p.coloncolon_token()).map(ted::remove);
if let Some(a) = prefix.parent_path().and_then(|p| p.coloncolon_token()) {
ted::remove(a)
}
ted::remove(prefix.syntax());
Some(())
}
Expand Down Expand Up @@ -976,7 +980,9 @@ enum Foo {

fn check_add_variant(before: &str, expected: &str, variant: ast::Variant) {
let enum_ = ast_mut_from_text::<ast::Enum>(before);
enum_.variant_list().map(|it| it.add_variant(variant));
if let Some(it) = enum_.variant_list() {
it.add_variant(variant)
}
let after = enum_.to_string();
assert_eq_text!(&trim_indent(expected.trim()), &trim_indent(after.trim()));
}
Expand Down
1 change: 0 additions & 1 deletion crates/syntax/src/ast/make.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,6 @@ pub fn record_field(
ast_from_text(&format!("struct S {{ {visibility}{name}: {ty}, }}"))
}

// TODO
pub fn block_expr(
stmts: impl IntoIterator<Item = ast::Stmt>,
tail_expr: Option<ast::Expr>,
Expand Down
36 changes: 17 additions & 19 deletions crates/syntax/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,29 +181,27 @@ impl ast::TokenTree {
let kind = t.kind();
if kind.is_trivia() {
was_joint = false
} else if kind == SyntaxKind::IDENT {
let token_text = t.text();
let contextual_kw =
SyntaxKind::from_contextual_keyword(token_text).unwrap_or(SyntaxKind::IDENT);
parser_input.push_ident(contextual_kw);
} else {
if kind == SyntaxKind::IDENT {
let token_text = t.text();
let contextual_kw = SyntaxKind::from_contextual_keyword(token_text)
.unwrap_or(SyntaxKind::IDENT);
parser_input.push_ident(contextual_kw);
} else {
if was_joint {
if was_joint {
parser_input.was_joint();
}
parser_input.push(kind);
// Tag the token as joint if it is float with a fractional part
// we use this jointness to inform the parser about what token split
// event to emit when we encounter a float literal in a field access
if kind == SyntaxKind::FLOAT_NUMBER {
if !t.text().ends_with('.') {
parser_input.was_joint();
}
parser_input.push(kind);
// Tag the token as joint if it is float with a fractional part
// we use this jointness to inform the parser about what token split
// event to emit when we encounter a float literal in a field access
if kind == SyntaxKind::FLOAT_NUMBER {
if !t.text().ends_with('.') {
parser_input.was_joint();
} else {
was_joint = false;
}
} else {
was_joint = true;
was_joint = false;
}
} else {
was_joint = true;
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/syntax/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ use crate::{ast, fuzz, AstNode, SourceFile, SyntaxError};

#[test]
fn parse_smoke_test() {
let code = r##"
let code = r#"
fn main() {
println!("Hello, world!")
}
"##;
"#;

let parse = SourceFile::parse(code);
// eprintln!("{:#?}", parse.syntax_node());
Expand Down