LegalizeAdulthood 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 ---------------- jrtc27 wrote: > 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 ();`) Well, in (modern) C you don't want to remove the `void` in `(void)` at all. This check applies only to C++ code, so we should be safe here. I'll add a test case just to cover it -- I think the correct result should be that the check leaves this instance of `(void)` intact. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116425/new/ https://reviews.llvm.org/D116425 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
