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

Better Debug for Vars and VarsOs #114132

Merged
merged 1 commit into from
Aug 12, 2023
Merged

Better Debug for Vars and VarsOs #114132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 8 additions & 4 deletions library/std/src/env.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,8 @@ impl Iterator for Vars {
#[stable(feature = "std_debug", since = "1.16.0")]
impl fmt::Debug for Vars {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("Vars").finish_non_exhaustive()
let Self { inner: VarsOs { inner } } = self;
f.debug_struct("Vars").field("inner", &inner.str_debug()).finish()
}
}

Expand All @@ -196,7 +197,8 @@ impl Iterator for VarsOs {
#[stable(feature = "std_debug", since = "1.16.0")]
impl fmt::Debug for VarsOs {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("VarOs").finish_non_exhaustive()
let Self { inner } = self;
f.debug_struct("VarsOs").field("inner", inner).finish()
}
}

Expand Down Expand Up @@ -829,7 +831,8 @@ impl DoubleEndedIterator for Args {
#[stable(feature = "std_debug", since = "1.16.0")]
impl fmt::Debug for Args {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("Args").field("inner", &self.inner.inner).finish()
let Self { inner: ArgsOs { inner } } = self;
f.debug_struct("Args").field("inner", inner).finish()
}
}

Expand Down Expand Up @@ -870,7 +873,8 @@ impl DoubleEndedIterator for ArgsOs {
#[stable(feature = "std_debug", since = "1.16.0")]
impl fmt::Debug for ArgsOs {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("ArgsOs").field("inner", &self.inner).finish()
let Self { inner } = self;
f.debug_struct("ArgsOs").field("inner", inner).finish()
}
}

Expand Down
20 changes: 20 additions & 0 deletions library/std/src/env/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,28 @@ fn args_debug() {
format!("Args {{ inner: {:?} }}", args().collect::<Vec<_>>()),
format!("{:?}", args())
);
}

#[test]
fn args_os_debug() {
assert_eq!(
format!("ArgsOs {{ inner: {:?} }}", args_os().collect::<Vec<_>>()),
format!("{:?}", args_os())
);
}

#[test]
fn vars_debug() {
assert_eq!(
format!("Vars {{ inner: {:?} }}", vars().collect::<Vec<_>>()),
format!("{:?}", vars())
);
}

#[test]
fn vars_os_debug() {
assert_eq!(
format!("VarsOs {{ inner: {:?} }}", vars_os().collect::<Vec<_>>()),
format!("{:?}", vars_os())
);
}
28 changes: 28 additions & 0 deletions library/std/src/sys/hermit/os.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,34 @@ pub struct Env {
iter: vec::IntoIter<(OsString, OsString)>,
}

// FIXME(https://github.com/rust-lang/rust/issues/114583): Remove this when <OsStr as Debug>::fmt matches <str as Debug>::fmt.
pub struct EnvStrDebug<'a> {
Copy link
Member

Choose a reason for hiding this comment

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

Could we avoid the code duplication here and move EnvStrDebug into common code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not without introducing additional allocations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps I should elaborate. The various Env structures contain iterators - even when they are backed by vectors - so it isn't possible to implement Debug without iterating those iterators. This means the iterators must be cloned. In the case of a vector-backed iterator, cloning would allocate, which this approach avoids.

slice: &'a [(OsString, OsString)],
}

impl fmt::Debug for EnvStrDebug<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self { slice } = self;
f.debug_list()
.entries(slice.iter().map(|(a, b)| (a.to_str().unwrap(), b.to_str().unwrap())))
.finish()
}
}

impl Env {
pub fn str_debug(&self) -> impl fmt::Debug + '_ {
let Self { iter } = self;
EnvStrDebug { slice: iter.as_slice() }
}
}

impl fmt::Debug for Env {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self { iter } = self;
f.debug_list().entries(iter.as_slice()).finish()
}
}

impl !Send for Env {}
impl !Sync for Env {}

Expand Down
51 changes: 49 additions & 2 deletions library/std/src/sys/sgx/os.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,61 @@ fn create_env_store() -> &'static EnvStore {
unsafe { &*(ENV.load(Ordering::Relaxed) as *const EnvStore) }
}

