Skip to content

Commit e629dba

Browse files
committed
Auto merge of #28256 - petrochenkov:conv, r=alexcrichton
This patch transforms functions of the form ``` fn f<Generic: AsRef<Concrete>>(arg: Generic) { let arg: &Concrete = arg.as_ref(); // Code using arg } ``` to the next form: ``` #[inline] fn f<Generic: AsRef<Concrete>>(arg: Generic) { fn f_inner(arg: &Concrete) { // Code using arg } f_inner(arg.as_ref()); } ``` Therefore, most of the code is concrete and not duplicated during monomorphisation (unless inlined) and only the tiny bit of conversion code is duplicated. This method was mentioned by @aturon in the Conversion Traits RFC (https://github.com/rust-lang/rfcs/blame/master/text/0529-conversion-traits.md#L249) and similar techniques are not uncommon in C++ template libraries. This patch goes to the extremes and applies the transformation even to smaller functions<sup>1</sup> for purity of the experiment. *Some of them can be rolled back* if considered too ridiculous. <sup>1</sup> However who knows how small are these functions are after inlining and everything. The functions in question are mostly `fs`/`os` functions and not used especially often with variety of argument types, so the code size reduction is rather small (but consistent). Here are the sizes of stage2 artifacts before and after the patch: https://gist.github.com/petrochenkov/e76a6b280f382da13c5d https://gist.github.com/petrochenkov/6cc28727d5256dbdfed0 Note: All the `inner` functions are concrete and unavailable for cross-crate inlining, some of them may need `#[inline]` annotations in the future. r? @aturon
2 parents bc6c397 + 1049021 commit e629dba

File tree

7 files changed

+101
-28
lines changed

7 files changed

+101
-28
lines changed

src/librustc_back/tempdir.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,12 @@ impl TempDir {
3838
#[allow(deprecated)] // rand usage
3939
pub fn new_in<P: AsRef<Path>>(tmpdir: P, prefix: &str)
4040
-> io::Result<TempDir> {
41+
Self::_new_in(tmpdir.as_ref(), prefix)
42+
}
43+
44+
fn _new_in(tmpdir: &Path, prefix: &str) -> io::Result<TempDir> {
4145
let storage;
42-
let mut tmpdir = tmpdir.as_ref();
46+
let mut tmpdir = tmpdir;
4347
if !tmpdir.is_absolute() {
4448
let cur_dir = try!(env::current_dir());
4549
storage = cur_dir.join(tmpdir);

src/libstd/env.rs

+19-3
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,10 @@ impl Iterator for VarsOs {
174174
/// ```
175175
#[stable(feature = "env", since = "1.0.0")]
176176
pub fn var<K: AsRef<OsStr>>(key: K) -> Result<String, VarError> {
177+
_var(key.as_ref())
178+
}
179+
180+
fn _var(key: &OsStr) -> Result<String, VarError> {
177181
match var_os(key) {
178182
Some(s) => s.into_string().map_err(VarError::NotUnicode),
179183
None => Err(VarError::NotPresent)
@@ -196,8 +200,12 @@ pub fn var<K: AsRef<OsStr>>(key: K) -> Result<String, VarError> {
196200
/// ```
197201
#[stable(feature = "env", since = "1.0.0")]
198202
pub fn var_os<K: AsRef<OsStr>>(key: K) -> Option<OsString> {
203+
_var_os(key.as_ref())
204+
}
205+
206+
fn _var_os(key: &OsStr) -> Option<OsString> {
199207
let _g = ENV_LOCK.lock();
200-
os_imp::getenv(key.as_ref())
208+
os_imp::getenv(key)
201209
}
202210

203211
/// Possible errors from the `env::var` method.
@@ -263,8 +271,12 @@ impl Error for VarError {
263271
/// ```
264272
#[stable(feature = "env", since = "1.0.0")]
265273
pub fn set_var<K: AsRef<OsStr>, V: AsRef<OsStr>>(k: K, v: V) {
274+
_set_var(k.as_ref(), v.as_ref())
275+
}
276+
277+
fn _set_var(k: &OsStr, v: &OsStr) {
266278
let _g = ENV_LOCK.lock();
267-
os_imp::setenv(k.as_ref(), v.as_ref())
279+
os_imp::setenv(k, v)
268280
}
269281

270282
/// Removes an environment variable from the environment of the currently running process.
@@ -294,8 +306,12 @@ pub fn set_var<K: AsRef<OsStr>, V: AsRef<OsStr>>(k: K, v: V) {
294306
/// ```
295307
#[stable(feature = "env", since = "1.0.0")]
296308
pub fn remove_var<K: AsRef<OsStr>>(k: K) {
309+
_remove_var(k.as_ref())
310+
}
311+
312+
fn _remove_var(k: &OsStr) {
297313
let _g = ENV_LOCK.lock();
298-
os_imp::unsetenv(k.as_ref())
314+
os_imp::unsetenv(k)
299315
}
300316

301317
/// An iterator over `Path` instances for parsing an environment variable

src/libstd/ffi/c_str.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,10 @@ impl CString {
181181
/// the position of the nul byte.
182182
#[stable(feature = "rust1", since = "1.0.0")]
183183
pub fn new<T: Into<Vec<u8>>>(t: T) -> Result<CString, NulError> {
184-
let bytes = t.into();
184+
Self::_new(t.into())
185+
}
186+
187+
fn _new(bytes: Vec<u8>) -> Result<CString, NulError> {
185188
match bytes.iter().position(|x| *x == 0) {
186189
Some(i) => Err(NulError(i, bytes)),
187190
None => Ok(unsafe { CString::from_vec_unchecked(bytes) }),

src/libstd/ffi/os_str.rs

+10-10
Original file line numberDiff line numberDiff line change
@@ -73,18 +73,18 @@ impl OsString {
7373
/// convert; non UTF-8 data will produce `None`.
7474
#[unstable(feature = "convert", reason = "recently added", issue = "27704")]
7575
pub fn from_bytes<B>(bytes: B) -> Option<OsString> where B: Into<Vec<u8>> {
76-
#[cfg(unix)]
77-
fn from_bytes_inner(vec: Vec<u8>) -> Option<OsString> {
78-
use os::unix::ffi::OsStringExt;
79-
Some(OsString::from_vec(vec))
80-
}
76+
Self::_from_bytes(bytes.into())
77+
}
8178

82-
#[cfg(windows)]
83-
fn from_bytes_inner(vec: Vec<u8>) -> Option<OsString> {
84-
String::from_utf8(vec).ok().map(OsString::from)
85-
}
79+
#[cfg(unix)]
80+
fn _from_bytes(vec: Vec<u8>) -> Option<OsString> {
81+
use os::unix::ffi::OsStringExt;
82+
Some(OsString::from_vec(vec))
83+
}
8684

87-
from_bytes_inner(bytes.into())
85+
#[cfg(windows)]
86+
fn _from_bytes(vec: Vec<u8>) -> Option<OsString> {
87+
String::from_utf8(vec).ok().map(OsString::from)
8888
}
8989

9090
/// Converts to an `OsStr` slice.

src/libstd/fs.rs

+18-5
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ impl File {
184184
/// ```
185185
#[stable(feature = "rust1", since = "1.0.0")]
186186
pub fn open<P: AsRef<Path>>(path: P) -> io::Result<File> {
187-
OpenOptions::new().read(true).open(path)
187+
OpenOptions::new().read(true).open(path.as_ref())
188188
}
189189

190190
/// Opens a file in write-only mode.
@@ -206,7 +206,7 @@ impl File {
206206
/// ```
207207
#[stable(feature = "rust1", since = "1.0.0")]
208208
pub fn create<P: AsRef<Path>>(path: P) -> io::Result<File> {
209-
OpenOptions::new().write(true).create(true).truncate(true).open(path)
209+
OpenOptions::new().write(true).create(true).truncate(true).open(path.as_ref())
210210
}
211211

212212
/// Attempts to sync all OS-internal metadata to disk.
@@ -494,7 +494,10 @@ impl OpenOptions {
494494
/// ```
495495
#[stable(feature = "rust1", since = "1.0.0")]
496496
pub fn open<P: AsRef<Path>>(&self, path: P) -> io::Result<File> {
497-
let path = path.as_ref();
497+
self._open(path.as_ref())
498+
}
499+
500+
fn _open(&self, path: &Path) -> io::Result<File> {
498501
let inner = try!(fs_imp::File::open(path, &self.0));
499502
Ok(File { inner: inner })
500503
}
@@ -1048,7 +1051,10 @@ pub fn remove_dir<P: AsRef<Path>>(path: P) -> io::Result<()> {
10481051
/// ```
10491052
#[stable(feature = "rust1", since = "1.0.0")]
10501053
pub fn remove_dir_all<P: AsRef<Path>>(path: P) -> io::Result<()> {
1051-
let path = path.as_ref();
1054+
_remove_dir_all(path.as_ref())
1055+
}
1056+
1057+
fn _remove_dir_all(path: &Path) -> io::Result<()> {
10521058
for child in try!(read_dir(path)) {
10531059
let child = try!(child).path();
10541060
let stat = try!(symlink_metadata(&*child));
@@ -1113,6 +1119,10 @@ pub fn read_dir<P: AsRef<Path>>(path: P) -> io::Result<ReadDir> {
11131119
as symlinks differently",
11141120
issue = "27707")]
11151121
pub fn walk_dir<P: AsRef<Path>>(path: P) -> io::Result<WalkDir> {
1122+
_walk_dir(path.as_ref())
1123+
}
1124+
1125+
fn _walk_dir(path: &Path) -> io::Result<WalkDir> {
11161126
let start = try!(read_dir(path));
11171127
Ok(WalkDir { cur: Some(start), stack: Vec::new() })
11181128
}
@@ -1272,7 +1282,10 @@ impl DirBuilder {
12721282
/// Create the specified directory with the options configured in this
12731283
/// builder.
12741284
pub fn create<P: AsRef<Path>>(&self, path: P) -> io::Result<()> {
1275-
let path = path.as_ref();
1285+
self._create(path.as_ref())
1286+
}
1287+
1288+
fn _create(&self, path: &Path) -> io::Result<()> {
12761289
if self.recursive {
12771290
self.create_dir_all(path)
12781291
} else {

src/libstd/io/error.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -191,10 +191,14 @@ impl Error {
191191
pub fn new<E>(kind: ErrorKind, error: E) -> Error
192192
where E: Into<Box<error::Error+Send+Sync>>
193193
{
194+
Self::_new(kind, error.into())
195+
}
196+
197+
fn _new(kind: ErrorKind, error: Box<error::Error+Send+Sync>) -> Error {
194198
Error {
195199
repr: Repr::Custom(Box::new(Custom {
196200
kind: kind,
197-
error: error.into(),
201+
error: error,
198202
}))
199203
}
200204
}

src/libstd/path.rs

+40-7
Original file line numberDiff line numberDiff line change
@@ -965,8 +965,10 @@ impl PathBuf {
965965
/// * if `path` has a prefix but no root, it replaces `self`.
966966
#[stable(feature = "rust1", since = "1.0.0")]
967967
pub fn push<P: AsRef<Path>>(&mut self, path: P) {
968-
let path = path.as_ref();
968+
self._push(path.as_ref())
969+
}
969970

971+
fn _push(&mut self, path: &Path) {
970972
// in general, a separator is needed if the rightmost byte is not a separator
971973
let mut need_sep = self.as_mut_vec().last().map(|c| !is_sep_byte(*c)).unwrap_or(false);
972974

@@ -1033,11 +1035,15 @@ impl PathBuf {
10331035
/// ```
10341036
#[stable(feature = "rust1", since = "1.0.0")]
10351037
pub fn set_file_name<S: AsRef<OsStr>>(&mut self, file_name: S) {
1038+
self._set_file_name(file_name.as_ref())
1039+
}
1040+
1041+
fn _set_file_name(&mut self, file_name: &OsStr) {
10361042
if self.file_name().is_some() {
10371043
let popped = self.pop();
10381044
debug_assert!(popped);
10391045
}
1040-
self.push(file_name.as_ref());
1046+
self.push(file_name);
10411047
}
10421048

10431049
/// Updates `self.extension()` to `extension`.
@@ -1048,14 +1054,17 @@ impl PathBuf {
10481054
/// is added; otherwise it is replaced.
10491055
#[stable(feature = "rust1", since = "1.0.0")]
10501056
pub fn set_extension<S: AsRef<OsStr>>(&mut self, extension: S) -> bool {
1057+
self._set_extension(extension.as_ref())
1058+
}
1059+
1060+
fn _set_extension(&mut self, extension: &OsStr) -> bool {
10511061
if self.file_name().is_none() { return false; }
10521062

10531063
let mut stem = match self.file_stem() {
10541064
Some(stem) => stem.to_os_string(),
10551065
None => OsString::new(),
10561066
};
10571067

1058-
let extension = extension.as_ref();
10591068
if !os_str_as_u8_slice(extension).is_empty() {
10601069
stem.push(".");
10611070
stem.push(extension);
@@ -1106,7 +1115,7 @@ impl<P: AsRef<Path>> iter::FromIterator<P> for PathBuf {
11061115
impl<P: AsRef<Path>> iter::Extend<P> for PathBuf {
11071116
fn extend<I: IntoIterator<Item = P>>(&mut self, iter: I) {
11081117
for p in iter {
1109-
self.push(p)
1118+
self.push(p.as_ref())
11101119
}
11111120
}
11121121
}
@@ -1452,7 +1461,11 @@ impl Path {
14521461
issue = "23284")]
14531462
pub fn relative_from<'a, P: ?Sized + AsRef<Path>>(&'a self, base: &'a P) -> Option<&Path>
14541463
{
1455-
iter_after(self.components(), base.as_ref().components()).map(|c| c.as_path())
1464+
self._relative_from(base.as_ref())
1465+
}
1466+
1467+
fn _relative_from<'a>(&'a self, base: &'a Path) -> Option<&'a Path> {
1468+
iter_after(self.components(), base.components()).map(|c| c.as_path())
14561469
}
14571470

14581471
/// Determines whether `base` is a prefix of `self`.
@@ -1472,7 +1485,11 @@ impl Path {
14721485
/// ```
14731486
#[stable(feature = "rust1", since = "1.0.0")]
14741487
pub fn starts_with<P: AsRef<Path>>(&self, base: P) -> bool {
1475-
iter_after(self.components(), base.as_ref().components()).is_some()
1488+
self._starts_with(base.as_ref())
1489+
}
1490+
1491+
fn _starts_with(&self, base: &Path) -> bool {
1492+
iter_after(self.components(), base.components()).is_some()
14761493
}
14771494

14781495
/// Determines whether `child` is a suffix of `self`.
@@ -1490,7 +1507,11 @@ impl Path {
14901507
/// ```
14911508
#[stable(feature = "rust1", since = "1.0.0")]
14921509
pub fn ends_with<P: AsRef<Path>>(&self, child: P) -> bool {
1493-
iter_after(self.components().rev(), child.as_ref().components().rev()).is_some()
1510+
self._ends_with(child.as_ref())
1511+
}
1512+
1513+
fn _ends_with(&self, child: &Path) -> bool {
1514+
iter_after(self.components().rev(), child.components().rev()).is_some()
14941515
}
14951516

14961517
/// Extracts the stem (non-extension) portion of `self.file_name()`.
@@ -1552,6 +1573,10 @@ impl Path {
15521573
/// ```
15531574
#[stable(feature = "rust1", since = "1.0.0")]
15541575
pub fn join<P: AsRef<Path>>(&self, path: P) -> PathBuf {
1576+
self._join(path.as_ref())
1577+
}
1578+
1579+
fn _join(&self, path: &Path) -> PathBuf {
15551580
let mut buf = self.to_path_buf();
15561581
buf.push(path);
15571582
buf
@@ -1571,6 +1596,10 @@ impl Path {
15711596
/// ```
15721597
#[stable(feature = "rust1", since = "1.0.0")]
15731598
pub fn with_file_name<S: AsRef<OsStr>>(&self, file_name: S) -> PathBuf {
1599+
self._with_file_name(file_name.as_ref())
1600+
}
1601+
1602+
fn _with_file_name(&self, file_name: &OsStr) -> PathBuf {
15741603
let mut buf = self.to_path_buf();
15751604
buf.set_file_name(file_name);
15761605
buf
@@ -1590,6 +1619,10 @@ impl Path {
15901619
/// ```
15911620
#[stable(feature = "rust1", since = "1.0.0")]
15921621
pub fn with_extension<S: AsRef<OsStr>>(&self, extension: S) -> PathBuf {
1622+
self._with_extension(extension.as_ref())
1623+
}
1624+
1625+
fn _with_extension(&self, extension: &OsStr) -> PathBuf {
15931626
let mut buf = self.to_path_buf();
15941627
buf.set_extension(extension);
15951628
buf

0 commit comments

Comments
 (0)