| From: | Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | Daniel Gustafsson <daniel(at)yesql(dot)se>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Proposal: adding --enable-shadows-warning |
| Date: | 2025-11-28 11:16:45 |
| Message-ID: | CAN55FZ2v5cTsYQ4tzTeLcPb-7iaiDXygmWD3MrsTtMn3S0nu2A@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Fri, 28 Nov 2025 at 12:40, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
> > On Nov 28, 2025, at 17:20, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
> >
> >> On 28 Nov 2025, at 10:02, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> >
> >> To make hacker’s life easier, it would make sense to always enable “-Wshadow”, however, that may have a risk of breaking some compilers. So thinking over, I just fell adding an opt-in “—enable-shadows-warnings” could be a solution.
> >
> > I'm not sure that adding buildsystem flags for compiler behaviour is the right
> > direction, isn't the environment variables like CFLAGS already covering this?
> >
>
> Like I explained in my first email, using PROFILE can append extra options to CFLAGS, I am always use this method to append ‘-O0’ to CFLAGS. However, this way is easy to missing the extra option to add. As CommitFest CI always enable -Wshadows, we should allow developers to always enable the option in local.
I think there might be misunderstanding here. Commitfest CI does not
enable '-Wshadow', it just sets '-Werror' before running the compiler
warnings checks. There is the '-Wshadow=compatible-local' flag which
is added at 0fe954c285 and it is always enabled when it is supported.
--
Regards,
Nazir Bilal Yavuz
Microsoft
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Eisentraut | 2025-11-28 11:38:59 | Re: headerscheck ccache support |
| Previous Message | jian he | 2025-11-28 11:00:08 | refactor ExecInitPartitionInfo |