Track expression-list paren tokens (not just positions), see comment thread for #193

This commit paves the way for in the future (not now) possibly experimenting with allowing expression-list to have braces, if it's important to provide explicit initializer-list expression syntax (mostly for function arguments)

Author: hsutter

regression-tests/test-results/clang-12/pure2-cpp1-multitoken-fundamental-types.cpp.output

@@ -1,9 +1,8 @@
 pure2-bounds-safety-pointer-arithmetic-error.cpp2...
 pure2-bounds-safety-pointer-arithmetic-error.cpp2(15,13): error: 'delete' and owning raw pointers are not supported in Cpp2
 pure2-bounds-safety-pointer-arithmetic-error.cpp2(15,13): error:   - use unique.new<T>, shared.new<T>, or gc.new<T> instead (in that order)
-pure2-bounds-safety-pointer-arithmetic-error.cpp2(15,16): error: expected ; at end of statement (at 'first')
-pure2-bounds-safety-pointer-arithmetic-error.cpp2(15,16): error: invalid statement in compound-statement (at 'first')
-pure2-bounds-safety-pointer-arithmetic-error.cpp2(15,16): error: invalid while loop body (at 'first')
-pure2-bounds-safety-pointer-arithmetic-error.cpp2(17,1): error: unexpected text at end of Cpp2 code section (at '}')
+pure2-bounds-safety-pointer-arithmetic-error.cpp2(9,25): error: invalid while loop body (at '{')
+pure2-bounds-safety-pointer-arithmetic-error.cpp2(3,3): error: ill-formed initializer (at '{')
+pure2-bounds-safety-pointer-arithmetic-error.cpp2(2,1): error: unexpected text at end of Cpp2 code section (at 'main')
 pure2-bounds-safety-pointer-arithmetic-error.cpp2(1,0): error: parse failed for section starting here
 

regression-tests/test-results/gcc-10/pure2-cpp1-multitoken-fundamental-types.cpp.output

@@ -1,7 +1,4 @@
 @echo off
-REM This is intended to be run in the /test-results directory
-REM A copy is in the parent directory just for convenience in case of accidental deletion.
-REM
 del *.cpp *.output
 copy ..\*.cpp2 .
 set count=0

regression-tests/test-results/msvc-2022/pure2-cpp1-multitoken-fundamental-types.cpp.output

@@ -2108,12 +2108,7 @@ class cppfront
     {
         auto add_parens = should_add_expression_list_parens() && !n.inside_initializer;
         if (add_parens) {
-            if (n.expressions.empty()) {
-                printer.print_cpp2( "{", n.position());
-            }
-            else {
-                printer.print_cpp2( "(", n.position());
-            }
+            printer.print_cpp2( *n.open_paren, n.position());
         }
 
         auto first = true;
@@ -2163,12 +2158,7 @@ class cppfront
         }
 
         if (add_parens) {
-            if (n.expressions.empty()) {
-                printer.print_cpp2( "}", n.position());
-            }
-            else {
-                printer.print_cpp2( ")", n.position());
-            }
+            printer.print_cpp2( *n.close_paren, n.position());
         }
         //  We want to consume only one of these
         consumed_expression_list_parens();
@@ -2346,7 +2336,8 @@ class cppfront
             printer.print_cpp2( "{\n", n.position() );
         }
         else {
-            printer.print_cpp2( "(", n.pos_open_paren );
+            assert(n.open_paren);
+            printer.print_cpp2( "(", n.open_paren->position() );
         }
 
         //  So we don't get cute about text-aligning the first parameter when it's on a new line
@@ -2372,8 +2363,9 @@ class cppfront
         else {
             //  Position heuristic (aka hack): Avoid emitting extra whitespace before )
             //  beyond column 10
-            auto col = std::min( n.pos_close_paren.colno, colno_t{10} );
-            printer.print_cpp2( ")", { n.pos_close_paren.lineno, col } );
+            assert(n.close_paren);
+            auto col = std::min( n.close_paren->position().colno, colno_t{10});
+            printer.print_cpp2( ")", { n.close_paren->position().lineno, col});
         }
 
         in_parameter_list = false;

regression-tests/test-results/pure2-bounds-safety-pointer-arithmetic-error.cpp2.output

