Unused lambda capture source ranges are slightly off

[tbaederr]

See: https://godbolt.org/z/MKjzsYb9b

int main() {

    int a = 0, b = 0;

    auto F = [&a, &b]() {

    };
}

The output is:

<source>:7:16: warning: lambda capture 'a' is not used [-Wunused-lambda-capture]
    7 |     auto F = [&a, &b]() {
      |               ~^~
<source>:7:20: warning: lambda capture 'b' is not used [-Wunused-lambda-capture]
    7 |     auto F = [&a, &b]() {
      |                 ~~~^
<source>:7:10: warning: unused variable 'F' [-Wunused-variable]
    7 |     auto F = [&a, &b]() {
      |          ^

1. The source range for &a includes the comma after it
2. The source range for &b includes both the comma and the whitespace between that and the capture

The second point is especially fun when inserting some useless whitespace:

    auto F = [&a,         
    
     &b]() {

    };

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Timm Baeder (tbaederr)

See: https://godbolt.org/z/MKjzsYb9b

int main() {

    int a = 0, b = 0;

    auto F = [&a, &b]() {

    };
}

The output is:

<source>:7:16: warning: lambda capture 'a' is not used [-Wunused-lambda-capture]
    7 |     auto F = [&a, &b]() {
      |               ~^~
<source>:7:20: warning: lambda capture 'b' is not used [-Wunused-lambda-capture]
    7 |     auto F = [&a, &b]() {
      |                 ~~~^
<source>:7:10: warning: unused variable 'F' [-Wunused-variable]
    7 |     auto F = [&a, &b]() {
      |          ^

1. The source range for &a includes the comma after it
2. The source range for &b includes both the comma and the whitespace between that and the capture

The second point is especially fun when inserting some useless whitespace:

    auto F = [&a,         
    
     &b]() {

    };

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
3. Fix the issue locally.
4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
5. Create a Git commit.
6. Run git clang-format HEAD~1 to format your changes.
7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

[llvmbot]

@llvm/issue-subscribers-good-first-issue

Author: Timm Baeder (tbaederr)

See: https://godbolt.org/z/MKjzsYb9b

int main() {

    int a = 0, b = 0;

    auto F = [&a, &b]() {

    };
}

The output is:

<source>:7:16: warning: lambda capture 'a' is not used [-Wunused-lambda-capture]
    7 |     auto F = [&a, &b]() {
      |               ~^~
<source>:7:20: warning: lambda capture 'b' is not used [-Wunused-lambda-capture]
    7 |     auto F = [&a, &b]() {
      |                 ~~~^
<source>:7:10: warning: unused variable 'F' [-Wunused-variable]
    7 |     auto F = [&a, &b]() {
      |          ^

1. The source range for &a includes the comma after it
2. The source range for &b includes both the comma and the whitespace between that and the capture

The second point is especially fun when inserting some useless whitespace:

    auto F = [&a,         
    
     &b]() {

    };

[shafik]

Note the source range for a does not include the comma after it if you have whitespace: https://godbolt.org/z/sGEYPaThs

[c8ef]

Hi, I would like to work on this.

[c8ef]

I believe the issue was introduced in 832f49b. Interpreting the source range as the fix-it range rather than the diagnostic range actually makes some kind of sense.

[c8ef]

So I’m wondering if this is the intended behavior. If we want to fix it, where should the changes be made? From the perspective of the fix-it suggestion, the comma still needs to be bound to a variable, either the former or the latter.

[tbaederr]

Right, those aren't just ranges, but delete ranges from the fixme. In that case, I'm not sure what to do. 🙃

[jhendle2]

For https://godbolt.org/z/8zdKPxY6K

int main() {

    int a = 0, b = 0;

    auto F = [&a, &b]() {

    };
}

The output is:

<source>:11:16: warning: lambda capture 'a' is not used [-Wunused-lambda-capture]
   11 |     auto F = [&a, &b]() {
      |               ~^~
<source>:11:20: warning: lambda capture 'b' is not used [-Wunused-lambda-capture]
   11 |     auto F = [&a, &b]() {
      |                 ~~~^
<source>:11:10: warning: unused variable 'F' [-Wunused-variable]
   11 |     auto F = [&a, &b]() {
      |          ^

So, I assume the third warning output should be:

<source>:11:20: warning: lambda capture 'b' is not used [-Wunused-lambda-capture]
   11 |     auto F = [&a, &b]() {
      |                   ~^

Because this matches the behavior for a lambda with a single unsued capture?

<source>:11:16: warning: lambda capture 'a' is not used [-Wunused-lambda-capture]
   11 |     auto F = [&a]() {
      |               ~^

[tbaederr]

Kinda, yes. But the output is already correct as it is, when we consider that the source ranges aren't supposed to just highlight the unused parameter. They are fix-it hints that will remove the underlined text when applied.

[jhendle2]

Hi, can this ticket be assigned to me?