Add test for unrecognized statement error with vector braces (#472)

The old code using `take_till_inclusive` assumed that a right brace would be the end of a block and therefore never part of a statement. However, some PTX statements can include vector operands. This meant that any unrecognized statement with a vector operand would backtrace and eventually produce an unhelpful context error rather than an `UnrecognizedStatement` error.

This pull request also adds a mechanism for testing parser errors.

ptx/src/test/mod.rs

@@ -3,6 +3,27 @@ use ptx_parser as ast;
 
 mod spirv_run;
 
+#[macro_export]
+macro_rules! read_test_file {
+    ($file:expr) => {
+        {
+            if cfg!(feature = "ci_build") {
+                include_str!($file).to_string()
+            } else {
+                use std::path::PathBuf;
+                // CARGO_MANIFEST_DIR is the crate directory (ptx), but file! is relative to the workspace root (and therefore also includes ptx).
+                let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+                path.pop();
+                path.push(file!());
+                path.pop();
+                path.push($file);
+                std::fs::read_to_string(path).unwrap()
+            }
+        }
+    };
+}
+pub(crate) use read_test_file;
+
 fn parse_and_assert(ptx_text: &str) {
     ast::parse_module_checked(ptx_text).unwrap();
 }

ptx/src/test/spirv_run/mod.rs

@@ -1,3 +1,4 @@
+use super::read_test_file;
 use crate::pass;
 use comgr::Comgr;
 use cuda_types::cuda::CUstream;
@@ -10,32 +11,10 @@ use std::fmt::{self, Debug, Display, Formatter};
 use std::fs::{self, File};
 use std::io::Write;
 use std::mem;
-use std::path::{Path, PathBuf};
+use std::path::Path;
 use std::ptr;
 use std::str;
 
-#[cfg(not(feature = "ci_build"))]
-macro_rules! read_test_file {
-    ($file:expr) => {
-        {
-            // CARGO_MANIFEST_DIR is the crate directory (ptx), but file! is relative to the workspace root (and therefore also includes ptx).
-            let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
-            path.pop();
-            path.push(file!());
-            path.pop();
-            path.push($file);
-            std::fs::read_to_string(path).unwrap()
-        }
-    };
-}
-
-#[cfg(feature = "ci_build")]
-macro_rules! read_test_file {
-    ($file:expr) => {
-        include_str!($file).to_string()
-    };
-}
-
 macro_rules! test_ptx_llvm {
     ($fn_name:ident) => {
         paste::item! {

ptx_parser/src/lib.rs

@@ -732,79 +732,52 @@ fn statement<'a, 'input>(
             pragma.map(|_| None),
             block_statement.map(Some),
         )),
-        take_till_inclusive(
+        take_till_end_of_statement(),
-            |(t, _)| *t == Token::RBrace,
-            |(t, _)| match t {
-                Token::Semicolon | Token::Colon => true,
-                _ => false,
-            },
-        ),
         |text| PtxError::UnrecognizedStatement(text.unwrap_or("")),
     )
     .map(Option::flatten)
     .parse_next(stream)
 }
 
-fn take_till_inclusive<I: Stream, E: ParserError<I>>(
+fn take_till_end_of_statement<
-    backtrack_token: impl Fn(&I::Token) -> bool,
+    'a,
-    end_token: impl Fn(&I::Token) -> bool,
+    I: Stream<Token = (Token<'a>, std::ops::Range<usize>)>,
-) -> impl Parser<I, <I as Stream>::Slice, E> {
+    E: ParserError<I>,
-    fn get_offset<I: Stream, E: ParserError<I>>(
+>() -> impl Parser<I, <I as Stream>::Slice, E> {
-        input: &mut I,
+    trace("take_till_end_of_statement", move |stream: &mut I| {
-        backtrack_token: &impl Fn(&I::Token) -> bool,
+        let mut depth = 0;
-        end_token: &impl Fn(&I::Token) -> bool,
+
-        should_backtrack: &mut bool,
+        let mut iterator = stream.iter_offsets().peekable();
-    ) -> Result<usize, E> {
+        while let Some((current_offset, (token, _))) = iterator.next() {
-        *should_backtrack = false;
+            match token {
-        let mut hit = false;
+                Token::LBrace => {
-        for (offset, token) in input.iter_offsets() {
+                    depth += 1;
-            if hit {
-                return Ok(offset);
-            } else {
-                if backtrack_token(&token) {
-                    *should_backtrack = true;
-                    return Ok(offset);
                 }
-                if end_token(&token) {
+                Token::RBrace => {
-                    hit = true;
+                    if depth == 0 {
+                        return Err(ErrMode::from_error_kind(
+                            stream,
+                            winnow::error::ErrorKind::Token,
+                        ));
+                    }
+                    depth -= 1;
                 }
+                Token::Semicolon | Token::Colon => {
+                    let offset = if let Some((next_offset, _)) = iterator.peek() {
+                        *next_offset
+                    } else {
+                        current_offset
+                    };
+                    return Ok(stream.next_slice(offset));
+                }
+                _ => {}
             }
         }
-        Err(ParserError::from_error_kind(input, ErrorKind::Eof))
+
-    }
+        Err(ParserError::from_error_kind(stream, ErrorKind::Eof))
-    trace("take_till_inclusive", move |stream: &mut I| {
-        let mut should_backtrack = false;
-        let offset = get_offset(stream, &backtrack_token, &end_token, &mut should_backtrack)?;
-        let result = stream.next_slice(offset);
-        if should_backtrack {
-            Err(ErrMode::from_error_kind(
-                stream,
-                winnow::error::ErrorKind::Token,
-            ))
-        } else {
-            Ok(result)
-        }
     })
 }
 
-/*
-pub fn take_till_or_backtrack_eof<Set, Input, Error>(
-    set: Set,
-) -> impl Parser<Input, <Input as Stream>::Slice, Error>
-where
-    Input: StreamIsPartial + Stream,
-    Set: winnow::stream::ContainsToken<<Input as Stream>::Token>,
-    Error: ParserError<Input>,
-{
-    move |stream: &mut Input| {
-        if stream.eof_offset() == 0 {
-            return ;
-        }
-        take_till(0.., set)
-    }
-}
-     */
-
 fn with_recovery<'a, 'input: 'a, T>(
     mut parser: impl Parser<PtxParser<'a, 'input>, T, ContextError>,
     recovery: impl Parser<PtxParser<'a, 'input>, &'a [(Token<'input>, logos::Span)], ContextError>,
@@ -1344,7 +1317,7 @@ impl<Ident> ast::ParsedOperand<Ident> {
     }
 }
 
-#[derive(Debug, thiserror::Error, strum::AsRefStr)]
+#[derive(Debug, thiserror::Error, PartialEq, strum::AsRefStr)]
 pub enum PtxError<'input> {
     #[error("{source}")]
     ParseInt {
@@ -3867,6 +3840,26 @@ mod tests {
         ));
     }
 
+    #[test]
+    fn report_unknown_instruction_with_braces() {
+        let text = "
+            .version 6.5
+            .target sm_60
+            .address_size 64
+
+            .visible .entry unrecognized_braces(
+            )
+            {
+                mov.u32 foo, {} {};
+                ret;
+            }";
+        let errors = parse_module_checked(text).err().unwrap();
+        assert_eq!(
+            errors,
+            vec![PtxError::UnrecognizedStatement("mov.u32 foo, {} {};")]
+        );
+    }
+
     #[test]
     fn report_unknown_directive() {
         let text = "