This bug tracker entry is to track adding "Argument Clinic" to CPython trunk.

Please see http://mail.python.org/pipermail/python-dev/2012-December/122920.html for more information.

Sorry for the delay; this head cold is slowing me down.  Here's the current state of Argument Clinic as a patch for review.  I look forward to your comments!

Can't you propose a PEP for this? I'd like to see the DSL officially discussed (or at least validated).

It seems the clinic.txt DSL document should be proofread for proper/consistent use of argument/parameter (e.g. as described in the recently added http://docs.python.org/dev/glossary.html#term-parameter ).  To choose a couple random examples--

+Argument declaration |       type name = default
+Argument flags       |       flag flag2 flag3=value

Shouldn't this be "Type declaration", etc?

+Functions With Positional-Only Arguments
+========================================

Positional-Only Parameters

I second Antoine's request for a PEP. I'm not only concerned about the DSL:

This approach may work for modules like posixmodule.c, where each function
is largely a self-contained unit wrapping some API function.

On the other hand, I don't want to imagine how _decimal.c will look like:
There are > 100 functions that are closely related. Many of those functions
are *already* generated by macros. I think that vertical space is a precious
resource, so all docstrings (cruft as far as understanding the program is
concerned) are in a header file.

The clinic.py preprocessor would add at least 3000 lines to an already 
large module, tearing apart groups of functions that form logical units.


Apologies for sounding negative, but IIRC this hasn't been brought up in 
the python-dev discussion at all.

Antoine, Stefan: There doesn't appear to be a bright line separating "this should get a PEP" from "this doesn't need a PEP".  That said, changes to the C API for CPython don't seem to merit PEPs very often, the recent "Stable ABI" being the lone exception to the rule I can recall.  Also, Guido already said (in python-dev) he didn't think it merited a PEP, and I tend to agree.

What I don't understand is what advantage writing a PEP would bring.  If you're worried that it needs discussion--okay, let's discuss it.  Surely we can do that here?  Or in python-dev?  What's on your mind?


Stefan: It sounds like Argument Clinic won't work well for _cdecimal, nor would any alternate DSL grammar work any better, really.  But have no fear--I doubt anyone is going to force you to accept Argument-Clinic-ization of _cdecimal against your will.  If you're comfortable maintaining the _cdecimal code without it, you certainly have my blessing.  Though I do hope you'll add the data needed to generate Signature objects once we figure out what that looks like.


Chris: clinic.txt is available for review as part of the patch I posted, please feel free to make your comments there.  I could file a new patch with the two changes you already suggest if you prefer.  (Though I'm not changing the name of the project to "Parameter Clinic"--the Monty Python reference is simply irresistible.)


Finally, an admission.  I wrote "Argument Clinic" thinking that the inspect.Signature Parameter objects had a per-Parameter docstring.  Well, they don't.  (I feel like an idiot.)  So we could theoretically remove the per-parameter docstrings from the DSL.  However, I'm pretty happy with the current structure of the DSL.  My inclination therefore is to leave them in.  (And who knows, maybe the Parameter object could grow a docstring.)  Still, if you're going to propose changes to the DSL, I submit that removing the per-parameter docstring is wholly viable.

> Antoine, Stefan: There doesn't appear to be a bright line separating
> "this should get a PEP" from "this doesn't need a PEP".  That said,
> changes to the C API for CPython don't seem to merit PEPs very often,
> the recent "Stable ABI" being the lone exception to the rule I can
> recall.  Also, Guido already said (in python-dev) he didn't think it
> merited a PEP, and I tend to agree.

My argument for requiring a PEP is that this change will not only affect
the people maintaining the clinic code. It will affect everyone
contributing and maintaining code in the CPython codebase. It's much
more than "just an addition to the C API", it's a change in how CPython
is developed.

Writing and proposing a PEP brings public scrutiny in a much more
visible, and also better-recorded, way than a bug entry on a tracker.

Note that writing a PEP doesn't mean that there'll be a huge discussion
about it. And you needn't post it on python-ideas, you can post it on
python-dev instead.

For what it's worth, I'm  not as concerned with the process of the PEP, as having a single document we can review and discuss.

(and, oh, I agree there isn't a bright line separating "this should get
a PEP" from "this doesn't need a PEP")

Here are some things that I'm interested in right now:

   1) From what kind of data structure are signature objects generated?

   2) Is there an easy way to do this manually?

   3) Can at least part of the verboseness go into header files (virtually
      all compilers support "static inline" in header files)?

   4) Why don't approaches work that look simpler on the surface, like
      http://mail.python.org/pipermail/python-dev/2012-December/122934.html ?

   5) Are C contributors in general happy (or at least tolerate) the new
      verboseness?


Sure, we can discuss 1) to 3) here. For recording objections or approaches
that don't work (think FAQ "Why don't you just do X?") a PEP would be nice
to have.


If I'm the only one who is slightly bothered by the aesthetic and readability
aspects, then 5) obviously need not be discussed. Most people, however, will
probably find out about the change only once Python 3.4 is released.

I disagree that the Clinic DSL is "verbose".  Certainly I find it
more succinct than what we do now.

On the other hand, the syntax you proposed in the python-dev message
you cite is insufficient to the task.  Consider a function that
takes a "char *".  How might you want Python to convert the Python
value into that char *?  Should it only accept bytes?  Should it
accept bytes and Unicode?  Should it accept objects exposing the
buffer protocol?  Do you want the length too?  If you want the
length, does that mean that you accept nulls inside the string?
If it's Unicode, do you want to use a default encoding, or do you
want to specify an encoding?  Or do you want to specify your own
conversion function?

