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

yaml: Implement IndexMut #19

Closed
wants to merge 1 commit into from
Closed

yaml: Implement IndexMut #19

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 56 additions & 1 deletion src/yaml.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use std::borrow::Cow;
use std::ops::ControlFlow;
use std::{collections::BTreeMap, convert::TryFrom, mem, ops::Index};
use std::{collections::BTreeMap, convert::TryFrom, mem, ops::Index, ops::IndexMut};

use encoding_rs::{Decoder, DecoderResult, Encoding};
use hashlink::LinkedHashMap;
Expand Down Expand Up @@ -478,6 +478,23 @@ pub fn $name(&self) -> Option<$t> {
);
);

macro_rules! define_as_mut_ref (
($name:ident, $t:ty, $yt:ident) => (
/// Get a mutable reference to the inner object in the YAML enum if it is a `$t`.
///
/// # Return
/// If the variant of `self` is `Yaml::$yt`, return `Some(&$t)` with the `$t` contained. Otherwise,
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
/// If the variant of `self` is `Yaml::$yt`, return `Some(&$t)` with the `$t` contained. Otherwise,
/// If the variant of `self` is `Yaml::$yt`, return `Some(&mut $t)` with the `$t` contained. Otherwise,

/// return `None`.
#[must_use]
pub fn $name(&mut self) -> Option<$t> {
match *self {
Yaml::$yt(ref mut v) => Some(v),
_ => None
}
}
);
);

macro_rules! define_into (
($name:ident, $t:ty, $yt:ident) => (
/// Get the inner object in the YAML enum if it is a `$t`.
Expand All @@ -503,6 +520,9 @@ impl Yaml {
define_as_ref!(as_hash, &Hash, Hash);
define_as_ref!(as_vec, &Array, Array);

define_as_mut_ref!(as_mut_hash, &mut Hash, Hash);
define_as_mut_ref!(as_mut_vec, &mut Array, Array);

define_into!(into_bool, bool, Boolean);
define_into!(into_i64, i64, Integer);
define_into!(into_string, String, String);
Expand Down Expand Up @@ -641,6 +661,16 @@ impl<'a> Index<&'a str> for Yaml {
}
}

impl<'a> IndexMut<&'a str> for Yaml {
fn index_mut(&mut self, idx: &'a str) -> &mut Yaml {
let key = Yaml::String(idx.to_owned());
match self.as_mut_hash() {
Some(h) => h.get_mut(&key).unwrap(),
None => panic!("Not a hash type"),
}
}
}

impl Index<usize> for Yaml {
type Output = Yaml;

Expand All @@ -656,6 +686,31 @@ impl Index<usize> for Yaml {
}
}

impl IndexMut<usize> for Yaml {
fn index_mut(&mut self, idx: usize) -> &mut Yaml {
if let Some(v) = self.as_mut_vec() {
let yaml = v.get_mut(idx).unwrap();

// This transmute just casts the lifetime away. Since Rust only
// has SESE regions, this early return cannot be worked out and
// such that the borrow region of self includes the whole block.
// The explixit lifetimes in the function signature ensure that
// this is safe.
// From: https://github.com/image-rs/image-gif/blob/c814b40079a36af0b26adc36237f2c742d69c994/src/reader/decoder.rs#L227-L229
unsafe {
return mem::transmute::<&mut Yaml, &mut Yaml>(yaml);
}
}

if let Some(v) = self.as_mut_hash() {
let key = Yaml::Integer(i64::try_from(idx).unwrap());
v.get_mut(&key).unwrap()
} else {
panic!("Not a hash or verctor type");
}
}
Comment on lines +690 to +711
Copy link
Owner

Choose a reason for hiding this comment

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

My other comment didn't get through.

You do not need the unsafe block if you only borrow self once.

While you're allowed to panic as per IndexMut's interface, you're only allowed to do so if the index it out of range.
In this particular case, we have an additional condition in which we may panic which is the variant contained in self and this should be documented.

Suggested change
fn index_mut(&mut self, idx: usize) -> &mut Yaml {
if let Some(v) = self.as_mut_vec() {
let yaml = v.get_mut(idx).unwrap();
// This transmute just casts the lifetime away. Since Rust only
// has SESE regions, this early return cannot be worked out and
// such that the borrow region of self includes the whole block.
// The explixit lifetimes in the function signature ensure that
// this is safe.
// From: https://github.com/image-rs/image-gif/blob/c814b40079a36af0b26adc36237f2c742d69c994/src/reader/decoder.rs#L227-L229
unsafe {
return mem::transmute::<&mut Yaml, &mut Yaml>(yaml);
}
}
if let Some(v) = self.as_mut_hash() {
let key = Yaml::Integer(i64::try_from(idx).unwrap());
v.get_mut(&key).unwrap()
} else {
panic!("Not a hash or verctor type");
}
}
/// Perform indexing if `self` is a sequence or a mapping.
///
/// # Panics
/// This function panics if the index given is out of range (as per [`IndexMut`]). If `self` i
/// a [`Yaml::Array`], this is when the index is bigger or equal to the length of the
/// underlying `Vec`. If `self` is a [`Yaml::Hash`], this is when the mapping sequence does no
/// contain [`Yaml::Integer`]`(idx)` as a key.
///
/// This function also panics if `self` is not a [`Yaml::Array`] nor a [`Yaml::Hash`].
fn index_mut(&mut self, idx: usize) -> &mut Yaml {
match self {
Yaml::Array(sequence) => sequence.index_mut(idx),
Yaml::Hash(mapping) => {
let key = Yaml::Integer(i64::try_from(idx).unwrap());
mapping.get_mut(&key).unwrap()
}
_ => panic!("Attempting to index but `self` is not a sequence nor a mapping"),
}
}

}

impl IntoIterator for Yaml {
type Item = Yaml;
type IntoIter = YamlIter;
Expand Down