[PATCH] D53069: [analyzer][www] Update avaible_checks.html

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov requested changes to this revision.
george.karpenkov added inline comments.
This revision now requires changes to proceed.


================
Comment at: www/analyzer/available_checks.html:459
 
+
 </tbody></table>
----------------
Spurious newline


================
Comment at: www/analyzer/available_checks.html:496
+<div class="example"><pre>
+<!-- TODO: Add examples. And also test files for this checker. -->
+</pre></div></div></td></tr>
----------------
If we don't have a description, let's just drop it.


================
Comment at: www/analyzer/available_checks.html:677
+<div class="example"><pre>
+void use_semaphor_antipattern() {
+  dispatch_semaphore_t sema = dispatch_semaphore_create(0);
----------------
I have a longer description:

This checker finds a common performance anti-pattern in a code that uses Grand Central dispatch.
The anti-pattern involves emulating a synchronous call from an asynchronous API using semaphores,
as in the snippet below, where the `requestCurrentTaskName` function makes an XPC call and then uses the semaphore to
block until the XPC call returns:

```
+ (NSString *)requestCurrentTaskName {
    __block NSString *taskName = nil;
    dispatch_semaphore_t sema = dispatch_semaphore_create(0);
    NSXPCConnection *connection = [[NSXPCConnection alloc] initWithServiceName:@"MyConnection"];
    id remoteObjectProxy = connection.remoteObjectProxy;
    [remoteObjectProxy requestCurrentTaskName:^(NSString *task) {
        taskName = task;
        dispatch_semaphore_signal(sema);
    }];
    dispatch_semaphore_wait(sema, DISPATCH_TIME_FOREVER);
    return taskName;
}
```

Usage of such a pattern in production code running on the main thread is discouraged, as the main queue gets blocked waiting for the background queue,
which could be running at a lower priority, and unnecessary threads are spawned in the process.

In order to avoid the anti-pattern, the available alternatives are:

 - Use the synchronous version of the API, if available.
In the example above, the synchronous XPC proxy object can be used:

```
+ (NSString *)requestCurrentTaskName {
    __block NSString *taskName = nil;
    NSXPCConnection *connection = [[NSXPCConnection alloc] initWithServiceName:@"MyConnection"];
    id remoteObjectProxy = [connection synchronousRemoteObjectProxyWithErrorHandler:^(NSError *error) {
      NSLog(@"Error = %@", error);

    }];
    [remoteObjectProxy requestCurrentTaskName:^(NSString *task) {
        taskName = task;
    }];
    return taskName;
}
```

 - Alternatively, the API can be used in the asynchronous way.



================
Comment at: www/analyzer/available_checks.html:768
+Check for proper uses of Objective-C properties</div></div></td>
+<td><div class="exampleContainer expandable">
+<div class="example"><pre>
----------------
If we don't have proper description, let's drop.


================
Comment at: www/analyzer/available_checks.html:877
+(ObjC)</span><div class="descr">
+Warn about potentially crashing writes to autoreleasing objects from different
+autoreleasing pools in Objective-C.</div></div></td>
----------------
I have a longer description:

Under ARC, function parameters which are pointers to pointers (e.g. NSError**) are `__autoreleasing`.
Writing to such parameters inside autoreleasing pools might crash whenever the parameter outlives the pool. Detecting such crashes may be difficult, as usage of autorelease pool is usually hidden inside the called functions implementation. Examples include:

```
BOOL writeToErrorWithIterator(NSError *__autoreleasing* error, NSArray *a) { [a enumerateObjectsUsingBlock:^{
    *error = [NSError errorWithDomain:1];
    }];
}
```

and

```
BOOL writeToErrorInBlockFromCFunc(NSError *__autoreleasing* error) {
    dispatch_semaphore_t sem = dispatch_semaphore_create(0l);
    dispatch_async(queue, ^{
        if (error) {
            *error = [NSError errorWithDomain:1];
        }
        dispatch_semaphore_signal(sem);
    });
 
    dispatch_semaphore_wait(sem, 100);
  return 0;
}
```



================
Comment at: www/analyzer/available_checks.html:1071
+(ObjC)</span><div class="descr">
+Model the APIs that are guaranteed to return a non-nil value.</div></div></td>
+<td><div class="exampleContainer expandable">
----------------
Probably should be dropped.


================
Comment at: www/analyzer/available_checks.html:1119
+<div class="example"><pre>
+<!-- TODO: Add examples. -->
+</pre></div></div></td></tr>
----------------
Top of the checker file has a somewhat reasonable description:

// A checker for detecting leaks resulting from allocating temporary
// autoreleased objects before starting the main run loop.
//
// Checks for two antipatterns:
// 1. ObjCMessageExpr followed by [[NSRunLoop mainRunLoop] run] in the same
// autorelease pool.
// 2. ObjCMessageExpr followed by [[NSRunLoop mainRunLoop] run] in no
// autorelease pool.
//
// Any temporary objects autoreleased in code called in those expressions
// will not be deallocated until the program exits, and are effectively leaks.



https://reviews.llvm.org/D53069