[PATCH] D12473: [clang-tidy] Add old style function check

[Aaron Ballman via cfe-commits]

On Sun, Aug 30, 2015 at 4:53 PM, Piotr Dziwinski <piotrdz at gmail.com> wrote:
> @Aaron: Yes, I'm aware of that. I wanted to show that my check does not take
> this into account.
>
> In C++ this code is equivalent, so I think nothing should be reported,
> unless we really want to get rid of that void, but I suppose this other
> check does it already.

I think that in C++, it would make sense for this to be reported as a
consistency issue. For instance, the declaration without void could be
in a header while the definition is in a source file.

> And when it comes to C, in well-formed C code, we should never see those two
> definitions of allright() and allright(void), as they are declarations of
> different functions. If I try to add empty bodies {} to define them, this
> example won't even compile.

Correct, but it also strikes me as weird that your test case is a .c
file and there are no diagnostics triggered helping the user to
understand that these are declaring different things. It's not until
you add the definition that you get a diagnostic. This shouldn't be a
clang-tidy diagnostic, but a frontend diagnostic. I'm not suggesting
you have to do that work, either. Merely commenting that 1) they're
different in C, 2) C++ programmer may be unaware of those differences,
3) this can cause real bugs, and 4) we currently fail to help the user
in a meaningful way.

http://coliru.stacked-crooked.com/a/59ba90b4d3ec5828

~Aaron

>
> So anyway, I still don't understand the problem here, unless somebody
> explains it in clearer terms.
>
> Best regards,
> Piotr Dziwinski
>
>
> On 2015-08-30 22:43, Aaron Ballman wrote:
>>
>> On Sun, Aug 30, 2015 at 4:39 PM, Piotr Dziwinski via cfe-commits
>> <cfe-commits at lists.llvm.org> wrote:
>>>
>>> piotrdz added a comment.
>>>
>>> @Eugene: I don't understand, what does declaring function with "void"
>>> argument have in common with this review? I only check here variable
>>> declarations inside functions.
>>>
>>> Maybe you meant my other review for inconsistent declaration parameter
>>> names? If so, this is how it behaves currently:
>>>
>>>    $ cat test.c
>>>    void allright();
>>>    void allright(void);
>>>
>>>    void notallright(int a);
>>>    void notallright(int b);
>>>
>>>    $ clang-tidy
>>> -checks='-*,readability-inconsistent-declaration-parameter-name' test.c --
>>> -std=c11
>>>    1 warning generated.
>>>    /work/clang-trunk/test.c:4:6: warning: function 'notallright' has
>>> other declaration with different parameter name(s)
>>> [readability-inconsistent-declaration-parameter-name]
>>>    void notallright(int a);
>>>         ^
>>>    /work/clang-trunk/test.c:5:6: note: other declaration seen here
>>>    void notallright(int b);
>>>
>>> So I see no reason to add specific checks for functions with "void"
>>> parameter, unless you see something wrong with this behavior?
>>
>> In C, void allright(); is a function accepting a variable number of
>> arguments, and void alright(void); is a function accepting no
>> arguments. In C++, they are both functions accepting no arguments.
>>
>> ~Aaron
>
>