[clang] Handle block scope when using lambda inside if initializer

[gousemoodhin]

Summary

[clang] Handle block scope when using lambda inside if initializer

When we use lambda inside "if" initializer, clang should honor block
scope for the names defined inside the block. clang should not produce
compilation error like "redefinition of <names>".

Code example:
 if(int foo = []() -> int {auto foo = int{}; return foo;}(); foo)
 {
 }

Bug: 46242

[riccibruno]

Hi! You need at least:

1. Tests.
2. A better description of the problem and why this is the right fix.
3. A more descriptive title prefixed with [clang].

[gousemoodhin]

In D82266#2105481, @riccibruno wrote:

Hi! You need at least:

1. Tests.
2. A better description of the problem and why this is the right fix.
3. A more descriptive title prefixed with [clang].

I will add test cases, and will provide a description of the problem in detail.

[gousemoodhin]

Add new test case

[NekoPes]

I have further reduced the problematic example. The gist is as follows:
Any name that appear in the init-statement of an if statement can no longer be redeclared inside a nested initializer lambda. This error happens even when the lambda does not capture any variables, i.e. a new block scope is introduced and the name can be legally reused.

The updated scope.cpp example is as follows:

void test() {
  if (int foo = []() -> int { 
        auto foo = int{};  // expected-error{{redefinition of 'foo'}}
        return foo;
      }();
      foo) {
  }
}

The "[]() -> int" suppressed one additional compiler error which is only a side effect of this bug.

I have tested the patch and it seems to works OK. However I am not that familiar with the clang/LLVM codebase.

Looking at C++17 draft standard, 9.4.1p3 the above init-statement can be transformed into the equivalent expression:

void test() {
  {
    int foo = []() -> int { 
      auto foo = int{};  // expected-error{{redefinition of 'foo'}}
      return foo;
    }();
    if (foo) {
    }
  }
}

Which Clang compiles without and error and, thereby, confirming the deficiency.

[dblaikie]

This might be marginally simpler:

void test() {
  if (int foo = [] { // expected-error{{redefinition of 'foo'}}
        int foo = 0;
        return 0;
      }();
      true)
    ;
}

Also the test should probably test some functionality more specific than "does anything other than crash". I'll leave that up to @rsmith to determine though.

[gousemoodhin]

Yes, I agree with David.
In current scenario, we are avoiding compiler crash.

@ Richard Smith - Please review changes, let me know your opinions.

clang/lib/Sema/IdentifierResolver.cpp

if (LangOpt.CPlusPlus) {
       // Names declared in the for-init-statement, and in the condition of if,
       // while, for, and switch statements are local to the if, while, for, or
       // switch statement (including the controlled statement), and shall not be
       // redeclared in a subsequent condition of that statement nor in the
       // outermost block (or, for the if statement, any of the outermost blocks)
       // of the controlled statement.
       //
       assert(S->getParent() && "No TUScope?");
-      if (S->getParent()->getFlags() & Scope::ControlScope) {
+      if ((S->getParent()->getFlags() & Scope::ControlScope) &&
+          !(S->getFnParent()->getFlags() & Scope::BlockScope)) {
         S = S->getParent();
         if (S->isDeclScope(D))
           return true;
       }
       if (S->getFlags() & Scope::FnTryCatchScope)
         return S->getParent()->isDeclScope(D);
     }
     return false;

clang/test/Misc/scope.cpp

+// RUN: not %clang_cc1 -c -std=c++17 -verify %s
+int test() {
+  if (int foo = [&]() { // expected-error{{redefinition of 'foo'}}
+        int foo = 0;
+        return foo;
+      }();
+      foo) {
+    return 0;
+  }
+}
 No newline at end of file