[PATCH] D116425: [clang-tidy] Improve modernize-redundant-void-arg to recognize macro uses

Jessica Clarke via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 6 17:44:01 PST 2022


jrtc27 added inline comments.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-redundant-void-arg.cpp:561
+#define return_t(T) T
+return_t(void) func(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: redundant void argument list in function declaration
----------------
LegalizeAdulthood wrote:
> LegalizeAdulthood wrote:
> > aaron.ballman wrote:
> > > LegalizeAdulthood wrote:
> > > > aaron.ballman wrote:
> > > > > Can you also add a test for:
> > > > > ```
> > > > > void func(return_t(void));
> > > > > ```
> > > > `:-)`
> > > > 
> > > > What are you suggesting the result should be?  Honestly, looking at that, I'm not sure myself `:)`
> > > > 
> > > > IMO, if I saw this in a code review, I would flag it because you're using a macro called "return type" to specify the type of an argument.
> > > LoL, yeah, the name `return_t` would certainly be novel to use in a parameter list, but what I was hoping to test is whether we try to fix the use of the macro within the parameter list or not. I *think* it probably makes sense to issue the diagnostic, but I don't think it makes sense to try to fix it because the macro could be defined differently for different configurations. But the diagnostic is silenced as well as the fix-it, I wouldn't lose a whole lot of sleep over it.
> > Well it could conceivably be used to declare a function pointer argument like this:
> > 
> > `void func(return_t(void) (*fp)(void));`
> > 
> > In that case, my expectation is that the check would fix the void arg, but not the arg to the macro.
> OK, that was a good idea to add the test I described above because it failed `:)`,
> so let me improve the check some more.
If you want a less-contrived example that shows up all over the place in crusty old C code that supports (or, perhaps, supported and let bitrot support for) pre-ANSI C compilers:

```
#define __P(x) x
void foo __P((void));
```

(the idea being that, for pre-ANSI C compilers, you'd instead define __P(x) as () to get `void foo ();`)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116425/new/

https://reviews.llvm.org/D116425



More information about the cfe-commits mailing list