Feature request: annotation for maybe deleted this [clang:temporal-safety]
- Dominant language
- LLVM
- Stars
- 40.5k
- Forks
- 18.7k
- PR merge metrics
- PR metrics pending
Description
In Chromium network code, it is common to have a method like `HandleMessage()` that may synchronously send a response message. Sending a response message may fail if there is no longer a route to the remote host, in which case cleanup code may have run and deleted the current object.
We generally handle this in one of two ways
1) Use a weak pointer to keep track of whether `this` is still valid, or
2) Since 1) is expensive, in simpler cases just have a comment saying `// this may be deleted here` after the call.
For the second case, it would be useful to have an annotation instead of a comment so that that the compiler can check that members are not accessed afterwards. For example:
```
HandleMessage(message);
[[this_maybe_deleted]];
return;
```
If someone then added some code before `return` that relied on `this`, the compiler could issue a warning.
Another issue is that another method further up the stack may not be aware of the comment and may add code that touches `this`. To help prevent this, the annotation would also be useful as an attribute on function declarations, eg.
```
[[this_maybe_deleted]] void ReadThenHandleMessage();
```
Then code like
```
ReadThenHandleMessage();
RecordHistogram("Net.MessageCount", message_count_);
```
would also issue a warning.
Further enhancements could issue a warning if `ReadThenHandleMessage()` calls `HandleMessage()` which has the `[[this_maybe_deleted]]` annotation but `ReadThenHandleMessage()` itself doesn't have the annotation. Calling such a method from the destructor could also issue a warning.
Another possible enhancement would be to warn on incorrect use of the annotation. For example,
```
HandleMessage();
LOG(VERBOSE) << "Handled message with id " << last_id_;
[[this_maybe_deleted]];
```
is a bug which could be caught, assuming we can see inside the LOG macro to see that it will never delete `this`.
A common pattern in Chromium IPC code is to have an IPC handler which is owned by a factory object, and to call a method on the factory object to request deletion, something like:
```
factory_->RemoveHandler(this);
// this is deleted here.
```
In this case, `this` is always deleted, but it's probably not necessary to have a separate annotation for this case, as the warnings that need to be given are the same.
Contributor guide
Research direction
No implementation file, test, or entry point is named. Start by reviewing the existing clang temporal-safety work and how Clang represents declaration attributes and statement annotations, then determine the diagnostic and propagation rules needed for the examples in the issue. Done means the annotation design is implemented with diagnostics and tests covering the listed use cases.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 28/100