Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Variable initialized after equally named capture rejected #1312

Open
JohelEGP opened this issue Oct 11, 2024 · 1 comment
Open

[BUG] Variable initialized after equally named capture rejected #1312

JohelEGP opened this issue Oct 11, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@JohelEGP
Copy link
Contributor

JohelEGP commented Oct 11, 2024

Title: Variable initialized after equally named capture rejected.

Minimal reproducer (https://cpp2.godbolt.org/z/MPYnY1qbM):

main: () = {
  test_overloads := 0;
  _ = :() = { test_overloads := test_overloads$; };
}
Commands:
cppfront main.cpp2
clang++-18 -std=c++26 -stdlib=libc++ -lc++abi -pedantic-errors -Wall -Wextra -Wconversion -Werror=unused-result -Werror=unused-value -Werror=unused-parameter -Werror=unused-variable -I . main.cpp

Expected result: A working program.

Actual result and error:

Output:
main.cpp2...
main.cpp2(3,33): error: local variable test_overloads cannot be used in its own initializer

See also:

@gregmarr
Copy link
Contributor

For context, this is the discussion where the current error was introduced: #1028

I swore this reproducer code should not work, because of a variety of reasons, such as this having to require that the name lookup for test_overloads$ exclude the variable currently being defined, violating reading left to right where the new variable is set up before the initializer is processed. I thought it was just trying to have the initializer not take the new name into account since it's still being defined.

I was going to argue that the new name exists as soon as the : and thus this shouldn't be expected to work, just as the code below doesn't work.

main: () = {
  test_overloads : std::string = "";
  _ = :() = { 
    test_overloads := 1; 
    to : std::string = test_overloads$; 
  };
}

Then it actually worked. This is when my brain broke.

auto main() -> int{
  std::string test_overloads {""}; 
  static_cast<void>([_0 = cpp2::move(test_overloads)]() mutable -> void{
    auto test_overloads {1}; 
    std::string to {_0}; 
  });
}

So the $ at the end of test_overloads not only says "capture this value", it also says "change the lookup scope for this name to be the outer scope, bypassing any local variables with the same name." It turns out that this is sort-of documented, though not explicitly, see below.

Right this minute I am very much not in favor of this. I am a bit okay with the local test_overloads shadowing the outer test_overloads not being detected right now, though it would be nice in the future if shadowing wasn't hit or miss like this.

I don't like at all the local scope bypass on $. The second example should find the local test_overloads using normal lookup scope, and then say "the local variable test_overloads is not eligible for capture" (see the end for what actually happens when you try to capture a local variable in a lambda).

I am also fine with the original example failing with either the self-assignment error or the not eligible for capture error, since there are technically two errors there if $ doesn't skip the local scope.

Another reason to not like this is that this $ changed lookup scope behavior would only be for lambda capture, not string interpolation.

main: () = {
  test_overloads : std::string = "";
  _ = :() = { 
    test_overloads := 1; 
    to : std::string = test_overloads$; 
    onestr := "(test_overloads)$";
  };
}
auto main() -> int{
  std::string test_overloads {""}; 
  static_cast<void>([_0 = test_overloads]() mutable -> void{
    auto test_overloads {1}; 
    std::string to {_0}; 
    auto onestr {"" + cpp2::to_string(cpp2::move(test_overloads)) + ""}; 
  });
}

Documentation:

The design and syntax are selected so that capture is spelled the same way in all contexts. For details, see Design note: Capture.

Capture in function expressions (aka lambdas)
Any capture in a function expression body is evaluated at the point where the function expression is written, at the declaration of the function expression.

Capture in string interpolation
Any capture in a string literal is evaluated at the point where the string literal is written. The string literal can be used repeatedly later, and includes the captured value.

I know that this says that the capture is evaluated at the point where the function expression is written and the point where the string literal is written, and I was 100% on board with that until now. It doesn't explicitly mention lookup scope, and I didn't expect it to until now. While capture is spelled the same way in all contexts, it doesn't have the same scope in all contexts. It means that if I am in a lambda and take the first line below and refactor it into the second, I get a different variable, and that's just way too subtle.

    str1 := std::to_string((test_overloads)$);
    str2 := "(test_overloads)$";

PS. Here is the error when you attempt to do a lambda capture of a local variable, it's not what you'd expect:

main: () = {
  _ = :() = { 
    to := 1; 
    too := to$
  };
}

error: ill-formed initializer (at 'to')
error: $ (capture) cannot appear here - it must appear in an anonymous expression function, a postcondition, or an interpolated string literal (at '$')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants