On Mon, Feb 28, 2022 at 10:12 AM Reid Kleckner <r...@google.com> 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 <dblai...@gmail.com> 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 &&FallbackGen = [this, OutlinedFn, &D, &CapturedVars,
>>> RequiresOuterTask,
>>> -                        &CS, OffloadingMandatory](CodeGenFunction &CGF)
>>> {
>>> +  auto &&FallbackGen = [this, &D, OutlinedFn, &CapturedVars,
>>> RequiresOuterTask, &CS,
>>> +                        OffloadingMandatory](CodeGenFunction &CGF) {
>>>      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 &&ThenGen = [this, Device, OutlinedFn, OutlinedFnID, &D,
>>> &InputInfo,
>>> -                    &MapTypesArray, &MapNamesArray, &CS,
>>> RequiresOuterTask,
>>> -                    &CapturedVars, SizeEmitter,
>>> +  auto &&ThenGen = [this, Device, OutlinedFnID, &D, &InputInfo,
>>> +                    &MapTypesArray, &MapNamesArray, SizeEmitter,
>>>                      FallbackGen](CodeGenFunction &CGF, 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 &&ElseGen = [this, &D, OutlinedFn, &CS, &CapturedVars,
>>> RequiresOuterTask,
>>> -                    FallbackGen](CodeGenFunction &CGF, PrePostActionTy
>>> &) {
>>> +  auto &&ElseGen = [FallbackGen](CodeGenFunction &CGF, 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

Reply via email to