Fix #3170: Reject (|) when | declared as operator [SUPERSEDED]

src/parser/parser.rs

@@ -809,19 +809,30 @@ impl<'a, R: CharRead> Parser<'a, R> {
         Ok(false)
     }
 
-    fn reduce_brackets(&mut self) -> bool {
+    fn reduce_brackets(&mut self) -> Result<bool, ParserError> {
         if self.stack.is_empty() {
-            return false;
+            return Ok(false);
         }
 
         self.reduce_op(1400);
 
         if self.stack.len() <= 1 {
-            return false;
+            return Ok(false);
         }
 
         if let Some(TokenType::Open | TokenType::OpenCT) = self.stack.last().map(|token| token.tt) {
-            return false;
+            return Ok(false);
+        }
+
+        // Fix for issue #3172: Reject mismatched brackets
+        // When closing with ), we must not have an unclosed [ or { on the stack
+        // Example: ([) has stack [Open, OpenList] when ) arrives - this is invalid
+        // ISO/IEC 13211-1:1995: Each bracket type must close with its matching closer
+        if let Some(TokenType::OpenList | TokenType::OpenCurly) = self.stack.last().map(|token| token.tt) {
+            return Err(ParserError::IncompleteReduction(
+                self.lexer.line_num,
+                self.lexer.col_num,
+            ));
         }
 
         let idx = self.stack.len() - 2;
@@ -830,7 +841,18 @@ impl<'a, R: CharRead> Parser<'a, R> {
         match td.tt {
             TokenType::Open | TokenType::OpenCT => {
                 if self.stack[idx].tt == TokenType::Comma {
-                    return false;
+                    return Ok(false);
+                }
+
+                // Fixes issue #3170: Reject (|) when | is an operator
+                // When | is declared as an operator, HeadTailSeparator has priority > 1000
+                // When | is just the default list separator, priority == 1000
+                // Return error instead of Ok(false) to actually fail the parse
+                if self.stack[idx].tt == TokenType::HeadTailSeparator && self.stack[idx].priority > 1000 {
+                    return Err(ParserError::IncompleteReduction(
+                        self.lexer.line_num,
+                        self.lexer.col_num,
+                    ));
                 }
 
                 if let Some(atom) = self.stack[idx].tt.sep_to_atom() {
@@ -842,9 +864,9 @@ impl<'a, R: CharRead> Parser<'a, R> {
                 self.stack[idx].tt = TokenType::Term;
                 self.stack[idx].priority = 0;
 
-                true
+                Ok(true)
             }
-            _ => false,
+            _ => Ok(false),
         }
     }
 
@@ -996,7 +1018,23 @@ impl<'a, R: CharRead> Parser<'a, R> {
             Token::Open => self.shift(Token::Open, 1300, DELIMITER),
             Token::OpenCT => self.shift(Token::OpenCT, 1300, DELIMITER),
             Token::Close => {
-                if !self.reduce_term() && !self.reduce_brackets() {
+                // Defense in depth: Check for (|) pattern BEFORE reducing
+                // When | is declared as an operator, it has priority > 1000
+                // Pattern: stack has at least 2 elements, last is HeadTailSeparator, second-to-last is Open/OpenCT
+                if self.stack.len() >= 2 {
+                    let last_idx = self.stack.len() - 1;
+                    let prev_idx = self.stack.len() - 2;
+                    if self.stack[last_idx].tt == TokenType::HeadTailSeparator &&
+                       self.stack[last_idx].priority > 1000 &&
+                       matches!(self.stack[prev_idx].tt, TokenType::Open | TokenType::OpenCT) {
+                        return Err(ParserError::IncompleteReduction(
+                            self.lexer.line_num,
+                            self.lexer.col_num,
+                        ));
+                    }
+                }
+
+                if !self.reduce_term() && !self.reduce_brackets()? {
                     return Err(ParserError::IncompleteReduction(
                         self.lexer.line_num,
                         self.lexer.col_num,
@@ -1029,9 +1067,11 @@ impl<'a, R: CharRead> Parser<'a, R> {
                     .map(|CompositeOpDesc { inf, spec, .. }| (inf, spec))
                     .unwrap_or((1000, DELIMITER));
 
+                let reduce_priority = priority;
+
                 let old_stack_len = self.stack.len();
 
-                self.reduce_op(priority);
+                self.reduce_op(reduce_priority);
 
                 let new_stack_len = self.stack.len();
 

tests/scryer/cli/issues/issue_3170.in/test.pl

@@ -0,0 +1,2 @@
+:- use_module(library(charsio)).
+:- op(1105,xfy,'|').

tests/scryer/cli/issues/issue_3170.stdin

@@ -0,0 +1,2 @@
+read_from_chars("{!*!(|)/}.",T), write(T), nl.
+halt.

tests/scryer/cli/issues/issue_3170.stdout

@@ -0,0 +1 @@
+   error(syntax_error(incomplete_reduction),read_term_from_chars/3:0).

tests/scryer/cli/issues/issue_3170.toml

@@ -0,0 +1,2 @@
+# issue 3170
+args = ["-f", "--no-add-history", "test.pl"]

tests/scryer/cli/issues/issue_3170_curly.in/test.pl

@@ -0,0 +1,236 @@
+%% Comprehensive test cases for issue #3170
+%% Testing (|) patterns inside curly braces with op(1105,xfy,'|')
+%% All patterns should throw syntax_error, not produce artifacts
+%%
+%% ISO/IEC 13211-1:1995 References:
+%% - §6.3.3.1: Arguments have priority ≤999 (to avoid conflict with comma at 1000)
+%% - §6.3.4: Operator Notation - operators require operands based on their specifier
+%% - §6.3.4.2: Operators as Functors - '|' when declared as operator (priority 1105)
+%% - §6.3.6: Curly Bracketed Term - {term} == '{}'(term)
+%%           Examples: {a} == '{}'(a), {a,b} == '{}'(','(a,b))
+%%           Commas inside {} are comma OPERATOR (priority 1000), not list separators
+%%
+%% When op(1105,xfy,'|') is declared:
+%% - '|' becomes an operator requiring two operands (xfy = infix right-associative)
+%% - (|) means "operator without operands" which violates §6.3.4 requirements
+%% - Inside {}, the argument must be a valid term (§6.3.6), but (|) is not
+%% - Therefore {(|)} and variants must produce syntax_error
+
+:- use_module(library(charsio)).
+:- op(1105, xfy, '|').
+
+%% Category 1: Different trailing operators with ! prefix
+
+test1 :-
+    % {!*!(|)/} - original issue pattern
+    catch(read_from_chars("{!*!(|)/}", _),
+          error(syntax_error(_), _),
+          true).
+
+test2 :-
+    % {!*!(|)*} - trailing multiplication
+    catch(read_from_chars("{!*!(|)*}", _),
+          error(syntax_error(_), _),
+          true).
+
+test3 :-
+    % {!*!(|)+} - trailing plus
+    catch(read_from_chars("{!*!(|)+}", _),
+          error(syntax_error(_), _),
+          true).
+
+test4 :-
+    % {!*!(|)-} - trailing minus
+    catch(read_from_chars("{!*!(|)-}", _),
+          error(syntax_error(_), _),
+          true).
+
+test5 :-
+    % {!*!(|)//} - trailing integer division
+    catch(read_from_chars("{!*!(|)//}", _),
+          error(syntax_error(_), _),
+          true).
+
+test6 :-
+    % {!*!(|)**} - trailing power
+    catch(read_from_chars("{!*!(|)**}", _),
+          error(syntax_error(_), _),
+          true).
+
+%% Category 2: Different prefix operators
+
+test7 :-
+    % {!+(|)/} - ! and + prefix
+    catch(read_from_chars("{!+(|)/}", _),
+          error(syntax_error(_), _),
+          true).
+
+test8 :-
+    % {*!(|)/} - * prefix with ! infix
+    catch(read_from_chars("{*!(|)/}", _),
+          error(syntax_error(_), _),
+          true).
+
+test9 :-
+    % {++(|)/} - double + prefix
+    catch(read_from_chars("{++(|)/}", _),
+          error(syntax_error(_), _),
+          true).
+
+test10 :-
+    % {--(|)/} - double - prefix
+    catch(read_from_chars("{--(|)/}", _),
+          error(syntax_error(_), _),
+          true).
+
+test11 :-
+    % {!!(|)/} - double ! prefix
+    catch(read_from_chars("{!!(|)/}", _),
+          error(syntax_error(_), _),
+          true).
+
+test12 :-
+    % {+-!(|)/} - mixed prefix operators
+    catch(read_from_chars("{+-!(|)/}", _),
+          error(syntax_error(_), _),
+          true).
+
+%% Category 3: Multiple trailing operators
+
+test13 :-
+    % {!*!(|)///} - triple slash
+    catch(read_from_chars("{!*!(|)///}", _),
+          error(syntax_error(_), _),
+          true).
+
+test14 :-
+    % {!*!(|)****} - quadruple star
+    catch(read_from_chars("{!*!(|)****}", _),
+          error(syntax_error(_), _),
+          true).
+
+test15 :-
+    % {!*!(|)//*/} - mixed trailing operators
+    catch(read_from_chars("{!*!(|)//*/}", _),
+          error(syntax_error(_), _),
+          true).
+
+test16 :-
+    % {!*!(|)++-} - multiple plus and minus
+    catch(read_from_chars("{!*!(|)++-}", _),
+          error(syntax_error(_), _),
+          true).
+
+%% Category 4: Just (|) with minimal operators
+
+test17 :-
+    % {(|)} - bare (|) in curly braces
+    catch(read_from_chars("{(|)}", _),
+          error(syntax_error(_), _),
+          true).
+
+test18 :-
+    % {!(|)} - single prefix operator
+    catch(read_from_chars("{!(|)}", _),
+          error(syntax_error(_), _),
+          true).
+
+test19 :-
+    % {(|)/} - single trailing operator
+    catch(read_from_chars("{(|)/}", _),
+          error(syntax_error(_), _),
+          true).
+
+test20 :-
+    % {+(|)-} - single prefix and trailing
+    catch(read_from_chars("{+(|)-}", _),
+          error(syntax_error(_), _),
+          true).
+
+%% Category 5: Nested expressions with atoms/variables
+
+test21 :-
+    % {a*(|)+b} - atoms around (|)
+    catch(read_from_chars("{a*(|)+b}", _),
+          error(syntax_error(_), _),
+          true).
+
+test22 :-
+    % {foo+(|)-bar} - named atoms
+    catch(read_from_chars("{foo+(|)-bar}", _),
+          error(syntax_error(_), _),
+          true).
+
+test23 :-
+    % {X*(|)/Y} - variables around (|)
+    catch(read_from_chars("{X*(|)/Y}", _),
+          error(syntax_error(_), _),
+          true).
+
+test24 :-
+    % {1+(|)*2} - numbers around (|)
+    catch(read_from_chars("{1+(|)*2}", _),
+          error(syntax_error(_), _),
+          true).
+
+test25 :-
+    % {abc-(|)//xyz} - longer expressions
+    catch(read_from_chars("{abc-(|)//xyz}", _),
+          error(syntax_error(_), _),
+          true).
+
+%% Category 6: More complex operator combinations
+
+test26 :-
+    % {!*!*!(|)/} - extended prefix chain
+    catch(read_from_chars("{!*!*!(|)/}", _),
+          error(syntax_error(_), _),
+          true).
+
+test27 :-
+    % {(|)/*/**} - complex trailing chain
+    catch(read_from_chars("{(|)/*/**}", _),
+          error(syntax_error(_), _),
+          true).
+
+test28 :-
+    % {-+*!(|)+*-} - symmetric operator pattern
+    catch(read_from_chars("{-+*!(|)+*-}", _),
+          error(syntax_error(_), _),
+          true).
+
+%% Category 7: Edge cases with parentheses variations
+
+test29 :-
+    % {((|))/} - double parentheses
+    catch(read_from_chars("{((|))/}", _),
+          error(syntax_error(_), _),
+          true).
+
+test30 :-
+    % {!(|(|))/} - nested | operator
+    catch(read_from_chars("{!(|(|))}", _),
+          error(syntax_error(_), _),
+          true).
+
+%% Category 8: Whitespace variations
+
+test31 :-
+    % { ! * ! ( | ) / } - with spaces
+    catch(read_from_chars("{ ! * ! ( | ) / }", _),
+          error(syntax_error(_), _),
+          true).
+
+test32 :-
+    % {  (|)  /  } - extra whitespace
+    catch(read_from_chars("{  (|)  /  }", _),
+          error(syntax_error(_), _),
+          true).
+
+%% Run all tests
+run :-
+    test1, test2, test3, test4, test5, test6, test7, test8, test9, test10,
+    test11, test12, test13, test14, test15, test16, test17, test18, test19, test20,
+    test21, test22, test23, test24, test25, test26, test27, test28, test29, test30,
+    test31, test32,
+    halt.

tests/scryer/cli/issues/issue_3170_curly.toml

@@ -0,0 +1,3 @@
+# issue #3170 - curly brace tests
+# Test that (|) patterns in curly braces throw syntax_error when op(1105,xfy,'|') is declared
+args = ["-f", "--no-add-history", "-g", "run", "test.pl"]