From b0497554d873fc80372a67ecf620a808eae8520a Mon Sep 17 00:00:00 2001
From: Lysandros Nikolaou <lisandrosnik@gmail.com>
Date: Sun, 17 May 2020 18:21:35 +0300
Subject: [PATCH 1/4] bpo-40334: Produce better error messages for
 non-parenthesized genexps

The error message, generated for non-parenthesized generator expression
in function calls, was still the generic `invalid syntax`, when the
generator expression wasn't appearing as the first argument in the call.
Now, even on input like `f(a, b, c for c in d, e)`, the correct error
message gets produced.
---
 Grammar/python.gram     |  1 +
 Lib/test/test_syntax.py |  8 +++-----
 Parser/pegen/parse.c    | 19 +++++++++++++++++++
 Parser/pegen/pegen.c    | 23 ++++++++++++++++++++++-
 Parser/pegen/pegen.h    |  1 +
 5 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/Grammar/python.gram b/Grammar/python.gram
index cca92090546265..1d45ee5d61d950 100644
--- a/Grammar/python.gram
+++ b/Grammar/python.gram
@@ -624,6 +624,7 @@ incorrect_arguments:
     | args ',' '*' { RAISE_SYNTAX_ERROR("iterable argument unpacking follows keyword argument unpacking") }
     | a=expression for_if_clauses ',' [args | expression for_if_clauses] {
         RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "Generator expression must be parenthesized") }
+    | a=args for_if_clauses { _PyPegen_nonparen_genexp_in_call(p, a) }
     | a=args ',' args { _PyPegen_arguments_parsing_error(p, a) }
 invalid_kwarg:
     | a=expression '=' {
diff --git a/Lib/test/test_syntax.py b/Lib/test/test_syntax.py
index 60c7d9fd3868e8..4ab159b4f7847f 100644
--- a/Lib/test/test_syntax.py
+++ b/Lib/test/test_syntax.py
@@ -206,11 +206,9 @@
 >>> f(x for x in L, **{})
 Traceback (most recent call last):
 SyntaxError: Generator expression must be parenthesized
-
-# >>> f(L, x for x in L)
-# Traceback (most recent call last):
-# SyntaxError: Generator expression must be parenthesized
-
+>>> f(L, x for x in L)
+Traceback (most recent call last):
+SyntaxError: Generator expression must be parenthesized
 >>> f(x for x in L, y for y in L)
 Traceback (most recent call last):
 SyntaxError: Generator expression must be parenthesized
diff --git a/Parser/pegen/parse.c b/Parser/pegen/parse.c
index 2a9dad7d1d7ef5..2234c7ee76185a 100644
--- a/Parser/pegen/parse.c
+++ b/Parser/pegen/parse.c
@@ -10594,6 +10594,7 @@ t_atom_rule(Parser *p)
 // incorrect_arguments:
 //     | args ',' '*'
 //     | expression for_if_clauses ',' [args | expression for_if_clauses]
+//     | args for_if_clauses
 //     | args ',' args
 static void *
 incorrect_arguments_rule(Parser *p)
@@ -10649,6 +10650,24 @@ incorrect_arguments_rule(Parser *p)
         }
         p->mark = _mark;
     }
+    { // args for_if_clauses
+        expr_ty a;
+        asdl_seq* for_if_clauses_var;
+        if (
+            (a = args_rule(p))  // args
+            &&
+            (for_if_clauses_var = for_if_clauses_rule(p))  // for_if_clauses
+        )
+        {
+            _res = _PyPegen_nonparen_genexp_in_call ( p , a );
+            if (_res == NULL && PyErr_Occurred()) {
+                p->error_indicator = 1;
+                return NULL;
+            }
+            goto done;
+        }
+        p->mark = _mark;
+    }
     { // args ',' args
         Token * _literal;
         expr_ty a;
diff --git a/Parser/pegen/pegen.c b/Parser/pegen/pegen.c
index 7f3e4561de6055..3a1dec67f1b755 100644
--- a/Parser/pegen/pegen.c
+++ b/Parser/pegen/pegen.c
@@ -2099,4 +2099,25 @@ _PyPegen_get_invalid_target(expr_ty e)
         default:
             return e;
     }