My solution for the above is to add "flags".  Which is,
semantically, the only important distinction between what you
propose and Clinic's DSL.  (That and the per-argument docstrings,
which I already suggest are negotiable.)  The remaining differences
are that you want to cram everything onto one line, wheras Clinic's
DSL spreads things over multiple lines.  I suggest the latter is
more readable, particularly when the number of arguments grows
large.

To answer 5), you're the only C contributor who comes to mind who
seems unhappy.  Do I understand you correctly that your main pain
point is that you generate scores of identical functions with the
C preprocessor, and you see no way to do that with Clinic?  If we
figured out a way to make that work with Clinic, would that reduce
your concerns?

Le dimanche 09 décembre 2012 à 19:20 +0000, Stefan Krah a écrit :
> 
> If I'm the only one who is slightly bothered by the aesthetic and readability
> aspects, then 5) obviously need not be discussed.

You're not the only one. The vertical space argument also resonates with
me, although I'm not sure how easy it would be to put the generated
boilerplate into separate include files.

The DSL is a bit weird, it doesn't seem to have consistent typography
rules, some lines end with a colon, some lines don't, some assignment
signs ("=") have spaces around them, some don't.

What lines end with a colon?

> What lines end with a colon?

He probably means semicolon, for example:

+    int dir_fd = DEFAULT_DIR_FD;
+    default=None

> > What lines end with a colon?
> 
> He probably means semicolon, for example:
> 
> +    int dir_fd = DEFAULT_DIR_FD;
> +    default=None

Oops, yes, sorry. Semicolon indeed.

Ah.  In that case, may I rewrite your critique as

"[...] some lines may end with a semicolon, some lines may not, some assignment signs ("=") permit spaces around them, some don't."

The semicolon is optional, permitted explicitly so you can copy-and-paste the original C variable declarations in and you don't have to go hunting for semicolons.  The spaces are permitted on those lines for the same reason; they aren't on the "flag" lines because of how flags are parsed.

I concede that this is inconsistent.  If this inconsistency bothers you, I invite you to propose a different (and presumably more consistent) syntax for the DSL.  I happen to think the current syntax has a motley "practicality beats purity" charm, but then I would, wouldn't I.

> The semicolon is optional, permitted explicitly so you can
> copy-and-paste the original C variable declarations in and you don't
> have to go hunting for semicolons.  The spaces are permitted on
> those lines for the same reason; they aren't on the "flag" lines
> because of how flags are parsed.

The syntax being liberal sounds ok, but I think our coding style should
be consistent.

> I happen to think the current
> syntax has a motley "practicality beats purity" charm, but then I
> would, wouldn't I.

Are we a motley crew?

How Argument Clinic solves problems like #16490?

Larry Hastings <report@bugs.python.org> wrote:
> I disagree that the Clinic DSL is "verbose".  Certainly I find it
> more succinct than what we do now.

I was referring to the total size of the generated output, not to the DSL.

> On the other hand, the syntax you proposed in the python-dev message
> you cite is insufficient to the task.
[...]

Actually this was Stefan Behnel's suggestion. Thanks for the explanation.

> To answer 5), you're the only C contributor who comes to mind who
> seems unhappy.  Do I understand you correctly that your main pain
> point is that you generate scores of identical functions with the
> C preprocessor, and you see no way to do that with Clinic?  If we
> figured out a way to make that work with Clinic, would that reduce
> your concerns?

No, the main problem is the amount of vertical space that's consumed.
I've looked at os_access etc., and it seems that about 100 lines will
be produced per function. The size of _decimal.c would go up from 5700
to something like 15000 lines.

You can do similar math for posixmodule.c.

Even with tools like ctags, I find code with a huge proportion of boilerplate
harder to understand and to navigate.

Also, I think that specifications of a declarative nature logically belong into
header files, but currently I don't see a nice way to make that happen.

doesn't the size of code setting up ParseTupleWithKeywords and checking the values afterwards bother you?  that's the same thing only much less consistent.

Gregory, I'm talking about the size of the *total output* of the tool, not
about the size of the actual DSL parts:

posixmodule.c:     11382    11514    +132     (2 functions)
_cursesmodule.c:    3434     3499     +65     (1 function)
zlibmodule.c:       1295     1336     +41     (1 function)
_dbmmodule.c:        437      488     +51     (1 function)

To this you have to add the proposed data structure for the signature
information (one per function, if I understand correctly).

For _decimal.c the situation is worse, since all docstrings are currently
in a header file. So I think that my estimate of +100 lines per function
is not exaggerated.

I don't think we can solve the problem of the output being too long for your liking.  Personally I like the output; I find it eminently readable, and making it shorter would impair that.  I think in the majority of uses Clinic will be a win for readability.

I do have one suggestion that might mitigate the problem for you.  What if you put all the argument processing in a separate file (with .c or .h extension) and #include that in _decimal.c?  You don't *have* to have the body for the impl function immediately after the DSL block; you could just have a semicolon there after the block and the C compiler would interpret it as a (redundant) declaration.  You'd have to maintain declaring the impl yourself in _decimal.c, but I'm guessing you'd rather do that than have all the Clinic stuff blowing up _decimal.c.

Finally, since you have not addressed it directly, let me ask you: are you interested in Clinic having a more boilerplate-friendly macro processing mode, with templates or recycling old entries or something?  Or do you not care?  If it doesn't help you then I may as well not bother.

