Skip to content

First take at removing HeaderError from YaccParser internals #554

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

Merged
ltratt merged 3 commits into from
Apr 20, 2025
Merged

First take at removing HeaderError from YaccParser internals #554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
277ae03
Remove `Header` as parameters to `YaccParser` and others.
ratmice Apr 17, 2025
0f15c0e
Make `HeaderError` convert into `YaccGrammarError`
ratmice Apr 19, 2025
e5e3b65
Hide the entire header and markmap modules.
ratmice Apr 18, 2025
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
172 changes: 120 additions & 52 deletions cfgrammar/src/lib/header.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::{
markmap::{Entry, MarkMap},
yacc::{ParserError, YaccKind, YaccOriginalActionKind},
yacc::{
parser::SpansKind, YaccGrammarError, YaccGrammarErrorKind, YaccKind, YaccOriginalActionKind,
},
Location, Span,
};
use lazy_static::lazy_static;
Expand All @@ -14,21 +16,37 @@ use std::{error::Error, fmt};
/// * An error during parsing the section.
/// * An error resulting from a value in the section having an invalid value.
#[derive(Debug, Clone)]
pub struct HeaderError {
#[doc(hidden)]
pub struct HeaderError<T> {
pub kind: HeaderErrorKind,
pub locations: Vec<Location>,
pub locations: Vec<T>,
}

impl Error for HeaderError {}
impl fmt::Display for HeaderError {
impl<T: fmt::Debug> Error for HeaderError<T> {}
impl<T> fmt::Display for HeaderError<T> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{}", self.kind)
}
}

