-
-
Notifications
You must be signed in to change notification settings - Fork 11
Implement optional modifiers #105
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.
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
base: main
Are you sure you want to change the base?
Changes from 6 commits
d8b5bed
efff982
048790a
779af55
0d6127f
3dfecd3
3514300
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -107,7 +107,7 @@ impl Symbol { | |||||||||||||||||||||||||||
/// Possible modifiers for this symbol. | ||||||||||||||||||||||||||||
pub fn modifiers(&self) -> impl Iterator<Item = &str> + '_ { | ||||||||||||||||||||||||||||
self.variants() | ||||||||||||||||||||||||||||
.flat_map(|(m, _, _)| m.into_iter()) | ||||||||||||||||||||||||||||
.flat_map(|(m, _, _)| m.into_iter().map(|m| m.name())) | ||||||||||||||||||||||||||||
.collect::<std::collections::BTreeSet<_>>() | ||||||||||||||||||||||||||||
.into_iter() | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
@@ -170,14 +170,17 @@ mod test { | |||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||
let variants = s | ||||||||||||||||||||||||||||
.variants() | ||||||||||||||||||||||||||||
.map(|(m, v, _)| (m.into_iter().collect::<BTreeSet<_>>(), v)) | ||||||||||||||||||||||||||||
.map(|(m, v, _)| { | ||||||||||||||||||||||||||||
(m.into_iter().map(|m| m.as_str()).collect::<BTreeSet<_>>(), v) | ||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||
.collect::<BTreeSet<_>>(); | ||||||||||||||||||||||||||||
let control = control | ||||||||||||||||||||||||||||
.iter() | ||||||||||||||||||||||||||||
.map(|&(m, v)| { | ||||||||||||||||||||||||||||
( | ||||||||||||||||||||||||||||
ModifierSet::from_raw_dotted(m) | ||||||||||||||||||||||||||||
.into_iter() | ||||||||||||||||||||||||||||
.map(|m| m.as_str()) | ||||||||||||||||||||||||||||
.collect::<BTreeSet<_>>(), | ||||||||||||||||||||||||||||
v, | ||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||
|
@@ -187,4 +190,89 @@ mod test { | |||||||||||||||||||||||||||
assert_eq!(variants, control); | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
#[test] | ||||||||||||||||||||||||||||
fn no_overlap() { | ||||||||||||||||||||||||||||
recur("", ROOT); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
/// Iterate over all symbols in a module, recursing into submodules. | ||||||||||||||||||||||||||||
fn recur(prefix: &str, m: Module) { | ||||||||||||||||||||||||||||
for (name, b) in m.iter() { | ||||||||||||||||||||||||||||
match b.def { | ||||||||||||||||||||||||||||
Def::Module(m) => { | ||||||||||||||||||||||||||||
let new_prefix = if prefix.is_empty() { | ||||||||||||||||||||||||||||
name.to_string() | ||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||
prefix.to_string() + "." + name | ||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||
recur(&new_prefix, m); | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
Def::Symbol(s) => check_symbol(prefix, name, s), | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
/// Check the no overlap rule for a single symbol | ||||||||||||||||||||||||||||
fn check_symbol(prefix: &str, name: &str, sym: Symbol) { | ||||||||||||||||||||||||||||
// maximum number of modifiers per variant (we don't need to check more than this). | ||||||||||||||||||||||||||||
let max_modifs = | ||||||||||||||||||||||||||||
sym.variants().map(|(m, ..)| m.iter().count()).max().unwrap(); | ||||||||||||||||||||||||||||
let modifs = sym.modifiers().collect::<Vec<_>>(); | ||||||||||||||||||||||||||||
let max_index = modifs.len().saturating_sub(1); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
for k in 0..=max_modifs { | ||||||||||||||||||||||||||||
let mut indices = (0..k).collect::<Vec<_>>(); | ||||||||||||||||||||||||||||
loop { | ||||||||||||||||||||||||||||
let mset = indices.iter().map(|i| modifs[*i]).fold( | ||||||||||||||||||||||||||||
ModifierSet::<String>::default(), | ||||||||||||||||||||||||||||
|mut res, m| { | ||||||||||||||||||||||||||||
res.insert_raw(m); | ||||||||||||||||||||||||||||
res | ||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||
Comment on lines
+226
to
+232
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should have an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I really don't like that, see #46 (comment). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh indeed you are right! I had forgotten I already suggested a similar thing before. I think having |
||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if sym.variants().filter(|(m, ..)| mset.is_candidate(*m)).count() > 1 | ||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||
panic!( | ||||||||||||||||||||||||||||
"Overlap in symbol {prefix}.{name} for modifiers {}", | ||||||||||||||||||||||||||||
mset.as_str() | ||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if next_subseq(&mut indices, max_index).is_none() { | ||||||||||||||||||||||||||||
break; | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
/// Produce the next term in a sequence like | ||||||||||||||||||||||||||||
/// ```text | ||||||||||||||||||||||||||||
/// [0,1,2], [0,1,3], [0,1,4], [0,2,3], [0,2,4], [0,3,4], [1,2,3], [1,2,4], [1,3,4], [2,3,4] | ||||||||||||||||||||||||||||
/// ``` | ||||||||||||||||||||||||||||
T0mstone marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||
fn next_subseq(indices: &mut [usize], max_index: usize) -> Option<()> { | ||||||||||||||||||||||||||||
// invariant: indices.len() <= max_index + 1 | ||||||||||||||||||||||||||||
T0mstone marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||
match indices { | ||||||||||||||||||||||||||||
[] => None, | ||||||||||||||||||||||||||||
[single] => { | ||||||||||||||||||||||||||||
if *single < max_index { | ||||||||||||||||||||||||||||
*single += 1; | ||||||||||||||||||||||||||||
Some(()) | ||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||
None | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
[left @ .., last] => { | ||||||||||||||||||||||||||||
assert_ne!(max_index, 0); | ||||||||||||||||||||||||||||
if *last < max_index { | ||||||||||||||||||||||||||||
*last += 1; | ||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||
next_subseq(left, max_index - 1)?; | ||||||||||||||||||||||||||||
*last = left.last().copied().map_or(*last, |x| x + 1); | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
Some(()) | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This seems more idiomatic to me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I disagree. |
||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} |
Uh oh!
There was an error while loading. Please reload this page.