Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better error handling in grammar #78

Merged
merged 4 commits into from
Mar 23, 2022
Merged

Better error handling in grammar #78

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7d8aee3
Better error handling in grammar
tirix Mar 22, 2022
f305801
Make `UnknownToken(Span)` an error.
tirix Mar 22, 2022
1913480
`UnknownCommandToken` to `UndefinedToken`
tirix Mar 22, 2022
afa15f8
Static error messages in case Grammar can't be built
tirix Mar 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
167 changes: 112 additions & 55 deletions src/grammar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,8 +547,11 @@ impl Grammar {
let mut tokens = sref.math_iter().peekable();

// Atom for this axiom's label.
let this_label = names.lookup_label(sref.label()).unwrap().atom;
let this_typecode = nset.get_atom(tokens.next().unwrap().slice);
let this_label = names
.lookup_label(sref.label())
.ok_or_else(|| Diagnostic::UnknownLabel(sref.label_span()))?
.atom;
let this_typecode = nset.get_atom(tokens.next().ok_or(Diagnostic::UnknownToken(0))?.slice);

// In case of a non-syntax axiom, skip it.
if this_typecode == self.provable_type {
Expand Down Expand Up @@ -623,7 +626,10 @@ impl Grammar {
let mut tokens = sref.math_iter();

// Atom for this float's label.
let this_label = names.lookup_label(sref.label()).unwrap().atom;
let this_label = names
.lookup_label(sref.label())
.ok_or_else(|| Diagnostic::UnknownLabel(sref.label_span()))?
.atom;
let this_typecode =
nset.get_atom(tokens.next().ok_or(Diagnostic::NotActiveSymbol(0))?.slice);

Expand All @@ -637,8 +643,7 @@ impl Grammar {
.nodes
.create_leaf(Reduce::new_variable(this_label), this_typecode);

// If is safe to unwrap here since parser has already checked.
let token = tokens.next().unwrap();
let token = tokens.next().ok_or(Diagnostic::BadFloating)?;
let symbol = names
.lookup_symbol(token.slice)
.ok_or(Diagnostic::FloatNotVariable(1))?;
Expand Down Expand Up @@ -698,7 +703,7 @@ impl Grammar {
leaf_label,
},
)
.unwrap();
.map_err(|_| Diagnostic::GrammarCantBuild)?;
}
Some(existing_next_node) => {
// A branch for the converted type already exist: add the conversion to that branch!
Expand All @@ -715,7 +720,7 @@ impl Grammar {
existing_next_node_id,
Reduce::new(label, 1),
)
.unwrap();
.map_err(|_| Diagnostic::GrammarCantBuild)?;
}
}
}
Expand Down Expand Up @@ -752,8 +757,9 @@ impl Grammar {
db: &Database,
mut stored_reduces: ReduceVec,
make_final: F,
) where
F: FnOnce(&ReduceVec, TypeCode) -> NextNode + Copy,
) -> Result<(), Diagnostic>
where
F: FnOnce(&ReduceVec, TypeCode) -> Result<NextNode, Diagnostic> + Copy,
{
debug!("Clone {} to {}", add_from_node_id, add_to_node_id);
debug!("{:?}", self.node_id(db, add_from_node_id));
Expand Down Expand Up @@ -785,7 +791,7 @@ impl Grammar {
"LEAF for {} to {} {:?}",
add_from_node_id, add_to_node_id, reduce_vec
);
let final_node = make_final(&reduce_vec, typecode);
let final_node = make_final(&reduce_vec, typecode)?;
match self.add_branch(add_to_node_id, symbol, stype, &final_node) {
Ok(_) => {}
Err(conflict_node_id) => {
Expand All @@ -798,7 +804,7 @@ impl Grammar {
final_node.next_node_id,
conflict_node_id,
&reduce_vec,
);
)?;
}
}
} else {
Expand Down Expand Up @@ -830,10 +836,11 @@ impl Grammar {
db,
new_stored_reduces,
make_final,
);
)?;
}
}
}
Ok(())
}

// compare this with "copy_branches"!
Expand All @@ -842,10 +849,10 @@ impl Grammar {
add_from_node_id: NodeId,
add_to_node_id: NodeId,
reduce_vec: &ReduceVec,
) {
) -> Result<(), Diagnostic> {
if add_from_node_id == add_to_node_id {
// nothing to clone here!
return;
return Ok(());
}
debug!(
"Clone with reduce {} to {}",
Expand All @@ -859,8 +866,9 @@ impl Grammar {
stype,
&next_node.with_reduce_vec(reduce_vec),
)
.expect("Double conflict!");
.map_err(|_| Diagnostic::GrammarCantBuild)?; // Double conflict
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.map_err(|_| Diagnostic::GrammarCantBuild)?; // Double conflict
.map_err(|_| Diagnostic::GrammarCantBuild("Double conflict"))?;

It's a shame to lose these comments in the output. If it stores a &'static str or Cow<'static, str> then you don't have to go to the trouble of creating an enum for these messages but you can still display them

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good suggestion. I'll do that.

}
Ok(())
}