impl From<HeaderError> for ParserError {
fn from(e: HeaderError) -> ParserError {
ParserError::HeaderError(e)
impl From<HeaderError<Span>> for YaccGrammarError {
fn from(e: HeaderError<Span>) -> YaccGrammarError {
YaccGrammarError {
kind: YaccGrammarErrorKind::Header(e.kind, e.spanskind()),
spans: e.locations,
}
}
}

// This is essentially a tuple that needs a newtype so we can implement `From` for it.
// Thus we aren't worried about it being `pub`.
#[derive(Debug, PartialEq)]
#[doc(hidden)]
pub struct HeaderValue<T>(pub T, pub Value<T>);
Copy link
Copy Markdown
Collaborator Author

@ratmice ratmice Apr 18, 2025

Choose a reason for hiding this comment

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

Likely needs #[doc(hidden)], and I should just go through the docs again, to ensure there isn't anything else I added that also needs it.

Copy link
Copy Markdown
Collaborator Author

@ratmice ratmice Apr 19, 2025

Choose a reason for hiding this comment

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

I believe all the doc changes should be fixed across various commits,
this one in 7895675


impl From<HeaderValue<Span>> for HeaderValue<Location> {
fn from(hv: HeaderValue<Span>) -> HeaderValue<Location> {
HeaderValue(hv.0.into(), hv.1.into())
}
}

Expand Down Expand Up @@ -60,6 +78,16 @@ impl fmt::Display for HeaderErrorKind {
}
}

impl<T> HeaderError<T> {
/// Returns the [SpansKind] associated with this error.
pub fn spanskind(&self) -> SpansKind {
match self.kind {
HeaderErrorKind::DuplicateEntry => SpansKind::DuplicationError,
_ => SpansKind::Error,
}
}
}

/// Indicates a value prefixed by an optional namespace.
/// `Foo::Bar` with optional `Foo` specified being
/// ```rust,ignore
Expand All @@ -78,24 +106,24 @@ impl fmt::Display for HeaderErrorKind {
/// ```
#[derive(Debug, Eq, PartialEq)]
#[doc(hidden)]
pub struct Namespaced {
pub namespace: Option<(String, Location)>,
pub member: (String, Location),
pub struct Namespaced<T> {
pub namespace: Option<(String, T)>,
pub member: (String, T),
}

#[derive(Debug, Eq, PartialEq)]
#[doc(hidden)]
pub enum Setting {
pub enum Setting<T> {
/// A value like `YaccKind::Grmtools`
Unitary(Namespaced),
Unitary(Namespaced<T>),
/// A value like `YaccKind::Original(UserActions)`.
/// In that example the field ctor would be: `Namespaced { namespace: "YaccKind", member: "Original" }`.
/// The field would be `Namespaced{ None, UserActions }`.
Constructor {
ctor: Namespaced,
arg: Namespaced,
ctor: Namespaced<T>,
arg: Namespaced<T>,
},
Num(u64, Location),
Num(u64, T),
}

/// Parser for the `%grmtools` section
Expand All @@ -111,9 +139,48 @@ pub struct GrmtoolsSectionParser<'input> {
/// like booleans, numeric types, and string values.
#[derive(Debug, Eq, PartialEq)]
#[doc(hidden)]
pub enum Value {
Flag(bool, Location),
Setting(Setting),
pub enum Value<T> {
Flag(bool, T),
Setting(Setting<T>),
}

impl From<Value<Span>> for Value<Location> {
fn from(v: Value<Span>) -> Value<Location> {
match v {
Value::Flag(flag, u) => Value::Flag(flag, u.into()),
Value::Setting(s) => Value::Setting(match s {
Setting::Unitary(Namespaced {
namespace,
member: (m, ml),
}) => Setting::Unitary(Namespaced {
namespace: namespace.map(|(n, nl)| (n, nl.into())),
member: (m, ml.into()),
}),
Setting::Constructor {
ctor:
Namespaced {
namespace: ctor_ns,
member: (ctor_m, ctor_ml),
},
arg:
Namespaced {
namespace: arg_ns,
member: (arg_m, arg_ml),
},
} => Setting::Constructor {
ctor: Namespaced {
namespace: ctor_ns.map(|(ns, ns_l)| (ns, ns_l.into())),
member: (ctor_m, ctor_ml.into()),
},
arg: Namespaced {
namespace: arg_ns.map(|(ns, ns_l)| (ns, ns_l.into())),
member: (arg_m, arg_ml.into()),
},
},
Setting::Num(num, num_loc) => Setting::Num(num, num_loc.into()),
}),
}
}
}

lazy_static! {
Expand All @@ -127,11 +194,11 @@ lazy_static! {

const MAGIC: &str = "%grmtools";

fn add_duplicate_occurrence(
errs: &mut Vec<HeaderError>,
fn add_duplicate_occurrence<T: Eq + PartialEq + Clone>(
errs: &mut Vec<HeaderError<T>>,
kind: HeaderErrorKind,
orig_loc: Location,
dup_loc: Location,
orig_loc: T,
dup_loc: T,
) {
if !errs.iter_mut().any(|e| {
if e.kind == kind && e.locations[0] == orig_loc {
Expand All @@ -152,18 +219,18 @@ impl<'input> GrmtoolsSectionParser<'input> {
pub fn parse_value(
&'_ self,
mut i: usize,
) -> Result<(String, Location, Value, usize), HeaderError> {
) -> Result<(String, Span, Value<Span>, usize), HeaderError<Span>> {
if let Some(j) = self.lookahead_is("!", i) {
let (flag_name, k) = self.parse_name(j)?;
Ok((
flag_name,
Location::Span(Span::new(j, k)),
Value::Flag(false, Location::Span(Span::new(i, k))),
Span::new(j, k),
Value::Flag(false, Span::new(i, k)),
self.parse_ws(k),
))
} else {
let (key_name, j) = self.parse_name(i)?;
let key_span = Location::Span(Span::new(i, j));
let key_span = Span::new(i, j);
i = self.parse_ws(j);
if let Some(j) = self.lookahead_is(":", i) {
i = self.parse_ws(j);
Expand All @@ -173,7 +240,7 @@ impl<'input> GrmtoolsSectionParser<'input> {
let num_str = &self.src[num_span.start()..num_span.end()];
// If the above regex matches we expect this to succeed.
let num = str::parse::<u64>(num_str).unwrap();
let val = Setting::Num(num, Location::Span(num_span));
let val = Setting::Num(num, num_span);
i = self.parse_ws(num_span.end());
Ok((key_name, key_span, Value::Setting(val), i))
}
Expand All @@ -197,7 +264,7 @@ impl<'input> GrmtoolsSectionParser<'input> {
} else {
Err(HeaderError {
kind: HeaderErrorKind::ExpectedToken(')'),
locations: vec![Location::Span(Span::new(i, i))],
locations: vec![Span::new(i, i)],
})
}
} else {
Expand All @@ -211,20 +278,23 @@ impl<'input> GrmtoolsSectionParser<'input> {
}
}
} else {
Ok((key_name, key_span.clone(), Value::Flag(true, key_span), i))
Ok((key_name, key_span, Value::Flag(true, key_span), i))
}
}
}

fn parse_namespaced(&self, mut i: usize) -> Result<(Namespaced, usize), HeaderError> {
fn parse_namespaced(
&self,
mut i: usize,
) -> Result<(Namespaced<Span>, usize), HeaderError<Span>> {
// Either a name alone, or a namespace which will be followed by a member.
let (name, j) = self.parse_name(i)?;
let name_span = Location::Span(Span::new(i, j));
let name_span = Span::new(i, j);
i = self.parse_ws(j);
if let Some(j) = self.lookahead_is("::", i) {
i = self.parse_ws(j);
let (member_val, j) = self.parse_name(i)?;
let member_val_span = Location::Span(Span::new(i, j));
let member_val_span = Span::new(i, j);
i = self.parse_ws(j);
Ok((
Namespaced {
Expand Down Expand Up @@ -258,7 +328,7 @@ impl<'input> GrmtoolsSectionParser<'input> {
}

#[allow(clippy::type_complexity)]
pub fn parse(&'_ self) -> Result<(Header, usize), Vec<HeaderError>> {
pub fn parse(&'_ self) -> Result<(Header<Span>, usize), Vec<HeaderError<Span>>> {
let mut errs = Vec::new();
if let Some(mut i) = self.lookahead_is(MAGIC, self.parse_ws(0)) {
let mut ret = Header::new();
Expand All @@ -276,16 +346,16 @@ impl<'input> GrmtoolsSectionParser<'input> {
};
match ret.entry(key) {
Entry::Occupied(orig) => {
let (orig_loc, _): &(Location, Value) = orig.get();
let HeaderValue(orig_loc, _): &HeaderValue<Span> = orig.get();
add_duplicate_occurrence(
&mut errs,
HeaderErrorKind::DuplicateEntry,
orig_loc.clone(),
*orig_loc,
key_loc,
)
}
Entry::Vacant(entry) => {
entry.insert((key_loc, val));
entry.insert(HeaderValue(key_loc, val));
}
}
if let Some(j) = self.lookahead_is(",", j) {
Expand All @@ -305,32 +375,29 @@ impl<'input> GrmtoolsSectionParser<'input> {
} else {
errs.push(HeaderError {
kind: HeaderErrorKind::ExpectedToken('}'),
locations: vec![Location::Span(Span::new(
section_start_pos,
self.src.len(),
))],
locations: vec![Span::new(section_start_pos, self.src.len())],
});
Err(errs)
}
} else {
errs.push(HeaderError {
kind: HeaderErrorKind::ExpectedToken('{'),
locations: vec![Location::Span(Span::new(i, i))],
locations: vec![Span::new(i, i)],
});
Err(errs)
}
} else if self.required {
errs.push(HeaderError {
kind: HeaderErrorKind::MissingGrmtoolsSection,
locations: vec![Location::Span(Span::new(0, 0))],
locations: vec![Span::new(0, 0)],
});
Err(errs)
} else {
Ok((Header::new(), 0))
}
}

fn parse_name(&self, i: usize) -> Result<(String, usize), HeaderError> {
fn parse_name(&self, i: usize) -> Result<(String, usize), HeaderError<Span>> {
match RE_NAME.find(&self.src[i..]) {
Some(m) => {
assert_eq!(m.start(), 0);
Expand All @@ -341,7 +408,7 @@ impl<'input> GrmtoolsSectionParser<'input> {
}
None => Err(HeaderError {
kind: HeaderErrorKind::IllegalName,
locations: vec![Location::Span(Span::new(i, i))],
locations: vec![Span::new(i, i)],
}),
}
}
Expand All @@ -363,11 +430,12 @@ impl<'input> GrmtoolsSectionParser<'input> {
}

/// A data structure representation of the %grmtools section.
pub type Header = MarkMap<String, (Location, Value)>;
#[doc(hidden)]
pub type Header<T> = MarkMap<String, HeaderValue<T>>;

impl TryFrom<YaccKind> for Value {
type Error = HeaderError;
fn try_from(kind: YaccKind) -> Result<Value, HeaderError> {
impl TryFrom<YaccKind> for Value<Location> {
type Error = HeaderError<Location>;
fn try_from(kind: YaccKind) -> Result<Value<Location>, HeaderError<Location>> {
let from_loc = Location::Other("From<YaccKind>".to_string());
Ok(match kind {
YaccKind::Grmtools => Value::Setting(Setting::Unitary(Namespaced {
Expand Down Expand Up @@ -402,9 +470,9 @@ impl TryFrom<YaccKind> for Value {
}
}

impl TryFrom<&Value> for YaccKind {
type Error = HeaderError;
fn try_from(value: &Value) -> Result<YaccKind, HeaderError> {
impl<T: Clone> TryFrom<&Value<T>> for YaccKind {
type Error = HeaderError<T>;
fn try_from(value: &Value<T>) -> Result<YaccKind, HeaderError<T>> {
let mut err_locs = Vec::new();
match value {
Value::Flag(_, loc) => Err(HeaderError {
Expand Down
Loading