employer
/
tame
1
0
Fork 0
Code Packages Releases Activity

tamer: ld::xmle::lower: Diagnostic message for cycles 

This moves the special handling of circular dependencies out of
`poc.rs`---and to be clear, everything needs to be moved out of there---and
into the source of the error.  The diagnostic system did not exist at the
time.

This is one example of how easy it will be to create robust diagnostics once
we have the spans on the graph.  Once the spans resolve to the proper source
locations rather than the `xmlo` file, it'll Just Work.

It is worth noting, though, that this detection and error will ultimately
need to be moved so that it can occur when performing other operation on the
graph during compilation, such as type inference and unification.  I don't
expect to go out of my way to detect cycles, though, since the linker will.

DEV-13430
main
Mike Gerwitz 2022-12-16 15:02:57 -05:00
parent f3135940c1
commit 8c4923274a
2 changed files with 128 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
tamer/src/ld/poc.rs
View File

@ -81,33 +81,7 @@ pub fn xmle(package_path: &str, output: &str) -> Result<(), TameldError> {
..
} = state;
let sorted = match sort(&depgraph, Sections::new()) {
Ok(sections) => sections,
Err(SortError::Cycles(cycles)) => {
return Err(TameldError::CycleError(
cycles
.into_iter()
.map(|cycle| {
let mut path: Vec<SymbolId> = cycle
.into_iter()
.filter_map(|obj| {
depgraph
.get(obj)
.unwrap()
.as_ident_ref()
.map(|ident| ident.name().symbol())
})
.collect();
path.reverse();
path.push(path[0].clone());
path
})
.collect(),
))
}
Err(e) => return Err(e.into()),
};
let sorted = sort(&depgraph, Sections::new())?;
output_xmle(
sorted,
@ -289,7 +263,6 @@ pub enum TameldError {
AirLowerError(ParseError<Air, AsgError>),
XirWriterError(XirWriterError),
FinalizeError(FinalizeError),
CycleError(Vec<Vec<SymbolId>>),
Fmt(fmt::Error),
}
@ -365,21 +338,6 @@ impl Display for TameldError {
Self::AirLowerError(e) => Display::fmt(e, f),
Self::XirWriterError(e) => Display::fmt(e, f),
Self::FinalizeError(e) => Display::fmt(e, f),
Self::CycleError(cycles) => {
for cycle in cycles {
writeln!(
f,
"cycle: {}",
cycle
.iter()
.map(|sym| sym.lookup_str())
.collect::<Vec<&str>>()
.join(" -> ")
)?;
}
Ok(())
}
Self::Fmt(e) => Display::fmt(e, f),
}
}
@ -397,7 +355,6 @@ impl Error for TameldError {
Self::AirLowerError(e) => Some(e),
Self::XirWriterError(e) => Some(e),
Self::FinalizeError(e) => Some(e),
Self::CycleError(_) => None,
Self::Fmt(e) => Some(e),
}
}
@ -414,10 +371,7 @@ impl Diagnostic for TameldError {
Self::FinalizeError(e) => e.describe(),
Self::SortError(e) => e.describe(),
Self::Io(_)
| Self::XirWriterError(_)
| Self::CycleError(_)
| Self::Fmt(_) => vec![],
Self::Io(_) | Self::XirWriterError(_) | Self::Fmt(_) => vec![],
}
}
}

151
tamer/src/ld/xmle/lower.rs
View File