pub type Env = vec::IntoIter<(OsString, OsString)>;
pub struct Env {
iter: vec::IntoIter<(OsString, OsString)>,
}

// FIXME(https://github.com/rust-lang/rust/issues/114583): Remove this when <OsStr as Debug>::fmt matches <str as Debug>::fmt.
pub struct EnvStrDebug<'a> {
slice: &'a [(OsString, OsString)],
}

impl fmt::Debug for EnvStrDebug<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self { slice } = self;
f.debug_list()
.entries(slice.iter().map(|(a, b)| (a.to_str().unwrap(), b.to_str().unwrap())))
.finish()
}
}

impl Env {
pub fn str_debug(&self) -> impl fmt::Debug + '_ {
let Self { iter } = self;
EnvStrDebug { slice: iter.as_slice() }
}
}

impl fmt::Debug for Env {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self { iter } = self;
f.debug_list().entries(iter.as_slice()).finish()
}
}

impl !Send for Env {}
impl !Sync for Env {}

impl Iterator for Env {
type Item = (OsString, OsString);
fn next(&mut self) -> Option<(OsString, OsString)> {
self.iter.next()
}
fn size_hint(&self) -> (usize, Option<usize>) {
self.iter.size_hint()
}
}

pub fn env() -> Env {
let clone_to_vec = |map: &HashMap<OsString, OsString>| -> Vec<_> {
map.iter().map(|(k, v)| (k.clone(), v.clone())).collect()
};

get_env_store().map(|env| clone_to_vec(&env.lock().unwrap())).unwrap_or_default().into_iter()
let iter = get_env_store()
.map(|env| clone_to_vec(&env.lock().unwrap()))
.unwrap_or_default()
.into_iter();
Env { iter }
}

pub fn getenv(k: &OsStr) -> Option<OsString> {
Expand Down
28 changes: 28 additions & 0 deletions library/std/src/sys/solid/os.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,34 @@ pub struct Env {
iter: vec::IntoIter<(OsString, OsString)>,
}

// FIXME(https://github.com/rust-lang/rust/issues/114583): Remove this when <OsStr as Debug>::fmt matches <str as Debug>::fmt.
pub struct EnvStrDebug<'a> {
slice: &'a [(OsString, OsString)],
}

impl fmt::Debug for EnvStrDebug<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self { slice } = self;
f.debug_list()
.entries(slice.iter().map(|(a, b)| (a.to_str().unwrap(), b.to_str().unwrap())))
.finish()
}
}

impl Env {
pub fn str_debug(&self) -> impl fmt::Debug + '_ {
let Self { iter } = self;
EnvStrDebug { slice: iter.as_slice() }
}
}

impl fmt::Debug for Env {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self { iter } = self;
f.debug_list().entries(iter.as_slice()).finish()
}
}

impl !Send for Env {}
impl !Sync for Env {}

Expand Down
28 changes: 28 additions & 0 deletions library/std/src/sys/unix/os.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,34 @@ pub struct Env {
iter: vec::IntoIter<(OsString, OsString)>,
}

// FIXME(https://github.com/rust-lang/rust/issues/114583): Remove this when <OsStr as Debug>::fmt matches <str as Debug>::fmt.
pub struct EnvStrDebug<'a> {
slice: &'a [(OsString, OsString)],
}

impl fmt::Debug for EnvStrDebug<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self { slice } = self;
f.debug_list()
.entries(slice.iter().map(|(a, b)| (a.to_str().unwrap(), b.to_str().unwrap())))
.finish()
}
}

impl Env {
pub fn str_debug(&self) -> impl fmt::Debug + '_ {
let Self { iter } = self;
EnvStrDebug { slice: iter.as_slice() }
}
}

impl fmt::Debug for Env {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self { iter } = self;
f.debug_list().entries(iter.as_slice()).finish()
}
}

impl !Send for Env {}
impl !Sync for Env {}

