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

2015-11-06 Thread Piotr Dziwinski via cfe-commits
piotrdz abandoned this revision. piotrdz added a comment. @alexfh: Ah, I forgot about this review. I will mark it as abandoned, because I have already started work on new check for localizing variables. It will have the wider scope that Eugene proposed originally, that is to move variable decla

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

2015-11-03 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. Apparently, I forgot to submit the comments a looong time ago. Sorry for the delay. In http://reviews.llvm.org/D12473#236401, @alexfh wrote: > A high-level comment: > > It seems that the scope of the check is artificially made too narrow. The > check could actually look

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

2015-08-31 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. A high-level comment: It seems that the scope of the check is artificially made too narrow. The check could actually look at any POD variables declared unnecessarily far from their initialization and usages. And here the value of the check would also be much higher, if

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

2015-08-30 Thread Eugene Zelenko via cfe-commits
Eugene.Zelenko added a comment. Sorry, I was mislead by check name. http://reviews.llvm.org/D12473 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2015-08-30 Thread 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@lists.llvm.org> wrote: > On Sun, Aug

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

2015-08-30 Thread Aaron Ballman via cfe-commits
On Sun, Aug 30, 2015 at 4:53 PM, Piotr Dziwinski 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

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

2015-08-30 Thread Piotr Dziwinski via cfe-commits
@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. And when it comes to C, in wel

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

2015-08-30 Thread Aaron Ballman via cfe-commits
On Sun, Aug 30, 2015 at 4:39 PM, Piotr Dziwinski via cfe-commits 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

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

2015-08-30 Thread Piotr Dziwinski via cfe-commits
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

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

2015-08-30 Thread Eugene Zelenko via cfe-commits
Eugene.Zelenko added a comment. Just for reference: - Previous attempt: http://reviews.llvm.org/D7639. - Bugzilla request . http://reviews.llvm.org/D12473 ___ cfe-commits mailing list cfe-commits@lists.

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

2015-08-30 Thread Eugene Zelenko via cfe-commits
Eugene.Zelenko added a subscriber: Eugene.Zelenko. Eugene.Zelenko added a comment. I think extern "C" functions should be kept with (void). I may be mistaken, but looks like code doesn't check for in C++ mode. http://reviews.llvm.org/D12473 ___ cfe