@ -21,10 +21,17 @@
//!
//! See the [parent module](super) for more information.
use std::iter::once;
use super::section::{SectionsError, XmleSections};
use crate::{
asg::{Asg, Ident, IdentKind, Object, ObjectRef},
diagnose::Diagnostic,
asg::{Asg, Ident, IdentKind, Object},
diagnose::{Annotate, Diagnostic},
fmt::{
AndConjList, DisplayWrapper, JoinListWrap, ListDisplayWrapper, Raw,
TtQuote,
},
parse::util::SPair,
sym::{st, GlobalSymbolResolve, SymbolId},
};
use petgraph::visit::DfsPostOrder;
@ -100,7 +107,7 @@ fn check_cycles(asg: &Asg) -> SortResult<()> {
// impact.
let sccs = petgraph::algo::tarjan_scc(&asg.graph);
let cycles: Vec<_> = sccs
let cycles: Vec<Vec<SPair>> = sccs
.into_iter()
.filter_map(|scc| {
// For single-node SCCs, we just need to make sure they are
@ -122,8 +129,17 @@ fn check_cycles(asg: &Asg) -> SortResult<()> {
if is_all_funcs {
None
} else {
let cycles =
scc.iter().map(|nx| ObjectRef::from(*nx)).collect();
// TODO: ...these aren't references, they're the actual
// identifiers,
// so the diagnostic message isn't that great of a guide
// yet!
// Use reference spans once they're available.
let cycles = scc
.iter()
.filter_map(|nx| {
asg.get(*nx).unwrap().as_ident_ref().map(Ident::name)
})
.collect();
Some(cycles)
}
})
@ -148,7 +164,7 @@ pub enum SortError {
SectionsError(SectionsError),
/// The graph has a cyclic dependency.
Cycles(Vec<Vec<ObjectRef>>),
Cycles(Vec<Vec<SPair>>),
}
impl From<SectionsError> for SortError {
@ -158,10 +174,31 @@ impl From<SectionsError> for SortError {
}
impl std::fmt::Display for SortError {
fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
match self {
Self::SectionsError(err) => err.fmt(fmt),
Self::Cycles(_) => write!(fmt, "cyclic dependencies"),
Self::SectionsError(err) => err.fmt(f),
Self::Cycles(cycles) => {
let cycles_fmt = cycles
.iter()
.map(|cycle| {
let cycle_fmt = cycle
.iter()
.rev()
.chain(once(cycle.last().unwrap()))
.collect::<Vec<_>>();
// TODO: Wrappers ought to support nested lists.
JoinListWrap::<" -> ", Raw>::wrap(&cycle_fmt)
.to_string()
})
.collect::<Vec<_>>();
write!(
f,
"circular dependencies: {}",
AndConjList::<TtQuote>::wrap(&cycles_fmt),
)
}
}
}
}
@ -178,8 +215,51 @@ impl Diagnostic for SortError {
match self {
SectionsError(e) => e.describe(),
// TODO
Cycles(_) => vec![],
// TODO: In future it'd be far less confusing to have a single
// error per cycle.
Cycles(cycles) => cycles
.iter()
.map(|cycle| {
let n = cycle.len();
let ident = cycle.last().unwrap();
cycle.iter().rev().enumerate().map(|(i, spair)| {
spair.note(match i {
0 => format!(
"[0/{n}] the cycle begins here, depending on..."
),
_ => format!(
"[{i}/{n}] ...this identifier, which depends on..."
)
})
})
.chain(once(ident.error(format!(
"[{n}/{n}] ...the first identifier once again, \
creating the cycle"
))))
.chain(vec![
ident.help(format!(
"the value of {} cannot be computed because its",
TtQuote::wrap(ident),
)),
ident.help(
" definition requires first computing itself.",
),
ident.help(
"in the future the above output will emphasize the "
),
ident.help(
" references to the identifiers rather than their "
),
ident.help(
" definition sites."
)
])
.collect::<Vec<_>>()
})
.flatten()
.collect(),
}
}
}
@ -422,8 +502,11 @@ mod test {
let result = sort(&asg, Sections::new());
let expected: Vec<Vec<ObjectRef>> =
vec![vec![dep_node.into(), sym_node.into()]];
let expected = vec![[dep_node, sym_node]
.into_iter()
.map(|o| asg.get(o).unwrap().as_ident_ref().unwrap().name())
.collect::<Vec<_>>()];
match result {
Ok(_) => panic!("sort did not detect cycle"),
Err(SortError::Cycles(scc)) => assert_eq!(expected, scc),
@ -500,10 +583,16 @@ mod test {
let result = sort(&asg, Sections::new());
let expected: Vec<Vec<ObjectRef>> = vec![
vec![dep_node.into(), sym_node.into()],
vec![dep2_node.into(), sym2_node.into()],
];
let expected = [[dep_node, sym_node], [dep2_node, sym2_node]]
.into_iter()
.map(|cycle| {
cycle
.into_iter()
.map(|o| asg.get(o).unwrap().as_ident_ref().unwrap().name())
.collect::<Vec<_>>()
})
.collect::<Vec<_>>();
match result {
Ok(_) => panic!("sort did not detect cycle"),
Err(SortError::Cycles(scc)) => assert_eq!(expected, scc),
@ -612,8 +701,11 @@ mod test {
let result = sort(&asg, Sections::new());
let expected: Vec<Vec<ObjectRef>> =
vec![vec![sym3_node.into(), sym2_node.into(), sym1_node.into()]];
let expected = vec![[sym3_node, sym2_node, sym1_node]
.into_iter()
.map(|o| asg.get(o).unwrap().as_ident_ref().unwrap().name())
.collect::<Vec<_>>()];
match result {
Ok(_) => panic!("sort did not detect cycle"),
Err(SortError::Cycles(scc)) => assert_eq!(expected, scc),
@ -677,8 +769,11 @@ mod test {
let result = sort(&asg, Sections::new());
let expected: Vec<Vec<ObjectRef>> =
vec![vec![sym3_node.into(), sym2_node.into(), sym1_node.into()]];
let expected = vec![[sym3_node, sym2_node, sym1_node]
.into_iter()
.map(|o| asg.get(o).unwrap().as_ident_ref().unwrap().name())
.collect::<Vec<_>>()];
match result {
Ok(_) => panic!("sort did not detect cycle"),
Err(SortError::Cycles(scc)) => assert_eq!(expected, scc),
@ -741,8 +836,11 @@ mod test {
let result = sort(&asg, Sections::new());
let expected: Vec<Vec<ObjectRef>> =
vec![vec![sym3_node.into(), sym2_node.into(), sym1_node.into()]];
let expected = vec![[sym3_node, sym2_node, sym1_node]
.into_iter()
.map(|o| asg.get(o).unwrap().as_ident_ref().unwrap().name())
.collect::<Vec<_>>()];
match result {
Ok(_) => panic!("sort did not detect cycle"),
Err(SortError::Cycles(scc)) => assert_eq!(expected, scc),
@ -851,8 +949,11 @@ mod test {
let result = sort(&asg, Sections::new());
let expected: Vec<Vec<ObjectRef>> =
vec![vec![sym3_node.into(), sym2_node.into(), sym1_node.into()]];
let expected = vec![[sym3_node, sym2_node, sym1_node]
.into_iter()
.map(|o| asg.get(o).unwrap().as_ident_ref().unwrap().name())
.collect::<Vec<_>>()];
match result {
Ok(_) => panic!("sort did not detect cycle"),
Err(SortError::Cycles(scc)) => assert_eq!(expected, scc),