Page MenuHomePhabricator

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

Authored by gousemoodhin on Jun 20 2020, 2:16 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
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

Diff Detail

Event Timeline

gousemoodhin created this revision.Jun 20 2020, 2:16 PM

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].

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 retitled this revision from clang: Handle block scope to [clang] Handle block scope when using lambda inside if initializer.
gousemoodhin edited the summary of this revision. (Show Details)

Add new test case

NekoPes edited the summary of this revision. (Show Details)Jun 30 2020, 4:35 AM

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 added a subscriber: rsmith.Jul 1 2020, 1:23 PM

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.

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

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