Larry Hastings <report@bugs.python.org> wrote:
> Finally, since you have not addressed it directly, let me ask you:
> are you interested in Clinic having a more boilerplate-friendly macro
> processing mode, with templates or recycling old entries or something?
> Or do you not care?  If it doesn't help you then I may as well not bother.

Thanks for the offer. -- Currently I think that for me the easiest option is
to add the signature information manually. So unless other people want this
feature, I'd say don't bother implementing it.

I'll ask again, do you plan to write a PEP to iron out the functional
details? I don't think a tracker entry is the right format for this.

Le vendredi 14 décembre 2012 à 07:21 +0000, Larry Hastings a écrit :
> Larry Hastings added the comment:
> 
> I don't think we can solve the problem of the output being too long
> for your liking.  Personally I like the output; I find it eminently
> readable, and making it shorter would impair that.  I think in the
> majority of uses Clinic will be a win for readability.

I have no current plan to write a PEP.  I don't know how to settle the difference of opinion here; the easiest thing would be if you convinced Guido we needed one.

There's a bit of an impasse here on the PEP front.  Guido said it wasn't needed.  Antoine brings up the point that CPython developers will have to live with the result of this work so maybe something more easy to read and see in one place like a PEP would be valuable...

That said, I think a lot of that has been done if you look at the clinic.txt file within the patch.  That already contains the meat of what would go into a PEP.

What more is required?  I personally think we should iterate on this via checked in code in 3.4.

> I have no current plan to write a PEP.  I don't know how to settle the
> difference of opinion here; the easiest thing would be if you
> convinced Guido we needed one.

This is silly. Nobody is opposed to you writing a PEP and a couple
people would like you to do it. What needs settling exactly?

PEPs are perceived as a hurdle. Regardless, clinic.txt would turn into one
pretty easily so just doing it may be easiest. :)

On Fri, Dec 14, 2012 at 12:11 PM, Antoine Pitrou <report@bugs.python.org>wrote:

>
> Antoine Pitrou added the comment:
>
> > I have no current plan to write a PEP.  I don't know how to settle the
> > difference of opinion here; the easiest thing would be if you
> > convinced Guido we needed one.
>
> This is silly. Nobody is opposed to you writing a PEP and a couple
> people would like you to do it. What needs settling exactly?
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue16612>
> _______________________________________
>

> That said, I think a lot of that has been done if you look at the
> clinic.txt file within the patch.  That already contains the meat of
> what would go into a PEP.

Certainly. A PEP doesn't have to be a 100% new text.

First off, thanks for all the work so far. This has proven incredibly useful to me in a personal project.

However, I think there needs to be some additional discussion of how to
handle situations where the arguments passed to PyArg_ParseTuple require
additional cleanup.

As a prototypical example, I'll consider filename arguments.
The Python docs recommend that filename arguments be handled with
`O&` and PyUnicode_FSConverter. How can we handle this in clinic?

1. No special handling in clinic:
  /*[clinic]
  foo -> None

      PyObject *filename
  [clinic]*/
  ...
  foo_impl(PyObject *self, PyObject *filename)
  /*[clinic end:...*/
  {
      char *c_filename;
      PyObject *b_filename;
      if (!PyUnicode_FSConverter(filename, &b_filename))
          return NULL;

      c_filename = PyBytes_AsString(b_filename);

      // ...

      Py_DECREF(b_filename);
  }

This offloads all of the processing to the impl function and leaves us
no better off. Unacceptable.

2. Use PyObject* and a converter option:

  /*[clinic]
  foo -> None

      PyBytesObject *filename
      converter=PyUnicode_FSConverter
  [clinic]*/
  ...
  foo_impl(PyObject *self, PyBytesObject *filename)
  /*[clinic end:...]*/
  {
      char *c_filename = PyBytes_AsString(filename);

      ...
      Py_DECREF(filename);
  }

This is much more convenient, but the `_impl` function now steals the
filename reference, which is unexpected (and confusing).

3. "The dream"

Ideally `foo_impl` would have a signature like:

  static PyObject *
  foo_impl(PyObject *self, char *filename);

And `foo` would be automatically generated as:

  static PyObject *
  foo(PyObject *self, PyObject *args, PyObject *kwargs) {
    PyObject *_ret;
    PyObject *filename;
    static char *_keywords[] = {"filename", NULL};
    if (!PyArg_ParseTupleAndKeywords(args, kwargs,
        "O&:foo", _keywords,
        PyUnicode_FSConverter, &filename))
        return NULL;

    _ret = foo_impl(self, PyBytes_AsString(filename));
    Py_DECREF(filename);
    return _ret;
  }

It's not clear to me how one would extend the clinic syntax to support
this. In particular, clinic would need to know:
 - The type of the intermediate object (i.e., PyObject * or 
   PyBytesObject *).
 - The converter to use in PyArg_ParseTupleAndKeywords (i.e.,
   PyUnicode_FSConverter)
 - The impl type (i.e, char *)
 - How to convert the intermediate object to the impl type (i.e.,
   PyBytes_AsString(filename)).
 - How to cleanup in the end (i.e., Py_DECREF(filename)).

This seems like too much data to encode in the clinic syntax.

