classification
Title: Argument Clinic: multiple macro definition
Type: behavior Stage: resolved
Components: Argument Clinic, Build, Demos and Tools Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: larry Nosy List: larry, python-dev, serhiy.storchaka
Priority: normal Keywords: needs review, patch

Created on 2015-02-22 10:37 by serhiy.storchaka, last changed 2015-04-03 20:35 by larry. This issue is now closed.

Files
File name Uploaded Description Edit
sample.c serhiy.storchaka, 2015-02-22 11:08
clinic_append.patch serhiy.storchaka, 2015-03-04 15:10 review
sample.c serhiy.storchaka, 2015-03-04 15:12 Fixed sample file
larry.ac_multiple_macro_definitions.diff.1.txt larry, 2015-03-15 04:20 review
larry.ac_multiple_macro_definitions.diff.2.txt larry, 2015-03-15 04:25 Diff after running "make clinic" with this patch review
clinic_append_2.patch serhiy.storchaka, 2015-03-15 07:22 review
sample.c serhiy.storchaka, 2015-03-15 07:22
larry.ac_multiple_macro_definitions.diff.3.txt larry, 2015-03-18 08:51 Another approach for cleaning up the multiple macros problem. review
larry.ac_multiple_macro_definitions.diff.4.txt larry, 2015-04-03 16:11 review
larry.ac_multiple_macro_definitions.diff.5.txt larry, 2015-04-03 17:12 review
Messages (27)
msg236403 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-02-22 10:37
Argument Clinic generates multiple definitions in non-block mode for functions with alternating signature. See example file. If FOO is not defines, SPAM_METHODDEF is defined twice.

First time in:

#ifndef SPAM_METHODDEF
    #define SPAM_METHODDEF
#endif /* !defined(SPAM_METHODDEF) */

Second time in:

#if !defined(FOO)
...
#define SPAM_METHODDEF    \
msg236405 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-02-22 10:56
Can you give me a test case?
msg236407 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-02-22 11:08
Oh, sorry. Here is a sample. Other sample see in issue23501.
msg237183 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-04 15:10
Here is a patch which moves all methoddef_ifndef to the end of the buffer or file, so they are not conflicts with other definitions.
msg237185 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-04 15:12
Here is a sample file generated by fixed clinic.py.
msg237192 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-03-04 16:59
In your sample output we still get two #ifndef SPAM_METHODDEF stanzas.  Wouldn't it be better to only have one?

Maybe Clinic needs to be smarter about generating those anyway.  Let me think about it.
msg237201 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-04 19:04
Actually in this sample output no one #ifndef SPAM_METHODDEF stanza is needed, because SPAM_METHODDEF is defined in any case.
msg238076 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-14 10:23
May be first commit this non-perfect solution? Generated code is correct, it is just not optimal. This issue is a dependency of issue23501, that is a dependency of issue23492, that is a dependency of my patch for optimizing argument parsing in 1-argument functions.
msg238119 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-03-15 04:20
How about this approach?  Only ever emit the #ifndef stanza once per symbol.
msg238120 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-03-15 04:21
(see larry.ac_multiple_macro_definitions.diff.1.txt posted above)
msg238122 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-03-15 04:25
Oops, I should have run "make clinic", so you could see all the changes that result from this patch.
msg238126 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-15 06:35
It doesn't fix the issue, because the #ifndef stanza is emitted before second definition. Try to run clinic.py with your patch on sample.c. But may be this idea can be used with my patch.
msg238128 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-15 07:22
Yes, this works. Here is combined patch and proceeded sample file.
msg238384 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-03-18 08:51
What do you think of this approach?  Now a "Destination" object behaves like an array of text accumulators.  If you ask for one that doesn't exist it's created for you.  When the Destination is dumped, the output from each accumulator is joined together, like buffer[0] + buffer[1] + buffer[2].  (You can even specify negative indices, if you want text that goes *before* the default text accumulator.)

With this approach, all the #ifndef stanzas are at the end of the emitted text.
msg238420 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-18 11:21
This looks as generalization of my patch. It produces the same output. I left comments on Rietveld, but in any case the patch LGTM.
msg239576 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-30 07:18
Could you please commit your patch (may be with changes) Larry?
msg239596 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-03-30 11:53
I want to redo it--it's smelly.  I hope to get it done this week.
msg240008 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-04-03 16:11
As promised, here's a cleaned-up version of the patch.  The taxonomy of objects now makes sense: a Destination contains a BufferSeries object, rather than Destinations weirdly supporting __getitem__ to reference different objects.

