Argument Clinic: multiple macro definition

[serhiy-storchaka]

BPO	23500
Nosy	@larryhastings, @serhiy-storchaka
Files	sample.c clinic_append.patch sample.c: Fixed sample file larry.ac_multiple_macro_definitions.diff.1.txt larry.ac_multiple_macro_definitions.diff.2.txt: Diff after running "make clinic" with this patch clinic_append_2.patch sample.c larry.ac_multiple_macro_definitions.diff.3.txt: Another approach for cleaning up the multiple macros problem. larry.ac_multiple_macro_definitions.diff.4.txt larry.ac_multiple_macro_definitions.diff.5.txt

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/larryhastings'
closed_at = <Date 2015-04-03.20:35:59.511>
created_at = <Date 2015-02-22.10:37:42.845>
labels = ['type-bug', 'expert-argument-clinic', 'build']
title = 'Argument Clinic: multiple macro definition'
updated_at = <Date 2015-04-03.20:35:59.509>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2015-04-03.20:35:59.509>
actor = 'larry'
assignee = 'larry'
closed = True
closed_date = <Date 2015-04-03.20:35:59.511>
closer = 'larry'
components = ['Build', 'Demos and Tools', 'Argument Clinic']
creation = <Date 2015-02-22.10:37:42.845>
creator = 'serhiy.storchaka'
dependencies = []
files = ['38205', '38328', '38329', '38491', '38492', '38494', '38495', '38538', '38821', '38822']
hgrepos = []
issue_num = 23500
keywords = ['patch', 'needs review']
message_count = 27.0
messages = ['236403', '236405', '236407', '237183', '237185', '237192', '237201', '238076', '238119', '238120', '238122', '238126', '238128', '238384', '238420', '239576', '239596', '240008', '240009', '240010', '240012', '240016', '240028', '240029', '240030', '240031', '240035']
nosy_count = 3.0
nosy_names = ['larry', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue23500'
versions = ['Python 3.5']

[serhiy-storchaka]

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    \

[larryhastings]

Can you give me a test case?

[serhiy-storchaka]

Oh, sorry. Here is a sample. Other sample see in bpo-23501.

[serhiy-storchaka]

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.

[serhiy-storchaka]

Here is a sample file generated by fixed clinic.py.

[larryhastings]

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.

[serhiy-storchaka]

Actually in this sample output no one #ifndef SPAM_METHODDEF stanza is needed, because SPAM_METHODDEF is defined in any case.

[serhiy-storchaka]

May be first commit this non-perfect solution? Generated code is correct, it is just not optimal. This issue is a dependency of bpo-23501, that is a dependency of bpo-23492, that is a dependency of my patch for optimizing argument parsing in 1-argument functions.

[larryhastings]

How about this approach? Only ever emit the #ifndef stanza once per symbol.

[larryhastings]

(see larry.ac_multiple_macro_definitions.diff.1.txt posted above)

[larryhastings]

Oops, I should have run "make clinic", so you could see all the changes that result from this patch.

[serhiy-storchaka]

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.

[serhiy-storchaka]

Yes, this works. Here is combined patch and proceeded sample file.

[larryhastings]

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.

[serhiy-storchaka]

This looks as generalization of my patch. It produces the same output. I left comments on Rietveld, but in any case the patch LGTM.

[serhiy-storchaka]

Could you please commit your patch (may be with changes) Larry?

[larryhastings]

I want to redo it--it's smelly. I hope to get it done this week.

[larryhastings]

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.

[serhiy-storchaka]

Looks as you didn't notice my comments on Rietveld.

[larryhastings]

I did, I just didn't respond. I'll do that now.

[larryhastings]

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.

[serhiy-storchaka]

larry.ac_multiple_macro_definitions.diff.5.txt LGTM.

[python-dev]

New changeset 25eef0ecb9c1 by Larry Hastings in branch 'default':
Issue bpo-23500: Argument Clinic is now smarter about generating the "#ifndef"
https://hg.python.org/cpython/rev/25eef0ecb9c1

[larryhastings]

Does this really need a backport to 3.4?

[serhiy-storchaka]

I think this is not needed.

[serhiy-storchaka]

Thank you Larry.

[larryhastings]

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).