@gvanrossum @pablogsal One question on this:

In the macro #define CHECK(result) (result == NULL ? SET_ERROR(p) : result) result might be a call to some costly function, which in the positive case will be called twice. Are we confident enough the the compiler is going to optimize those doubled calls away?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this does not work because the ternary operator deduces the type as void*. From the C99 standard:

since C does not have a type hierarchy for pointer types, pointer operands may only be used if they are of the same type (ignoring type qualifiers) or one is void or NULL.

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

The grammar needs to be fixed as well. If you substitute the macro for an identity:

# define CHECK(result) result

you will start to see these errors:

n file included from Parser/pegen.h:7,
                 from Parser/parser.c:2:
./Include/Python-ast.h:647:50: note: expected ‘asdl_expr_seq *’ but argument is of type ‘asdl_seq *’
  647 | expr_ty _Py_BoolOp(boolop_ty op, asdl_expr_seq * values, int lineno, int
      |                                  ~~~~~~~~~~~~~~~~^~~~~~
In file included from Parser/parser.c:2:

The reason these errors are not present now is that the macros pass void* and that can be cast to anything, but that's what we try to avoid.

In a chat I had with @pablogsal, he explained to me why this doesn't work. Basically the different pointer types will get cast to void *s with this solution as well. The easiest to grasp solution (Pablo's idea) would be to also pass the return type to the macro and cast to that type. That would require a good amount of changes to the grammar though.

Bottom line is we should probably discuss this a bit more. Another idea Pablo mentioned is to pass the type to the macro in the generator level, but that hardly seems feasible for nested call to CHECK.

Hm, it's just a bit of work, we should just do it.