I tripped over myself a little with the "two-pass" destination / preset.  It was an experiment long ago but nobody's using it.  So rather than fix it I just turned it off.
msg240009 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-04-03 16:47
Looks as you didn't notice my comments on Rietveld.
msg240010 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-04-03 16:51
I did, I just didn't respond.  I'll do that now.
msg240012 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-04-03 17:12
Updated patch, removed all references to two-pass.  Also realized I needed to make the default behavior for methoddef_ifndef go to the end.  And, finally, I forgot to merge the "only print each #ifndef block once" code I wrote before when I redid the patch, so that's in now.
msg240016 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-04-03 17:46
larry.ac_multiple_macro_definitions.diff.5.txt LGTM.
msg240028 - (view) Author: Roundup Robot (python-dev) Date: 2015-04-03 20:09
New changeset 25eef0ecb9c1 by Larry Hastings in branch 'default':
Issue #23500: Argument Clinic is now smarter about generating the "#ifndef"
https://hg.python.org/cpython/rev/25eef0ecb9c1
msg240029 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-04-03 20:09
Does this really need a backport to 3.4?
msg240030 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-04-03 20:16
I think this is not needed.
msg240031 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-04-03 20:17
Thank you Larry.
msg240035 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-04-03 20:35
Removing 3.4 from the version list as I close the bug, then.  If we decide we need it backported please reopen (or create a new bug, either is fine).
History
Date User Action Args
2015-04-03 20:35:59larrysetstatus: open -> closed
versions: - Python 3.4
messages: + msg240035

resolution: fixed
stage: commit review -> resolved
2015-04-03 20:17:22serhiy.storchakasetmessages: + msg240031
2015-04-03 20:16:55serhiy.storchakasetmessages: + msg240030
2015-04-03 20:09:58larrysetmessages: + msg240029
2015-04-03 20:09:18python-devsetnosy: + python-dev
messages: + msg240028
2015-04-03 18:43:52serhiy.storchakasetassignee: larry
stage: patch review -> commit review
2015-04-03 17:46:36serhiy.storchakasetmessages: + msg240016
2015-04-03 17:12:00larrysetfiles: + larry.ac_multiple_macro_definitions.diff.5.txt

messages: + msg240012
2015-04-03 16:51:32larrysetmessages: + msg240010
2015-04-03 16:47:02serhiy.storchakasetmessages: + msg240009
2015-04-03 16:11:34larrysetfiles: + larry.ac_multiple_macro_definitions.diff.4.txt

messages: + msg240008
2015-03-30 11:53:07larrysetmessages: + msg239596
2015-03-30 07:18:58serhiy.storchakasetmessages: + msg239576
2015-03-18 11:21:16serhiy.storchakasetmessages: + msg238420
2015-03-18 08:51:59larrysetfiles: + larry.ac_multiple_macro_definitions.diff.3.txt

messages: + msg238384
2015-03-15 07:22:30serhiy.storchakasetfiles: + clinic_append_2.patch, sample.c

messages: + msg238128
2015-03-15 06:35:09serhiy.storchakasetmessages: + msg238126
2015-03-15 04:25:07larrysetfiles: + larry.ac_multiple_macro_definitions.diff.2.txt

messages: + msg238122
2015-03-15 04:21:19larrysetmessages: + msg238120
2015-03-15 04:20:56larrysetfiles: + larry.ac_multiple_macro_definitions.diff.1.txt

messages: + msg238119
2015-03-14 10:23:28serhiy.storchakasetmessages: + msg238076
2015-03-04 19:04:23serhiy.storchakasetmessages: + msg237201
2015-03-04 16:59:43larrysetmessages: + msg237192
2015-03-04 15:12:42serhiy.storchakasetfiles: + sample.c

messages: + msg237185
2015-03-04 15:10:30serhiy.storchakasetkeywords: + needs review, patch
files: + clinic_append.patch
messages: + msg237183

stage: patch review
2015-02-25 15:30:19serhiy.storchakasetcomponents: + Argument Clinic
2015-02-22 11:08:43serhiy.storchakasetfiles: + sample.c

messages: + msg236407
2015-02-22 11:05:50serhiy.storchakalinkissue23501 dependencies
2015-02-22 10:56:53larrysetmessages: + msg236405
2015-02-22 10:38:05serhiy.storchakasetnosy: + larry
2015-02-22 10:37:42serhiy.storchakacreate