-}
\ No newline at end of file
+}
+
+void *
+_PyPegen_nonparen_genexp_in_call(Parser *p, expr_ty args)
+{
+    /* The rule that calls this function is 'args for_if_clauses'.
+       For the input f(L, x for x in y), L and x are in args and
+       the for is parsed as a for_if_clause. We have to check if
+       len == 1, so that input like dict((a, b) for a, b in x)
+       gets successfully parsed and then we pass the last
+       argument (x in the above example) as the location of the
+       error */
+    Py_ssize_t len = asdl_seq_LEN(args->v.Call.args);
+    if (len == 1) {
+        return NULL;
+    }
+
+    return RAISE_SYNTAX_ERROR_KNOWN_LOCATION(
+        ((expr_ty) asdl_seq_GET(args->v.Call.args, len - 1)),
+        "Generator expression must be parenthesized"
+    );
+}
diff --git a/Parser/pegen/pegen.h b/Parser/pegen/pegen.h
index b9d4c048bb52b0..c2e8eebe333de5 100644
--- a/Parser/pegen/pegen.h
+++ b/Parser/pegen/pegen.h
@@ -263,6 +263,7 @@ mod_ty _PyPegen_make_module(Parser *, asdl_seq *);
 // Error reporting helpers
 
 expr_ty _PyPegen_get_invalid_target(expr_ty e);
+void *_PyPegen_nonparen_genexp_in_call(Parser *p, expr_ty args);
 
 void *_PyPegen_parse(Parser *);
 

From c545e5f6c691f317f4b54d08348e4a423ca75c20 Mon Sep 17 00:00:00 2001
From: Lysandros Nikolaou <lisandrosnik@gmail.com>
Date: Sun, 17 May 2020 18:31:36 +0300
Subject: [PATCH 2/4] Fix RAISE_SYNTAX_ERROR_KNOWN_LOCATION macro

---
 Parser/pegen/pegen.c | 2 +-
 Parser/pegen/pegen.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Parser/pegen/pegen.c b/Parser/pegen/pegen.c
index 3a1dec67f1b755..22f510468ee0bb 100644
--- a/Parser/pegen/pegen.c
+++ b/Parser/pegen/pegen.c
@@ -2117,7 +2117,7 @@ _PyPegen_nonparen_genexp_in_call(Parser *p, expr_ty args)
     }
 
     return RAISE_SYNTAX_ERROR_KNOWN_LOCATION(
-        ((expr_ty) asdl_seq_GET(args->v.Call.args, len - 1)),
+        (expr_ty) asdl_seq_GET(args->v.Call.args, len - 1),
         "Generator expression must be parenthesized"
     );
 }
