El dilluns, 29 de desembre del 2025, a les 17:46:32 (Hora està ndard d’Europa central), Friedrich W. H. Kossebau va escriure: > 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 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?
We enabled it in poppler both for gcc and clang. After the initial cleanup work I feel it gives more readable code. Cheers, Albert > > Cheers > Friedrich
