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

[don hinton via cfe-commits]

gcc has -Wstrict-prototypes which will catch this, but clang doesn't
implement it.

however, both gcc and clang have -Wmissing-prototypes which should catch
these if users enable it.


On Sun, Aug 30, 2015 at 5:32 PM, Aaron Ballman via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> 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
> >
> >
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150830/0892c85f/attachment.html>