Merge pull request #69 from DaveMcEwan/ppErrorPreprocess

Use `Error::Preprocess` on pp parsing error, instead of `Error::Parse`.

sv-parser-error/src/lib.rs

@@ -7,29 +7,38 @@ use thiserror::Error;
 pub enum Error {
     #[error("IO error: {0}")]
     Io(#[from] std::io::Error),
+
     #[error("File error: {path:?}")]
     File {
         #[source]
         source: std::io::Error,
         path: PathBuf,
     },
+
     #[error("Include error")]
     Include {
         #[from]
         source: Box<Error>,
     },
+
     #[error("Parse error: {0:?}")]
     Parse(Option<(PathBuf, usize)>),
-    #[error("Preprocess error")]
-    Preprocess,
+
+    #[error("Preprocess error: {0:?}")]
+    Preprocess(Option<(PathBuf, usize)>),
+
     #[error("Define argument not found: {0}")]
     DefineArgNotFound(String),
+
     #[error("Define not found: {0}")]
     DefineNotFound(String),
+
     #[error("Define must have argument")]
     DefineNoArgs,
+
     #[error("Exceed recursive limit")]
     ExceedRecursiveLimit,
+
     #[error("Include line can't have other items")]
     IncludeLine,
 }

sv-parser-pp/src/preprocess.rs

@@ -255,19 +255,19 @@ pub fn preprocess_str<T: AsRef<Path>, U: AsRef<Path>, V: BuildHasher>(
 
     let span = Span::new_extra(&s, SpanInfo::default());
     let (_, pp_text) = all_consuming(pp_parser)(span).map_err(|x| match x {
-        nom::Err::Incomplete(_) => Error::Parse(None),
+        nom::Err::Incomplete(_) => Error::Preprocess(None),
         nom::Err::Error(e) => {
             if let Some(pos) = error_position(&e) {
-                Error::Parse(Some((PathBuf::from(path.as_ref()), pos)))
+                Error::Preprocess(Some((PathBuf::from(path.as_ref()), pos)))
             } else {
-                Error::Parse(None)
+                Error::Preprocess(None)
             }
         }
         nom::Err::Failure(e) => {
             if let Some(pos) = error_position(&e) {
-                Error::Parse(Some((PathBuf::from(path.as_ref()), pos)))
+                Error::Preprocess(Some((PathBuf::from(path.as_ref()), pos)))
             } else {
-                Error::Parse(None)
+                Error::Preprocess(None)
             }
         }
     })?;
@@ -1447,6 +1447,16 @@ mod tests {
         );
     } // }}}
 
+    #[test]
+    #[ignore = "Exposes unfixed PP parser bug."]
+    fn macro_comment_embedded() { // {{{
+        let (ret, _) = preprocess_usualargs("macro_comment_embedded.sv").unwrap();
+        assert_eq!(
+            ret.text(),
+            testfile_contents("expected/macro_comment_embedded.sv")
+        );
+    } // }}}
+
     #[test]
     fn macro_delimiters() { // {{{
         let (ret, _) = preprocess_usualargs("macro_delimiters.sv").unwrap();

sv-parser-pp/testcases/expected/macro_comment_embedded.sv

@@ -0,0 +1,54 @@
+
+// Unusual case where a preprocessor comment (same symbol as a single-line
+// comment, "//") is embedded in an emitted C-style comment.
+// This should not preprocess, but not parse, i.e. in the emitted text, there
+// will be an opening // C-style comment symbol "/*" but no closing symbol "*/".
+`define A \
+  A1 \
+  A2 /* emitted */ \
+  A3 /* // not emitted, unclosed C comment */ \
+  A4
+
+// Same as A, but without space before "//".
+// This may catch bad parsers where the first "/" in "//" is treated as part of
+// the closing "*/".
+`define B \
+  B1 \
+  B2 /* emitted */ \
+  B3 /*// not emitted, unclosed C comment */ \
+  B4
+
+// Another variation on B.
+`define C \
+  C1 \
+  C2 /* emitted */ \
+  C3 //* not emitted, not a C comment */ \
+  C4
+
+// Another variation on B.
+`define D \
+  D1 \
+  D2 /* emitted */ \
+  D3 /* emitted, unclosed C comment *// \
+  D4
+
+  A1 
+  A2 /* emitted */ 
+  A3 /* 
+  A4
+
+  B1 
+  B2 /* emitted */ 
+  B3 /*
+  B4
+
+  C1 
+  C2 /* emitted */ 
+  C3 //
+  C4
+
+  D1 
+  D2 /* emitted */ 
+  D3 /* emitted, unclosed C comment *
+  D4
+

sv-parser-pp/testcases/macro_comment_embedded.sv

@@ -0,0 +1,42 @@
+
+// Unusual case where a preprocessor comment (same symbol as a single-line
+// comment, "//") is embedded in an emitted C-style comment.
+// This should not preprocess, but not parse, i.e. in the emitted text, there
+// will be an opening // C-style comment symbol "/*" but no closing symbol "*/".
+`define A \
+  A1 \
+  A2 /* emitted */ \
+  A3 /* // not emitted, unclosed C comment */ \
+  A4
+
+// Same as A, but without space before "//".
+// This may catch bad parsers where the first "/" in "//" is treated as part of
+// the closing "*/".
+`define B \
+  B1 \
+  B2 /* emitted */ \
+  B3 /*// not emitted, unclosed C comment */ \
+  B4
+
+// Another variation on B.
+`define C \
+  C1 \
+  C2 /* emitted */ \
+  C3 //* not emitted, C comment is closed */ \
+  C4
+
+// Another variation on B.
+`define D \
+  D1 \
+  D2 /* emitted */ \
+  D3 /* emitted, unclosed C comment *// \
+  D4
+
+`A
+
+`B
+
+`C
+
+`D
+