@@ -96,27 +96,35 @@ enum class lexeme : std::int8_t {
     Keyword,
     Cpp1MultiKeyword,
     Cpp2FixedType,
-    Identifier
+    Identifier,
+    None = 127
 };
 
-auto is_literal(lexeme l) {
+auto is_literal(lexeme l) -> bool {
     switch (l) {
-    break;case lexeme::FloatLiteral:        
-          case lexeme::BinaryLiteral:       
-          case lexeme::DecimalLiteral:      
-          case lexeme::HexadecimalLiteral:  
-          case lexeme::StringLiteral:       
+    break;case lexeme::FloatLiteral:
+          case lexeme::BinaryLiteral:
+          case lexeme::DecimalLiteral:
+          case lexeme::HexadecimalLiteral:
+          case lexeme::StringLiteral:
           case lexeme::CharacterLiteral:    return true;
     break;default:                          return false;
     }
 }
 
-//  TODO: Remove the duplication above/below with a variadic macro,
-//        but for now it's simpler just to write it out
+auto close_paren_type(lexeme l) -> lexeme {
+    switch (l) {
+    break;case lexeme::LeftBrace:   return lexeme::RightBrace;
+    break;case lexeme::LeftBracket: return lexeme::RightBracket;
+    break;case lexeme::LeftParen:   return lexeme::RightParen;
+    break;default:                  return lexeme::None;
+    }
+}
+
 
 template<typename T>
     requires std::is_same_v<T, std::string>
-auto as(lexeme l)
+auto as(lexeme l) -> std::string
 {
     switch (l) {
     break;case lexeme::SlashEq:             return "SlashEq";
@@ -181,6 +189,7 @@ auto as(lexeme l)
     break;case lexeme::Cpp1MultiKeyword:    return "Cpp1MultiKeyword";
     break;case lexeme::Cpp2FixedType:       return "Cpp2FixedType";
     break;case lexeme::Identifier:          return "Identifier";
+    break;case lexeme::None:                return "(NONE)";
     break;default:                          return "INTERNAL-ERROR";
     }
 };

regression-tests/test-results/run-tests.bat

@@ -256,9 +256,9 @@ auto to_string_view(passing_style pass) -> std::string_view {
 
 struct expression_list_node
 {
-    source_position open_paren  = {};
-    source_position close_paren = {};
-    bool inside_initializer     = false;
+    token const* open_paren  = {};
+    token const* close_paren = {};
+    bool inside_initializer  = false;
 
     struct term {
         passing_style                    pass = {};
@@ -277,8 +277,8 @@ struct expression_list_node
     auto position() const -> source_position
     {
         //  Make sure this got set
-        assert (open_paren != source_position());
-        return open_paren;
+        assert (open_paren);
+        return open_paren->position();
     }
 
     auto visit(auto& v, int depth) -> void
@@ -910,8 +910,8 @@ auto compound_statement_node::visit(auto& v, int depth) -> void
 
 struct parameter_declaration_node
 {
-    source_position pos;
-    passing_style pass = passing_style::in;
+    source_position pos = {};
+    passing_style pass  = passing_style::in;
 
     enum class modifier { none=0, implicit, virtual_, override_, final_ };
     modifier mod = modifier::none;
@@ -926,14 +926,15 @@ struct parameter_declaration_node
 
 struct parameter_declaration_list_node
 {
-    source_position pos_open_paren;
-    source_position pos_close_paren;
+    token const* open_paren  = {};
+    token const* close_paren = {};
 
     std::vector<std::unique_ptr<parameter_declaration_node>> parameters;
 
     auto position() const -> source_position
     {
-        return pos_open_paren;
+        assert(open_paren);
+        return open_paren->position();
     }
 
     auto visit(auto& v, int depth) -> void
@@ -1507,6 +1508,7 @@ class parser
     //G     id-expression
     //G     literal
     //G     '(' expression-list ')'
+    //G     '{' expression-list '}'
     //G     unnamed-declaration
     //G
     auto primary_expression()
@@ -1539,23 +1541,28 @@ class parser
             return n;
         }
 
-        if (curr().type() == lexeme::LeftParen)
+        if (curr().type() == lexeme::LeftParen
+            //  If in the future (not now) we want to experiment with braced-expressions
+            //      || curr().type() == lexeme::LeftBrace
+            )
         {
             bool inside_initializer = (peek(-1)->type() == lexeme::Assignment);
-            auto open_paren = curr().position();
+            auto open_paren = &curr();
+            auto close = close_paren_type(open_paren->type());
+            auto close_text = [&] () -> std::string { if (close == lexeme::RightParen) { return ")"; } return "}"; }();
             next();
             auto expr_list = expression_list(open_paren, inside_initializer);
             if (!expr_list) {
                 error("unexpected text - ( is not followed by an expression-list");
                 next();
                 return {};
             }
-            if (curr().type() != lexeme::RightParen) {
-                error("unexpected text - expression-list is not terminated by )");
+            if (curr().type() != close_paren_type(open_paren->type())) {
+                error("unexpected text - expression-list is not terminated by " + close_text);
                 next();
                 return {};
             }
-            expr_list->close_paren = curr().position();
+            expr_list->close_paren = &curr();
             next();
             n->expr = std::move(expr_list);
             return n;
@@ -1656,7 +1663,7 @@ class parser
 
             if (term.op->type() == lexeme::LeftBracket)
             {
-                term.expr_list = expression_list(term.op->position());
+                term.expr_list = expression_list(term.op);
                 if (!term.expr_list) {
                     error("subscript expression [ ] must not be empty");
                     return {};
@@ -1665,13 +1672,13 @@ class parser
                     error("unexpected text - [ is not properly matched by ]");
                     return {};
                 }
-                term.expr_list->close_paren = curr().position();
+                term.expr_list->close_paren = &curr();
                 term.op_close = &curr();
                 next();
             }
             else if (term.op->type() == lexeme::LeftParen)
             {
-                term.expr_list = expression_list(term.op->position());
+                term.expr_list = expression_list(term.op);
                 if (!term.expr_list) {
                     error("( is not followed by a valid expression list");
                     return {};
@@ -1680,7 +1687,7 @@ class parser
                     error("unexpected text - ( is not properly matched by )");
                     return {};
                 }
-                term.expr_list->close_paren = curr().position();
+                term.expr_list->close_paren = &curr();
                 term.op_close = &curr();
                 next();
             }
@@ -2008,7 +2015,7 @@ class parser
     //G     parameter-direction? expression
     //G     expression-list ',' expression
     //G
-    auto expression_list(source_position open_paren, bool inside_initializer = false) -> std::unique_ptr<expression_list_node> {
+    auto expression_list(token const* open_paren, bool inside_initializer = false) -> std::unique_ptr<expression_list_node> {
         auto pass = passing_style::in;
         auto n = std::make_unique<expression_list_node>();
         n->open_paren = open_paren;
@@ -2391,7 +2398,6 @@ class parser
                 //  it doesn't destabilize any regression tests
             )
         {
-            error("expected ; at end of statement");
             return {};
         }
         if (curr().type() == lexeme::Semicolon) {
@@ -2848,7 +2854,6 @@ class parser
         }
 
         else {
-            //next();
             return {};
         }
     }
@@ -2870,6 +2875,9 @@ class parser
 
         auto n = std::make_unique<compound_statement_node>();
 
+        //  Remember current position, in case this isn't a valid statement
+        auto start_pos = pos;
+
         //  In the case where this is a declaration initializer with
         //      = {
         //  on the same line, we want to remember our start position
@@ -2886,7 +2894,7 @@ class parser
         while (curr().type() != lexeme::RightBrace) {
             auto s = statement(true);
             if (!s) {
-                error("invalid statement in compound-statement");
+                pos = start_pos;    // backtrack
                 return {};
             }
             n->statements.push_back( std::move(s) );
@@ -3011,7 +3019,7 @@ class parser
         }
 
         auto n = std::make_unique<parameter_declaration_list_node>();
-        n->pos_open_paren = curr().position();
+        n->open_paren = &curr();
         next();
 
         auto param = std::make_unique<parameter_declaration_node>();
@@ -3035,7 +3043,7 @@ class parser
             return {};
         }
 
-        n->pos_close_paren = curr().position();
+        n->close_paren = &curr();
         next();
         return n;
     }