4. Extend clinic to add a cleanup flag which would produce code like:

  /*[clinic]
  foo

      PyBytesObject *filename
      converter=PyUnicode_FSConverter
      cleanup=Py_DECREF

  [clinic]*/
  ...
  static PyObject *
  foo(PyObject *self, PyObject *args, PyObject *kwargs) {
    PyObject *_ret;
    PyBytesObject *filename;
    static char *_keywords[] = {"filename", NULL};
    if (!PyArg_ParseTupleAndKeywords(args, kwargs,
        "O&:foo", _keywords,
        PyUnicode_FSConverter, &filename))
        return NULL;

    _ret = foo_impl(self, filename);
    Py_DECREF(filename);
    return _ret;
  }

  static PyObject *
  foo_impl(PyObject *self, PyBytesObject *filename)
  /*[clinic end:...]*/
  {
     char *c_filename = PyBytes_AsString(filename);
     // ...
  }

This seems like a relatively modest addition, which might also work for
other cleanup functions like PyBuffer_Release.

----

Additionally, there are a few other bugs I've noticed:
- The s* and z* codes should be of type Py_buffer (and not Py_buffer *)
- Since Py_buffer is a relatively large struct, zlib_decompress_impl
  should probably take a pointer to a Py_buffer.  This, however, would
  likely require extending the clinic syntax.

+1 for documenting the clinic DSL as a PEP. It's a bit too hard to find the rationale for toolchain changes like this when they're hidden away in tracker issues.

It doesn't need to be an extended essay like most of the PEPs I write, it can be relatively short and sweet (consider the PEP that approved the addition of unittest.mock: http://www.python.org/dev/peps/pep-0417/)

As Greg suggested, taking the current clinic.txt and publishing it as a "DSL for C API argument processing" PEP would get you most of the way there.

Here's a proposal for an alternative without parameter docstrings and a
different DSL (see os_stat.c). I guess it's easiest to present my thoughts
in list form.


Changes and rationale:
======================

  Split docstring into function header and rest
  ---------------------------------------------

    - Since the docstrings aren't repeated, less vertical space is used.

    - The main part of the docstring can go into a header file.

    - It's (IMO) easier to compare the generated header (see OS_STAT_HEADER)
      to the specification in the comment.


  More formal DSL
  ---------------

    This is my personal opinion: The existing DSL is fine for a configuration
    file (think .hgrc), but I have trouble with it in the context of a C file.

    Most importantly, I'm unable to take in the required information at a
    single glance.

    So I propose to make the structure of the specification explicit. For
    me the result is more readable. Also, it's already pretty close to a formal
    grammar and can be optionally condensed into single lines.

  Logical grouping
  ----------------

    The preprocessor comment, OS_STAT_HEADER and the os_stat() definition are
    close together and fit on a single screen.

> Here's a proposal for an alternative without parameter docstrings and
> a
> different DSL (see os_stat.c). I guess it's easiest to present my
> thoughts
> in list form.

It's a bit difficult to give an opinion without a more formal definition.
For example it seems you are using REQUIRED and KEYWORD as opposites,
but a required argument can also be a keyword argument.

As for the docstring: I would like it better if I could avoid typing
the cumbersome "\n\"s.

As for the general parameter declaration syntax: I think it shouldn't
be too verbose, otherwise it will quickly become tiring.
(also I don't think it should be required to write "[preprocessor]"
twice)

> As for the docstring: I would like it better if I could avoid typing
> the cumbersome "\n\"s.

I agree with Stefan that the file is a lot more readable if the docstring
is not repeated twice. It's unfortunate that C doesn't have a notion of
a raw string (as opposed to C++11 with the new R"(...)" syntax) but I
think it is something we'll have to live with. I would have expected
that a good text editor would be able to convert a selected region into a
C string, but I've never actually seen such a feature.

In general I think we should aim for clarity in scope of the arguments in
the DSL -- either by using curly-braces (a C construct) or indentation (a
Python construct). To minimize the usage of vertical space, I'd like to
see the DSL not require a blank line between arguments.

In a project I worked on recently I ended up writing a parser to go
through a list of C arguments and automatically produce the
PyArg_ParseTuple / PyArg_ParseTupleAndKeywords lines. It's not as
full featured as what we are looking for here, but it did have the
benefit of minimizing the number of extra vertical lines.  For example::

    static PyObject *
    w_rktime(PyObject *self, PyObject *args, PyObject *kwargs)
    {
        /*[kwargs rktime]*/
        darray u;
        meshdata msh;
        double dt;
        int nsteps=1;
        /*[/kwargs]*/
        static char *_keywords[] = {"u", "msh", "dt", "nsteps", NULL};
        if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O&O&d|i:rktime", _keywords, view_as_darray, &u, DgMeshData_Converter, &msh, &dt, &nsteps))
            return NULL;
        /*[checksum=...]*/
        ...
    }

I could imagine extending such a syntax to allow custom converters
and other extensions using comments::

    path_t path = PATH_T_INITIALIZE("stat", 0, 1)
        /* converter = path_converter; default = None */;
    int dir_fd = DEFAULT_DIR_FD
        /* converter = OS_STAT_DIR_FD_CONVERTER */;

The biggest downside to this approach would be that the parser could
not inject C code directly into the global scope -- instead it would
be limited to producing #define lines.

-Brad

I'm not sure I correctly understand skrah's proposal.  If I do, then

(1)  The first several lines ( "/* pymacro.h */" until "/* could go into a separate header file */" ) would not be written at all, and are just there to help reviewers understand.

