[Feature request] Warn when a macro argument is not used

[ISSOtm]

Macros will, logically, error out when they are passed too few arguments. But, by nature, extraneous arguments are simply ignored. This can be intentional, but I expect that this would be rare; so, I'm suggesting -Wunused-macro-arg enabled by -Wall.

In theory, we could also add some syntax for macros to specify how many arguments they want to be given; but I expect that adoption would be low, and thus I'm not sure it would be worth the implementation effort.

[Rangi42]

There are definitely some macros which just take a "flag" argument whose actual value doesn't matter (often it's 1, could be TRUE if the codebase uses such a constant, but 0 or foobar or whatever would also work) and just influences behavior of a _NARG check. So if we have a warning like this, there should be a convenient way to disable the warning, i.e. one that could be done in the definition of the macro, not at every usage site.

Syntax "to specify how many arguments they want to be given" sounds like it would develop into #912.

[Rangi42]

So, in this hypothetical:

macro foo
  pusho Wno-unused-macro-arg
    db \1, \3
  popo
endm
  foo 1, 2, 3

The popo would undo the Wno-unused-macro-arg before reaching the endm, but the unused argument check would have to occur at the endm. So this would still warn about \2 being unused.

On the other hand, users could just put a ; unused: \2 comment, if they want the warning enabled in general but not for this particular macro (the equivalent of (void)unused_arg; in C). Instances like this are rare enough that I think that would be acceptable.

(You could also do ; \# if you're not sure which args might be unused. Or /* \# */ if some of them might have newlines.)

[Rangi42]

Oops, we don't expand things in comments, so ; \# or /* \# */ wouldn't work. (And on reflection, that's a good thing, since if an argument contained a newline or */ then ending a comment on expansion would be really annoying.)

This also does not work:

if 0
  println "\#"
endc

(In hindsight that also makes sense. If you did if _NARG < 2 ... else ... endc, you wouldn't want \3 to expand in the else block. And we don't allow trickery like an arg that expands to a control-flow-changing endc anyway.)

Things that do work:

redef _discard equs "\#"

assert isconst("\#")

Or, of course, local opt Wno-unused-macro-arg or global -Wno-unused-macro-arg.

[mid-kid]

Would shift be considered using an argument? Because if so, I figure that would be enough.

I'm also not entirely sure who the warning is for. Is it for the user of the macro, who might've specified too many arguments? Or, is it for the implementor of the macro, who might've forgotten to do something?
RGBDS has no real macro specification, they're all positional, and the error for specifying too few only happens when it's referenced.

In my opinion, given this constraint, I see no real circumstance in which shift 3 would mean the implementor has forgotten to use the first three arguments, as they're clearly being acknowledged (a macro invocation with less than 3 arguments would error out).

[Rangi42]

Good point about shift; the current PR does not count that as a "use". Although I think most cases of false-positive unused-arg warnings don't even involve shift (note the update PR test cases, and the new complaints about building pokecrystal).