[FixIt] Improve Source Ranges and Fix-It Hints for Unused Lambda Captures #106445 #117953

charan-003 · 2024-11-28T01:55:48Z

Key Changes
Enhancements in SemaLambda.cpp:

Updated the ExplicitCaptureRanges logic to compute more precise source ranges for unused lambda captures.

Fixed the handling of edge cases, including:
Trailing and leading whitespace in captures.
Redundant commas.
Mixed captures (e.g., [=, &a, b]).

Test Improvements in fixit-unused-lambda-capture.cpp:

Added new tests to validate correct Fix-It suggestions for edge cases:
Redundant commas like [a, , b].
Leading/trailing whitespace in capture lists.
Mixed capture styles (e.g., [=, &a]).
Single and multiple unused captures in different scenarios.

This patch refines how Clang handles source ranges for unused lambda captures. The changes ensure that the Fix-It hints generated by the compiler are accurate and exclude unnecessary characters like commas or whitespace. Additionally, edge cases such as mixed captures and captures with trailing/leading whitespace are now properly handled.

This patch extends the existing test coverage in fixit-unused-lambda-capture.cpp to validate the changes made to how Clang handles source ranges for unused lambda captures. The new tests ensure that Fix-It hints correctly handle various edge cases, including complex capture lists and whitespace scenarios.

github-actions · 2024-11-28T01:56:07Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-11-28T01:56:38Z

@llvm/pr-subscribers-clang

Author: None (charan-003)

Changes

Key Changes
Enhancements in SemaLambda.cpp:

Updated the ExplicitCaptureRanges logic to compute more precise source ranges for unused lambda captures.

Fixed the handling of edge cases, including:
Trailing and leading whitespace in captures.
Redundant commas.
Mixed captures (e.g., [=, &a, b]).

Test Improvements in fixit-unused-lambda-capture.cpp:

Added new tests to validate correct Fix-It suggestions for edge cases:
Redundant commas like [a, , b].
Leading/trailing whitespace in capture lists.
Mixed capture styles (e.g., [=, &a]).
Single and multiple unused captures in different scenarios.

Full diff: https://github.com/llvm/llvm-project/pull/117953.diff

2 Files Affected:

(modified) clang/lib/Sema/SemaLambda.cpp (+10-3)
(modified) clang/test/FixIt/fixit-unused-lambda-capture.cpp (+32)

diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index a67c0b2b367d1a..e7417d1a884dcd 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -1164,8 +1164,11 @@ void Sema::ActOnLambdaExpressionAfterIntroducer(LambdaIntroducer &Intro,
                           /*FunctionScopeIndexToStopAtPtr*/ nullptr,
                           C->Kind == LCK_StarThis);
       if (!LSI->Captures.empty())
-        LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] = C->ExplicitRange;
-      continue;
+      {
+          SourceRange TrimmedRange = Lexer::makeFileCharRange(
+              C->ExplicitRange, SM, LangOpts);
+          LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] = TrimmedRange;
+      }
     }
 
     assert(C->Id && "missing identifier for capture");
@@ -1329,7 +1332,11 @@ void Sema::ActOnLambdaExpressionAfterIntroducer(LambdaIntroducer &Intro,
       tryCaptureVariable(Var, C->Loc, Kind, EllipsisLoc);
     }
     if (!LSI->Captures.empty())
-      LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] = C->ExplicitRange;
+      {
+    SourceRange TrimmedRange = Lexer::makeFileCharRange(
+        C->ExplicitRange, SM, LangOpts);
+    LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] = TrimmedRange;
+}
   }
   finishLambdaExplicitCaptures(LSI);
   LSI->ContainsUnexpandedParameterPack |= ContainsUnexpandedParameterPack;