(2)  The "#define OS_STAT_DOC ..." line is the docstring, and would be needed, but could easily go into a separate header file, like os_stat__doc.h.  For something like cdecimal, there might be only a single _cdecimal__doc.h containing all the docstrings.  There might even be a build switch that (at a minimum) replaced anything from those __doc.h files with "" for space-constrained builds.

(3)  The human-maintained code would be the DSL between "/*[preprocessor]" and "[preprocessor]*/".

(4)  The lines between "[preprocessor]*/" and "/*[preprocessor end:f3e6b328245207c240825782d908d3ff3a9c11c0]*/" would NOT be written or maintained by a human, but WOULD be checked into hg for the benefit builders without the argument-clinic tool.

(5)  The only C code written or maintained by a human (or another macro system) would be the last 5 lines (the system call, the path cleanup, and the return).

If I'm wrong about the above assumptions, then I think your proposal is insufficiently ambitious.  

If I'm correct, then your proposal boils down to

(1)  Allow (require?) the function-level docstring to be defined somewhere else, possibly in another file.

(2)  Change the DSL
  (2a)  Get rid of function flags?  (Not sure this is workable)
  (2b)  Replace the (previously proposed) parameter declarations with literal C code forming an array of [parameter kind, array-of-setup-instructions-and-or-magically-named-variable-settings]


More flexibility in building the docstring is probably good.  

The other changes -- I'm not sure I see the advantage, except that it might simplify writing the thing as a C pre-processing macro.

Jim Jewett <report@bugs.python.org> wrote:
> (1)  The first several lines ( "/* pymacro.h */" until "/* could go into a separate header file */" ) would not be written at all, and are just there to help reviewers understand.

Yes, they should ultimately go into Include/pymacro.h.

> (2)  The "#define OS_STAT_DOC ..." line is the docstring, and would be needed, but could easily go into a separate header file, like os_stat__doc.h.  For something like cdecimal, there might be only a single _cdecimal__doc.h containing all the docstrings.  There might even be a build switch that (at a minimum) replaced anything from those __doc.h files with "" for space-constrained builds.

Yes, it's supposed to be the main part of the docstring. The docstring header
containing the function signature would be autogenerated.

> (3)  The human-maintained code would be the DSL between "/*[preprocessor]" and "[preprocessor]*/".

Yes.

> (4)  The lines between "[preprocessor]*/" and "/*[preprocessor end:f3e6b328245207c240825782d908d3ff3a9c11c0]*/" would NOT be written or maintained by a human, but WOULD be checked into hg for the benefit builders without the argument-clinic tool.

Yes, all that code would be generated by clinic.py.

> (5)  The only C code written or maintained by a human (or another macro system) would be the last 5 lines (the system call, the path cleanup, and the return).

Correct.

> If I'm correct, then your proposal boils down to
> 
> (1)  Allow (require?) the function-level docstring to be defined somewhere else, possibly in another file.

Yes.

> (2)  Change the DSL
>   (2a)  Get rid of function flags?  (Not sure this is workable)

I didn't intend to but you're right, they were missing.

>   (2b)  Replace the (previously proposed) parameter declarations with literal C code forming an array of [parameter kind, array-of-setup-instructions-and-or-magically-named-variable-settings]

Regarding the DSL: I wanted to change the syntax, not the functionality.
Unfortunately, as Antoine pointed out, I didn't get it quite right.

(2b): Perhaps I misunderstand, but the snippets of literal C code are also
present in Larry's original:

/*[clinic]
os.stat -> stat result

    path_t path = PATH_T_INITIALIZE("stat", 0, 1);
    required
    converter=path_converter

    int dir_fd = DEFAULT_DIR_FD;
    default=None
    converter=OS_STAT_DIR_FD_CONVERTER
    keyword-only

    int follow_symlinks = 1;
    default=True
    types=bool
[clinic]*/

The motivation for trying to change the DSL is that I'd like to see
a) something that looks more like a C declaration, b) something that
is easily compressible vertically and c) some visual hints that
subdivide the declaration into sections.

Stefan, would you be willing to write up your version as a PEP? (FWIW, I
personally favour your suggested C'ish style for the preprocessor, since my
brain would be in that mode anyway when hacking on C code)

> I agree with Stefan that the file is a lot more readable if the 
> docstring
> is not repeated twice.

Couldn't the generated C docstring end up in a separate file? It's only a 
constant after all.
It seems to me that one reason we don't give much love to C docstrings is
that they're painful to edit, compared to Python docstrings.

I would be willing to write an (alternative) PEP, but I'm not quite satisfied
with my own proposal yet. Also, I'd need to understand the positional-only
part better, but the _cursesmodule.c example does not seem to compile here. :)

So, *disregarding* the positional-only situation, I've been thinking about
the os.stat example and what I want from the DSL:

1) We already have a partial DSL, namely "O&|$O&p". This is concise,
   unambiguous and well understood.

2) "O&" denotes a custom converter function with a user specified type.
    For example, path_converter() has type [string, bytes, int] -> path_t,
    where [string, bytes, int] are the alternatives that path_converter()
    accepts.

   "p" can be seen as a default converter function with type bool -> int.

3) Specifying the kind (required, etc.) and the default values of arguments
   is most readable in the standard Python form:

     os.stat(path, *, dir_fd=None, follow_symlinks=True) -> stat_result

I've tried to come up with a declaration that merges 2) and 3), while
reusing the existing conversion specifiers. It emphasizes the converter
functions (which ultimately decide the static typing):

