I was reviewing a patch by Larry and I started commenting on some rather tasteless code, until I realized it was generated by Argument Clinic.

It would be nice if Argument Clinic added some markers, such as:

/* Start of code generated by Argument Clinic */

/* End of code generated by Argument Clinic */

You find the big
/*[clinic checksum: b6ded2204fd0aab263564feb5aae6bac840b5b94]*/
marker insufficient?

Perhaps this is simply something we will quickly get used to.  How about we let this sit for a while and see what other people think.

p.s. I accept your critique of Clinic's autogenerated code.

> You find the big
> /*[clinic checksum: b6ded2204fd0aab263564feb5aae6bac840b5b94]*/
> marker insufficient?

Well, it doesn't say anything except "checksum", and it doesn't seem
geared at humans (except perhaps those with a hex calculator in their
head :-)).

Clinic-generated code looks at first sight like code that would be
written by a human, so it's easy to get miffed.

I think it's possible to get used to the markers.  However, to bikeshed a
little, I would prefer "preprocessor" instead of "clinic", since jokes
tend to wear off if one sees then too often.

> However, to bikeshed a little, I would prefer
> "preprocessor" instead of "clinic", since jokes
> tend to wear off if one sees then too often.

"Argument Clinic" is the name of the tool.  The marker /*[clinic]*/
was chosen deliberately:

* If someone says "what the hell is this" a quick online search
  should turn up the answer quickly.

* It differentiates it from /*[python]*/, which allows one to embed
  raw Python code inside files.  ("print" is redirected to the output
  section.)

On 23 Nov 2013 08:05, "Larry Hastings" <report@bugs.python.org> wrote:
>
>
> Larry Hastings added the comment:
>
> > However, to bikeshed a little, I would prefer
> > "preprocessor" instead of "clinic", since jokes
> > tend to wear off if one sees then too often.
>
> "Argument Clinic" is the name of the tool.  The marker /*[clinic]*/
> was chosen deliberately:
>
> * If someone says "what the hell is this" a quick online search
>   should turn up the answer quickly.
>
> * It differentiates it from /*[python]*/, which allows one to embed
>   raw Python code inside files.  ("print" is redirected to the output
>   section.)

It also differentiates it clearly from the C preprocessor. If Guido can
name the language after a comedy troupe, Larry can certainly name the
custom code generator after one of their sketches :)

FWIW, I tend to start editing the generated code as well, so I agree this
is something for us to keep an eye on. If the habit doesn't go away, we may
want some more prominent markers.

Cheers,
Nick.

>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue19723>
> _______________________________________

I grepped for clinic in the source code and I have a hunch why this confusing: each clinic-generated block has *three* marker comments, each containing [clinic] or [clinic checksum: ...].

TBH I can't always tell on which side of the comment the generated code sits, so I agree it would be nice if there was an additional keyword clearly indicating begin/end.

Looking more carefully it seems the pattern is

/*[clinic]
.
. (this seems to be the clinic input)
.
[clinic]*/
.
. (this seems to be generated)
.
/*[clinic checksum: da39a3ee5e6b4b0d3255bfef95601890afd80709]*/

I expect things would be clearer to the uninitiated if instead they said something like:

/*[clinic input]
.
.
.
[clinic start generated code]*/
.
.
.
/*[clinic end generated code; checksum: da39a3ee5e6b4b0d3255bfef95601890afd80709]*/

Larry, would that be easy?

And for Python blocks would you suggest

   /*[python input]
   ...
   [python start generated code]*/
   ...
   /*[python end generated code; checksum: da39a3ee5e6b4b0d3255bfef95601890afd80709]*/

To answer your question: no, it wouldn't be hard.

Yes. Then I suggest working on a patch before people get too deep into the conversion project.

Why not just move most (except implementation function headers) generated code out to separate file? Right now the list of symbols in the Kate editor contains three names for every function: BINASCII_A2B_UU_METHODDEF, binascii_a2b_uu and binascii_a2b_uu_impl. This makes navigation cumbersome. Also when I search function name 
a2b_uu, I found 13 occurrences (it will be decreased to 2 or 3 if move generated code out). So search is not helpful too,

I use Emacs, so I have no idea what would help for Kate.  I suppose Kate is programmable?  Maybe you can add some kind of filter for this purpose?

If e.g. binascii_a2b_uu_impl were moved to a different file, how would binascii_a2b_uu call it without exposing the symbol externally?

