[LifetimeAnalysis] Do not forbid void deref type in gsl::Pointer/gsl::Owner annotations

[xazax.hun]

Summary

It turns out it is useful to be able to define the deref type as void. In case we have a type erased owner, we want to express that the pointee can be basically any type. I think it should not be unnatural to have a void deref type as we already familiar with "pointers to void".

[gribozavr2]

Could you provide a more fleshed out example of a case where it is useful?

Could you update the documentation for the attribute? I'm forgetting if there's a spec for this attribute -- is this extension part of that spec?

Do you foresee any issues or further special cases? For example, IIRC, 'operator*' is required to return DerefType. I have a hard time understanding how one would define 'operator*' in such an owner.

[xazax.hun]

In D72097#1801777, @gribozavr2 wrote:

Could you provide a more fleshed out example of a case where it is useful?

Sure! The std::any could be one example (std::variant is similar, but ultimately, we migh want to be able to enumerate multiple types in that case rather than just using void).

The specific example I found was an allocator: https://fuchsia-review.googlesource.com/c/fuchsia/+/340159/3/zircon/kernel/lib/fbl/include/fbl/arena.h#25

This might not be super important as it might be unlikely that we will ever find a problem where an arena is destroyed before an object allocated there is dereferenced, but still, it is good to cover this use case as well.

Could you update the documentation for the attribute? I'm forgetting if there's a spec for this attribute -- is this extension part of that spec?

void is currently not mentioned in the documentation, but adding an example and the interpretation would help indeed, thanks!

Do you foresee any issues or further special cases? For example, IIRC, 'operator*' is required to return DerefType. I have a hard time understanding how one would define 'operator*' in such an owner.

I think those types that have void as deref types are unlikely to implement operator*. There might be some skeletons but I am not aware of anything at the moment.

[xazax.hun]

• Update the documentation.

[Charusso]

clang/include/clang/Basic/AttrDocs.td

4619

cna typo

[xazax.hun]

• Fix typo.

[xazax.hun]

clang/include/clang/Basic/AttrDocs.td

4619

Thanks!

[aaron.ballman]

LGTM!

[gribozavr2]

Sorry, just getting back to this review. Your justification makes sense, and the patch LGTM.

clang/include/clang/Basic/AttrDocs.td

.. code-block:: c++
   private:
     int value;
   public:
     int *getInt() { return &value; }
   };
 
 The argument ``T`` is optional and is ignored.
 This attribute may be used by analysis tools and has no effect on code