/*[preprocessor]
# Declaration
os.stat (
  { path, path_converter : [string, bytes, int] -> path_t },
  *,
  { dir_fd=None, OS_STAT_DIR_FD_CONVERTER : [int, None] -> int },
  { follow_symlinks=True, "p" : bool -> int }
) -> stat_result

# User code
path_t path = PATH_T_INITIALIZE("stat", 0, 1);
int dir_fd = DEFAULT_DIR_FD;
int follow_symlinks = 1;
[preprocessor]*/

The declaration should be read as follows:

Declare os.stat as a function ...

   1) taking the required argument "path", to which the user-supplied function
      path_converter() of type [string, bytes, int] -> path_t will be applied
      for conversion.

   2) taking a keyword-only argument "dir_fd" with the default value None,
      to which the user-supplied function OS_STAT_DIR_FD_CONVERTER() of
      type [int, None] -> int will be applied for conversion.

   3) taking a keyword-only argument "follow_symlinks" with the default
      value True, to which the default converter "p" of type bool -> int
      will be applied for conversion.

... returning stat_result.

The user code section could be type-checked against the declaration.

What are the advantages?

  1) The structure of the Python function is part of the declaration.

  2) AFAICS, using the conversion specifiers directly eliminates the
     need for these flags:

       encoding, length, zeroes, bitwise, nullable, types, keyword-only,
       required, default, converter.

  3) The converter-oriented statically typed spec forces the user to think
     about the types (a good thing IMO).

  4) Due to the rigid structure (better) error messages should be easier
     to produce.

If this sounds like a viable alternative, I can try to write a PEP.

Stefan, that proposal definitely looks like it is worth writing up as a PEP to me. One thing that I particularly like about it is that it should be possible to pluck out the first element of the {} entries fairly easily in order to get the ordinary Python signature to feed to Cython or a "from string" constructor for signature objects.

As far as the "positional-only" parameters problem goes, at one point Guido was kicking around the idea of allowing "/" as a separator in Python function declarations to indicate positional only arguments. So the signature of a function that didn't accept keyword arguments at all would look like:

    def addch(x, y, ch, attr, /):
        ...

He eventually dropped it because positional only arguments (and the need to avoid colliding with arbitrary keyword arguments) are relatively rare in Python code, and using an inner function together with *args is a reasonable way to get decent error messages. However, as a way of concisely indicating positional-only arguments in the signature of *C* functions, the idea may be worth reconsidering.

OK, I'll have a go at the PEP then. In addition to the proposed syntax
in my previous mail, I'm going to suggest this alternative:

/*[preprocessor]
# Declaration
os.stat [PyOs_Stat] (
  { path: [string, bytes, int] => path_converter => path_t },
  *,
  { dir_fd: [int, None] = None => OS_STAT_DIR_FD_CONVERTER => int },
  { follow_symlinks: bool = True => "p" => int }
) -> stat_result

# User code
path_t path = PATH_T_INITIALIZE("stat", 0, 1);
int dir_fd = DEFAULT_DIR_FD;
int follow_symlinks = 1;
[preprocessor]*/

It's a slight abuse of notation, since it looks like function composition.
The advantage is that the LHS of the first arrow is a complete legal Python
argument spec together with the type annotation.

The construct should be viewed like a Unix pipe and be read somewhat like:

Pass argument dir_fd of type [int, None], initialized to the default value
None to function OS_STAT_DIR_FD_CONVERTER, which yields a C int.

Both versions carry precisely the same amount of information. I have no strong
preference for either version, but perhaps other people do.

Regarding the positional arguments with groups, I'm suggesting this construct
for window.addch():

/*[preprocessor]
# Declaration
window.addch [addch] (
  { y: int => "i" => int },
  { x: int => "i" => int },
  { ch: int => "O" => PyObject * },
  { attr: int => "l" => long }
) -> None

WHERE groups = { [ch], [ch, attr], [y, x, ch], [y, x, ch, attr] }
[preprocessor]*/

The top part is largely the same as in the os.stat() example, the WHERE
clause means:

  - If you pass a single argument, it will be interpreted as "ch"

  - If you pass two arguments, they will be interpreted as "ch, attr"

  [and so forth ...]

>  { path: [string, bytes, int] => path_converter => path_t },
>  *,
>  { dir_fd: [int, None] = None => OS_STAT_DIR_FD_CONVERTER => int },
>  { follow_symlinks: bool = True => "p" => int }

Why not just:

  path: path_t
  *
  dir_fd: dir_fd_t = None => DEFAULT_DIR_FD
  follow_symlinks: bool = True => 1

?

And register types somewhere:

clinic.register('path_t', restype='path_t', converter='path_converter', signature='[string, bytes, int]')
clinic.register('dir_fd_t', restype='int', converter='OS_STAT_DIR_FD_CONVERTER', signature='[int, None]')
clinic.register('bool', restype='int', converter='_PyBool_Converter', signature='bool')
...
clinic.register('string', restype='PyObject *', converter='_PyUnicode_Converter', signature='string')
clinic.register('buffer', restype='PyBuffer', converter='_PyBuffer_Converter', signature='buffer')
...
clinic.register('int', restype='int', converter='_Py_int_Converter', signature='int')
clinic.register('unsigned long', restype='unsigned long', converter='_Py_long_Converter', signature='int')

If you use path_converter, then definitely input types are [string, bytes, int] and an output C type is path_t. You need only name all converters and register them.

Serhiy Storchaka <report@bugs.python.org> wrote:
> >  { path: [string, bytes, int] => path_converter => path_t },
>
> And register types somewhere:

I must admit that I had a similar thought when I first heard about the project:
If we're going through all this anyway, why not have a registry with signatures
and type annotations for all functions?

But since we're dealing with C files, intuitively I think it's better to stick
to C conventions.

Imagine that the preprocessor had full-blown C parser support, so we could
declare:

  static int
  path_converter(/* [string, bytes, int] */ PyObject *o, void *p) {

Later we tell the preprocessor to generate a function taking an argument "path"
of type [string, bytes, int] that will be passed on to path_converter().

In the absence of type inference, it feels natural to me to declare the
type in both places.

What *is* a problem is that we need some discipline in case the type of
path_converter() changes at some point, since the C compiler won't know
about [string, bytes, int].

Okay, I have finally addressed all the comments so far.  Changes described below are my patch #2.  They're also checked in to https://bitbucket.org/larry/python-clinic/ .

* Antoine, Nick, et al: I've converted clinic.txt into a PEP.  I've
  already sent it to the PEP maintainer dudes.

* Chris Jerdonek: I've taken a stab at clearing up "parameter" versus
  "argument".  Please give it a fresh once-over (and let me know if
  I miscounted the angels dancing on the head of that pin :-p ).

* Bradley Froehle: I've added an experimental(!) extension mechanism,
  exactly for path_t arguments.  Please see Modules/posixmodule.c
  to see what it looks like.  (I knew I'd have a use for the [python]
  sections someday!)

* Antoine: I've implemented shunting the bulk of Argument Clinic's
  output into a separate file.  Please see Modules/zlibmodule.c
  and Modules/zlibmodule_clinic.c for more.

* Stefan Krah: I look forward to reading your PEP.  Hopefully I've saved
  you some work in the rationale, if referring to mine works for you.

I don't suppose we can get some decisions made and some code checked in before the PyCon sprints...?

Argument Clinic is now PEP 436.

http://www.python.org/dev/peps/pep-0436/

A quick note about the extension mechanism.  Currently the only way to extend PyArg_Parse* is via O&.  Therefore, any extended type you add will use O&, and will have a "converter".  So internally all I did was say "if the parameter has a converter, ignore the type and use the O& format unit".

This may not be the perfect extension API, if we change Clinic to using some new yet-to-be-defined API for argument parsing.  But I suspect it's pretty close.  Extension types will need a conversion function from PyObject to their type, they'll need a way of defining the C default value, and they'll need a way of cleaning up afterwards, all of which I already do.  So I guess I'm interested in feedback on the extension API too.

I still can't see a reference to Cython in the PEP. Larry, I don't really mind adding a newly designed DSL for this, but regarding the implementation of the actual argument parser, could you at least add a short paragraph to the PEP that mentions it as worth being looked at? I mean, it already does all of this and generates optimised and streamlined code for fast unpacking of args and kwargs, although just from a typed Python function signature, not a full fledged DSL. I mean, you don't have to reuse that code, but it's certainly relevant to see how others have solved this. Years ago.

As a rule I'm unlikely to mention things I haven't heard about.  I've never used Cython, and I don't recall anyone mentioning this technology previously.  Once skrah posts his alternative DSL proposal, I'll amend the PEP to discuss both these alternatives.

Can you point me to documentation on this Cython feature?

And, FWIW, the DSL was the interesting part for me.  I'm hoping someone else will step up and write the shiny new fast argument parsing API.  If it can be adapted out of existing working code, so much the better.

I mentioned it in a couple of places during the discussion, you might just have missed them.

The code that generates the unpacking C code starts here:

https://github.com/cython/cython/blob/4ebc647c2fa54179d25335b2dcf5d845e7fc9a79/Cython/Compiler/Nodes.py#L3068

and extends to here:

https://github.com/cython/cython/blob/4ebc647c2fa54179d25335b2dcf5d845e7fc9a79/Cython/Compiler/Nodes.py#L3641

As I said, Cython actually takes its signature information from normal Python signatures, which you can augment with type annotations in normal C style. So it's not really some fancy DSL behind it, just straight forward stuff like this:

    def func(a, int b, bytes c, unicode s, *, bint flag=False):
        ...

If you want to take a closer look, it's best to just write down a Python function and let Cython translate it for you to see the code that it generates for it.

And it's based on Cython's type system, so argument type conversions use Cython's support for things like copying mappings into structs, for example, or optimised builtin types unboxing.

> * Antoine: I've implemented shunting the bulk of Argument Clinic's
>   output into a separate file.  Please see Modules/zlibmodule.c
>   and Modules/zlibmodule_clinic.c for more.

Interesting, thanks :-)
Why do you need both clinic.write_clinic_file() and clinic.include_clinic_file() ?

> I don't suppose we can get some decisions made and some code checked
> in before the PyCon sprints...?

I suppose that depends on how quick everyone is.
But code can be written before the PEP is finally accepted, of course.

write_clinic_file tells Clinic to start writing to the clinic file, which I think is best to do at the very top of the file.  include_clinic_file spits out the #include, which you probably want near the bottom of the file, just before the static module declarations.  You can't put the #include at the top because it may refer to typedefs declared further down. 

Antoine Pitrou <report@bugs.python.org> wrote:

>
>Antoine Pitrou added the comment:
>
>> * Antoine: I've implemented shunting the bulk of Argument Clinic's
>>   output into a separate file.  Please see Modules/zlibmodule.c
>>   and Modules/zlibmodule_clinic.c for more.
>
>Interesting, thanks :-)
>Why do you need both clinic.write_clinic_file() and clinic.include_clinic_file() ?
>
>> I don't suppose we can get some decisions made and some code checked
>> in before the PyCon sprints...?
>
>I suppose that depends on how quick everyone is.
>But code can be written before the PEP is finally accepted, of course.
>
>----------
>title: Integrate "Argument Clinic" specialized preprocessor into CPython trunk -> Integrate "Argument Clinic" specialized preprocessor	into CPython trunk
>
>_______________________________________
>Python tracker <report@bugs.python.org>
><http://bugs.python.org/issue16612>
>_______________________________________

I've started working on the alternative DSL PEP, but I hit a DSL-independent roadblock:

The preprocessor passes copies of structs to the _impl functions,
which could lead to subtle bugs. I pointed out a benign example on
Rietveld, but in general I think this should not be done.

So the question is whether to add another flag or symbol that
determines if a pointer should be passed to the _impl function.

Thanks for pointing that out!  I've fixed it in my local branch, though I'm fooling around with some new syntax so I'm not ready to publish it yet.

It's not a general problem, exactly; it's a problem with extension types.  After just a little tweaking and refactoring, the extension interface now lets you override the behavior in a fine-grained way.  So path_t is declared on the stack, but a pointer is passed in to the impl.

Larry has requested privately that I send the counter proposal PEP and additional information, so here it is:

I've send the PEP draft to Nick. The patch that I uploaded contains
DSL examples, an ml-yacc grammar and token specifications.

Two prototype tools are available. Usage:

   ./printsemant Tools/preprocess/testcases/posix_stat.c

   ./preprocess Tools/preprocess/testcases/posix_stat.c 


The first tool dumps the semantically checked parse tree to stdout,
the second tool dumps the preprocessed .c file to stdout.

These are *prototypes* written in Standard ML, since I'm not familiar
with the PLY toolchain.


Known deficiencies:

  * The Python 'test' expression is not semantically checked. The syntax
    however is checked since it is part of the grammar.

  * The lexer does not handle triple quoted strings.

  * C declarations are parsed in a primitive way. The final implementation
    should utilize 'declarator' and 'init-declarator' from the C grammar.

  * The *preprocess* tool does not emit code for the left-and-right optional
    arguments case (addch.c). The *printsemant* tool can deal with this case.

  * Since the *preprocess* tool generates the output from the parse
    tree, the original indentation of the define block is lost.

Stefan's draft is now published: http://www.python.org/dev/peps/pep-0437/

Attached is a patch against current trunk merging Argument Clinic.  I already got the go-ahead to merge (took a poll in python-dev, got lots of +1s), but I still wanted to post it here to let it soak a little, even only if it's just a day.  Unless something amazing happens I plan to merge about 24 hours from now.

Ladies and gentlemen, fire up your Rietvelds!

Spelling corrections from Arfrever (thanks Arfrever!), and some eentsy teensy weentsy bugfixes from me.  And with a dramatic new patch number just so everybody can keep it straight.

Incorporated suggestions from Antoine and Berker Peksag.  Thanks!

This is awesome!

Clinic adds too much visual garbage. Could we move all generated content into separated *_clinic.h files?

I played with that at one point.  Guido suggested it was a bad idea.
If you redirect stuff into a second file then you have way more points
of failure--the files could get out sync, for example.  I tend to agree.

We could, however, change Clinic so it generated most of the junk at the bottom of the file, where you don't need to look at it.

Or we could copy a page out of the PyPy playbook: generate a mandlebrot
as a comment at column 0, then put the real text at column 80+.  In the
right editor you'd never see it again!

In any case let's leave it for now.  We can revisit it later once we've
had a little more experience living with Clinic.

> If you redirect stuff into a second file then you have way more points
of failure--the files could get out sync, for example.

Clinic can define a macro with a random value in a main file before including generated file and check this value by preprocessor in generated file. It will guard us from unintentional desynchronization.

I have added a handful of nitpicks on Rietveld.

> Or we could copy a page out of the PyPy playbook: generate a mandlebrot
> as a comment at column 0, then put the real text at column 80+.  In the
> right editor you'd never see it again!

That's only good if editing the file needs 3GB RAM.

Incorporated a small change from Antoine and a ton of small changes
from Serhiy.  Thanks guys!

New changeset 8fde1a2c94dc by Larry Hastings in branch 'default':
Issue #16612: Add "Argument Clinic", a compile-time preprocessor
http://hg.python.org/cpython/rev/8fde1a2c94dc

Checked in.  Thanks everybody!

_curses and _curses_panel fail to build after this commit on koobs-freebsd10 buildslave, with some lovely clang warnings too.

Attaching full log here.

New changeset 47618b00405b by Serhiy Storchaka in branch 'default':
Fix compilation of the curses module (broken by issue #16612).
http://hg.python.org/cpython/rev/47618b00405b

Here is a patch with yet some tiny changes (but they are too large for just comment). It corrects misleading docstrings of permute_*_option_groups() and simplifies rstrip_lines() and permute_optional_groups(). Fill free to apply those of them which look good to you.

New changeset f35b3a86ade3 by Meador Inge in branch 'default':
Fix minor bug in dict.__contains__ docstring.
http://hg.python.org/cpython/rev/f35b3a86ade3

Meador Inge: In the future, please create new issues for these sorts of fixes.

Will do, but in this case I didn't think a one character diff was worth
it.

If it's not worth a new issue, then you can check it in without citing an issue at all.