[PATCH] D63954: Add lifetime categories attributes

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:4164
+  let Content = [{
+When annotating a class ``O`` with ``[[gsl::Owner(T)]]``, then each function
+that returns cv-qualified ``T&`` or ``T*`` is assumed to return a
----------------
Slightly better: "In a class annotated with ..., each function that returns ... is assumed to ..."

However, the "is assumed" part throws me off. There's no assumption here -- this is what this attribute means, intentionally, by design, no guessing involved.

So how about this?

The attribute `[[gsl::Owner(T)]]` applies to structs and classes that own an object of type `T`:

```
[[gsl::Owner(int)]] class IntOwner {
private:
  int value;
public:
  int *getInt() { return &value; }
  long *getLong();
};
```

In a class annotated like this each member function that returns `T&` or `T*` returns a pointer/reference to the data owned by the class instance:

```
IntOwner int_owner;
int *x = int_owner.getInt();
// `x` points to memory owned by `int_owner`.
```

Therefore, that the lifetime of that data ends when ... For example:

```
int *x = IntOwner().getInt();
// `x` points to memory owned by the temporary `IntOwner()`, which has been destroyed now.

long *y = IntOwner().getLong();
// Can't make any conclusion about validity of `y`.
```



================
Comment at: clang/include/clang/Basic/AttrDocs.td:4177
+  let Content = [{
+When annotating a class ``P`` with ``[[gsl::Pointer(T)]]``, it assumed to be a
+non-owning type whose objects can point to ``T`` type objects or dangle.
----------------
Ditto, "A class annotated with ... is assumed to be ..."

However, again, I feel like "assume" is the wrong word to use.

"The attribute `[[gsl::Pointer(T)]]` applies to structs and classes that behave like pointers to an object of type `T`:

```
class [[gsl::Pointer(int)]] IntPointer {
private:
  int *valuePointer;
public:
  // fill it in with appropriate APIs
};
```

Classes annotated like this behave like regular pointers: they can either point to objects of type `T`, or dangle (<<<explain what it means>>>).

For example: ... sample code and a static analysis message...


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4546-4547
+
+  if (ParmType->isVoidType()) {
+    S.Diag(AL.getLoc(), diag::err_attribute_invalid_argument) << "'void'" << AL;
+    return;
----------------
xazax.hun wrote:
> aaron.ballman wrote:
> > Is `void` really the only problematic type? What about incomplete types, for instance?
> I think incomplete types are fine. The only information we actually need here is the name the pointee types. E.g. for a `SomeOwner<Incomplete>` which is annotated `Owner(Incomplete)`, we will assume that a method returning `Incomplete*`, `std::reference_wrapper<Incomplete>` etc will return an object pointing to the memory owned by `SomeOwner`.
What about array types? References?

gsl::Owner(int[])
gsl::Owner(int&)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63954