> If e.g. binascii_a2b_uu_impl were moved to a different file, how would
> binascii_a2b_uu call it without exposing the symbol externally?

I think "stitching it with #include" is the only workable solution.
We already do this for e.g. stringlib.

And the stringlib situation confuses the hell out of me already.

> I use Emacs, so I have no idea what would help for Kate.  I suppose Kate is programmable?  Maybe you can add some kind of filter for this purpose?

In comparision with Emacs it is not programmable.

> If e.g. binascii_a2b_uu_impl were moved to a different file, how would binascii_a2b_uu call it without exposing the symbol externally?

No, I propose to move generated binascii_a2b_uu__doc__, BINASCII_A2B_UU_METHODDEF and binascii_a2b_uu to separate file, and left clinic input and binascii_a2b_uu_impl in main file. Then #include "binascii.clinic" should be added before the list of module functions.

In contrast to stringlib programmers should not look in these generated files.

Then the separate file version looks much worse to me.

If it's source code, programmers will need to examine it from time to time.  A more important distinction imo: stringlib is type-parameterized like some sort of prehistoric C++ template specialization.  Thankfully the gunk generated by Argument Clinic is free of such dizzying technology.


Another approach to making Clinic output easier to read--which I thought I also proposed at one point--would be to save up most of the Clinic output in an "accumulator", then emit the contents of the accumulator on demand.  Clinic could probably get away with only generating a prototype for the generated function, the macro for the methoddef, and the prototype (sans semicolon) for the impl.  The bulk of the generated code, the implementation of the generated function and the docstring, would go in the "accumulator" and could be tucked away anywhere in the file you like.  I could even make it idiot-proof; if you haven't emptied the accumulator before the end of the file, it could tack the correct Clinic block on the end for you.

Here's a quick mockup of what that would look like:

/*[clinic]

    zlib.compress
        bytes: Py_buffer
            Binary data to be compressed.
        [
        level: int
            Compression level, in 0-9.
        ]
        /

    Returns compressed string.

    [clinic]*/

    #define ZLIB_COMPRESS_METHODDEF    \
        {"compress", (PyCFunction)zlib_compress, METH_VARARGS, zlib_compress__doc__},

    static PyObject *
    zlib_compress(PyModuleDef *module, PyObject *args);

    static PyObject *
    zlib_compress_impl(PyModuleDef *module, Py_buffer *bytes, int group_right_1, int level)
    /*[clinic checksum: 9f055a396620bc1a8a13d74c3496249528b32b0d]*/
    {
        PyObject *ReturnVal = NULL;
        Byte *input, *output = NULL;
        unsigned int length;
        ...


Not sure how far this suggestion ever got; I think maybe I only showed it to Stefan Krah, who said it wouldn't help his use case so I dropped it.

Any good?

Antoine just suggested that, if we used this "accumulator" thing, we'd want a convention for where the generated text should go.  I actually have an answer for that: near the end, below the implementations of the module / class methods, but above the methoddef/type structures and the module init function.

My reasoning: when I navigate CPython C files implementing a module or a type, when I know what entry point I want I just search for its name.  When I don't know what I want, I jump to the end, then scroll up until I find the name in the init function or the structures.  So I wouldn't want the code at the very end; that would screw up that navigation mode.

> I think maybe I only showed it to Stefan Krah, who said it wouldn't help his use case so I dropped it.

I think we were talking about _decimal, where any tool will interfere with the 100% code coverage patches.  But that's a special case.

Patch attached.  I tweaked the punctuation in the last line, from this:

  /*[clinic end generated code; checksum: {checksum}]*/
                              ^         ^
to this:                      |         |
                              v         v
  /*[clinic end generated code: checksum={checksum}]*/

Guido, can you live with that?  I think it's a slight improvement but I'm not committed enough to fight about it.  I'll change it back if you want it your way.

The new syntax is fine; I was only giving an example.

I'm assuming this is sufficient.  If further bikeshedding is needed please reopen the issue.

New changeset aa153113d273 by Zachary Ware in branch 'default':
Issue #19723: Fix issue number typo in Misc/NEWS
http://hg.python.org/cpython/rev/aa153113d273

New changeset e634b377d5cc by Larry Hastings in branch 'default':
Issue #19723: Missed one conversion to the new Argument Clinic syntax.
http://hg.python.org/cpython/rev/e634b377d5cc