[clang] -Wshadow option produces a warning when explicitly capturing class member using the same name

[flotsamarch]

Starting from Clang 17.0.0 (built from source, but this also reproduces on Compiler Explorer) using -Wshadow flag now produces diagnostics for this code:

struct A
{
    int b = 5;

    int foo()
    {
        return [b = b] () 
        {
            return b;
        } ();
    }
};

I couldn't find this change in changelogs and there is no test for this type of capture in warn-shadow-in-lambdas.cpp, so I think this is a bug that was introduced by the commit that fixes #61105.

While technically this might be the case of 'shadowing' (I can't make any references to standard that either support or contradict my opinion), I don't think it has ever been considered a 'shadowing' case which should produce diagnostics and has been a somewhat common C++ pattern.

I don't see much value in that warning, since the name b referring to original class member is not available inside lambda body, and and the name b referring to the lambda member becomes available only when the other b is already not. Should it be considered shadowing?

Expected behavior: No warning generated
Actual behavior: warning: declaration shadows a field of 'A' [-Wshadow] diagnostics produced

[myrrc]

This also reproduces with me on clang 15 and 16:

struct A {
  int a;
  void foo() {
    auto b = [a = this->a] {
 
    };
  }
};

In this scenario, a isn't available in lambda without capturing this so there is no real shadowing. Unfortunately, our project https://github.com/ClickHouse/ClickHouse/ is also built with -Werror so workarounds like [a1 = this->a] have to be written

[shafik]

CC @Fznamznon

[Rinn]

We're also encountering this in the UnrealEngine codebase as many of our compiles are done with -Wshadow and this has caused hundreds of new warnings. At the very least if this could be put on a separate warning flag that would be really appreciated, for the moment this will block us from updating clang as we don't want to disable -Wshadow entirely to work around it.

[Fznamznon]

I don't think it is 0fb84bc that is guilty since it adds an orthogonal error message, but I can take a look anyway.

[Fznamznon]

The new warning appeared due to https://reviews.llvm.org/D124351 . It added a new call to CheckShadow here:

llvm-project/clang/lib/Sema/SemaLambda.cpp

Line 1155 in 410bf5e

CheckShadow(CurrentScope, V);

I don't fully understand why it is required.
If it is, I wonder if we should put this new warning under -Wshadow-uncaptured-local like it happens in case of a function parameter or just a variable (not field decl) - https://godbolt.org/z/GWY9qe79r. Or for example check that the shadowed variable/field can't be referenced from lambda body anyway (due to it or this not being captured explicitly or not).
@cor3ntin I was wondering if you could help to understand the best way to fix, please?

[cor3ntin]

CheckShadow fails to filter out fields for captures.

--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -8351,10 +8351,12 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
 
   unsigned WarningDiag = diag::warn_decl_shadow;
   SourceLocation CaptureLoc;
-  if (isa<VarDecl>(D) && isa<VarDecl>(ShadowedDecl) && NewDC &&
+  if (isa<VarDecl>(D) && NewDC &&
       isa<CXXMethodDecl>(NewDC)) {
     if (const auto *RD = dyn_cast<CXXRecordDecl>(NewDC->getParent())) {
       if (RD->isLambda() && OldDC->Encloses(NewDC->getLexicalParent())) {
+        if(!isa<VarDecl>(ShadowedDecl))
+            return;
         if (RD->getLambdaCaptureDefault() == LCD_None) {
           // Try to avoid warnings for lambdas with an explicit capture list.
           const auto *LSI = cast<LambdaScopeInfo>(getCurFunction());

I was able to silence the warning with something along the lines of that.
I am not sure I will have time to submit a PR today so feel free to look more into it if you have time