Expand Down
18 changes: 17 additions & 1 deletion library/std/src/sys/unsupported/os.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,26 @@ pub fn current_exe() -> io::Result<PathBuf> {

pub struct Env(!);

impl Env {
// FIXME(https://github.com/rust-lang/rust/issues/114583): Remove this when <OsStr as Debug>::fmt matches <str as Debug>::fmt.
pub fn str_debug(&self) -> impl fmt::Debug + '_ {
let Self(inner) = self;
match *inner {}
}
}

impl fmt::Debug for Env {
fn fmt(&self, _: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self(inner) = self;
match *inner {}
}
}

impl Iterator for Env {
type Item = (OsString, OsString);
fn next(&mut self) -> Option<(OsString, OsString)> {
self.0
let Self(inner) = self;
match *inner {}
}
}

Expand Down
29 changes: 29 additions & 0 deletions library/std/src/sys/wasi/os.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,39 @@ impl StdError for JoinPathsError {
pub fn current_exe() -> io::Result<PathBuf> {
unsupported()
}

pub struct Env {
iter: vec::IntoIter<(OsString, OsString)>,
}

// FIXME(https://github.com/rust-lang/rust/issues/114583): Remove this when <OsStr as Debug>::fmt matches <str as Debug>::fmt.
pub struct EnvStrDebug<'a> {
slice: &'a [(OsString, OsString)],
}

impl fmt::Debug for EnvStrDebug<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self { slice } = self;
f.debug_list()
.entries(slice.iter().map(|(a, b)| (a.to_str().unwrap(), b.to_str().unwrap())))
.finish()
}
}

impl Env {
pub fn str_debug(&self) -> impl fmt::Debug + '_ {
let Self { iter } = self;
EnvStrDebug { slice: iter.as_slice() }
}
}

impl fmt::Debug for Env {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self { iter } = self;
f.debug_list().entries(iter.as_slice()).finish()
}
}

impl !Send for Env {}
impl !Sync for Env {}

Expand Down
54 changes: 49 additions & 5 deletions library/std/src/sys/windows/os.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,25 +85,69 @@ pub fn error_string(mut errnum: i32) -> String {

pub struct Env {
base: c::LPWCH,
cur: c::LPWCH,
iter: EnvIterator,
}

// FIXME(https://github.com/rust-lang/rust/issues/114583): Remove this when <OsStr as Debug>::fmt matches <str as Debug>::fmt.
pub struct EnvStrDebug<'a> {
iter: &'a EnvIterator,
}

impl fmt::Debug for EnvStrDebug<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self { iter } = self;
let iter: EnvIterator = (*iter).clone();
let mut list = f.debug_list();
for (a, b) in iter {
list.entry(&(a.to_str().unwrap(), b.to_str().unwrap()));
}
list.finish()
}
}

impl Env {
pub fn str_debug(&self) -> impl fmt::Debug + '_ {
let Self { base: _, iter } = self;
EnvStrDebug { iter }
}
}

impl fmt::Debug for Env {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self { base: _, iter } = self;
f.debug_list().entries(iter.clone()).finish()
}
}

impl Iterator for Env {
type Item = (OsString, OsString);

fn next(&mut self) -> Option<(OsString, OsString)> {
let Self { base: _, iter } = self;
iter.next()
}
}

#[derive(Clone)]
struct EnvIterator(c::LPWCH);

impl Iterator for EnvIterator {
type Item = (OsString, OsString);

fn next(&mut self) -> Option<(OsString, OsString)> {
let Self(cur) = self;
loop {
unsafe {
if *self.cur == 0 {
if **cur == 0 {
return None;
}
let p = self.cur as *const u16;
let p = *cur as *const u16;
let mut len = 0;
while *p.add(len) != 0 {
len += 1;
}
let s = slice::from_raw_parts(p, len);
self.cur = self.cur.add(len + 1);
*cur = cur.add(len + 1);

// Windows allows environment variables to start with an equals
// symbol (in any other position, this is the separator between
Expand Down Expand Up @@ -137,7 +181,7 @@ pub fn env() -> Env {
if ch.is_null() {
panic!("failure getting env string from OS: {}", io::Error::last_os_error());
}
Env { base: ch, cur: ch }
Env { base: ch, iter: EnvIterator(ch) }
}
}

Expand Down