TAMER: Linker: Human-readable unresolved object errors

Example output:

  error: unresolved extern `taxHomeState` of type `rate[float; 0]`, declared
  in `common/tax/state/dc`
master
Mike Gerwitz 2020-07-02 12:43:17 -04:00
commit e0356324bd
8 changed files with 805 additions and 387 deletions

View File

@ -14,6 +14,15 @@ commits that introduce the changes. To make a new release, run
`tools/mkrelease`, which will handle updating the heading for you.
NEXT
====
Linker
------
- Provide useful error for unresolved identifiers.
- This was previously falling through to an `unreachable!` block,
producing a very opaque and useless internal error message.
v17.4.2 (2020-05-13)
====================
This release adds GraphML output for linked objects to allow us to

File diff suppressed because it is too large Load Diff

View File

@ -22,6 +22,7 @@
use super::ident::IdentKind;
use super::object::{
FragmentText, IdentObjectData, IdentObjectState, Source, TransitionError,
UnresolvedError,
};
use super::Sections;
use crate::sym::Symbol;
@ -86,7 +87,7 @@ where
name: &'i Symbol<'i>,
kind: IdentKind,
src: Source<'i>,
) -> AsgResult<ObjectRef<Ix>, Ix>;
) -> AsgResult<ObjectRef<Ix>>;
/// Declare an abstract identifier.
///
@ -113,7 +114,7 @@ where
name: &'i Symbol<'i>,
kind: IdentKind,
src: Source<'i>,
) -> AsgResult<ObjectRef<Ix>, Ix>;
) -> AsgResult<ObjectRef<Ix>>;
/// Set the fragment associated with a concrete identifier.
///
@ -124,7 +125,7 @@ where
&mut self,
identi: ObjectRef<Ix>,
text: FragmentText,
) -> AsgResult<ObjectRef<Ix>, Ix>;
) -> AsgResult<ObjectRef<Ix>>;
/// Retrieve an object from the graph by [`ObjectRef`].
///
@ -186,17 +187,28 @@ where
O: IdentObjectData<'i>,
Ix: IndexType,
{
/// Sort graph into [`Sections`].
///
/// Sorting will fail if the graph contains unresolved objects,
/// or identifiers whose kind cannot be determined
/// (see [`UnresolvedError`]).
fn sort(
&'i self,
roots: &[ObjectRef<Ix>],
) -> AsgResult<Sections<'i, O>, Ix>;
) -> SortableAsgResult<Sections<'i, O>, Ix>;
}
/// A [`Result`] with a hard-coded [`AsgError`] error type.
///
/// This is the result of every [`Asg`] operation that could potentially
/// fail in error.
pub type AsgResult<T, Ix> = Result<T, AsgError<Ix>>;
pub type AsgResult<T> = Result<T, AsgError>;
/// A [`Result`] with a hard-coded [`SortableAsgError`] error type.
///
/// This is the result of every [`SortableAsg`] operation that could
/// potentially fail in error.
pub type SortableAsgResult<T, Ix> = Result<T, SortableAsgError<Ix>>;
/// Reference to an [object][super::object] stored within the [`Asg`].
///
@ -240,7 +252,7 @@ pub type Node<O> = Option<O>;
/// so this stores only owned values.
/// The caller will know the problem values.
#[derive(Debug, PartialEq)]
pub enum AsgError<Ix: IndexType> {
pub enum AsgError {
/// An object could not change state in the manner requested.
///
/// See [`Asg::declare`] and [`Asg::set_fragment`] for more
@ -250,24 +262,20 @@ pub enum AsgError<Ix: IndexType> {
/// The node was not expected in the current context
UnexpectedNode(String),
/// The graph has a cyclic dependency
Cycles(Vec<Vec<ObjectRef<Ix>>>),
}
impl<Ix: IndexType> std::fmt::Display for AsgError<Ix> {
impl std::fmt::Display for AsgError {
fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
match self {
Self::ObjectTransition(err) => std::fmt::Display::fmt(&err, fmt),
Self::UnexpectedNode(msg) => {
write!(fmt, "unexpected node: {}", msg)
}
Self::Cycles(_) => write!(fmt, "cyclic dependencies"),
}
}
}
impl<Ix: IndexType> std::error::Error for AsgError<Ix> {
impl std::error::Error for AsgError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match self {
Self::ObjectTransition(err) => err.source(),
@ -276,12 +284,72 @@ impl<Ix: IndexType> std::error::Error for AsgError<Ix> {
}
}
impl<Ix: IndexType> From<TransitionError> for AsgError<Ix> {
impl From<TransitionError> for AsgError {
fn from(err: TransitionError) -> Self {
Self::ObjectTransition(err)
}
}
/// Error during graph sorting.
///
/// These errors reflect barriers to meaningfully understanding the
/// properties of the data in the graph with respect to sorting.
/// It does not represent bad underlying data that does not affect the
/// sorting process.
#[derive(Debug, PartialEq)]
pub enum SortableAsgError<Ix: IndexType> {
/// An unresolved object was encountered during sorting.
///
/// An unresolved object means that the graph has an incomplete picture
/// of the program,
/// and so sorting cannot be reliably performed.
/// Since all objects are supposed to be resolved prior to sorting,
/// this represents either a problem with the program being compiled
/// or a bug in the compiler itself.
UnresolvedObject(UnresolvedError),
/// The kind of an object encountered during sorting could not be
/// determined.
///
/// Sorting uses the object kind to place objects into their appropriate
/// sections.
/// It should never be the case that a resolved object has no kind,
/// so this likely represents a compiler bug.
MissingObjectKind(String),
/// The graph has a cyclic dependency.
Cycles(Vec<Vec<ObjectRef<Ix>>>),
}
impl<Ix: IndexType> From<UnresolvedError> for SortableAsgError<Ix> {
fn from(err: UnresolvedError) -> Self {
Self::UnresolvedObject(err)
}
}
impl<Ix: IndexType> std::fmt::Display for SortableAsgError<Ix> {
fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
match self {
Self::UnresolvedObject(err) => std::fmt::Display::fmt(err, fmt),
Self::MissingObjectKind(name) => write!(
fmt,
"internal error: missing object kind for object `{}` (this may be a compiler bug!)",
name,
),
Self::Cycles(_) => write!(fmt, "cyclic dependencies"),
}
}
}
impl<Ix: IndexType> std::error::Error for SortableAsgError<Ix> {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match self {
Self::UnresolvedObject(err) => Some(err),
_ => None,
}
}
}
#[cfg(test)]
mod test {
use super::*;

View File

@ -196,11 +196,14 @@ mod ident;
mod object;
mod section;
pub use graph::{Asg, AsgError, AsgResult, IndexType, ObjectRef, SortableAsg};
pub use graph::{
Asg, AsgError, AsgResult, IndexType, ObjectRef, SortableAsg,
SortableAsgError,
};
pub use ident::{DataType, Dim, IdentKind, IdentKindError};
pub use object::{
FragmentText, IdentObject, IdentObjectData, IdentObjectState, Source,
TransitionError, TransitionResult,
TransitionError, TransitionResult, UnresolvedError,
};
pub use section::{Section, SectionIterator, Sections};

View File

@ -212,6 +212,23 @@ where
/// since rules may vary between implementations.
fn resolve(self, kind: IdentKind, src: Source<'i>) -> TransitionResult<T>;
/// Assertion to return self if identifier is resolved,
/// otherwise failing with [`UnresolvedError`].
///
/// This simplifies working with identifiers without having to match on
/// specific variants,
/// and will continue to work if new variants are added in the
/// future that are considered to be unresolved.
///
/// Since this does not cause a state transition and is useful in
/// contexts where ownership over the identifier is not possible,
/// this accepts and returns a reference to the identifier.
///
/// At present,
/// both [`IdentObject::Missing`] and [`IdentObject::Extern`] are
/// considered to be unresolved.
fn resolved(&self) -> Result<&T, UnresolvedError>;
/// Resolve identifier against an extern declaration or produce an
/// extern.
///
@ -373,6 +390,25 @@ impl<'i> IdentObjectState<'i, IdentObject<'i>> for IdentObject<'i> {
}
}
fn resolved(&self) -> Result<&IdentObject<'i>, UnresolvedError> {
match self {
IdentObject::Missing(name) => Err(UnresolvedError::Missing {
name: name.to_string(),
}),
IdentObject::Extern(name, ref kind, ref src) => {
Err(UnresolvedError::Extern {
name: name.to_string(),
kind: kind.clone(),
pkg_name: src.pkg_name.map(|s| s.to_string()),
})
}
IdentObject::Ident(_, _, _)
| IdentObject::IdentFragment(_, _, _, _) => Ok(self),
}
}
fn extern_(
self,
kind: IdentKind,
@ -524,6 +560,52 @@ impl std::error::Error for TransitionError {
}
}
/// Resolved identifier was expected.
#[derive(Clone, Debug, PartialEq)]
pub enum UnresolvedError {
/// Expected identifier is missing and nothing about it is known.
Missing { name: String },
/// Expected identifier has not yet been resolved with a concrete
/// definition.
Extern {
/// Identifier name.
name: String,
/// Expected identifier type.
kind: IdentKind,
/// Name of package where the extern was defined.
pkg_name: Option<String>,
},
}
impl std::fmt::Display for UnresolvedError {
fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
match self {
UnresolvedError::Missing { name } => {
write!(fmt, "missing expected identifier `{}`", name,)
}
UnresolvedError::Extern {
name,
kind,
pkg_name,
} => write!(
fmt,
"unresolved extern `{}` of type `{}`, declared in `{}`",
name,
kind,
pkg_name.as_ref().unwrap_or(&"<unknown>".into()),
),
}
}
}
impl std::error::Error for UnresolvedError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
None
}
}
/// Compiled fragment for identifier.
///
/// This represents the text associated with an identifier.
@ -782,8 +864,24 @@ mod test {
}
#[test]
fn ident_object_ident() {
fn resolved_on_missing() {
let sym = symbol_dummy!(1, "missing");
let result = IdentObject::declare(&sym)
.resolved()
.expect_err("expected error asserting resolved() on missing");
match result {
UnresolvedError::Missing { name: e_name } => {
assert_eq!(sym.to_string(), e_name);
}
_ => panic!("expected UnresolvedError {:?}", result),
}
}
#[test]
fn ident_object_ident() {
let sym = symbol_dummy!(1, "ident");
let kind = IdentKind::Meta;
let src = Source {
desc: Some("ident ctor".into()),
@ -798,6 +896,25 @@ mod test {
);
}
#[test]
fn resolved_on_ident() {
let sym = symbol_dummy!(1, "ident resolve");
let kind = IdentKind::Meta;
let src = Source {
desc: Some("ident ctor".into()),
..Default::default()
};
assert_eq!(
&IdentObject::Ident(&sym, kind.clone(), src.clone()),
IdentObject::declare(&sym)
.resolve(kind.clone(), src.clone())
.unwrap()
.resolved()
.unwrap(),
);
}
// Note that we don't care about similar sources. It's expected
// that the system populating the ASG will only resolve local
// symbols, and so redeclarations should represent that multiple
@ -834,7 +951,7 @@ mod test {
#[test]
fn ident_object() {
let sym = symbol_dummy!(1, "missing");
let sym = symbol_dummy!(1, "extern");
let kind = IdentKind::Class(Dim::from_u8(1));
let src = Source {
desc: Some("extern".into()),
@ -847,6 +964,72 @@ mod test {
);
}
#[test]
fn resolved_on_extern() {
let sym = symbol_dummy!(1, "extern resolved");
let kind = IdentKind::Class(Dim::from_u8(1));
let pkg_name = symbol_dummy!(2, "pkg/name");
let src = Source {
pkg_name: Some(&pkg_name),
desc: Some("extern".into()),
..Default::default()
};
let result =
IdentObject::Extern(&sym, kind.clone(), src.clone())
.resolved()
.expect_err(
"expected error asserting resolved() on extern",
);
match result {
UnresolvedError::Extern {
name: e_name,
kind: e_kind,
pkg_name: e_pkg_name,
} => {
assert_eq!(sym.to_string(), e_name);
assert_eq!(kind, e_kind);
assert_eq!(Some(pkg_name.to_string()), e_pkg_name);
}
_ => panic!("expected UnresolvedError: {:?}", result),
}
}
#[test]
fn resolved_on_extern_error_fmt_without_pkg() {
let meta = IdentKind::Meta;
let err = UnresolvedError::Extern {
name: "foo".into(),
kind: IdentKind::Meta,
pkg_name: None,
};
let msg = format!("{}", err);
assert!(msg.contains("`foo`"));
assert!(msg.contains("in `<unknown>`"));
assert!(msg.contains(&format!("`{}`", meta)));
}
#[test]
fn resolved_on_extern_error_fmt_with_pkg() {
let meta = IdentKind::Meta;
let pkg = "pkg".to_string();
let err = UnresolvedError::Extern {
name: "foo".into(),
kind: IdentKind::Meta,
pkg_name: Some(pkg.clone()),
};
let msg = format!("{}", err);
assert!(msg.contains("`foo`"));
assert!(msg.contains(&format!("in `{}`", pkg)));
assert!(msg.contains(&format!("`{}`", meta)));
}
// Extern first, then identifier
#[test]
fn redeclare_compatible_resolves() {
@ -1020,6 +1203,27 @@ mod test {
);
}
#[test]
fn resolved_on_fragment() {
let sym = symbol_dummy!(1, "tofrag resolved");
let src = Source {
generated: true,
..Default::default()
};
let kind = IdentKind::Meta;
let ident = IdentObject::declare(&sym)
.resolve(kind.clone(), src.clone())
.unwrap();
let text = FragmentText::from("a fragment for resolved()");
let ident_with_frag = ident.set_fragment(text.clone());
assert_eq!(
Ok(&IdentObject::IdentFragment(&sym, kind, src, text)),
ident_with_frag.unwrap().resolved(),
);
}
#[test]
fn add_fragment_to_fragment_fails() {
let sym = symbol_dummy!(1, "badsym");

View File

@ -25,8 +25,8 @@ use crate::fs::{
};
use crate::global;
use crate::ir::asg::{
Asg, AsgError, DefaultAsg, IdentObject, IdentObjectData, Sections,
SortableAsg,
Asg, DefaultAsg, IdentObject, IdentObjectData, Sections, SortableAsg,
SortableAsgError,
};
use crate::obj::xmle::writer::XmleWriter;
use crate::obj::xmlo::{AsgBuilder, AsgBuilderState, XmloReader};
@ -72,7 +72,7 @@ pub fn xmle(package_path: &str, output: &str) -> Result<(), Box<dyn Error>> {
let mut sorted = match depgraph.sort(&roots) {
Ok(sections) => sections,
Err(AsgError::Cycles(cycles)) => {
Err(SortableAsgError::Cycles(cycles)) => {
let msg: Vec<String> = cycles
.into_iter()
.map(|cycle| {

View File

@ -294,7 +294,10 @@ impl<W: Write> XmleWriter<W> {
self.writer.write_event(Event::Empty(sym))?;
}
_ => unreachable!("filtered out during sorting"),
_ => unreachable!(
"identifier should have been filtered out during sorting: {:?}",
ident,
),
}
}

View File

@ -81,7 +81,7 @@ use std::fmt::Display;
use std::hash::BuildHasher;
pub type Result<'i, S, Ix> =
std::result::Result<AsgBuilderState<'i, S, Ix>, AsgBuilderError<Ix>>;
std::result::Result<AsgBuilderState<'i, S, Ix>, AsgBuilderError>;
/// Builder state between imports.
///
@ -289,7 +289,7 @@ where
/// Error populating graph with [`XmloResult`]-derived data.
#[derive(Debug, PartialEq)]
pub enum AsgBuilderError<Ix: IndexType> {
pub enum AsgBuilderError {
/// Error with the source `xmlo` file.
XmloError(XmloError),
@ -297,7 +297,7 @@ pub enum AsgBuilderError<Ix: IndexType> {
IdentKindError(IdentKindError),
/// [`Asg`] operation error.
AsgError(AsgError<Ix>),
AsgError(AsgError),
/// Fragment encountered for an unknown identifier.
MissingFragmentIdent(String),
@ -309,7 +309,7 @@ pub enum AsgBuilderError<Ix: IndexType> {
BadEligRef(String),
}
impl<Ix: IndexType> Display for AsgBuilderError<Ix> {
impl Display for AsgBuilderError {
fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
match self {
Self::XmloError(e) => e.fmt(fmt),
@ -331,25 +331,25 @@ impl<Ix: IndexType> Display for AsgBuilderError<Ix> {
}
}
impl<Ix: IndexType> From<XmloError> for AsgBuilderError<Ix> {
impl From<XmloError> for AsgBuilderError {
fn from(src: XmloError) -> Self {
Self::XmloError(src)
}
}
impl<Ix: IndexType> From<IdentKindError> for AsgBuilderError<Ix> {
impl From<IdentKindError> for AsgBuilderError {
fn from(src: IdentKindError) -> Self {
Self::IdentKindError(src)
}
}
impl<Ix: IndexType> From<AsgError<Ix>> for AsgBuilderError<Ix> {
fn from(src: AsgError<Ix>) -> Self {
impl From<AsgError> for AsgBuilderError {
fn from(src: AsgError) -> Self {
Self::AsgError(src)
}
}
impl<Ix: IndexType> Error for AsgBuilderError<Ix> {
impl Error for AsgBuilderError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match self {
Self::XmloError(e) => Some(e),