tamer: asg::graph::object::ident::ObjectIndex::<Ident>::bind_definition: Replace ident span

I noticed this while working on a graph traversal.  The unit test used the
same span for both the reference _and_ the binding, so I didn't notice. -_-

The problem with this, though, is that we do not have a separate span
representing the source location of the identifier reference.  The reason is
that we decided to re-use an existing node rather than creating another one,
which would add another inconvenient layer of indirection (and complexity).

So, I may have to add (optional?) spans to edges.

DEV-13708
main
Mike Gerwitz 2023-02-07 10:23:48 -05:00
parent 89700aa949
commit 39e98210be
3 changed files with 16 additions and 5 deletions

View File

@ -557,7 +557,7 @@ impl ParseState for AirAggregate {
// It is important that we do not mark this expression as
// reachable unless we successfully bind the identifier.
match oi_ident.bind_definition(asg, oi) {
match oi_ident.bind_definition(asg, id, oi) {
Ok(_) => Transition(BuildingExpr(
oi_pkg,
es.reachable_by(id),

View File

@ -984,8 +984,9 @@ fn expr_ref_to_ident() {
let toks = vec![
Air::ExprOpen(ExprOp::Sum, S1),
Air::ExprIdent(id_foo),
// Reference to an as-of-yet-undefined id (okay)
Air::ExprRef(id_bar),
// Reference to an as-of-yet-undefined id (okay),
// with a different span than `id_bar`.
Air::ExprRef(SPair("bar".into(), S3)),
Air::ExprClose(S4),
//
// Another expression to reference the first
@ -1018,13 +1019,18 @@ fn expr_ref_to_ident() {
let oi_ident_bar =
foo_rels.pop().and_then(ExprRel::narrow::<Ident>).unwrap();
let ident_bar = oi_ident_bar.resolve(&asg);
assert_eq!(ident_bar.span(), id_bar.span());
// The identifier will have originally been `Missing`,
// since it did not exist at the point of reference.
// But it should now properly identify the other expression.
assert_matches!(ident_bar, Ident::Transparent(..));
// The span of the identifier must be updated with the defining
// `ExprIdent`,
// otherwise it'll be the location of the `ExprRef` that originally
// added it as `Missing`.
assert_eq!(ident_bar.span(), id_bar.span());
let oi_expr_bar = asg.expect_ident_oi::<Expr>(id_bar);
assert!(oi_ident_bar.is_bound_to(&asg, oi_expr_bar));
}

View File

@ -1052,6 +1052,7 @@ impl ObjectIndex<Ident> {
pub fn bind_definition<O: ObjectKind>(
self,
asg: &mut Asg,
id: SPair,
definition: ObjectIndex<O>,
) -> Result<ObjectIndex<Ident>, AsgError>
where
@ -1088,7 +1089,11 @@ impl ObjectIndex<Ident> {
}
// We are okay to proceed to add an edge to the `definition`.
Missing(id) => Ok(Transparent(id)),
// Discard the original span
// (which is the location of the first reference _to_ this
// identifier before it was defined),
// and use the newly provided `id` and its span.
Missing(_) => Ok(Transparent(id)),
})
.map(|ident_oi| ident_oi.add_edge_to(asg, definition))
}