Skip to content

Commit

Permalink
Rollup merge of rust-lang#130962 - nyurik:opts-libs, r=cuviper
Browse files Browse the repository at this point in the history
Migrate lib's `&Option<T>` into `Option<&T>`

Trying out my new lint rust-lang/rust-clippy#13336 - according to the [video](https://www.youtube.com/watch?v=6c7pZYP_iIE), this could lead to some performance and memory optimizations.

Basic thoughts expressed in the video that seem to make sense:
* `&Option<T>` in an API breaks encapsulation:
  * caller must own T and move it into an Option to call with it
  * if returned, the owner must store it as Option<T> internally in order to return it
* Performance is subject to compiler optimization, but at the basics, `&Option<T>` points to memory that has `presence` flag + value, whereas `Option<&T>` by specification is always optimized to a single pointer.
  • Loading branch information
tgross35 authored Oct 12, 2024
2 parents 76ce3a9 + 7153288 commit de76914
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 20 deletions.
14 changes: 7 additions & 7 deletions std/src/sys/pal/sgx/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,8 @@ fn io_err_to_addr(result: io::Result<&SocketAddr>) -> io::Result<String> {
}
}

fn addr_to_sockaddr(addr: &Option<String>) -> io::Result<SocketAddr> {
addr.as_ref()
.ok_or(io::ErrorKind::AddrNotAvailable)?
fn addr_to_sockaddr(addr: Option<&str>) -> io::Result<SocketAddr> {
addr.ok_or(io::ErrorKind::AddrNotAvailable)?
.to_socket_addrs()
// unwrap OK: if an iterator is returned, we're guaranteed to get exactly one entry
.map(|mut it| it.next().unwrap())
Expand Down Expand Up @@ -161,11 +160,11 @@ impl TcpStream {
}

pub fn peer_addr(&self) -> io::Result<SocketAddr> {
addr_to_sockaddr(&self.peer_addr)
addr_to_sockaddr(self.peer_addr.as_deref())
}

pub fn socket_addr(&self) -> io::Result<SocketAddr> {
addr_to_sockaddr(&self.inner.local_addr)
addr_to_sockaddr(self.inner.local_addr.as_deref())
}

pub fn shutdown(&self, _: Shutdown) -> io::Result<()> {
Expand Down Expand Up @@ -255,13 +254,14 @@ impl TcpListener {
}

pub fn socket_addr(&self) -> io::Result<SocketAddr> {
addr_to_sockaddr(&self.inner.local_addr)
addr_to_sockaddr(self.inner.local_addr.as_deref())
}

pub fn accept(&self) -> io::Result<(TcpStream, SocketAddr)> {
let (fd, local_addr, peer_addr) = usercalls::accept_stream(self.inner.inner.raw())?;
let peer_addr = Some(peer_addr);
let ret_peer = addr_to_sockaddr(&peer_addr).unwrap_or_else(|_| ([0; 4], 0).into());
let ret_peer =
addr_to_sockaddr(peer_addr.as_deref()).unwrap_or_else(|_| ([0; 4], 0).into());
Ok((TcpStream { inner: Socket::new(fd, local_addr), peer_addr }, ret_peer))
}

Expand Down
4 changes: 2 additions & 2 deletions std/src/sys/pal/unix/process/process_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,8 @@ impl Command {
}

#[allow(dead_code)]
pub fn get_cwd(&self) -> &Option<CString> {
&self.cwd
pub fn get_cwd(&self) -> Option<&CStr> {
self.cwd.as_deref()
}
#[allow(dead_code)]
pub fn get_uid(&self) -> Option<uid_t> {
Expand Down
2 changes: 1 addition & 1 deletion std/src/sys/pal/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ impl Command {
cvt(libc::setuid(u as uid_t))?;
}
}
if let Some(ref cwd) = *self.get_cwd() {
if let Some(cwd) = self.get_cwd() {
cvt(libc::chdir(cwd.as_ptr()))?;
}

Expand Down
2 changes: 1 addition & 1 deletion std/src/sys/pal/unix/process/process_vxworks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl Command {
t!(cvt_r(|| libc::dup2(fd, libc::STDERR_FILENO)));
}

if let Some(ref cwd) = *self.get_cwd() {
if let Some(cwd) = self.get_cwd() {
t!(cvt(libc::chdir(cwd.as_ptr())));
}

Expand Down
11 changes: 6 additions & 5 deletions test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,8 +650,8 @@ fn run_test_in_process(
io::set_output_capture(None);

let test_result = match result {
Ok(()) => calc_result(&desc, Ok(()), &time_opts, &exec_time),
Err(e) => calc_result(&desc, Err(e.as_ref()), &time_opts, &exec_time),
Ok(()) => calc_result(&desc, Ok(()), time_opts.as_ref(), exec_time.as_ref()),
Err(e) => calc_result(&desc, Err(e.as_ref()), time_opts.as_ref(), exec_time.as_ref()),
};
let stdout = data.lock().unwrap_or_else(|e| e.into_inner()).to_vec();
let message = CompletedTest::new(id, desc, test_result, exec_time, stdout);
Expand Down Expand Up @@ -712,7 +712,8 @@ fn spawn_test_subprocess(
formatters::write_stderr_delimiter(&mut test_output, &desc.name);
test_output.extend_from_slice(&stderr);

let result = get_result_from_exit_code(&desc, status, &time_opts, &exec_time);
let result =
get_result_from_exit_code(&desc, status, time_opts.as_ref(), exec_time.as_ref());
(result, test_output, exec_time)
})();

Expand All @@ -724,8 +725,8 @@ fn run_test_in_spawned_subprocess(desc: TestDesc, runnable_test: RunnableTest) -
let builtin_panic_hook = panic::take_hook();
let record_result = Arc::new(move |panic_info: Option<&'_ PanicHookInfo<'_>>| {
let test_result = match panic_info {
Some(info) => calc_result(&desc, Err(info.payload()), &None, &None),
None => calc_result(&desc, Ok(()), &None, &None),
Some(info) => calc_result(&desc, Err(info.payload()), None, None),
None => calc_result(&desc, Ok(()), None, None),
};

// We don't support serializing TrFailedMsg, so just
Expand Down
8 changes: 4 additions & 4 deletions test/src/test_result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ pub enum TestResult {
pub fn calc_result<'a>(
desc: &TestDesc,
task_result: Result<(), &'a (dyn Any + 'static + Send)>,
time_opts: &Option<time::TestTimeOptions>,
exec_time: &Option<time::TestExecTime>,
time_opts: Option<&time::TestTimeOptions>,
exec_time: Option<&time::TestExecTime>,
) -> TestResult {
let result = match (&desc.should_panic, task_result) {
(&ShouldPanic::No, Ok(())) | (&ShouldPanic::Yes, Err(_)) => TestResult::TrOk,
Expand Down Expand Up @@ -96,8 +96,8 @@ pub fn calc_result<'a>(
pub fn get_result_from_exit_code(
desc: &TestDesc,
status: ExitStatus,
time_opts: &Option<time::TestTimeOptions>,
exec_time: &Option<time::TestExecTime>,
time_opts: Option<&time::TestTimeOptions>,
exec_time: Option<&time::TestExecTime>,
) -> TestResult {
let result = match status.code() {
Some(TR_OK) => TestResult::TrOk,
Expand Down

0 comments on commit de76914

Please # to comment.