Skip to content

Commit bc96471

Browse files
DaniPopesDaniPopes
authored
chore: remove most impl AsRef<str,Path> (#157)
These mostly only bloat code size and compilation times, which are already not great when we are already codegenning a billion structs and ASTs with serde derives For AsRef<str>, the only change is having to specify an extra 1 (one) character (&) when using `String` so it derefs to `str`, otherwise it's the same For AsRef<Path> it's the same as AsRef<str> when using Path/PathBuf, but it's a bit worse for string literals; but most of these functions are called with variables in the first place. except in tests which will have to use .as_ref() or Path::new I've not removed these when using with &[] or impl IntoIterator since that does help quite a bit, e.g if you have an owning iterator of Strings, you cannot borrow inside of a map closure I've also kept Into<PathBuf> since that makes more sense, you want to avoid cloning if you can pass in an owned thing already, otherwise you'll clone it yourself Also fixes some inconsistencies in function signatures for str vs path
1 parent 121f284 commit bc96471

File tree

26 files changed

+457
-637
lines changed

26 files changed

+457
-637
lines changed

‎crates/artifacts/solc/src/ast/lowfidelity.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,9 @@ pub struct Node {
5353

5454
impl Node {
5555
/// Deserialize a serialized node attribute.
56-
pub fn attribute<D: DeserializeOwned>(&self, key: impl AsRef<str>) -> Option<D> {
56+
pub fn attribute<D: DeserializeOwned>(&self, key: &str) -> Option<D> {
5757
// TODO: Can we avoid this clone?
58-
self.other.get(key.as_ref()).and_then(|v| serde_json::from_value(v.clone()).ok())
58+
self.other.get(key).and_then(|v| serde_json::from_value(v.clone()).ok())
5959
}
6060
}
6161

‎crates/artifacts/solc/src/ast/mod.rs‎

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,30 +1100,28 @@ ast_node!(
11001100
#[cfg(test)]
11011101
mod tests {
11021102
use super::*;
1103-
use std::{fs, path::PathBuf};
1103+
use std::{fs, path::Path};
11041104

11051105
#[test]
11061106
fn can_parse_ast() {
1107-
fs::read_dir(
1108-
PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../../../test-data").join("ast"),
1109-
)
1110-
.unwrap()
1111-
.for_each(|path| {
1112-
let path = path.unwrap().path();
1113-
let path_str = path.to_string_lossy();
1114-
1115-
let input = fs::read_to_string(&path).unwrap();
1116-
let deserializer = &mut serde_json::Deserializer::from_str(&input);
1117-
let result: Result<SourceUnit, _> = serde_path_to_error::deserialize(deserializer);
1118-
match result {
1119-
Err(e) => {
1120-
println!("... {path_str} fail: {e}");
1121-
panic!();
1107+
fs::read_dir(Path::new(env!("CARGO_MANIFEST_DIR")).join("../../../test-data").join("ast"))
1108+
.unwrap()
1109+
.for_each(|path| {
1110+
let path = path.unwrap().path();
1111+
let path_str = path.to_string_lossy();
1112+
1113+
let input = fs::read_to_string(&path).unwrap();
1114+
let deserializer = &mut serde_json::Deserializer::from_str(&input);
1115+
let result: Result<SourceUnit, _> = serde_path_to_error::deserialize(deserializer);
1116+
match result {
1117+
Err(e) => {
1118+
println!("... {path_str} fail: {e}");
1119+
panic!();
1120+
}
1121+
Ok(_) => {
1122+
println!("... {path_str} ok");
1123+
}
11221124
}
1123-
Ok(_) => {
1124-
println!("... {path_str} ok");
1125-
}
1126-
}
1127-
})
1125+
})
11281126
}
11291127
}

‎crates/artifacts/solc/src/bytecode.rs‎

Lines changed: 13 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -66,18 +66,11 @@ impl CompactBytecode {
6666
///
6767
/// Returns true if the bytecode object is fully linked, false otherwise
6868
/// This is a noop if the bytecode object is already fully linked.
69-
pub fn link(
70-
&mut self,
71-
file: impl AsRef<str>,
72-
library: impl AsRef<str>,
73-
address: Address,
74-
) -> bool {
69+
pub fn link(&mut self, file: &str, library: &str, address: Address) -> bool {
7570
if !self.object.is_unlinked() {
7671
return true;
7772
}
7873

79-
let file = file.as_ref();
80-
let library = library.as_ref();
8174
if let Some((key, mut contracts)) = self.link_references.remove_entry(file) {
8275
if contracts.remove(library).is_some() {
8376
self.object.link(file, library, address);
@@ -148,8 +141,8 @@ impl Bytecode {
148141
}
149142

150143
/// Same as `Bytecode::link` but with fully qualified name (`file.sol:Math`)
151-
pub fn link_fully_qualified(&mut self, name: impl AsRef<str>, addr: Address) -> bool {
152-
if let Some((file, lib)) = name.as_ref().split_once(':') {
144+
pub fn link_fully_qualified(&mut self, name: &str, addr: Address) -> bool {
145+
if let Some((file, lib)) = name.split_once(':') {
153146
self.link(file, lib, addr)
154147
} else {
155148
false
@@ -161,18 +154,11 @@ impl Bytecode {
161154
///
162155
/// Returns true if the bytecode object is fully linked, false otherwise
163156
/// This is a noop if the bytecode object is already fully linked.
164-
pub fn link(
165-
&mut self,
166-
file: impl AsRef<str>,
167-
library: impl AsRef<str>,
168-
address: Address,
169-
) -> bool {
157+
pub fn link(&mut self, file: &str, library: &str, address: Address) -> bool {
170158
if !self.object.is_unlinked() {
171159
return true;
172160
}
173161

174-
let file = file.as_ref();
175-
let library = library.as_ref();
176162
if let Some((key, mut contracts)) = self.link_references.remove_entry(file) {
177163
if contracts.remove(library).is_some() {
178164
self.object.link(file, library, address);
@@ -195,7 +181,7 @@ impl Bytecode {
195181
T: AsRef<str>,
196182
{
197183
for (file, lib, addr) in libs.into_iter() {
198-
if self.link(file, lib, addr) {
184+
if self.link(file.as_ref(), lib.as_ref(), addr) {
199185
return true;
200186
}
201187
}
@@ -209,7 +195,7 @@ impl Bytecode {
209195
S: AsRef<str>,
210196
{
211197
for (name, addr) in libs.into_iter() {
212-
if self.link_fully_qualified(name, addr) {
198+
if self.link_fully_qualified(name.as_ref(), addr) {
213199
return true;
214200
}
215201
}
@@ -316,9 +302,8 @@ impl BytecodeObject {
316302
/// This will replace all occurrences of the library placeholder with the given address.
317303
///
318304
/// See also: <https://docs.soliditylang.org/en/develop/using-the-compiler.html#library-linking>
319-
pub fn link_fully_qualified(&mut self, name: impl AsRef<str>, addr: Address) -> &mut Self {
305+
pub fn link_fully_qualified(&mut self, name: &str, addr: Address) -> &mut Self {
320306
if let Self::Unlinked(ref mut unlinked) = self {
321-
let name = name.as_ref();
322307
let place_holder = utils::library_hash_placeholder(name);
323308
// the address as hex without prefix
324309
let hex_addr = hex::encode(addr);
@@ -337,13 +322,8 @@ impl BytecodeObject {
337322
/// Links using the `file` and `library` names as fully qualified name `<file>:<library>`.
338323
///
339324
/// See [`link_fully_qualified`](Self::link_fully_qualified).
340-
pub fn link(
341-
&mut self,
342-
file: impl AsRef<str>,
343-
library: impl AsRef<str>,
344-
addr: Address,
345-
) -> &mut Self {
346-
self.link_fully_qualified(format!("{}:{}", file.as_ref(), library.as_ref()), addr)
325+
pub fn link(&mut self, file: &str, library: &str, addr: Address) -> &mut Self {
326+
self.link_fully_qualified(&format!("{file}:{library}"), addr)
347327
}
348328

349329
/// Links the bytecode object with all provided `(file, lib, addr)`.
@@ -354,15 +334,14 @@ impl BytecodeObject {
354334
T: AsRef<str>,
355335
{
356336
for (file, lib, addr) in libs.into_iter() {
357-
self.link(file, lib, addr);
337+
self.link(file.as_ref(), lib.as_ref(), addr);
358338
}
359339
self
360340
}
361341

362342
/// Returns whether the bytecode contains a matching placeholder using the qualified name.
363-
pub fn contains_fully_qualified_placeholder(&self, name: impl AsRef<str>) -> bool {
343+
pub fn contains_fully_qualified_placeholder(&self, name: &str) -> bool {
364344
if let Self::Unlinked(unlinked) = self {
365-
let name = name.as_ref();
366345
unlinked.contains(&utils::library_hash_placeholder(name))
367346
|| unlinked.contains(&utils::library_fully_qualified_placeholder(name))
368347
} else {
@@ -371,8 +350,8 @@ impl BytecodeObject {
371350
}
372351

373352
/// Returns whether the bytecode contains a matching placeholder.
374-
pub fn contains_placeholder(&self, file: impl AsRef<str>, library: impl AsRef<str>) -> bool {
375-
self.contains_fully_qualified_placeholder(format!("{}:{}", file.as_ref(), library.as_ref()))
353+
pub fn contains_placeholder(&self, file: &str, library: &str) -> bool {
354+
self.contains_fully_qualified_placeholder(&format!("{file}:{library}"))
376355
}
377356
}
378357

‎crates/artifacts/solc/src/lib.rs‎

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -158,15 +158,13 @@ impl SolcInput {
158158

159159
/// Sets the path of the source files to `root` adjoined to the existing path
160160
#[must_use]
161-
pub fn join_path(mut self, root: impl AsRef<Path>) -> Self {
162-
let root = root.as_ref();
161+
pub fn join_path(mut self, root: &Path) -> Self {
163162
self.sources = self.sources.into_iter().map(|(path, s)| (root.join(path), s)).collect();
164163
self
165164
}
166165

167166
/// Removes the `base` path from all source files
168-
pub fn strip_prefix(&mut self, base: impl AsRef<Path>) {
169-
let base = base.as_ref();
167+
pub fn strip_prefix(&mut self, base: &Path) {
170168
self.sources = std::mem::take(&mut self.sources)
171169
.into_iter()
172170
.map(|(path, s)| (path.strip_prefix(base).map(Into::into).unwrap_or(path), s))
@@ -490,8 +488,7 @@ impl Settings {
490488
self
491489
}
492490

493-
pub fn strip_prefix(&mut self, base: impl AsRef<Path>) {
494-
let base = base.as_ref();
491+
pub fn strip_prefix(&mut self, base: &Path) {
495492
self.remappings.iter_mut().for_each(|r| {
496493
r.strip_prefix(base);
497494
});
@@ -535,7 +532,7 @@ impl Settings {
535532
}
536533

537534
/// Strips `base` from all paths
538-
pub fn with_base_path(mut self, base: impl AsRef<Path>) -> Self {
535+
pub fn with_base_path(mut self, base: &Path) -> Self {
539536
self.strip_prefix(base);
540537
self
541538
}
@@ -624,8 +621,7 @@ impl Libraries {
624621

625622
/// Strips the given prefix from all library file paths to make them relative to the given
626623
/// `base` argument
627-
pub fn with_stripped_file_prefixes(mut self, base: impl AsRef<Path>) -> Self {
628-
let base = base.as_ref();
624+
pub fn with_stripped_file_prefixes(mut self, base: &Path) -> Self {
629625
self.libs = self
630626
.libs
631627
.into_iter()
@@ -1510,16 +1506,14 @@ impl CompilerOutput {
15101506
}
15111507

15121508
/// Finds the _first_ contract with the given name
1513-
pub fn find(&self, contract: impl AsRef<str>) -> Option<CompactContractRef<'_>> {
1514-
let contract_name = contract.as_ref();
1509+
pub fn find(&self, contract_name: &str) -> Option<CompactContractRef<'_>> {
15151510
self.contracts_iter().find_map(|(name, contract)| {
15161511
(name == contract_name).then(|| CompactContractRef::from(contract))
15171512
})
15181513
}
15191514

15201515
/// Finds the first contract with the given name and removes it from the set
1521-
pub fn remove(&mut self, contract: impl AsRef<str>) -> Option<Contract> {
1522-
let contract_name = contract.as_ref();
1516+
pub fn remove(&mut self, contract_name: &str) -> Option<Contract> {
15231517
self.contracts.values_mut().find_map(|c| c.remove(contract_name))
15241518
}
15251519

@@ -1586,16 +1580,14 @@ impl OutputContracts {
15861580
}
15871581

15881582
/// Finds the _first_ contract with the given name
1589-
pub fn find(&self, contract: impl AsRef<str>) -> Option<CompactContractRef<'_>> {
1590-
let contract_name = contract.as_ref();
1583+
pub fn find(&self, contract_name: &str) -> Option<CompactContractRef<'_>> {
15911584
self.contracts_iter().find_map(|(name, contract)| {
15921585
(name == contract_name).then(|| CompactContractRef::from(contract))
15931586
})
15941587
}
15951588

15961589
/// Finds the first contract with the given name and removes it from the set
1597-
pub fn remove(&mut self, contract: impl AsRef<str>) -> Option<Contract> {
1598-
let contract_name = contract.as_ref();
1590+
pub fn remove(&mut self, contract_name: &str) -> Option<Contract> {
15991591
self.0.values_mut().find_map(|c| c.remove(contract_name))
16001592
}
16011593
}
@@ -1920,7 +1912,7 @@ mod tests {
19201912

19211913
#[test]
19221914
fn can_parse_compiler_output() {
1923-
let dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../../../test-data/out");
1915+
let dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../../test-data/out");
19241916

19251917
for path in fs::read_dir(dir).unwrap() {
19261918
let path = path.unwrap().path();
@@ -1933,7 +1925,7 @@ mod tests {
19331925

19341926
#[test]
19351927
fn can_parse_compiler_input() {
1936-
let dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../../../test-data/in");
1928+
let dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../../test-data/in");
19371929

19381930
for path in fs::read_dir(dir).unwrap() {
19391931
let path = path.unwrap().path();
@@ -1946,7 +1938,7 @@ mod tests {
19461938

19471939
#[test]
19481940
fn can_parse_standard_json_compiler_input() {
1949-
let dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../../../test-data/in");
1941+
let dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../../test-data/in");
19501942

19511943
for path in fs::read_dir(dir).unwrap() {
19521944
let path = path.unwrap().path();
@@ -2078,7 +2070,7 @@ mod tests {
20782070

20792071
let libs = Libraries::parse(&libraries[..])
20802072
.unwrap()
2081-
.with_stripped_file_prefixes("/global/root")
2073+
.with_stripped_file_prefixes("/global/root".as_ref())
20822074
.libs;
20832075

20842076
assert_eq!(
@@ -2208,8 +2200,8 @@ mod tests {
22082200
// <https://github.com/foundry-rs/foundry/issues/3012>
22092201
#[test]
22102202
fn can_parse_compiler_output_spells_0_6_12() {
2211-
let path = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
2212-
.join("../../../test-data/0.6.12-with-libs.json");
2203+
let path =
2204+
Path::new(env!("CARGO_MANIFEST_DIR")).join("../../../test-data/0.6.12-with-libs.json");
22132205
let content = fs::read_to_string(path).unwrap();
22142206
let _output: CompilerOutput = serde_json::from_str(&content).unwrap();
22152207
}

0 commit comments

Comments
 (0)