hawq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul Guo <paul...@gmail.com>
Subject Re: Warnings and indent issues in HAWQ code.
Date Thu, 01 Dec 2016 06:17:37 GMT
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 <yyao@pivotal.io>:

> Great, +dev@hawq.incubator.apache.org
>
> On Thu, Dec 1, 2016 at 9:54 AM, Paul Guo <pguo@pivotal.io> 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 <rhuo@pivotal.io> wrote:
>>
>>> Strong +1 to move discussions about features, improvements, usages, etc
>>> in dev@hawq.incubator.apache.org and user@hawq.incubator.apache.org
>>> 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 <yyao@pivotal.io> wrote:
>>>
>>>> Thanks Paul for initiating this discussion, +1 for -Werror and using
>>>> pgindent.
>>>>
>>>> Besides, could we move such discussion to dev@hawq.incubator.apache.org,
>>>> it is perfect topic to discuss in open source community!
>>>>
>>>> On Wed, Nov 30, 2016 at 5:58 PM, Hong Wu <hwu@pivotal.io> 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 <pguo@pivotal.io> 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 <hwu@pivotal.io> 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 <pguo@pivotal.io>
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
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message