I am working on following warnings in my next relevant pull request, "-Wimplicit-function-declaration" "-Wgnu-variable-sized-type-not-at-end" "-Wtypedef-redefinition" "-Wincompatible-pointer-types" "-Wconstant-logical-operand" "-Wmemsize-comparison" "-Wnull-dereference" "-Wpointer-sign" "-Wint-conversion"
Please pick remaining warning types below: "-Wformat" "-Wformat-extra-args" "-Wuninitialized" "-Wunused-variable" "-Wunused-function" "-Wpointer-sign" "-Wmacro-redefined" "-Wmissing-prototypes" "-Wdeprecated-declarations" "-Wtautological-pointer-compare" Thanks, Hong 2016-12-05 13:52 GMT+08:00 Hong Wu <[email protected]>: > Ok, if we work together, that's sgreat! The remaining warning options > include: > > "-Wuninitialized" > "-Wtypedef-redefinition" > "-Wunused-variable" > "-Wunused-function" > "-Wgnu-variable-sized-type-not-at-end" > "-Wmissing-prototypes" > "-Wdeprecated-declarations" > "-Wmacro-redefined" > "-Wformat" > "-Wimplicit-function-declaration" > "-Wtautological-pointer-compare" > "-Wincompatible-pointer-types" > "-Wformat-extra-args" > "-Wconstant-logical-operand" > "-Wmemsize-comparison" > "-Wpointer-sign" > "-Wnumm-dereference" > "-Wpointer-sign" > "-Wint-conversion" > > If anybody picks some of them, please reply to this thread to avoid > repeated work. Thanks. > > > 2016-12-05 13:44 GMT+08:00 Lili Ma <[email protected]>: > >> I reviewed the PR too. >> >> This work is very important. Sometimes it reflects as a warning, but >> actually it's a potential bug which is not covered by our tests. >> >> Since there are a number of work there, let's work together to fix the >> warnings. >> >> Thanks Paul for heading up and thanks for Hong's work! >> >> Thanks >> Lili >> >> >> On Mon, Dec 5, 2016 at 1:26 PM, Paul Guo <[email protected]> wrote: >> >> > I just reviewed one of the warning fix related PRs which come from Hong. >> > >> > https://github.com/apache/incubator-hawq/pull/1036 >> > >> > it catches several obvious bugs. I guess applying -Werror and static >> code >> > check will keep helping the code quality of HAWQ. >> > >> > Thanks for Hong's work. >> > >> > >> > 2016-12-01 15:37 GMT+08:00 Hong Wu <[email protected]>: >> > >> > > Thanks Paul to point it out. I will start fixing these warnings as >> > possible >> > > as I can! >> > > >> > > Hong >> > > >> > > 2016-12-01 14:17 GMT+08:00 Paul Guo <[email protected]>: >> > > >> > > > I'm proposing the Macros below at first to help eliminate some >> "unused" >> > > > variable/argument warnings. >> > > > >> > > > Typical cases: >> > > > >> > > > 1) Variable is only used with some configurations, etc. val for >> > > > Assert(val). Then you could add the code below >> > > > to eliminate the warning when assert is not enabled. >> > > > >> > > > POSSIBLE_UNUSED_VAR(val); >> > > > >> > > > 2) For argument that is explicitly unused but might be kept for >> > > > compatibility, you could use UNUSED_ARG(). >> > > > >> > > > [pguo@host67:/data2/github/incubator-hawq-a/src/include]$ git diff >> > > > diff --git a/src/include/postgres.h b/src/include/postgres.h >> > > > index 1138f20..9391d6b 100644 >> > > > --- a/src/include/postgres.h >> > > > +++ b/src/include/postgres.h >> > > > @@ -513,6 +513,18 @@ extern void gp_set_thread_sigmasks(void); >> > > > >> > > > extern void OnMoveOutCGroupForQE(void); >> > > > >> > > > +#ifndef POSSIBLE_UNUSED_VAR >> > > > +#define POSSIBLE_UNUSED_VAR(x) ((void)x) >> > > > +#endif >> > > > + >> > > > +#ifndef POSSIBLE_UNUSED_ARG >> > > > +#define POSSIBLE_UNUSED_ARG(x) ((void)x) >> > > > +#endif >> > > > + >> > > > +#ifndef UNUSED_ARG >> > > > +#define UNUSED_ARG(x) ((void)x) >> > > > +#endif >> > > > + >> > > > #ifdef __cplusplus >> > > > } /* extern "C" */ >> > > > #endif >> > > > >> > > > 2016-12-01 10:26 GMT+08:00 Yandong Yao <[email protected]>: >> > > > >> > > > > Great, [email protected] >> > > > > >> > > > > On Thu, Dec 1, 2016 at 9:54 AM, Paul Guo <[email protected]> wrote: >> > > > > >> > > > >> Yes, strongly agree. >> > > > >> >> > > > >> By the way, We should keep updating your story progress on Apache >> > JIRA >> > > > if >> > > > >> your story is not confidential. >> > > > >> >> > > > >> We should try to avoid closing an Apache JIRA with just a simple >> > > > >> introduction, i.e. without analysis, without progress, without >> root >> > > > cause. >> > > > >> >> > > > >> On Thu, Dec 1, 2016 at 9:13 AM, Ruilong Huo <[email protected]> >> > wrote: >> > > > >> >> > > > >>> Strong +1 to move discussions about features, improvements, >> usages, >> > > etc >> > > > >>> in [email protected] and >> > [email protected] >> > > > >>> respectively! This improve interactions between us and >> developers >> > and >> > > > user, >> > > > >>> which in turn helps to grow hawq community in apache open source >> > > > society. >> > > > >>> >> > > > >>> >> > > > >>> >> > > > >>> Best regards, >> > > > >>> Ruilong Huo >> > > > >>> >> > > > >>> On Thu, Dec 1, 2016 at 8:59 AM, Yandong Yao <[email protected]> >> > wrote: >> > > > >>> >> > > > >>>> Thanks Paul for initiating this discussion, +1 for -Werror and >> > using >> > > > >>>> pgindent. >> > > > >>>> >> > > > >>>> Besides, could we move such discussion to >> > > > [email protected], >> > > > >>>> it is perfect topic to discuss in open source community! >> > > > >>>> >> > > > >>>> On Wed, Nov 30, 2016 at 5:58 PM, Hong Wu <[email protected]> >> wrote: >> > > > >>>> >> > > > >>>>> If we want to do this, I recommend to write the coding style >> > > inside a >> > > > >>>>> hawq.vim and a hawq.emacs file for developers to use. I agree >> > with >> > > > you that >> > > > >>>>> to fix compile warnings and coding styles including indents, >> we >> > do >> > > > this >> > > > >>>>> together with some bug fix which could minimize our effort. >> > > > >>>>> >> > > > >>>>> Hong >> > > > >>>>> >> > > > >>>>> On Wed, Nov 30, 2016 at 5:46 PM, Paul Guo <[email protected]> >> > wrote: >> > > > >>>>> >> > > > >>>>>> Sorry. I meant -Werror (Which means "warning as error"). >> > > > >>>>>> >> > > > >>>>>> I did not tried pgindent. Anyone knows this whether works for >> > us. >> > > By >> > > > >>>>>> the way, even this works I suspect there will be tons of >> > warnings >> > > > also then >> > > > >>>>>> we have to leave this to be fixed along with other fixes. >> > > > >>>>>> >> > > > >>>>>> Besides, pg have a coding style doc but that is very simple. >> I >> > saw >> > > > at >> > > > >>>>>> least our naming conventions are not uniform. This will be an >> > more >> > > > annoying >> > > > >>>>>> issue. >> > > > >>>>>> >> > > > >>>>>> On Wed, Nov 30, 2016 at 5:29 PM, Hong Wu <[email protected]> >> > wrote: >> > > > >>>>>> >> > > > >>>>>>> There is no harm to fix compile warnings. Just do it, paul! >> > > > >>>>>>> >> > > > >>>>>>> In "enforce -Werror (if gcc) in hawq" thread of dev mailing >> > list, >> > > > >>>>>>> we talked about opening "-Werror" but there were some >> concerns >> > > > about >> > > > >>>>>>> the portability issue. I think it's better to fix compile >> > > warnings >> > > > but do >> > > > >>>>>>> not open this option. >> > > > >>>>>>> >> > > > >>>>>>> For indent and code style, HAWQ contains a tool for check >> that >> > at >> > > > >>>>>>> "src/tools/pgindent" which is forked from original >> Postgres. I >> > > > think we >> > > > >>>>>>> should run the tool to check existing code and do some >> indent >> > > > fixes. >> > > > >>>>>>> Meanwhile, we should also use this tool to check our >> c-coding >> > > > indent before >> > > > >>>>>>> sending pull requests and reviewer should pay attention to >> > indent >> > > > too. >> > > > >>>>>>> >> > > > >>>>>>> Best, >> > > > >>>>>>> Hong >> > > > >>>>>>> >> > > > >>>>>>> On Wed, Nov 30, 2016 at 5:00 PM, Paul Guo <[email protected]> >> > > wrote: >> > > > >>>>>>> >> > > > >>>>>>>> Hi team, >> > > > >>>>>>>> >> > > > >>>>>>>> Long ago, I proposed to add "-Wall" (warning as error) >> option >> > in >> > > > >>>>>>>> the community and later found that is a big effort since >> > > warnings >> > > > are too >> > > > >>>>>>>> many, but I think the direction is right. I think we should >> > try >> > > > to fix >> > > > >>>>>>>> those warnings when you are modifying related source file, >> > > > besides, I found >> > > > >>>>>>>> indent issue is a bit severe, so if you encountered this >> > during >> > > > bug fixes >> > > > >>>>>>>> please fix them. All of these will improve the confidence >> of >> > > > users and >> > > > >>>>>>>> customers. >> > > > >>>>>>>> >> > > > >>>>>>>> Thanks. >> > > > >>>>>>>> >> > > > >>>>>>> >> > > > >>>>>>> >> > > > >>>>>> >> > > > >>>>> >> > > > >>>> >> > > > >>>> >> > > > >>>> -- >> > > > >>>> Best Regards, >> > > > >>>> Yandong >> > > > >>>> >> > > > >>> >> > > > >>> >> > > > >> >> > > > > >> > > > > >> > > > > -- >> > > > > Best Regards, >> > > > > Yandong >> > > > > >> > > > >> > > >> > >> > >
