Skip to content

Commit

Permalink
Fix #209 (#210)
Browse files Browse the repository at this point in the history 
This puts through a simple fix to prevent `defalias` definitions from
automatically marking their source columns as being used.
  • Loading branch information
@DavePearce
DavePearce authored Jun 20, 2024
1 parent 9559248 commit 375e724
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 30 deletions.
27 changes: 15 additions & 12 deletions src/compiler/generator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1547,11 +1547,11 @@ pub fn reduce(e: &AstNode, ctx: &mut Scope, settings: &CompileSettings) -> Resul
),
)),
Token::Symbol(name) => Ok(Some(
ctx.resolve_symbol(name)
ctx.resolve_symbol(name, true)
.with_context(|| make_ast_error(e))?,
)),
Token::IndexedSymbol { name, index } => {
let symbol = ctx.resolve_symbol(name)?;
let symbol = ctx.resolve_symbol(name, true)?;
if let Expression::ArrayColumn {
handle,
domain,
Expand All @@ -1568,7 +1568,7 @@ pub fn reduce(e: &AstNode, ctx: &mut Scope, settings: &CompileSettings) -> Resul
let name = handle.as_handle().ith(i.try_into().unwrap()).to_string();
// Resolve it properly this time.
Ok(Some(
ctx.resolve_symbol_with_path(&name)
ctx.resolve_symbol_with_path(&name, true)
.with_context(|| make_ast_error(e))?,
))
} else {
Expand Down Expand Up @@ -1619,26 +1619,29 @@ pub fn reduce(e: &AstNode, ctx: &mut Scope, settings: &CompileSettings) -> Resul
_ => Ok(None),
},
Token::DefInterleaving { target, froms } => {
let target_handle =
if let Expression::Column { handle, .. } = ctx.resolve_symbol(&target.name)?.e() {
handle.to_owned()
} else {
unreachable!()
};
let target_handle = if let Expression::Column { handle, .. } =
ctx.resolve_symbol(&target.name, true)?.e()
{
handle.to_owned()
} else {
unreachable!()
};

let mut from_handles = Vec::new();
for from in froms {
match &from.class {
Token::Symbol(name) => {
if let Expression::Column { handle, .. } = ctx.resolve_symbol(name)?.e() {
if let Expression::Column { handle, .. } =
ctx.resolve_symbol(name, true)?.e()
{
from_handles.push(handle.clone());
} else {
bail!("{} is not a column", name.white().bold());
}
}
Token::IndexedSymbol { name, index } => {
if let Expression::ArrayColumn { handle, domain, .. } =
ctx.resolve_symbol(name)?.e()
ctx.resolve_symbol(name, true)?.e()
{
let index_usize = reduce(index, ctx, settings)?
.and_then(|n| n.pure_eval().ok())
Expand Down Expand Up @@ -1875,7 +1878,7 @@ pub(crate) fn reduce_toplevel(

// TODO is this needed?
to.iter()
.map(|f| ctx.resolve_symbol(&f.name))
.map(|f| ctx.resolve_symbol(&f.name, true))
.collect::<Result<Vec<_>, errors::symbols::Error>>()
.with_context(|| anyhow!("while defining permutation"))?;
let suffix = hash_strings(froms.iter().map(|f| f.as_handle().name.clone()));
Expand Down
5 changes: 4 additions & 1 deletion src/compiler/parser/definitions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,11 @@ fn reduce(e: &AstNode, ctx: &mut Scope, settings: &CompileSettings) -> Result<()
)
}
Token::DefAlias(from, to) => {
// NOTE: false here (for used) is needed to prevent this
// symbol resolution from marking the column being aliased
// as used.
let _ = ctx
.resolve_symbol(to)
.resolve_symbol(to, false)
.with_context(|| anyhow!("while defining alias `{}`", from))?;

ctx.insert_alias(from, to)
Expand Down
45 changes: 28 additions & 17 deletions src/compiler/tables.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,13 +488,13 @@ impl Scope {
Ok(())
}

pub fn resolve_symbol(&mut self, name: &str) -> Result<Node, symbols::Error> {
pub fn resolve_symbol(&mut self, name: &str, used: bool) -> Result<Node, symbols::Error> {
let module = self.module();
let global = data!(self).global;

if name.contains('.') {
if global {
self.resolve_symbol_with_path(name)
self.resolve_symbol_with_path(name, used)
} else {
Err(symbols::Error::NotAGlobalScope(
name.split('.').next().unwrap().to_owned(),
Expand All @@ -513,6 +513,7 @@ impl Scope {
&mut self.tree.borrow_mut(),
name,
perspective,
used,
)
.map_err(|e| match e {
symbols::Error::SymbolNotFound(s, _, _) => {
Expand All @@ -531,14 +532,19 @@ impl Scope {
&mut HashSet::new(),
false,
false,
used,
)
.map_err(|_| symbols::Error::SymbolNotFound(name.to_owned(), module, None))
}
}

pub fn resolve_symbol_with_path(&mut self, name: &str) -> Result<Node, symbols::Error> {
pub fn resolve_symbol_with_path(
&mut self,
name: &str,
used: bool,
) -> Result<Node, symbols::Error> {
let components = name.split('.').collect::<Vec<_>>();
self.root()._resolve_symbol_with_path(&components)
self.root()._resolve_symbol_with_path(&components, used)
}

fn _resolve_symbol(
Expand All @@ -548,6 +554,7 @@ impl Scope {
ax: &mut HashSet<String>,
absolute_path: bool,
pure: bool,
used: bool,
) -> Result<Node, symbols::Error> {
if ax.contains(name) {
Err(symbols::Error::CircularDefinition(name.to_string()))
Expand All @@ -556,13 +563,15 @@ impl Scope {
match tree[n].unwrap_data_mut().symbols.get_mut(name) {
Some(Symbol::Alias(target)) => {
let target = target.to_owned();
Self::_resolve_symbol(n, tree, &target, ax, absolute_path, pure)
Self::_resolve_symbol(n, tree, &target, ax, absolute_path, pure, used)
}
Some(Symbol::Final(exp, ref mut visited)) => {
if pure && !matches!(exp.e(), Expression::Const(..)) {
Err(symbols::Error::UnavailableInPureContext(exp.to_string()))
} else {
*visited = true;
if used {
*visited = true;
}
Result::Ok(exp.clone())
}
}
Expand All @@ -580,6 +589,7 @@ impl Scope {
&mut HashSet::new(),
false,
tree[n].unwrap_data().closed || pure,
used,
)
},
)
Expand All @@ -594,31 +604,38 @@ impl Scope {
tree: &mut SymbolTableTree,
name: &str,
perspective: &str,
used: bool,
) -> Result<Node, symbols::Error> {
match tree.find_child(n, |o| {
o.perspective
.as_ref()
.map(|p| p == perspective)
.unwrap_or(false)
}) {
Some(o) => Self::_resolve_symbol(o, tree, name, &mut HashSet::new(), true, false),
Some(o) => Self::_resolve_symbol(o, tree, name, &mut HashSet::new(), true, false, used),
None => tree.parent(n).map_or(
Err(symbols::Error::PerspectiveNotFound(
perspective.into(),
tree[n].data().unwrap().module.clone(),
)),
|parent| Self::_resolve_symbol_in_perspective(parent, tree, name, perspective),
|parent| {
Self::_resolve_symbol_in_perspective(parent, tree, name, perspective, used)
},
),
}
}

fn _resolve_symbol_with_path(&mut self, path: &[&str]) -> Result<Node, symbols::Error> {
fn _resolve_symbol_with_path(
&mut self,
path: &[&str],
used: bool,
) -> Result<Node, symbols::Error> {
if path.len() == 1 {
self.resolve_symbol(path[0])
self.resolve_symbol(path[0], used)
} else {
for c in self.children() {
if data!(c).name == path[0] {
return self.at(c.id)._resolve_symbol_with_path(&path[1..]);
return self.at(c.id)._resolve_symbol_with_path(&path[1..], used);
}
}
return Err(symbols::Error::ModuleNotFound(
Expand Down Expand Up @@ -708,12 +725,6 @@ impl Scope {
}
}

// pub fn insert_used_symbol(&mut self, name: &str, e: Node) -> Result<()> {
// self.insert_symbol(name, e)?;
// let _ = self.resolve_symbol(name).unwrap();
// Ok(())
// }

pub fn insert_function(&mut self, name: &str, f: Function) -> Result<()> {
let my_name = data!(self).name.to_owned();
// User-defined function can be polymorphic on their input arguments and
Expand Down

0 comments on commit 375e724

Please sign in to comment.