/// Expand the tree at the given node, for the given symbol. This means cloning/inserting from the root tree at that symbol, until a given typecode is obtained, into the given node
Expand All @@ -871,34 +879,35 @@ impl Grammar {
symbol: Symbol,
stype: SymbolType,
db: &Database,
) -> NodeId {
) -> Result<NodeId, Diagnostic> {
let next_node_id_from_root = self
.nodes
.get(self.root)
.next_node(symbol, stype)
.expect("Expanded formula cannot be parsed from root node!")
.ok_or(Diagnostic::GrammarCantBuild)? // Expanded formula cannot be parsed from root node
.next_node_id;
let node_from_root = self.nodes.get(next_node_id_from_root).clone();
let new_node_id = self
.add_branch_with_reduce(to_node, symbol, stype, ReduceVec::new())
.unwrap();
.map_err(|_| Diagnostic::GrammarCantBuild)?;
self.clone_branches(
next_node_id_from_root,
new_node_id,
db,
ReduceVec::new(),
|rv, t| {
node_from_root
Ok(node_from_root
.next_node(t, SymbolType::Variable)
.expect("Expanded node's typecode not available!")
.with_reduce_vec(rv)
.ok_or(Diagnostic::GrammarCantBuild)? // Expanded node's typecode is not available
.with_reduce_vec(rv))
},
);
self.nodes
)?;
Ok(self
.nodes
.get(to_node)
.next_node(symbol, stype)
.unwrap()
.next_node_id
.ok_or(Diagnostic::GrammarCantBuild)?
.next_node_id)
}

/// Handle common prefixes (garden paths).
Expand Down Expand Up @@ -926,30 +935,39 @@ impl Grammar {
}
// TODO(tirix): use https://rust-lang.github.io/rfcs/2497-if-let-chains.html once it's out!
if let GrammarNode::Branch { map } = self.nodes.get(node_id) {
let prefix_symbol = db.name_result().lookup_symbol(prefix[index]).unwrap().atom;
let prefix_symbol = db
.name_result()
.lookup_symbol(prefix[index])
.ok_or(Diagnostic::UnknownToken(index as i32))?
.atom;
let next_node = map
.get(&(SymbolType::Constant, prefix_symbol))
.expect("Prefix cannot be parsed!");
node_id = next_node.next_node_id;
index += 1;
} else {
panic!("Leaf reached while parsing common prefixes!");
return Err(Diagnostic::GrammarCantBuild); // Leaf reached while parsing common prefixes!
}
}

// We note the typecode and next branch of the "shadowed" prefix
let shadowed_typecode = names.lookup_float(shadows[index]).unwrap().typecode_atom;
let shadowed_typecode = names
.lookup_float(shadows[index])
.ok_or(Diagnostic::UnknownToken(index as i32))?
.typecode_atom;
let (shadowed_next_node, _) = self
.next_var_node(node_id, shadowed_typecode)
.expect("Shadowed prefix cannot be parsed!");
.ok_or(Diagnostic::GrammarCantBuild)?; // Shadowed prefix cannot be parsed

// We note what comes after the shadowing typecode, both if we start from the prefix and if we start from the root
let mut add_from_node_id = self.root;
let mut shadowing_atom: Atom;
let mut shadowing_stype;
let mut missing_reduce = ReduceVec::new();
for token in &prefix[index..] {
let lookup_symbol = names.lookup_symbol(token).unwrap();
let lookup_symbol = names
.lookup_symbol(token)
.ok_or(Diagnostic::UnknownToken(index as i32))?;
debug!(
"Following prefix {}, at {} / {}",
as_str(token),
Expand All @@ -961,14 +979,17 @@ impl Grammar {
SymbolType::Constant => lookup_symbol.atom,
SymbolType::Variable => {
increment_offsets(&mut missing_reduce);
names.lookup_float(token).unwrap().typecode_atom
names
.lookup_float(token)
.ok_or(Diagnostic::UnknownToken(index as i32))?
.typecode_atom
}
};
let add_to_next_node = self
.nodes
.get(node_id)
.next_node(shadowing_atom, shadowing_stype)
.expect("Prefix cannot be parsed!");
.ok_or(Diagnostic::GrammarCantBuild)?; // Prefix cannot be parsed
node_id = add_to_next_node.next_node_id;
add_from_node_id = match self
.nodes
Expand All @@ -983,7 +1004,7 @@ impl Grammar {
)?;
next_node.next_node_id
}
None => self.expand_tree(add_from_node_id, shadowing_atom, shadowing_stype, db),
None => self.expand_tree(add_from_node_id, shadowing_atom, shadowing_stype, db)?,
}
}

