Proper fix to #42 (#45)

logos-derive/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "logos-derive"
-version = "0.9.0"
+version = "0.9.1"
 authors = ["maciejhirsz <maciej.hirsz@gmail.com>"]
 license = "MIT/Apache-2.0"
 description = "Create ridiculously fast Lexers"

logos-derive/src/regex.rs

@@ -459,8 +459,11 @@ impl Pattern {
         }
     }
 
-    // FIXME: this can be more robust
     pub fn intersect(&self, other: &Pattern) -> Option<Pattern> {
+        if self == other {
+            return None;
+        }
+
         if self.contains(other) {
             Some(other.clone())
         } else if other.contains(self) {
@@ -471,20 +474,24 @@ impl Pattern {
     }
 
     pub fn contains(&self, other: &Pattern) -> bool {
-        use self::Pattern::*;
-
-        if let Byte(x) = other {
-            match self {
-                Byte(_) => false,
-                Range(a, b) => {
-                    *a <= *x && *x <= *b
-                },
-                Class(class) => {
-                    class.iter().any(|pat| pat.contains(other))
-                },
-            }
-        } else {
-            false
+        match other {
+            Pattern::Byte(x) => self.contains_range(*x, *x),
+            Pattern::Range(a, b) => self.contains_range(*a, *b),
+            Pattern::Class(class) => {
+                class.iter().all(|pat| self.contains(pat))
+            },
+        }
+    }
+
+    fn contains_range(&self, xa: u8, xb: u8) -> bool {
+        match self {
+            Pattern::Byte(a) => *a == xa && *a == xb,
+            Pattern::Range(a, b) => {
+                (*a <= xa && xa <= *b) && (*a <= xb && xb <= *b)
+            },
+            Pattern::Class(class) => {
+                class.iter().any(|pat| pat.contains_range(xa, xb))
+            },
         }
     }
 

logos-derive/src/tree.rs

@@ -593,6 +593,8 @@ impl<'a> Node<'a> {
         }
     }
 
+    /// Checks if the fork contains one branch that is a generalization of all other branches,
+    /// and if so removes and returns that branch.
     pub fn fallback(&mut self) -> Option<Branch<'a>> {
         match self {
             Node::Fork(fork) => {
@@ -603,23 +605,35 @@ impl<'a> Node<'a> {
                 // This is a bit weird, but it basically checks if the fork
                 // has one and only one branch that is heavy and if so, it
                 // removes that branch and returns it.
-                //
-                // FIXME: This should check if all other branches in the tree
-                //        are specializations of that regex
                 let mut index = None;
+                let mut len = 1;
 
                 for (idx, branch) in fork.arms.iter().enumerate() {
-                    if branch.regex.first().weight() > 1 {
-                        // Make sure we only get one
-                        if index.is_some() {
-                            return None;
-                        }
+                    let other_len = branch.regex.first().len();
 
+                    if other_len > len {
                         index = Some(idx);
+                        len = other_len;
                     }
                 }
 
-                index.map(|idx| fork.arms.remove(idx))
+                let index = index?;
+                let selected = &fork.arms[index];
+
+                // FIXME: Specialization check should be deeper than first Pattern
+                let specialization = fork.arms.iter().enumerate().all(|(idx, branch)| {
+                    if idx == index {
+                        return true;
+                    }
+
+                    selected.regex.first().contains(branch.regex.first())
+                });
+
+                if specialization {
+                    Some(fork.arms.remove(index))
+                } else {
+                    None
+                }
             }
             _ => None,
         }

tests/Cargo.toml

@@ -9,4 +9,4 @@ edition = "2018"
 [dependencies]
 logos = { path = "../logos", default-features = false, features = ["nul_term_source"] }
 logos-derive = { path = "../logos-derive" }
-toolshed = "0.6"
+toolshed = "0.8"

tests/tests/advanced.rs

@@ -28,7 +28,7 @@ enum Token {
     #[regex="~[a-z][a-z]+"]
     LiteralUrbitAddress,
 
-    #[regex="~(m|h|s)[0-9]+"]
+    #[regex="~[mhs][0-9]+"]
     LiteralRelDate,
 
     #[regex = "🦀+"]