employer
/
tame
1
0
Fork 0
Code Packages Releases Activity

tamer: asg::air: Expression building error cases 

This addresses the two outstanding `todo!` match arms representing errors in
lowering expressions into the graph.  As noted in the comments, these errors
are unlikely to be hit when using TAME in the traditional way, since
e.g. XIR and NIR are going to catch the equivalent problems within their own
contexts (unbalanced tags and a valid expression grammar respectively).

_But_, the IR does need to stand on its own, and I further hope that some
tooling maybe can interact more directly with AIR in the future.

DEV-13160
main
Mike Gerwitz 2023-01-09 12:02:59 -05:00
parent dc3cd8bbc8
commit 8786ee74fa
4 changed files with 186 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
tamer/src/asg/air.rs
View File

@ -447,7 +447,9 @@ impl ParseState for AirAggregate {
Transition(BuildingExpr(es.push(poi), oi)).incomplete()
}
(Empty(_), CloseExpr(_)) => todo!("no matching expr to end"),
(st @ Empty(_), CloseExpr(span)) => {
Transition(st).err(AsgError::UnbalancedExpr(span))
}
(BuildingExpr(es, oi), CloseExpr(end)) => {
let start: Span = oi.into();
@ -475,7 +477,9 @@ impl ParseState for AirAggregate {
}
}
(Empty(_), IdentExpr(_)) => todo!("cannot bind ident to nothing"),
(st @ Empty(_), IdentExpr(ident)) => {
Transition(st).err(AsgError::InvalidExprBindContext(ident))
}
(BuildingExpr(es, oi), IdentExpr(id)) => {
// TODO: error on existing ident

90
tamer/src/asg/air/test.rs
View File

@ -254,7 +254,7 @@ fn ident_root_existing() {
}
#[test]
fn expr_empty() {
fn expr_empty_ident() {
let id = SPair("foo".into(), S2);
let toks = vec![
@ -275,7 +275,7 @@ fn expr_empty() {
}
#[test]
fn expr_non_empty() {
fn expr_non_empty_ident_root() {
let id_a = SPair("foo".into(), S2);
let id_b = SPair("bar".into(), S2);
@ -496,3 +496,89 @@ fn recovery_expr_reachable_after_dangling() {
// decide to do in the future.
// So we end here.
}
#[test]
fn expr_close_unbalanced() {
let id = SPair("foo".into(), S3);
let toks = vec![
// Close before _any_ open.
Air::CloseExpr(S1),
// Should recover,
// allowing for a normal expr.
Air::OpenExpr(ExprOp::Sum, S2),
Air::IdentExpr(id),
Air::CloseExpr(S4),
// And now an extra close _after_ a valid expr.
Air::CloseExpr(S5),
];
let mut sut = Sut::parse(toks.into_iter());
assert_eq!(
vec![
Err(ParseError::StateError(AsgError::UnbalancedExpr(S1))),
// Recovery should allow us to continue.
Ok(Parsed::Incomplete), // OpenExpr
Ok(Parsed::Incomplete), // IdentExpr
Ok(Parsed::Incomplete), // CloseExpr
// Another error after a successful expression.
Err(ParseError::StateError(AsgError::UnbalancedExpr(S5))),
],
sut.by_ref().collect::<Vec<_>>(),
);
let asg = sut.finalize().unwrap().into_context();
// Just verify that the expression was successfully added after recovery.
let expr = asg.expect_ident_obj::<Expr>(id);
assert_eq!(expr.span(), S2.merge(S4).unwrap());
}
#[test]
fn expr_bind_to_empty() {
let id_noexpr_a = SPair("noexpr_a".into(), S1);
let id_good = SPair("noexpr".into(), S3);
let id_noexpr_b = SPair("noexpr_b".into(), S5);
let toks = vec![
// No open expression to bind to.
Air::IdentExpr(id_noexpr_a),
// Post-recovery create an expression.
Air::OpenExpr(ExprOp::Sum, S2),
Air::IdentExpr(id_good),
Air::CloseExpr(S4),
// Once again we have nothing to bind to.
Air::IdentExpr(id_noexpr_b),
];
let mut sut = Sut::parse(toks.into_iter());
assert_eq!(
vec![
Err(ParseError::StateError(AsgError::InvalidExprBindContext(
id_noexpr_a
))),
// Recovery should allow us to continue.
Ok(Parsed::Incomplete), // OpenExpr
Ok(Parsed::Incomplete), // IdentExpr
Ok(Parsed::Incomplete), // CloseExpr
// Another error after a successful expression.
Err(ParseError::StateError(AsgError::InvalidExprBindContext(
id_noexpr_b
))),
],
sut.by_ref().collect::<Vec<_>>(),
);
let asg = sut.finalize().unwrap().into_context();
// Neither of the identifiers outside of expressions should exist on the
// graph.
assert_eq!(None, asg.get_ident_obj::<Expr>(id_noexpr_a));
assert_eq!(None, asg.get_ident_obj::<Expr>(id_noexpr_b));
// Verify that the expression was successfully added after recovery.
let expr = asg.expect_ident_obj::<Expr>(id_good);
assert_eq!(expr.span(), S2.merge(S4).unwrap());
}

47
tamer/src/asg/error.rs
View File

@ -26,6 +26,7 @@ use std::{
use crate::{
diagnose::{Annotate, AnnotatedSpan, Diagnostic},
parse::util::SPair,
span::Span,
};
@ -49,6 +50,26 @@ pub enum AsgError {
/// its span.
/// The span should encompass the entirety of the expression.
DanglingExpr(Span),
/// Attempted to close an expression with no corresponding opening
/// delimiter.
///
/// Note that the user may encounter an equivalent error in the source
/// document format
/// (e.g. XML via [XIR->NIR lowering](crate::nir))
/// and therefore may never see this error.
/// However,
/// a source IR _may_ choose to allow improperly nested expressions
/// through to this IR,
/// or may utilize this IR directly.
UnbalancedExpr(Span),
/// Attempted to bind the an identifier to an expression while not in an
/// expression context.
///
/// Note that the user may encounter an error from a higher-level IR
/// instead of this one.
InvalidExprBindContext(SPair),
}
impl Display for AsgError {
@ -58,6 +79,10 @@ impl Display for AsgError {
match self {
IdentTransition(err) => Display::fmt(&err, f),
DanglingExpr(_) => write!(f, "dangling expression"),
UnbalancedExpr(_) => write!(f, "unbalanced expression"),
InvalidExprBindContext(_) => {
write!(f, "invalid expression identifier binding context")
}
}
}
}
@ -68,7 +93,9 @@ impl Error for AsgError {
match self {
IdentTransition(err) => err.source(),
DanglingExpr(_) => None,
DanglingExpr(_) | UnbalancedExpr(_) | InvalidExprBindContext(_) => {
None
}
}
}
}
@ -83,6 +110,10 @@ impl Diagnostic for AsgError {
fn describe(&self) -> Vec<AnnotatedSpan> {
use AsgError::*;
// Before improving the diagnostic messages below,
// be sure that you have a use case in mind and that higher-level
// IRs do not preempt them in practice;
// your efforts may be better focused in those higher IRs.
match self {
// TODO: need spans
IdentTransition(_) => vec![],
@ -95,6 +126,20 @@ impl Diagnostic for AsgError {
),
span.help(" its value cannot referenced."),
],
UnbalancedExpr(span) => {
vec![span.error("there is no open expression to end here")]
}
InvalidExprBindContext(span) => vec![
span.error(
"there is no active expression to bind this identifier to",
),
span.help(
"an identifier must be bound to an expression before \
the expression is closed",
),
],
}
}
}

77
tamer/src/asg/graph.rs
View File

@ -26,6 +26,7 @@ use super::{
use crate::diagnose::panic::DiagnosticPanic;
use crate::diagnose::{Annotate, AnnotatedSpan};
use crate::f::Functor;
use crate::fmt::{DisplayWrapper, TtQuote};
use crate::global;
use crate::parse::util::SPair;
use crate::parse::Token;
@ -472,7 +473,6 @@ impl Asg {
///
/// This will return [`None`] if the identifier is either opaque or does
/// not exist.
#[cfg(test)]
fn get_ident_obj_oi<O: ObjectKind>(
&self,
ident: SPair,
@ -492,25 +492,57 @@ impl Asg {
.map(|ni| ObjectIndex::<O>::new(ni, ident.span()))
}
/// Retrieve the [`ObjectIndex`] to which the given `ident` is bound,
/// panicing if the identifier is opaque or does not exist.
/// Attempt to retrieve the [`Object`] to which the given `ident` is bound.
///
/// See [`Self::get_ident_obj_oi`] for more information;
/// this simply panics if its result is [`None`].
/// If the identifier either does not exist on the graph or is opaque
/// (is not bound to any expression),
/// then [`None`] will be returned.
///
/// If the system expects that the identifier must exist and would
/// otherwise represent a bug in the compiler,
/// see [`Self::expect_ident_obj`].
///
/// Panics
/// ======
/// 1. The identifier does not exist on the graph;
/// This method will panic if certain graph invariants are not met,
/// representing an invalid system state that should not be able to
/// occur through this API.
/// Violations of these invariants represent either a bug in the API
/// (that allows for the invariant to be violated)
/// or direct manipulation of the underlying graph.
pub fn get_ident_obj<O: ObjectKind>(&self, ident: SPair) -> Option<&O> {
self.get_ident_obj_oi::<O>(ident).map(|oi| {
let obj_container =
self.graph.node_weight(oi.into()).diagnostic_expect(
diagnostic_node_missing_desc(oi),
"invalid ObjectIndex: data are missing from the ASG",
);
obj_container.as_ref().diagnostic_expect(
diagnostic_borrowed_node_desc(oi),
"inaccessible ObjectIndex: object has not been returned to the ASG",
).into()
})
}
/// Attempt to retrieve the [`Object`] to which the given `ident` is bound,
/// panicing if the identifier is opaque or does not exist.
///
/// This method represents a compiler invariant;
/// it should _only_ be used when the identifier _must_ exist,
/// otherwise there is a bug in the compiler.
/// If this is _not_ the case,
/// use [`Self::get_ident_obj`] to get [`None`] in place of a panic.
///
/// Panics
/// ======
/// This method will panic if
///
/// 1. The identifier does not exist on the graph; or
/// 2. The identifier is opaque (has no edge to any object on the
/// graph).
#[cfg(test)]
fn expect_ident_obj_oi<O: ObjectKind>(
&self,
ident: SPair,
) -> ObjectIndex<O> {
use crate::fmt::{DisplayWrapper, TtQuote};
self.get_ident_obj_oi(ident).diagnostic_expect(
pub fn expect_ident_obj<O: ObjectKind>(&self, ident: SPair) -> &O {
self.get_ident_obj(ident).diagnostic_expect(
diagnostic_opaque_ident_desc(ident),
&format!(
"opaque identifier: {} has no object binding",
@ -519,22 +551,6 @@ impl Asg {
)
}
#[cfg(test)]
pub(super) fn expect_ident_obj<O: ObjectKind>(&self, ident: SPair) -> &O {
let oi = self.expect_ident_obj_oi::<O>(ident);
let obj_container =
self.graph.node_weight(oi.into()).diagnostic_expect(
diagnostic_node_missing_desc(oi),
"invalid ObjectIndex: data are missing from the ASG",
);
obj_container.as_ref().diagnostic_expect(
diagnostic_borrowed_node_desc(oi),
"inaccessible ObjectIndex: object has not been returned to the ASG",
).into()
}
/// Retrieve an identifier from the graph by [`ObjectIndex`].
///
/// If the object exists but is not an identifier,
@ -646,7 +662,6 @@ fn diagnostic_borrowed_node_desc<O: ObjectKind>(
]
}
#[cfg(test)]
fn diagnostic_opaque_ident_desc(ident: SPair) -> Vec<AnnotatedSpan<'static>> {
vec![
ident.internal_error(