[PATCH] D107026: [Clang] Add support for attribute 'escape'

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:260
+``escape`` placed on a function parameter of a pointer type is used to indicate
+that the pointer can escape the function. This means that a reference to the object
+the pointer points to that is derived from the parameter value may survive
----------------
aaron.ballman wrote:
> guitard0g wrote:
> > NoQ wrote:
> > > Are you sure you want "can escape" instead of "will escape"? In the former case it's impossible to implement the warning without false positives ("well, it says it may escape, but I know for sure that if I pass these other arguments then it won't escape"). In the latter case, of course, a lot of places where the value escapes conditionally won't be able to wear the attribute. Do I understand correctly that you want to proceed with the more aggressive diagnostic?
> > Hmm that's an interesting question. Initially my thought was that any possibility of escaping should eliminate the option of passing in a temporary pointer, which I think is reasonable in Swift's implicit bridging use-case. In terms of the API contract, I was thinking that any API author who annotates their function parameter with 'escaping' is effectively saying you shouldn't pass in a pointer that you're not comfortable escaping. I think it may be a little restrictive to say that the pointer will always escape for certain, but I do think that anybody using a function that takes an escaping parameter should treat this as if it were always escaping (if that makes sense). I suppose my question then would be, do you think it's better to say "will escape" if that's what the client calling into an API should expect (even though the pointer might not always escape in the implementation)? 
> > ``escape`` placed on a function parameter of a pointer type is used to indicate that the pointer can escape the function.
> 
> FWIW, this reads a bit strangely to me because that's already the case without the attribute. Anything that's not marked *can* escape. I think "will escape" more clearly identifies to the person reading the function signature that the parameter should be treated as though it will escape. If it does not escape, that's not necessarily an issue because that shouldn't change how the user treats the argument (for example, it may not escape today but it may start to escape in the future).
> 
> One thing the docs are missing is an explanation of what benefit you get from using this annotation. Does this improve optimizations? Enable better checking somewhere? That sort of thing (with examples) would be good to add.
If I understand correctly, the new attribute proclaims "Do not, under any circumstances, pass stack pointers here". Like, it doesn't necessarily escape every time, but you should expect the function to be able to stash that pointer somewhere without telling you, for indefinite amount of time, so if you put your temporaries into this function you're 100% doing something wrong and you want to be notified about this.

This take probably raises more questions though. Like, if the pointer is a heap pointer but we see a nearby `delete` of that pointer with our own eyes, do we warn too? Where exactly is the boundary between short-lived and long-lived objects? (IIUC the answer is yes, we do want to warn, because the function stashes the pointer for *indefinite* amount of time so it should be the judge for when to delete it, not you).

I think there could be a simple compiler warning associated with that attribute which would pattern-match the argument to see if it's an immediate stack address such as `&Class()` or `&Var` where `Var` is a local variable. A static analyzer check for the more sophisticated cases works too of course.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107026/new/

https://reviews.llvm.org/D107026