================
@@ -1641,6 +1641,17 @@ def warn_implicitly_retains_self : Warning <
   "block implicitly retains 'self'; explicitly mention 'self' to indicate "
   "this is intended behavior">,
   InGroup<DiagGroup<"implicit-retain-self">>, DefaultIgnore;
+def warn_blocks_capturing_this : Warning<"block implicitly captures 'this'">,
+                                 InGroup<DiagGroup<"blocks-capturing-this">>,
+                                 DefaultIgnore;
+def warn_blocks_capturing_reference
+    : Warning<"block implicitly captures a C++ reference">,
+      InGroup<DiagGroup<"blocks-capturing-reference">>,
+      DefaultIgnore;
+def warn_blocks_capturing_raw_pointer
+    : Warning<"block implicitly captures a raw pointer">,
----------------
sdefresne wrote:

The table with the data with true/false positive vs kind of capture.

kind | count | true positive | no escape block | __block | static pointer | 
function pointer | other | ratio
-- | -- | -- | -- | -- | -- | -- | -- | --
this | 28 | 20 | 0 | 0 | 0 | 0 | 8 | 71.43%
reference | 7 | 3 | 3 | 0 | 1 | 0 | 0 | 42.86%
pointer | 53 | 30 | 10 | 2 | 6 | 2 | 3 | 56.60%
total | 88 | 53 | 13 | 2 | 7 | 2 | 11 | 60.23%

Regarding the suppression mechanism, besides `#pragma clang diagnostic`, the 
developer can be explicit about the capture either by declaring a local struct 
to store the captured values, or using variables marked with `__block`.

So for example, one of the "false positive" is this [code in 
dawn](https://dawn.googlesource.com/dawn/+/refs/heads/main/src/dawn/native/metal/BufferMTL.mm#156)

```
    Ref<DeviceBase> deviceRef = GetDevice();
    wgpu::Callback callback = hostMappedDesc->disposeCallback;
    void* userdata = hostMappedDesc->userdata;
    auto dispose = ^(void*, NSUInteger) {
        deviceRef->GetCallbackTaskManager()->AddCallbackTask(
            [callback, userdata] { callback(userdata); });
    };
```

I think this can be fixed by writing either

```
    Ref<DeviceBase> deviceRef = GetDevice();
    __block wgpu::Callback callback = hostMappedDesc->disposeCallback;
    __block void* userdata = hostMappedDesc->userdata;
    auto dispose = ^(void*, NSUInteger) {
        deviceRef->GetCallbackTaskManager()->AddCallbackTask(
            [callback, userdata] { callback(userdata); });
    };
```

or

```
    Ref<DeviceBase> deviceRef = GetDevice();
    struct { wgpu::Callback callback; void* userdata; } captured = {
        hostMappedDesc->disposeCallback,
        hostMappedDesc->userdata,
    };
    auto dispose = ^(void*, NSUInteger) {
        deviceRef->GetCallbackTaskManager()->AddCallbackTask(
            [captured] { captured.callback(captured.userdata); });
    };
```

Since the warning does not inspect the captured objects, and only warn about 
pointers and reference, using a structure hides the capture from the warning. 
In the same way by using `__block` the pointer are copied in the block 
explicitly, and thus the warning should not fire (I have not tested, if it 
does, I would consider this a bug in my implementation and will fix it). The 
same pattern can also work for references.

Basically, to disable the warning, you would have to be explicit that the 
capture is intended.

https://github.com/llvm/llvm-project/pull/144388
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to