Expand All @@ -1001,17 +1022,19 @@ impl Grammar {

// Then we copy each of the next branch of the shadowed string to the shadowing branch
// If the next node is a leaf, instead, we add a leaf label, and point to the next
let make_final = |rv: &ReduceVec, _| NextNode {
next_node_id: shadowed_next_node,
leaf_label: *rv,
let make_final = |rv: &ReduceVec, _| {
Ok(NextNode {
next_node_id: shadowed_next_node,
leaf_label: *rv,
})
};
match self.nodes.get(add_from_node_id) {
GrammarNode::Branch { .. } => {
self.clone_branches(add_from_node_id, node_id, db, missing_reduce, make_final);
self.clone_branches(add_from_node_id, node_id, db, missing_reduce, make_final)?;
}
GrammarNode::Leaf { reduce, .. } => {
missing_reduce.push(*reduce);
self.clone_with_reduce_vec(shadowed_next_node, node_id, &missing_reduce);
self.clone_with_reduce_vec(shadowed_next_node, node_id, &missing_reduce)?;
}
}

Expand Down Expand Up @@ -1044,23 +1067,53 @@ impl Grammar {
let nset = db.name_result();
for sref in db.parse_result().segments(..) {
let buf = &**sref.buffer;
for &(ix, (_, ref command)) in &sref.j_commands {
for &(ix, (span, ref command)) in &sref.j_commands {
use CommandToken::*;
let address = StatementAddress::new(sref.id, ix);
if let (Keyword(k), rest) = command.split_first().expect("Empty parser command!") {
if let (Keyword(k), rest) = command
.split_first()
.ok_or((address, Diagnostic::BadCommand(span)))?
{
// Empty parser command
match k.as_ref(buf) {
b"syntax" => match rest {
[ty, Keyword(as_), code] if as_.as_ref(buf) == b"as" => {
// syntax '|-' as 'wff';
self.provable_type =
nset.lookup_symbol(ty.value(buf)).unwrap().atom;
self.typecodes
.push(nset.lookup_symbol(code.value(buf)).unwrap().atom);
self.provable_type = nset
.lookup_symbol(ty.value(buf))
.ok_or((
address,
Diagnostic::UndefinedToken(
ty.full_span(),
ty.value(buf).into(),
),
))?
.atom;
self.typecodes.push(
nset.lookup_symbol(code.value(buf))
.ok_or((
address,
Diagnostic::UndefinedToken(
code.full_span(),
code.value(buf).into(),
),
))?
.atom,
);
}
[ty] => {
// syntax 'setvar';
self.typecodes
.push(nset.lookup_symbol(ty.value(buf)).unwrap().atom);
self.typecodes.push(
nset.lookup_symbol(ty.value(buf))
.ok_or((
address,
Diagnostic::UndefinedToken(
ty.full_span(),
ty.value(buf).into(),
),
))?
.atom,
);
}
_ => {}
},
Expand All @@ -1069,7 +1122,7 @@ impl Grammar {
let split_index = rest
.iter()
.position(|t| matches!(t, Keyword(k) if k.as_ref(buf) == b"=>"))
.expect("'=>' not present in 'garden_path' command!");
.ok_or((address, Diagnostic::BadCommand(*k)))?; // '=>' not present in 'garden_path' command
let (prefix, shadows) = rest.split_at(split_index);
let prefix =
prefix.iter().map(|tk| tk.value(buf)).collect::<Box<[_]>>();
Expand Down Expand Up @@ -1145,7 +1198,7 @@ impl Grammar {

let mut formula_builder = FormulaBuilder::default();
let mut symbol_enum = symbol_iter.enumerate().peekable();
let mut ix;
let mut ix = 0;
let mut e = StackElement {
node_id: self.root,
expected_typecodes: expected_typecodes.to_vec().into_boxed_slice(),
Expand Down Expand Up @@ -1189,8 +1242,9 @@ impl Grammar {
return Ok(formula_builder.build(typecode));
} else {
// There are still symbols to parse, continue from root
let (next_node_id, leaf_label) =
self.next_var_node(self.root, typecode).unwrap(); // TODO(tirix): error case
let (next_node_id, leaf_label) = self
.next_var_node(self.root, typecode)
.ok_or(Diagnostic::UnparseableStatement(ix))?;
for &reduce in leaf_label {
Self::do_reduce(&mut formula_builder, reduce, nset);
}
Expand All @@ -1211,8 +1265,9 @@ impl Grammar {
}
// We have not found the expected typecode, continue from root
debug!(" ++ Wrong type obtained, continue.");
let (next_node_id, leaf_label) =
self.next_var_node(self.root, typecode).unwrap(); // TODO(tirix): error case
let (next_node_id, leaf_label) = self
.next_var_node(self.root, typecode)
.ok_or(Diagnostic::UnparseableStatement(ix))?;
for &reduce in leaf_label {
Self::do_reduce(&mut formula_builder, reduce, nset);
}
Expand Down Expand Up @@ -1516,9 +1571,11 @@ impl StmtParse {
for sps in self.segments.values() {
for (&sa, formula) in &sps.formulas {
let sref = sset.statement(sa);
let math_iter = sref
.math_iter()
.map(|token| nset.lookup_symbol(token.slice).unwrap().atom);
let math_iter = sref.math_iter().flat_map(|token| {
nset.lookup_symbol(token.slice)
.ok_or_else(|| (sref.address(), Diagnostic::UnknownToken(token.index())))
.map(|l| l.atom)
});
let fmla_iter = formula.as_ref(db).iter();
if math_iter.ne(fmla_iter) {
return Err((sa, Diagnostic::FormulaVerificationFailed));
Expand Down