-generation.
+generation. A ``void`` argument means that the class can own any type.
 
 See Pointer_ for an example.
 }];
 }
 
 def LifetimePointerDocs : Documentation {
   let Category = DocCatDecl;
   let Content = [{
.. code-block:: c++
   private:
     int *valuePointer;
   public:
     int *getInt() { return &valuePointer; }
   };
 
 The argument ``T`` is optional and is ignored.
 This attribute may be used by analysis tools and has no effect on code
-generation.
+generation. A ``void`` argument means that the pointer can point to any type.
 
 Example:
 When constructing an instance of a class annotated like this (a Pointer) from
 an instance of a class annotated with ``[[gsl::Owner]]`` (an Owner),
 then the analysis will consider the Pointer to point inside the Owner.
 When the Owner's lifetime ends, it will consider the Pointer to be dangling.
 
 .. code-block:: c++

clang/include/clang/Basic/DiagnosticSemaKinds.td

def warn_auto_var_is_id : Warning<
   InGroup<DiagGroup<"auto-var-id">>;
 
 // Attributes
 def err_nsobject_attribute : Error<
   "'NSObject' attribute is for pointer types only">;
 def err_attributes_are_not_compatible : Error<
   "%0 and %1 attributes are not compatible">;
 def err_attribute_invalid_argument : Error<
-  "%select{'void'|a reference type|an array type|a non-vector or "
+  "%select{a reference type|an array type|a non-vector or "
   "non-vectorizable scalar type}0 is an invalid argument to attribute %1">;
 def err_attribute_wrong_number_arguments : Error<
   "%0 attribute %plural{0:takes no arguments|1:takes one argument|"
   ":requires exactly %1 arguments}1">;
 def err_attribute_too_many_arguments : Error<
   "%0 attribute takes no more than %1 argument%s1">;
 def err_attribute_too_few_arguments : Error<
   "%0 attribute takes at least %1 argument%s1">;

clang/lib/Sema/SemaDeclAttr.cpp

static void handleVecTypeHint(Sema &S, Decl *D, const ParsedAttr &AL) {
 
   TypeSourceInfo *ParmTSI = nullptr;
   QualType ParmType = S.GetTypeFromParser(AL.getTypeArg(), &ParmTSI);
   assert(ParmTSI && "no type source info for attribute argument");
 
   if (!ParmType->isExtVectorType() && !ParmType->isFloatingType() &&
       (ParmType->isBooleanType() ||
        !ParmType->isIntegralType(S.getASTContext()))) {
-    S.Diag(AL.getLoc(), diag::err_attribute_invalid_argument) << 3 << AL;
+    S.Diag(AL.getLoc(), diag::err_attribute_invalid_argument) << 2 << AL;
     return;
   }
 
   if (VecTypeHintAttr *A = D->getAttr<VecTypeHintAttr>()) {
     if (!S.Context.hasSameType(A->getTypeHint(), ParmType)) {
       S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL;
       return;
     }

 
 static void handleLifetimeCategoryAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   TypeSourceInfo *DerefTypeLoc = nullptr;
   QualType ParmType;
   if (AL.hasParsedType()) {
     ParmType = S.GetTypeFromParser(AL.getTypeArg(), &DerefTypeLoc);
 
     unsigned SelectIdx = ~0U;
-    if (ParmType->isVoidType())
+    if (ParmType->isReferenceType())
       SelectIdx = 0;
-    else if (ParmType->isReferenceType())
-      SelectIdx = 1;
     else if (ParmType->isArrayType())
-      SelectIdx = 2;
+      SelectIdx = 1;
 
     if (SelectIdx != ~0U) {
       S.Diag(AL.getLoc(), diag::err_attribute_invalid_argument)
           << SelectIdx << AL;
       return;
     }
   }
 

clang/test/SemaCXX/attr-gsl-owner-pointer.cpp

 
 class [[gsl::Owner(int)]] [[gsl::Pointer(int)]] BothOwnerPointer{};
 // expected-error@-1 {{'Pointer' and 'Owner' attributes are not compatible}}
 // expected-note@-2 {{conflicting attribute is here}}
 // CHECK: CXXRecordDecl {{.*}} BothOwnerPointer
 // CHECK: OwnerAttr {{.*}} int
 
 class [[gsl::Owner(void)]] OwnerVoidDerefType{};
-// expected-error@-1 {{'void' is an invalid argument to attribute 'Owner'}}
+// CHECK: CXXRecordDecl {{.*}} OwnerVoidDerefType
+// CHECK: OwnerAttr {{.*}} void
 class [[gsl::Pointer(void)]] PointerVoidDerefType{};
-// expected-error@-1 {{'void' is an invalid argument to attribute 'Pointer'}}
+// CHECK: CXXRecordDecl {{.*}} PointerVoidDerefType
+// CHECK: PointerAttr {{.*}} void
 
 class [[gsl::Pointer(int)]] AddConflictLater{};
 // CHECK: CXXRecordDecl {{.*}} AddConflictLater
 // CHECK: PointerAttr {{.*}} int
 class [[gsl::Owner(int)]] AddConflictLater;
 // expected-error@-1 {{'Owner' and 'Pointer' attributes are not compatible}}
 // expected-note@-5 {{conflicting attribute is here}}
 // CHECK: CXXRecordDecl {{.*}} AddConflictLater