subject:"\[clang\] 1d1b089 \- Fix more unused lambda capture warnings, NFC"

Re: [clang] 1d1b089 - Fix more unused lambda capture warnings, NFC

2022-03-06 Thread David Blaikie via cfe-commits

On Mon, Feb 28, 2022 at 10:12 AM Reid Kleckner  wrote:

> I agree, but clearly this person chose a particular style, and I wasn't
> trying to revise their code style, just to fix some the Bazel build, which
> uses -Werror.
>

Oh, wow, yeah, this file/corner of the codebase is deep into lambdas, wow.
Sent https://reviews.llvm.org/D121077 for the broader change (both the
capture lists and at least one function that seemed to not really benefit
from taking a lambda - maybe other functions could have similar cleanup)


> The way that `assert` can affect a lambda's capture list seems like a
> particularly good argument for having a project wide style recommendation
> to never use explicit capture lists.
>

Yeah, not sure it's worth words in the style guide unless it's especially
contentious (the presence of code like this might argue that it is - but
hopefully even in this case only pointing it out will be enough) but maybe.

- Dave


>
> On Mon, Feb 28, 2022 at 9:45 AM David Blaikie  wrote:
>
>> FWIW, I think it's probably simpler/more maintainable to default capture
>> by reference ("[&]") if a lambda doesn't escape its scope (if it's never
>> type erased/put in a std::function or equivalent). Avoids assert/non-assert
>> unused issues, having to maintain/update the list when the code changes and
>> makes a capture live/dead/etc.
>>
>> On Wed, Feb 23, 2022 at 2:09 PM Reid Kleckner via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>>
>>> Author: Reid Kleckner
>>> Date: 2022-02-23T14:07:04-08:00
>>> New Revision: 1d1b089c5d503e2fc8697887411730105f66c774
>>>
>>> URL:
>>> https://github.com/llvm/llvm-project/commit/1d1b089c5d503e2fc8697887411730105f66c774
>>> DIFF:
>>> https://github.com/llvm/llvm-project/commit/1d1b089c5d503e2fc8697887411730105f66c774.diff
>>>
>>> LOG: Fix more unused lambda capture warnings, NFC
>>>
>>> Added:
>>>
>>>
>>> Modified:
>>> clang/lib/CodeGen/CGOpenMPRuntime.cpp
>>>
>>> Removed:
>>>
>>>
>>>
>>>
>>> 
>>> diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
>>> b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
>>> index b4033da890c4..3f4a78ddbf3c 100644
>>> --- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
>>> +++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
>>> @@ -10358,8 +10358,8 @@ void CGOpenMPRuntime::emitTargetCall(
>>>llvm::Value *MapTypesArray = nullptr;
>>>llvm::Value *MapNamesArray = nullptr;
>>>// Generate code for the host fallback function.
>>> -  auto & = [this, OutlinedFn, , ,
>>> RequiresOuterTask,
>>> -, OffloadingMandatory](CodeGenFunction )
>>> {
>>> +  auto & = [this, , OutlinedFn, ,
>>> RequiresOuterTask, ,
>>> +OffloadingMandatory](CodeGenFunction ) {
>>>  if (OffloadingMandatory) {
>>>CGF.Builder.CreateUnreachable();
>>>  } else {
>>> @@ -10371,9 +10371,8 @@ void CGOpenMPRuntime::emitTargetCall(
>>>  }
>>>};
>>>// Fill up the pointer arrays and transfer execution to the device.
>>> -  auto & = [this, Device, OutlinedFn, OutlinedFnID, ,
>>> ,
>>> -, , ,
>>> RequiresOuterTask,
>>> -, SizeEmitter,
>>> +  auto & = [this, Device, OutlinedFnID, , ,
>>> +, , SizeEmitter,
>>>  FallbackGen](CodeGenFunction , PrePostActionTy
>>> &) {
>>>  if (Device.getInt() == OMPC_DEVICE_ancestor) {
>>>// Reverse offloading is not supported, so just execute on the
>>> host.
>>> @@ -10392,6 +10391,7 @@ void CGOpenMPRuntime::emitTargetCall(
>>>
>>>  // From this point on, we need to have an ID of the target region
>>> defined.
>>>  assert(OutlinedFnID && "Invalid outlined function ID!");
>>> +(void)OutlinedFnID;
>>>
>>>  // Emit device ID if any.
>>>  llvm::Value *DeviceID;
>>> @@ -10529,8 +10529,7 @@ void CGOpenMPRuntime::emitTargetCall(
>>>};
>>>
>>>// Notify that the host version must be executed.
>>> -  auto & = [this, , OutlinedFn, , ,
>>> RequiresOuterTask,
>>> -FallbackGen](CodeGenFunction , PrePostActionTy
>>> &) {
>>> +  auto & = [FallbackGen](CodeGenFunction , PrePostActionTy
>>> &) {
>>>  FallbackGen(CGF);
>>>};
>>>
>>>
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang] 1d1b089 - Fix more unused lambda capture warnings, NFC

2022-02-28 Thread Reid Kleckner via cfe-commits

I agree, but clearly this person chose a particular style, and I wasn't
trying to revise their code style, just to fix some the Bazel build, which
uses -Werror.

The way that `assert` can affect a lambda's capture list seems like a
particularly good argument for having a project wide style recommendation
to never use explicit capture lists.

On Mon, Feb 28, 2022 at 9:45 AM David Blaikie  wrote:

> FWIW, I think it's probably simpler/more maintainable to default capture
> by reference ("[&]") if a lambda doesn't escape its scope (if it's never
> type erased/put in a std::function or equivalent). Avoids assert/non-assert
> unused issues, having to maintain/update the list when the code changes and
> makes a capture live/dead/etc.
>
> On Wed, Feb 23, 2022 at 2:09 PM Reid Kleckner via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>>
>> Author: Reid Kleckner
>> Date: 2022-02-23T14:07:04-08:00
>> New Revision: 1d1b089c5d503e2fc8697887411730105f66c774
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/1d1b089c5d503e2fc8697887411730105f66c774
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/1d1b089c5d503e2fc8697887411730105f66c774.diff
>>
>> LOG: Fix more unused lambda capture warnings, NFC
>>
>> Added:
>>
>>
>> Modified:
>> clang/lib/CodeGen/CGOpenMPRuntime.cpp
>>
>> Removed:
>>
>>
>>
>>
>> 
>> diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
>> b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
>> index b4033da890c4..3f4a78ddbf3c 100644
>> --- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
>> +++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
>> @@ -10358,8 +10358,8 @@ void CGOpenMPRuntime::emitTargetCall(
>>llvm::Value *MapTypesArray = nullptr;
>>llvm::Value *MapNamesArray = nullptr;
>>// Generate code for the host fallback function.
>> -  auto & = [this, OutlinedFn, , ,
>> RequiresOuterTask,
>> -, OffloadingMandatory](CodeGenFunction ) {
>> +  auto & = [this, , OutlinedFn, ,
>> RequiresOuterTask, ,
>> +OffloadingMandatory](CodeGenFunction ) {
>>  if (OffloadingMandatory) {
>>CGF.Builder.CreateUnreachable();
>>  } else {
>> @@ -10371,9 +10371,8 @@ void CGOpenMPRuntime::emitTargetCall(
>>  }
>>};
>>// Fill up the pointer arrays and transfer execution to the device.
>> -  auto & = [this, Device, OutlinedFn, OutlinedFnID, ,
>> ,
>> -, , ,
>> RequiresOuterTask,
>> -, SizeEmitter,
>> +  auto & = [this, Device, OutlinedFnID, , ,
>> +, , SizeEmitter,
>>  FallbackGen](CodeGenFunction , PrePostActionTy
>> &) {
>>  if (Device.getInt() == OMPC_DEVICE_ancestor) {
>>// Reverse offloading is not supported, so just execute on the
>> host.
>> @@ -10392,6 +10391,7 @@ void CGOpenMPRuntime::emitTargetCall(
>>
>>  // From this point on, we need to have an ID of the target region
>> defined.
>>  assert(OutlinedFnID && "Invalid outlined function ID!");
>> +(void)OutlinedFnID;
>>
>>  // Emit device ID if any.
>>  llvm::Value *DeviceID;
>> @@ -10529,8 +10529,7 @@ void CGOpenMPRuntime::emitTargetCall(
>>};
>>
>>// Notify that the host version must be executed.
>> -  auto & = [this, , OutlinedFn, , ,
>> RequiresOuterTask,
>> -FallbackGen](CodeGenFunction , PrePostActionTy
>> &) {
>> +  auto & = [FallbackGen](CodeGenFunction , PrePostActionTy
>> &) {
>>  FallbackGen(CGF);
>>};
>>
>>
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang] 1d1b089 - Fix more unused lambda capture warnings, NFC

2022-02-28 Thread David Blaikie via cfe-commits

FWIW, I think it's probably simpler/more maintainable to default capture by
reference ("[&]") if a lambda doesn't escape its scope (if it's never type
erased/put in a std::function or equivalent). Avoids assert/non-assert
unused issues, having to maintain/update the list when the code changes and
makes a capture live/dead/etc.

On Wed, Feb 23, 2022 at 2:09 PM Reid Kleckner via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Reid Kleckner
> Date: 2022-02-23T14:07:04-08:00
> New Revision: 1d1b089c5d503e2fc8697887411730105f66c774
>
> URL:
> https://github.com/llvm/llvm-project/commit/1d1b089c5d503e2fc8697887411730105f66c774
> DIFF:
> https://github.com/llvm/llvm-project/commit/1d1b089c5d503e2fc8697887411730105f66c774.diff
>
> LOG: Fix more unused lambda capture warnings, NFC
>
> Added:
>
>
> Modified:
> clang/lib/CodeGen/CGOpenMPRuntime.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
> b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
> index b4033da890c4..3f4a78ddbf3c 100644
> --- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
> +++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
> @@ -10358,8 +10358,8 @@ void CGOpenMPRuntime::emitTargetCall(
>llvm::Value *MapTypesArray = nullptr;
>llvm::Value *MapNamesArray = nullptr;
>// Generate code for the host fallback function.
> -  auto & = [this, OutlinedFn, , ,
> RequiresOuterTask,
> -, OffloadingMandatory](CodeGenFunction ) {
> +  auto & = [this, , OutlinedFn, ,
> RequiresOuterTask, ,
> +OffloadingMandatory](CodeGenFunction ) {
>  if (OffloadingMandatory) {
>CGF.Builder.CreateUnreachable();
>  } else {
> @@ -10371,9 +10371,8 @@ void CGOpenMPRuntime::emitTargetCall(
>  }
>};
>// Fill up the pointer arrays and transfer execution to the device.
> -  auto & = [this, Device, OutlinedFn, OutlinedFnID, ,
> ,
> -, , ,
> RequiresOuterTask,
> -, SizeEmitter,
> +  auto & = [this, Device, OutlinedFnID, , ,
> +, , SizeEmitter,
>  FallbackGen](CodeGenFunction , PrePostActionTy &)
> {
>  if (Device.getInt() == OMPC_DEVICE_ancestor) {
>// Reverse offloading is not supported, so just execute on the host.
> @@ -10392,6 +10391,7 @@ void CGOpenMPRuntime::emitTargetCall(
>
>  // From this point on, we need to have an ID of the target region
> defined.
>  assert(OutlinedFnID && "Invalid outlined function ID!");
> +(void)OutlinedFnID;
>
>  // Emit device ID if any.
>  llvm::Value *DeviceID;
> @@ -10529,8 +10529,7 @@ void CGOpenMPRuntime::emitTargetCall(
>};
>
>// Notify that the host version must be executed.
> -  auto & = [this, , OutlinedFn, , ,
> RequiresOuterTask,
> -FallbackGen](CodeGenFunction , PrePostActionTy &)
> {
> +  auto & = [FallbackGen](CodeGenFunction , PrePostActionTy &)
> {
>  FallbackGen(CGF);
>};
>
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 1d1b089 - Fix more unused lambda capture warnings, NFC

2022-02-23 Thread Reid Kleckner via cfe-commits


Author: Reid Kleckner
Date: 2022-02-23T14:07:04-08:00
New Revision: 1d1b089c5d503e2fc8697887411730105f66c774

URL: 
https://github.com/llvm/llvm-project/commit/1d1b089c5d503e2fc8697887411730105f66c774
DIFF: 
https://github.com/llvm/llvm-project/commit/1d1b089c5d503e2fc8697887411730105f66c774.diff

LOG: Fix more unused lambda capture warnings, NFC

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index b4033da890c4..3f4a78ddbf3c 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -10358,8 +10358,8 @@ void CGOpenMPRuntime::emitTargetCall(
   llvm::Value *MapTypesArray = nullptr;
   llvm::Value *MapNamesArray = nullptr;
   // Generate code for the host fallback function.
-  auto & = [this, OutlinedFn, , , RequiresOuterTask,
-, OffloadingMandatory](CodeGenFunction ) {
+  auto & = [this, , OutlinedFn, , 
RequiresOuterTask, ,
+OffloadingMandatory](CodeGenFunction ) {
 if (OffloadingMandatory) {
   CGF.Builder.CreateUnreachable();
 } else {
@@ -10371,9 +10371,8 @@ void CGOpenMPRuntime::emitTargetCall(
 }
   };
   // Fill up the pointer arrays and transfer execution to the device.
-  auto & = [this, Device, OutlinedFn, OutlinedFnID, , ,
-, , , RequiresOuterTask,
-, SizeEmitter,
+  auto & = [this, Device, OutlinedFnID, , ,
+, , SizeEmitter,
 FallbackGen](CodeGenFunction , PrePostActionTy &) {
 if (Device.getInt() == OMPC_DEVICE_ancestor) {
   // Reverse offloading is not supported, so just execute on the host.
@@ -10392,6 +10391,7 @@ void CGOpenMPRuntime::emitTargetCall(
 
 // From this point on, we need to have an ID of the target region defined.
 assert(OutlinedFnID && "Invalid outlined function ID!");
+(void)OutlinedFnID;
 
 // Emit device ID if any.
 llvm::Value *DeviceID;
@@ -10529,8 +10529,7 @@ void CGOpenMPRuntime::emitTargetCall(
   };
 
   // Notify that the host version must be executed.
-  auto & = [this, , OutlinedFn, , , 
RequiresOuterTask,
-FallbackGen](CodeGenFunction , PrePostActionTy &) {
+  auto & = [FallbackGen](CodeGenFunction , PrePostActionTy &) {
 FallbackGen(CGF);
   };
 



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang] 1d1b089 - Fix more unused lambda capture warnings, NFC

Re: [clang] 1d1b089 - Fix more unused lambda capture warnings, NFC

Re: [clang] 1d1b089 - Fix more unused lambda capture warnings, NFC

[clang] 1d1b089 - Fix more unused lambda capture warnings, NFC

4 matches

Site Navigation

Mail list logo

Footer information