tamer: xir::tree::attr::Attr::value_atom: Option<SymbolId>=>SymbolId

To maintain a proper abstraction, this cannot be the responsibility of the
caller; most callers should not know that fragments exist, letalone how to
handle them.
main
Mike Gerwitz 2021-11-16 12:41:03 -05:00
parent c9be1d613d
commit f519dab2b6
2 changed files with 21 additions and 16 deletions

View File

@ -37,7 +37,7 @@ type TestResult = Result<(), Box<dyn std::error::Error>>;
macro_rules! assert_attr{
($attrs:ident, $name:ident, $expected:expr, $($args:expr),*) => {
assert_eq!(
$attrs.find($name).and_then(|a| a.value_atom()),
$attrs.find($name).map(|a| a.value_atom()),
$expected,
$($args),*
)
@ -255,16 +255,16 @@ fn test_writes_deps() -> TestResult {
let attrs = ele.attrs().unwrap();
assert_eq!(
attrs.find(QN_NAME).and_then(|a| a.value_atom()),
attrs.find(QN_NAME).map(|a| a.value_atom()),
Some(ident.name()),
);
assert_eq!(
attrs.find(QN_TYPE).and_then(|a| a.value_atom()),
attrs.find(QN_TYPE).map(|a| a.value_atom()),
Some(ident.kind().unwrap().as_sym())
);
let generated = attrs.find(QN_GENERATED).and_then(|a| a.value_atom());
let generated = attrs.find(QN_GENERATED).map(|a| a.value_atom());
if let Some(Source {
generated: true, ..
@ -291,7 +291,7 @@ fn test_writes_deps() -> TestResult {
// uninterned and therefore cannot be compared as a
// `SymbolId`. Once the reader takes care of creating the
// symbol, we'll have no such problem.
match attrs.find(QN_DESC).and_then(|a| a.value_atom()) {
match attrs.find(QN_DESC).map(|a| a.value_atom()) {
Some(given) => {
assert_eq!(
desc.lookup_str(),
@ -430,8 +430,7 @@ fn test_writes_map_froms() -> TestResult {
.unwrap()
.find(QN_NAME)
.expect("expecting @name")
.value_atom()
.unwrap(),
.value_atom(),
);
});

View File

@ -113,26 +113,32 @@ impl Attr {
}
}
/// Attempt to retrieve a cost-free atom from the attribute.
/// Retrieve an atom from the attribute.
///
/// An atom is available if either
/// An atom is cost-free if either
/// (a) this is [`Attr::Simple`]; or
/// (b) this is [`Attr::Extensible`] with one fragment.
/// Otherwise,
/// rather than assuming what the caller may want to do,
/// return [`None`] and let the caller decide how to proceed with
/// deriving an atom.
/// this panics with a TODO,
/// since we haven't had a need for merging attributes [yet].
///
/// Since [`SymbolId`] implements [`Copy`],
/// this returns an owned value.
#[inline]
pub fn value_atom(&self) -> Option<SymbolId> {
pub fn value_atom(&self) -> SymbolId {
match self {
Self::Simple(attr) => Some(attr.value),
Self::Simple(attr) => attr.value,
Self::Extensible(attr) if attr.value_frags.len() == 1 => {
Some(attr.value_frags[0].0)
attr.value_frags[0].0
}
_ => None,
// We'll probably want to just merge and generate a new
// SymbolId,
// possibly having a separate variant of this method if we
// want to guarantee a cost-free conversion as a Result.
Self::Extensible(attr) => todo!(
"Multi-fragment attribute atom requested: {:?}",
attr.value_frags
),
}
}
}