Re: Function-like macro with the same name as a typedef confuses Coccinelle

[Philippe Mathieu-Daudé]

On 4/2/20 2:06 PM, Markus Armbruster wrote:
> I discovered that Vladimir's auto-propagated-errp.cocci leaves
> hw/arm/armsse.c unchanged, even though it clearly should change it.
> Running spatch with --debug prints (among lots of other things)
>
>      let's go
>      -----------------------------------------------------------------------
>      -----------------------------------------------------------------------
>      parse error
>       = error in hw/arm/armsse.c; set verbose_parsing for more info
>      badcount: 7
>      bad: }
>      bad:
>      bad: static void nsccfg_handler(void *opaque, int n, int level)
>      bad: {
>      BAD:!!!!!     ARMSSE *s = ARMSSE(opaque);
>      bad:
>      bad:     s->nsccfg = level;
>      bad: }
>      parse error
>
> Alright, what's wrong with this?  ARMSSE is both a function-like macro
> and a typedef:
>
>      #define ARMSSE(obj) OBJECT_CHECK(ARMSSE, (obj), TYPE_ARMSSE)
>
>      typedef struct ARMSSE {
>          ...
>      } ARMSSE;
>
> This appears to spook Coccinelle badly enough to skip code using ARMSSE.
>
> If I rename the macro to ARMSSE_() just for testing, it produces the
> transformation I'd expect.
>
> Grepping for typedef names is bothersome, so I used ctags -x to build a
> cross reference table, and searched that for function-like macros
> clashing with typedefs.  Result:
>
> include/hw/arm/armsse.h:#define ARMSSE(obj) OBJECT_CHECK(ARMSSE, (obj), 
> TYPE_ARMSSE)
> include/hw/arm/armsse.h:} ARMSSE;
> include/hw/block/swim.h:#define SWIM(obj) OBJECT_CHECK(SWIM, (obj), TYPE_SWIM)
> include/hw/block/swim.h:} SWIM;
> target/rx/cpu-qom.h:#define RXCPU(obj) \
> target/rx/cpu.h:typedef struct RXCPU RXCPU;
> target/s390x/translate.c:#define BD(N, BB, BD) { BB,  4, 0, FLD_C_b##N, 
> FLD_O_b##N }, \
> hw/audio/ac97.c:} BD;
>
> The last one is a name clash in unrelated files; should not bother
> Coccinelle.
>
> The first three are due to QOM.  By convention, the name of the
> function-like macro to convert to the QOM type is the QOM type in
> ALL_CAPS.  Clash when the QOM type is ALL_CAPS already.

To add to this list, another problem I'm having is with QOM interfaces.

For example this line:

  isa_bus_dma(bus, ISADMA(isa1), ISADMA(isa2));

The definitions are:

#define ISADMA(obj) INTERFACE_CHECK(IsaDma, (obj), TYPE_ISADMA)
typedef struct IsaDma IsaDma;

This is used as opaque pointer, so it compiles fine, but coccinelle is 
confused because the actual 'struct IsaDma' is never defined.

The only 'documentation' I found about this is commit 00ed3da9b5 which 
describes this as 'common practice'.

> Clearly, Coccinelle is getting spooked to easily.  Regardless, three
> questions:
>
> 1. Are ALL_CAPS typedef names a good idea?  We shout macros to tell
> readers "beware, possibly magic".  Shouting other stuff as well
> undermines that.
>
> 2. The compiler is quite cool with us using the same name for a
> function-like macro and a not-function-like non-macro.  But is it a good
> idea?
>
> 3. Do we want to work around Coccinelle's parsing flaw?  Having it skip
> so much code is clearly suboptimal, and potentially dangerous.  The
> obvious work-around is to rename the three problematic types so they
> contain at least one lower-case letter.