Add recursive define detection

CHANGELOG.md

@@ -2,18 +2,20 @@
 
 ## [Unreleased](https://github.com/dalance/sv-parser/compare/v0.4.6...Unreleased) - ReleaseDate
 
+* [Added] recursive define detection
+
 ## [v0.4.6](https://github.com/dalance/sv-parser/compare/v0.4.5...v0.4.6) - 2019-12-02
 
-* [Fix] constant_bit_select
+* [Fixed] constant_bit_select
-* [Fix] wrong linebreak at define macro usage
+* [Fixed] wrong linebreak at define macro usage
 
 ## [v0.4.5](https://github.com/dalance/sv-parser/compare/v0.4.4...v0.4.5) - 2019-11-28
 
-* [Fix] wrong space at define macro usage
+* [Fixed] wrong space at define macro usage
 
 ## [v0.4.4](https://github.com/dalance/sv-parser/compare/v0.4.3...v0.4.4) - 2019-11-22
 
-* [Fix] \`resetall wrongly clear define list
+* [Fixed] \`resetall wrongly clear define list
 
 ## [v0.4.3](https://github.com/dalance/sv-parser/compare/v0.4.2...v0.4.3) - 2019-11-15
 

sv-parser-error/src/lib.rs

@@ -24,6 +24,8 @@ pub enum ErrorKind {
     DefineNotFound(String),
     #[fail(display = "Define must have argument")]
     DefineNoArgs,
+    #[fail(display = "Recursive define is detected: {}", _0)]
+    DefineRecursive(String),
 }
 
 // -----------------------------------------------------------------------------

sv-parser-pp/src/preprocess.rs

@@ -124,7 +124,7 @@ pub fn preprocess<T: AsRef<Path>, U: AsRef<Path>>(
     let mut s = String::new();
     reader.read_to_string(&mut s)?;
 
-    preprocess_str(&s, path, pre_defines, include_paths)
+    preprocess_str(&s, path, pre_defines, include_paths, vec![])
 }
 
 pub fn preprocess_str<T: AsRef<Path>, U: AsRef<Path>>(
@@ -132,6 +132,7 @@ pub fn preprocess_str<T: AsRef<Path>, U: AsRef<Path>>(
     path: T,
     pre_defines: &Defines,
     include_paths: &[U],
+    resolved_defines: Vec<String>,
 ) -> Result<(PreprocessedText, Defines), Error> {
     let mut skip = false;
     let mut skip_nodes = vec![];
@@ -328,9 +329,14 @@ pub fn preprocess_str<T: AsRef<Path>, U: AsRef<Path>>(
                     IncludeCompilerDirective::TextMacroUsage(x) => {
                         let (_, _, ref x) = x.nodes;
                         skip_nodes.push(x.into());
-                        if let Some((p, _, _)) =
+                        if let Some((p, _, _)) = resolve_text_macro_usage(
-                            resolve_text_macro_usage(x, s, path.as_ref(), &defines, include_paths)?
+                            x,
-                        {
+                            s,
+                            path.as_ref(),
+                            &defines,
+                            include_paths,
+                            resolved_defines.clone(),
+                        )? {
                             let p = p.trim().trim_matches('"');
                             PathBuf::from(p)
                         } else {
@@ -355,9 +361,14 @@ pub fn preprocess_str<T: AsRef<Path>, U: AsRef<Path>>(
                 ret.merge(include);
             }
             NodeEvent::Enter(RefNode::TextMacroUsage(x)) if !skip => {
-                if let Some((text, origin, new_defines)) =
+                if let Some((text, origin, new_defines)) = resolve_text_macro_usage(
-                    resolve_text_macro_usage(x, s, path.as_ref(), &defines, include_paths)?
+                    x,
-                {
+                    s,
+                    path.as_ref(),
+                    &defines,
+                    include_paths,
+                    resolved_defines.clone(),
+                )? {
                     ret.push(&text, origin);
                     defines = new_defines;
                 }
@@ -433,10 +444,16 @@ fn resolve_text_macro_usage<T: AsRef<Path>, U: AsRef<Path>>(
     path: T,
     defines: &Defines,
     include_paths: &[U],
+    mut resolved_defines: Vec<String>,
 ) -> Result<Option<(String, Option<(PathBuf, Range)>, Defines)>, Error> {
     let (_, ref name, ref args) = x.nodes;
     let id = identifier((&name.nodes.0).into(), &s).unwrap();
 
+    if resolved_defines.contains(&id) {
+        return Err(ErrorKind::DefineRecursive(id).into());
+    }
+    resolved_defines.push(id.clone());
+
     let mut actual_args = Vec::new();
     let no_args = args.is_none();
     if let Some(args) = args {
@@ -504,8 +521,13 @@ fn resolve_text_macro_usage<T: AsRef<Path>, U: AsRef<Path>>(
             // remove leading whitespace
             replaced = String::from(replaced.trim_start());
 
-            let (replaced, new_defines) =
+            let (replaced, new_defines) = preprocess_str(
-                preprocess_str(&replaced, path.as_ref(), &defines, include_paths)?;
+                &replaced,
+                path.as_ref(),
+                &defines,
+                include_paths,
+                resolved_defines,
+            )?;
             Ok(Some((
                 String::from(replaced.text()),
                 text.origin.clone(),
@@ -681,4 +703,22 @@ endmodule
 "##
         );
     }
+
+    #[test]
+    fn test7() {
+        let ret = preprocess(get_testcase("test7.sv"), &HashMap::new(), &[] as &[String]);
+        assert_eq!(
+            format!("{:?}", ret),
+            "Err(Error { inner: \n\nRecursive define is detected: a })"
+        );
+    }
+
+    #[test]
+    fn test8() {
+        let ret = preprocess(get_testcase("test8.sv"), &HashMap::new(), &[] as &[String]);
+        assert_eq!(
+            format!("{:?}", ret),
+            "Err(Error { inner: \n\nRecursive define is detected: b })"
+        );
+    }
 }

sv-parser-pp/testcases/test7.sv

@@ -0,0 +1,3 @@
+`define a `a
+// direct recursion
+`a

sv-parser-pp/testcases/test8.sv

@@ -0,0 +1,6 @@
+`define b `c
+`define c `d
+`define d `e
+`define e `b
+// indirect recursion
+`b

sv-parser/src/lib.rs

@@ -130,7 +130,7 @@ pub fn parse_sv_str<T: AsRef<Path>, U: AsRef<Path>>(
     pre_defines: &HashMap<String, Option<Define>>,
     include_paths: &[U],
 ) -> Result<(SyntaxTree, Defines), Error> {
-    let (text, defines) = preprocess_str(s, path, pre_defines, include_paths)?;
+    let (text, defines) = preprocess_str(s, path, pre_defines, include_paths, vec![])?;
     let span = Span::new_extra(text.text(), SpanInfo::default());
     let result = all_consuming(sv_parser)(span);
     match result {
@@ -203,7 +203,7 @@ pub fn parse_lib_str<T: AsRef<Path>, U: AsRef<Path>>(
     pre_defines: &HashMap<String, Option<Define>>,
     include_paths: &[U],
 ) -> Result<(SyntaxTree, Defines), Error> {
-    let (text, defines) = preprocess_str(s, path, pre_defines, include_paths)?;
+    let (text, defines) = preprocess_str(s, path, pre_defines, include_paths, vec![])?;
     let span = Span::new_extra(text.text(), SpanInfo::default());
     let result = all_consuming(lib_parser)(span);
     match result {