[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In D70469#1777858 <https://reviews.llvm.org/D70469#1777858>, @aaron.ballman wrote:

> In D70469#1776523 <https://reviews.llvm.org/D70469#1776523>, @NoQ wrote:
>
> > It still mildly worries me that the attributes aren't truly reusable, as the exact meaning of the attribute depends on the domain. In particular, every domain is expected to have different approaches to error handling, i.e. a function that creates or destroys a handle may fail to, respectively, create or destroy the handle, but instead indicate the failure in a domain-specific manner, eg. through magical return values or null handle or errno or whatever.
> >
> > @aaron.ballman, do you think this is a problem? Should we rather go for an attribute name that's obviously domain-specific (eg., `__attribute__((fuchsia_acquire_handle))`), or is it ok to re-use attributes without re-using their exact meaning?
>
>
> That may be part of why I keep pushing back on this addition -- it seems like these are general purpose attributes that can be used to identify what a handle is and where a handle is obtained/released. Or these could be specific to a particular coding guideline's definition of a handle and associated semantics. If the goal is to only support a limited set of use cases, then I think something like `[[clang::fucschia_acquire_handle]]` makes more sense. If the goal is to provided general utilities for the static analyzer to reason about handles, then I think we want the more generalized names.


Basically, I think the exact semantics belong to the static analysis and the annotation is only there to supplement some information about the code that is not available in the type system. So in the sense I envisioned these attributes as a general way to encode knowledge about an API without relying on any semantics from any analysis. And the FuchsiaHandleChecker would be a first analysis to consume these handles. Inevitable, different analyses have to interpret these slightly differently when it comes to error handling and other corner cases. This is the result of APIs being fundamentally different. The question is, does this justify having a separate set of annotations for all handles, like `posix_handle_acquire`, `fuchsia_handle_acquire`? I do not have strong feelings and I am also OK with going a Fuchsia specific name. It is not that hard to add a new spelling without code repetition. Also, having separate spelling could provide some additional benefits, e.g. we could check of someone is trying to close a posix handle using a fuchsia API. I think both directions have some pros and cons the ultimate questions is what do we care about more. Assigning more specific semantics to annotations (as opposed to the analyses only) or reducing the annotation vocabulary.


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

https://reviews.llvm.org/D70469