RFC: Enabling -Wshadow, by default?

[Akseli Lahtinen]

Hi

On Monday 29 December 2025 18:46:32 Eastern European Standard Time Friedrich W. H. Kossebau wrote:
> Hi,
> 
> TL;DR  what is people's take on warnings about variable shadowing, useful or 
> going against naming habits? Would you prefer this being enabled by default 
> for any projects, at least with clang, would you at least for your project?
> 

I would not be against it, I've been bitten by it before. 

> 
> I just stumbled over a (non-effective) bug due to variable shadowing in 
> Okteta code, to discover that other than assumed by me by the default KDE 
> settings the compilers are not instructed to warn about shadowing of 
> variables. Checking with lxr.kde.org I only found some projects enable this 
> warning flag, like kmymoney or kdiff3. Now while not recently, I remember 
> that shadowing has been a category of issues causing bugs, especially on 
> refactoring existing code, where name usages are recombined in new scopes.
> 
> Playing around and talking to waqar, who just happened to have resolved some 
> shadowing issues in Kate (invent.kde.org/utilities/kate/-/merge_requests/
> 1971), it seems that while both gcc and clang support the -Wshadow flag, gcc 
> is complaining a bit too much, like over shadowing of private members of 
> base classes or around variables in lambdas vs. non-captured things (see 
> snippet below for gcc bug report links).
> 
> Clang warnings though seemed mostly reasonable, so for Okteta I now add 
> this, perhaps you might want to add this to your project as well:
> --- 8< ---
> if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
>     # Not setting for GNU due to too many warnings related to private 
> members of base classes or around lambdas
>     # see e.g. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56556 or 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79328
>     string(APPEND CMAKE_CXX_FLAGS " -Wshadow")
> endif()
> --- 8< ---
> 
> Now, fixing those warnings one finds there are some cases where the 
> shadowing might not really be a critical issue. E.g. names of local helper 
> variables which are later reused in another deeper scope, with little chance 
> to get this wrong:
> --- 8< ---
>     [...]
>     auto it = something();
>     if (it) 
>     [...]
>     for (...) {
>        [...]
>        auto it = somethingelse();
>        if (it)
>        [...]
>     }
> --- 8< ---
> In such cases it feels a bit annoying to have to invent a more complicated/
> non-obvious name just to avoid the warning, or seeing to move the first case 
> in a dedicated scope just for this sake.
> 
> Then, testing KCoreAddons, KConfig, KWidgetsAddons, & KXmlGui, with the 
> warning flag results in quite some related warnings. So one would assume 
> that quite some KDE projects would have a lot of places which trigger that 
> warning, and thus suddenly the build log would get noisy, while people are 
> currently not focused on this and no actual bugs might be uncovered by this.
> 
> So adding above snippet for everyone to KDECompilerSettings.cmake currently 
> might not be something to consider. Instead people first might want to 
> collect some experience with the flag when used on code across KDE projects? 
> And only once a critical numbers of projects have code prepared to deal with 
> that warning, one might switch this warning on by default.
> 
> What are your opinion on this?
> And what are your related experiences and approaches, also outside of KDE, 
> especially around code where the warning might conflict with non-risky 
> naming habits?
> 
> Cheers
> Friedrich
> 
> 
> 

Best regards,
- Akseli