diff --git a/Parser/pegen/pegen.h b/Parser/pegen/pegen.h
index c2e8eebe333de5..81b04287751687 100644
--- a/Parser/pegen/pegen.h
+++ b/Parser/pegen/pegen.h
@@ -152,7 +152,7 @@ RAISE_ERROR_KNOWN_LOCATION(Parser *p, PyObject *errtype, int lineno,
 #define RAISE_SYNTAX_ERROR(msg, ...) _PyPegen_raise_error(p, PyExc_SyntaxError, msg, ##__VA_ARGS__)
 #define RAISE_INDENTATION_ERROR(msg, ...) _PyPegen_raise_error(p, PyExc_IndentationError, msg, ##__VA_ARGS__)
 #define RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, msg, ...) \
-    RAISE_ERROR_KNOWN_LOCATION(p, PyExc_SyntaxError, a->lineno, a->col_offset, msg, ##__VA_ARGS__)
+    RAISE_ERROR_KNOWN_LOCATION(p, PyExc_SyntaxError, (a)->lineno, (a)->col_offset, msg, ##__VA_ARGS__)
 
 Py_LOCAL_INLINE(void *)
 CHECK_CALL(Parser *p, void *result)

From 9b8b22ddbeedbbd663e1066f0bc24c12d86191c8 Mon Sep 17 00:00:00 2001
From: Lysandros Nikolaou <lisandrosnik@gmail.com>
Date: Sun, 17 May 2020 18:51:33 +0300
Subject: [PATCH 3/4] Fix bug

---
 Parser/pegen/pegen.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Parser/pegen/pegen.c b/Parser/pegen/pegen.c
index 22f510468ee0bb..c3578a6790923d 100644
--- a/Parser/pegen/pegen.c
+++ b/Parser/pegen/pegen.c
@@ -2107,12 +2107,12 @@ _PyPegen_nonparen_genexp_in_call(Parser *p, expr_ty args)
     /* The rule that calls this function is 'args for_if_clauses'.
        For the input f(L, x for x in y), L and x are in args and
        the for is parsed as a for_if_clause. We have to check if
-       len == 1, so that input like dict((a, b) for a, b in x)
+       len <= 1, so that input like dict((a, b) for a, b in x)
        gets successfully parsed and then we pass the last
        argument (x in the above example) as the location of the
        error */
     Py_ssize_t len = asdl_seq_LEN(args->v.Call.args);
-    if (len == 1) {
+    if (len <= 1) {
         return NULL;
     }
 

From ed6487671ea081799142e4b9abf4f70c502440c4 Mon Sep 17 00:00:00 2001
From: Lysandros Nikolaou <lisandrosnik@gmail.com>
Date: Tue, 19 May 2020 02:19:47 +0300
Subject: [PATCH 4/4] Generate correct error message for generator expression
 following keyword arguments

---
 Grammar/python.gram  |  2 ++
 Parser/pegen/parse.c | 25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/Grammar/python.gram b/Grammar/python.gram
index 1d45ee5d61d950..99a7a284723a14 100644
--- a/Grammar/python.gram
+++ b/Grammar/python.gram
@@ -625,6 +625,8 @@ incorrect_arguments:
     | a=expression for_if_clauses ',' [args | expression for_if_clauses] {
         RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "Generator expression must be parenthesized") }
     | a=args for_if_clauses { _PyPegen_nonparen_genexp_in_call(p, a) }
+    | args ',' a=expression for_if_clauses {
+        RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "Generator expression must be parenthesized") }
     | a=args ',' args { _PyPegen_arguments_parsing_error(p, a) }
 invalid_kwarg:
     | a=expression '=' {
diff --git a/Parser/pegen/parse.c b/Parser/pegen/parse.c
index 2234c7ee76185a..bdb329f473e347 100644
--- a/Parser/pegen/parse.c
+++ b/Parser/pegen/parse.c
@@ -10595,6 +10595,7 @@ t_atom_rule(Parser *p)
 //     | args ',' '*'
 //     | expression for_if_clauses ',' [args | expression for_if_clauses]
 //     | args for_if_clauses
+//     | args ',' expression for_if_clauses
 //     | args ',' args
 static void *
 incorrect_arguments_rule(Parser *p)
@@ -10668,6 +10669,30 @@ incorrect_arguments_rule(Parser *p)
         }
         p->mark = _mark;
     }
+    { // args ',' expression for_if_clauses
+        Token * _literal;
+        expr_ty a;
+        expr_ty args_var;
+        asdl_seq* for_if_clauses_var;
+        if (
+            (args_var = args_rule(p))  // args
+            &&
+            (_literal = _PyPegen_expect_token(p, 12))  // token=','
+            &&
+            (a = expression_rule(p))  // expression
+            &&
+            (for_if_clauses_var = for_if_clauses_rule(p))  // for_if_clauses
+        )
+        {
+            _res = RAISE_SYNTAX_ERROR_KNOWN_LOCATION ( a , "Generator expression must be parenthesized" );
+            if (_res == NULL && PyErr_Occurred()) {
+                p->error_indicator = 1;
+                return NULL;
+            }
+            goto done;
+        }
+        p->mark = _mark;
+    }
     { // args ',' args
         Token * _literal;
         expr_ty a;