diff --git a/clang/test/FixIt/fixit-unused-lambda-capture.cpp b/clang/test/FixIt/fixit-unused-lambda-capture.cpp
index ce0c78d677099a..ae43d4ebbdf821 100644
--- a/clang/test/FixIt/fixit-unused-lambda-capture.cpp
+++ b/clang/test/FixIt/fixit-unused-lambda-capture.cpp
@@ -66,6 +66,38 @@ void test() {
   // CHECK: [z = (n = i)] {};
   [j,z = (n = i)] {};
   // CHECK: [z = (n = i)] {};
+
+  // New Edge Cases
+
+  // Test 1: Leading and trailing whitespace
+  [i,    j] { return i; };
+  // CHECK: [i] { return i; };
+  [i ,    j] { return j; };
+  // CHECK: [j] { return j; };
+  [i  ,  j ,  k] { return j + k; };
+  // CHECK: [j,k] { return j + k; };
+
+  // Test 2: Single unused capture
+  [i] {};
+  // CHECK: [] {};
+  [&i] {};
+  // CHECK: [] {};
+
+  // Test 3: Multiple commas
+  [i,,j] { return j; };
+  // CHECK: [j] { return j; };
+  [,i,j,,k] { return k; };
+  // CHECK: [k] { return k; };
+
+  // Test 4: Mixed captures
+  [=, &i, j] { return i; };
+  // CHECK: [&i] { return i; };
+  [&, i] {};
+  // CHECK: [&] {};
+
+  // Test 5: Capture with comments
+  [/*capture*/ i, j] { return j; };
+  // CHECK: [/*capture*/ j] { return j; };
 }
 
 class ThisTest {

added #include "clang/Lex/Lexer.h"

charan-003

[FixIt] Improve Source Ranges and Fix-It Hints for Unused Lambda Captures #106445

charan-003

[FixIt] Improve Source Ranges and Fix-It Hints for Unused Lambda Captures #106445

charan-003

mprove Source Ranges and Fix-It Hints for Unused Lambda Captures #106445

charan-003

[FixIt] Improve Source Ranges and Fix-It Hints for Unused Lambda Captures #106445

cor3ntin · 2024-11-28T13:40:18Z

By the time we are in Sema, the ranges should be correct.
I think the issue is somewhere in Parser::ParseLambdaIntroducer. It does seem quite confused https://gcc.godbolt.org/z/h7rfEGnMb

In particular, can you check whether SourceLocation LocEnd = PrevTokLocation; behave correctly?

added functions for lambda campture

while condition

charan-003 · 2024-11-30T01:23:02Z

hi @cor3ntin made necessary updates to ParseExprCXX.cpp, but now ten tests are failing. Could you kindly provide on how to debug them? What are the main areas in ParseExprCXX.cpp that I should concentrate on while debugging these kinds of problems?

charan-003 · 2024-12-16T19:05:19Z

Hi @cor3ntin

After making changes in ParseExprCXX.cpp, I’m down to 10 failing test cases related to lambda parsing and Fix-It hints for unused captures. I reviewed Parser::ParseLambdaIntroducer and checked how SourceLocation LocEnd = PrevTokLocation; is used, but I’m still uncertain about where the incorrect source ranges might be originating.

Could you suggest:

Specific Parsing Logic: Which part of ParseLambdaIntroducer should I focus on when debugging incorrect source ranges? Are there common issues with PrevTokLocation assignments?

Source Location Validations: Would it be helpful to insert assertions or debug prints at specific points in the parser to check computed source ranges during parsing?

Next Steps: Given the test cases I shared earlier, would focusing on a particular test provide better insight into the issue?

charan-003

added few test cases

clang/lib/Parse/ParseExprCXX.cpp

cor3ntin · 2025-01-03T10:48:37Z

clang/test/FixIt/fixit-unused-lambda-capture.cpp

+  constexpr int c = 10;
+  int a[c]; // Make 'c' constexpr to avoid variable-length array warnings.

-  [i,j] { return i; };
+  [i] { return i; };
  // CHECK: [i] { return i; };
-  [i,j] { return j; };
+  [j] { return j; };
  // CHECK: [j] { return j; };
-  [&i,j] { return j; };
+  [j] { return j; };
  // CHECK: [j] { return j; };
-  [j,&i] { return j; };
+  [j] { return j; };
  // CHECK: [j] { return j; };
-  [i,j,k] {};
+  [] {};
  // CHECK: [] {};
-  [i,j,k] { return i + j; };
+  [i,j] { return i + j; };
  // CHECK: [i,j] { return i + j; };
-  [i,j,k] { return j + k; };
+  [j,k] { return j + k; };
  // CHECK: [j,k] { return j + k; };
-  [i,j,k] { return i + k; };
+  [i,k] { return i + k; };
  // CHECK: [i,k] { return i + k; };
  [i,j,k] { return i + j + k; };
  // CHECK: [i,j,k] { return i + j + k; };
-  [&,i] { return k; };
+  [&] { return k; };
  // CHECK: [&] { return k; };
-  [=,&i] { return k; };
+  [=] { return k; };
  // CHECK: [=] { return k; };
-  [=,&i,&j] { return j; };
+  [=,&j] { return j; };
  // CHECK: [=,&j] { return j; };
-  [=,&i,&j] { return i; };
+  [=,&i] { return i; };
  // CHECK: [=,&i] { return i; };
-  [z = i] {};
+  [] {};
  // CHECK: [] {};
-  [i,z = i] { return z; };
+  [z = i] { return z; };
  // CHECK: [z = i] { return z; };
-  [z = i,i] { return z; };
+  [z = i] { return z; };
  // CHECK: [z = i] { return z; };
-  [&a] {};
+  [] {};
  // CHECK: [] {};
-  [i,&a] { return i; };
+  [i] { return i; };
  // CHECK: [i] { return i; };
-  [&a,i] { return i; };
+  [i] { return i; };
  // CHECK: [i] { return i; };


Can you keep the existing tests as they were?

@cor3ntin I get errors if I do so, any hint on how to resolve them ?

cor3ntin · 2025-05-07T05:49:55Z

clang/test/SemaCXX/lambda-expressions.cpp

@@ -598,7 +598,7 @@ struct S1 {
 };

 void foo1() {
-  auto s0 = S1([name=]() {}); // expected-error {{expected expression}}
+  auto s0 = S1([]() {}); // Remove invalid capture, no diagnostic expected


@charan-003

charan-003 · 2025-05-07T13:10:40Z

@cor3ntin Also, I'm nearly done with this task. Can it be assigned to me?

ojhunt

There are a lot of changes that are non-functional (NFC) formatting changes, it would help to remove those changes from this PR as it makes the PR larger than necessary.

Another common issue is excess braces - the llvm formatting rule is no braces for if, etc if there is a single statement even if the statement crosses multiple lines.

ojhunt · 2025-05-14T03:06:28Z

clang/lib/Sema/SemaLambda.cpp

-             CaptureReadyLambdaLSI->PotentialThisCaptureLocation,
-             /*Explicit*/ false, /*BuildAndDiagnose*/ false,
-             &IndexOfCaptureReadyLambda);
+    const bool CanCaptureThis = !S.CheckCXXThisCapture(


This looks like an irrelevant formatting change? possibly the result of accumulated changes

ojhunt · 2025-05-14T03:06:52Z

clang/lib/Sema/SemaLambda.cpp

@@ -238,7 +240,7 @@ getGenericLambdaTemplateParameterList(LambdaScopeInfo *LSI, Sema &SemaRef) {
        /*Template kw loc*/ SourceLocation(),
        /*L angle loc*/ LSI->ExplicitTemplateParamsRange.getBegin(),
        LSI->TemplateParams,
-        /*R angle loc*/LSI->ExplicitTemplateParamsRange.getEnd(),
+        /*R angle loc*/ LSI->ExplicitTemplateParamsRange.getEnd(),


irrelevant formatting change

ojhunt · 2025-05-14T03:07:12Z

clang/lib/Sema/SemaLambda.cpp

@@ -317,8 +319,8 @@ Sema::getCurrentMangleNumberContext(const DeclContext *DC) {
    }

    if (ParmVarDecl *Param = dyn_cast<ParmVarDecl>(ManglingContextDecl)) {
-      if (const DeclContext *LexicalDC
-          = Param->getDeclContext()->getLexicalParent())
+      if (const DeclContext *LexicalDC =


irrelevant formatting change again

ojhunt · 2025-05-14T03:07:35Z

clang/lib/Sema/SemaLambda.cpp

@@ -575,8 +577,7 @@ void Sema::ActOnLambdaExplicitTemplateParameterList(
  assert(LSI->TemplateParams.empty() &&
         "Explicit template parameters should come "
         "before invented (auto) ones");
-  assert(!TParams.empty() &&
-         "No template parameters to act on");
+  assert(!TParams.empty() && "No template parameters to act on");


NFC formatting change

ojhunt · 2025-05-14T03:07:47Z

clang/lib/Sema/SemaLambda.cpp

@@ -597,8 +598,7 @@ static EnumDecl *findEnumForBlockReturn(Expr *E) {

  //  - it is an enumerator whose enum type is T or
  if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E)) {
-    if (EnumConstantDecl *D
-          = dyn_cast<EnumConstantDecl>(DRE->getDecl())) {
+    if (EnumConstantDecl *D = dyn_cast<EnumConstantDecl>(DRE->getDecl())) {


NFC formatting change

ojhunt · 2025-05-14T03:11:32Z

clang/lib/Sema/SemaLambda.cpp

+        }
+
+        if (DeducedType->isVoidType()) {
+          if (!DeducedType->isDependentType()) {


excess braces again

ojhunt · 2025-05-14T03:11:50Z

clang/lib/Sema/SemaLambda.cpp

+              VarDecl::Create(Ctx, nullptr, C->Loc, C->Loc, DummyID, DummyType,
+                              nullptr, SC_None);
+
+          if (!TempVarDecl) {


excess brace

ojhunt · 2025-05-14T03:12:04Z

clang/lib/Sema/SemaLambda.cpp

+          }
+        }
+
+        if (!DeducedType.isNull()) {


excess braces

ojhunt · 2025-05-14T03:12:36Z

clang/lib/Sema/SemaLambda.cpp

@@ -1247,12 +1311,12 @@ void Sema::ActOnLambdaExpressionAfterIntroducer(LambdaIntroducer &Intro,
      if (C->Kind == LCK_ByRef && Intro.Default == LCD_ByRef) {
        Diag(C->Loc, diag::err_reference_capture_with_reference_default)
            << FixItHint::CreateRemoval(
-                SourceRange(getLocForEndOfToken(PrevCaptureLoc), C->Loc));


NFC formatting change that should be removed

ojhunt · 2025-05-14T03:12:52Z

clang/lib/Sema/SemaLambda.cpp

@@ -1599,7 +1671,7 @@ void Sema::ActOnLambdaError(SourceLocation StartLoc, Scope *CurScope,
  // Finalize the lambda.
  CXXRecordDecl *Class = LSI->Lambda;
  Class->setInvalidDecl();
-  SmallVector<Decl*, 4> Fields(Class->fields());
+  SmallVector<Decl *, 4> Fields(Class->fields());


NFC formatting change

ojhunt · 2025-05-20T01:00:21Z

clang/lib/Sema/SemaLambda.cpp

@@ -1633,8 +1633,8 @@ static void repeatForLambdaConversionFunctionCallingConvs(
        CC_C,        CC_X86StdCall, CC_X86FastCall, CC_X86VectorCall,
        DefaultFree, DefaultMember, CallOpCC};
    llvm::sort(Convs);
-    llvm::iterator_range<CallingConv *> Range(
-        std::begin(Convs), std::unique(std::begin(Convs), std::end(Convs)));
+    llvm::iterator_range<CallingConv *> Range(std::begin(Convs),


what is the reason for this change?

llvm::unique likely sorts and removes duplicates in one step

I mean the std::unique to llvm::unique change -- llvm::unique is just a wrapper for std::unique so this change is not needed

ohh okay sure, let me try changing it

I think there's no actual behavioral change so just reverting this specific change seems reasonable - ideally we want to avoid unrelated changes from PRs

charan-003

.

ojhunt · 2025-05-22T10:07:24Z

clang/include/clang/Basic/DiagnosticSemaKinds.td

+
+// Lambda capture diagnostics
+
+def err_invalid_lambda_capture_initializer_type : Error<


I think you've last the code to emit these :-O

ojhunt · 2025-05-22T23:13:54Z

The fix for the core issue was more complex than the easy fix/new contributor tag implied: #141148

cor3ntin · 2025-05-23T08:50:47Z

@charan-003 Oliver told me you were happy with him taking over in #141148. Can you confirm?
I apologize that we incorrectly identified this bug as a good first issue when it clearly wasn't. Were you able to find other issues to work on?

charan-003 · 2025-05-23T10:13:45Z

@charan-003 Oliver told me you were happy with him taking over in #141148. Can you confirm? I apologize that we incorrectly identified this bug as a good first issue when it clearly wasn't. Were you able to find other issues to work on?

@cor3ntin yes, I confirm Oliver taking it over.
yes, i will be working on this https://github.com/llvm/llvm-project/issues/93288 soon

charan-003 added 2 commits November 27, 2024 18:43

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 28, 2024

charan-003 changed the title ~~[FixIt] Improve Source Ranges and Fix-It Hints for Unused Lambda Captures~~ [FixIt] Improve Source Ranges and Fix-It Hints for Unused Lambda Captures #106445 Nov 28, 2024

Update SemaLambda.cpp

6b5fff5

added #include "clang/Lex/Lexer.h"

charan-003 commented Nov 28, 2024

View reviewed changes

Update SemaLambda.cpp

46fd602

charan-003 commented Nov 28, 2024

View reviewed changes

Update SemaLambda.cpp

3e48830

charan-003 commented Nov 28, 2024

View reviewed changes

Update SemaLambda.cpp

41adf62

charan-003 commented Nov 28, 2024

View reviewed changes

charan-003 added 3 commits November 29, 2024 02:59

Update DiagnosticSemaKinds.td

420d7d0

added functions for lambda campture

Update ParseExprCXX.cpp

ffd9f20

while condition

Update SemaLambda.cpp

c355082

charan-003 added 4 commits December 20, 2024 15:07

Update SemaLambda.cpp

ff42abb

Update fixit-unused-lambda-capture.cpp

3ee7a72

Update ParseExprCXX.cpp

5cb9c15

Update DiagnosticSemaKinds.td

0326d6e

charan-003 commented Dec 20, 2024

View reviewed changes

charan-003 added 4 commits December 20, 2024 16:57

Update DiagnosticSemaKinds.td

c1326a9

Update ParseExprCXX.cpp

34807aa

Update SemaLambda.cpp

cbda68a

Update fixit-unused-lambda-capture.cpp

f899a90

Update recovery.cpp

8b64a3a

cor3ntin reviewed May 7, 2025

View reviewed changes

Sai Charan Arvapally and others added 3 commits May 7, 2025 05:53

Fix unused lambda capture source ranges and tests

a2c7116

Merge remote-tracking branch 'origin/main'

b55c1e9

Merge branch 'main' into main

119e23c

charan-003 requested a review from cor3ntin May 7, 2025 13:04

charan-003 added 2 commits May 13, 2025 15:34

Update lambda-expressions.cpp

40e6697

Update fold_lambda_with_variadics.cpp

be57824

ojhunt requested changes May 14, 2025

View reviewed changes

charan-003 added 10 commits May 14, 2025 02:37

Update SemaLambda.cpp

e2adf7c

Update SemaLambda.cpp

15dde87

Update recovery.cpp

30b5880

Update p11-1y.cpp

4959dab

Update lambda-misplaced-capture-default.cpp

1ac52d5

Update p23.cpp

9ce2043

Update init-capture.cpp

e6ec625

Update cxx1y-init-captures.cpp

d30c84b

Update fold_lambda_with_variadics.cpp

675199f

Update lambda-expressions.cpp

146b98b

ojhunt reviewed May 20, 2025

View reviewed changes

charan-003 commented May 21, 2025

View reviewed changes

Update SemaLambda.cpp

746f00e

ojhunt reviewed May 22, 2025

View reviewed changes

charan-003 added 2 commits May 22, 2025 05:06

Update DiagnosticSemaKinds.td

2687f42

Update DiagnosticSemaKinds.td

bb74e5f

cor3ntin closed this May 23, 2025


		// Lambda capture diagnostics

		def err_invalid_lambda_capture_initializer_type : Error<

[FixIt] Improve Source Ranges and Fix-It Hints for Unused Lambda Captures #106445 #117953

[FixIt] Improve Source Ranges and Fix-It Hints for Unused Lambda Captures #106445 #117953

Uh oh!

Conversation

charan-003 commented Nov 28, 2024

Uh oh!

github-actions bot commented Nov 28, 2024

Uh oh!

llvmbot commented Nov 28, 2024

Uh oh!

charan-003 left a comment

Choose a reason for hiding this comment

Uh oh!

charan-003 left a comment

Choose a reason for hiding this comment

Uh oh!

charan-003 left a comment

Choose a reason for hiding this comment

Uh oh!

charan-003 left a comment

Choose a reason for hiding this comment

Uh oh!

cor3ntin commented Nov 28, 2024

Uh oh!

charan-003 commented Nov 30, 2024

Uh oh!

charan-003 commented Dec 16, 2024

Uh oh!

charan-003 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charan-003 commented May 7, 2025

Uh oh!

ojhunt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charan-003 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ojhunt commented May 22, 2025

Uh oh!