cppcheck icon indicating copy to clipboard operation
cppcheck copied to clipboard

fix #13378: syntax error for guessed macro value

Open ludviggunne opened this issue 1 year ago • 9 comments

Example output:

Checking tickets/13378.c ...
Checking tickets/13378.c: START...
tickets/13378.c:3:5: error: literal used as function. Macro 'START' expands to '1'. Use -DSTART=... to specify a value, or -USTART to undefine it. [unknownMacro]
    START();
    ^

With -DSTART the syntax error remains:

Checking tickets/13378.c ...
Checking tickets/13378.c: START=1...
tickets/13378.c:3:5: error: syntax error [syntaxError]
    START();
    ^

ludviggunne avatar Jan 13 '25 14:01 ludviggunne

You can specify actual macros via the CLI (at least with compilers): -D'name(args...)=definition'.

I used this recently somewhere in simplecpp or Cppcheck (most likely a local change) but I cannot remember where.

firewave avatar Jan 13 '25 14:01 firewave

You can specify actual macros via the CLI (at least with compilers): -D'name(args...)=definition'.

I used this recently somewhere in simplecpp or Cppcheck (most likely a local change) but I cannot remember where.

Hm, sorry but could you point out how this affects this PR?

ludviggunne avatar Jan 13 '25 15:01 ludviggunne

Use -DSTART=... to specify a value -> Use -DSTART(args...)=definition to specify it

And I guess having -USTART would only fix something if there was an #ifdef related to that macro. So maybe that should be dropped from the message.

This might also be addressed by fixing a missingInclude warning or providing a library.

On a side note - if we would apply that logic during the macro parsing we could (optionally) generate a configuration which already provides these - in some cases. That would require less user interaction. But that is obviously out of scope.

firewave avatar Jan 13 '25 15:01 firewave

And I guess having -USTART would only fix something if there was an #ifdef related to that macro. So maybe that should be dropped from the message.

Ah, maybe I should have provided more context. Here's the code from the ticket:

void foo(void) {
#ifdef START
    START();
#endif
}

ludviggunne avatar Jan 13 '25 15:01 ludviggunne

Ah, maybe I should have provided more context.

My fault. I did not look at the ticket. But to be fair there are also no tests added.

But I guess you will also get that message if the preprocessor check is missing.

I will file tickets about my notes.

firewave avatar Jan 13 '25 16:01 firewave

But to be fair there are also no tests added.

Oh yes, just thought it would be good to have it up as a draft to get som input on the messages.

ludviggunne avatar Jan 13 '25 16:01 ludviggunne

Oh yes, just thought it would be good to have it up as a draft to get som input on the messages.

I think it is a good change we can built upon.

firewave avatar Jan 13 '25 16:01 firewave

This might also be addressed by fixing a missingInclude warning or providing a library.

That's a good point. I suppose it could make the message quite long, would it be appropriate to split it into multiple messages?

ludviggunne avatar Jan 14 '25 14:01 ludviggunne

That's a good point. I suppose it could make the message quite long, would it be appropriate to split it into multiple messages?

I would keep it simple as it is not mention it at all. This is another case where the list of suggestions could be endless and misleading.

firewave avatar Jan 14 '25 15:01 firewave