Обсуждение: Proposal: adding --enable-shadows-warning
Hi Hackers, I had an experience where I made a patch ready locally, build passed without any warning, however, CommitFest CI failed witha compile waring of shadows variable. I felt strange at the time but now I understand that is because some compilers,like clang I am using, don’t enable -Wshadow by default. So obviously the CommitFest CI has enabled -Wshadow. Sucha situation is kinda wasting patch author’s time. To make hacker’s life easier, it would make sense to always enable “-Wshadow”, however, that may have a risk of breakingsome compilers. So thinking over, I just fell adding an opt-in “—enable-shadows-warnings” could be a solution. Before the option is added “./confgure”, a workaround is like: PROFILE=“-Wshadow” make, but that way is just a workaround,it’s easy to forget use the extra thing together with make. I have tried to add the option locally, but end up I gave up because of the issue I posted in a separate thread [1]. Once[1] is resolved and if no objection on this proposal, then I will be happy to create a patch to add this option. [1] https://postgr.es/m/913843DF-16BA-4241-8191-BF892049744A@gmail.com Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
> On 28 Nov 2025, at 10:02, Chao Li <li.evan.chao@gmail.com> wrote: > To make hacker’s life easier, it would make sense to always enable “-Wshadow”, however, that may have a risk of breakingsome 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? -- Daniel Gustafsson
> On Nov 28, 2025, at 17:20, Daniel Gustafsson <daniel@yesql.se> wrote: > >> On 28 Nov 2025, at 10:02, Chao Li <li.evan.chao@gmail.com> wrote: > >> To make hacker’s life easier, it would make sense to always enable “-Wshadow”, however, that may have a risk of breakingsome 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. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
Hi, On Fri, 28 Nov 2025 at 12:40, Chao Li <li.evan.chao@gmail.com> wrote: > > > On Nov 28, 2025, at 17:20, Daniel Gustafsson <daniel@yesql.se> wrote: > > > >> On 28 Nov 2025, at 10:02, Chao Li <li.evan.chao@gmail.com> wrote: > > > >> To make hacker’s life easier, it would make sense to always enable “-Wshadow”, however, that may have a risk of breakingsome 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