Improve documentation of the type safety attributes

[ygao]

Summary

Hi,
I was preparing our internal release documentation with our tech writers, and
when reviewing the clang manual for the current release, we think the current
documentation of type safety attributes are inadequate in a few aspects:

1. The arguments to these attributes are not described.
2. Some of the MPI code examples seem incomplete as they miss declarations.
3. Some references could be made more clear. For example, for the "type_tag_for_datatype" attribute, it is mentioned that its optional third argument determines "how the expression is compared to the type tag", but it is left unspecified what the "expression" is referring to.

We would like to share the edits we have made, both in hope that the edits are
useful to a wider audience, and also to gather some feedback that the new
documentation is still accurate.

Could you review?

• Gao

[aaron.ballman]

LGTM, thank you for improving the documentation!

include/clang/Basic/AttrDocs.td

 def DocCatTypeSafety : DocumentationCategory<"Type Safety Checking"> {
   let Content = [{
 Clang supports additional attributes to enable checking type safety properties
-that can't be enforced by the C type system.  Use cases include:
+that can't be enforced by the C type system. To see warnings produced by these
+checks, ensure that -Wtype-safety is enabled. Use cases include:
 
 * MPI library implementations, where these attributes enable checking that
   the buffer type matches the passed ``MPI_Datatype``;

 Use ``__attribute__((argument_with_type_tag(arg_kind, arg_idx,
 type_tag_idx)))`` on a function declaration to specify that the function
 accepts a type tag that determines the type of some other argument.
-``arg_kind`` is an identifier that should be used when annotating all
-applicable type tags.
 
 This attribute is primarily useful for checking arguments of variadic functions
 (``pointer_with_type_tag`` can be used in most non-variadic cases).
 
+In the attribute prototype above:
+* ``arg_kind`` is an identifier that should be used when annotating all
+  applicable type tags.
+* ``arg_idx`` provides the position of a function argument. The expected type of
+  this function argument will be determined by the function argument specified
+  by ``type_tag_idx``. In the code example below, "3" means that the type of the
+  function's third argument will be determined by ``type_tag_idx``.
+* ``type_tag_idx`` provides the position of a function argument. This function
+  argument will be a type tag. The type tag will determine the expected type of
+  the argument specified by ``arg_idx``. In the code example below, "2" means
+  that the type tag associated with the function's second argument should agree
+  with the type of the argument specified by ``arg_idx``.
+
 For example:
 
 .. code-block:: c++

 
   int fcntl(int fd, int cmd, ...)
       __attribute__(( argument_with_type_tag(fcntl,3,2) ));
+  // The function's second argument will be a type tag; this type tag will
+  // determine the expected type of the function's third argument.
   }];
 }
 

 on a function declaration to specify that the function accepts a type tag that
 determines the pointee type of some other pointer argument.
 
+In the attribute prototype above:
+* ``ptr_kind`` is an identifier that should be used when annotating all
+  applicable type tags.
+* ``ptr_idx`` provides the position of a function argument; this function
+  argument will have a pointer type. The expected pointee type of this pointer
+  type will be determined by the function argument specified by
+  ``type_tag_idx``. In the code example below, "1" means that the pointee type
+  of the function's first argument will be determined by ``type_tag_idx``.
+* ``type_tag_idx`` provides the position of a function argument; this function
+  argument will be a type tag. The type tag will determine the expected pointee
+  type of the pointer argument specified by ``ptr_idx``. In the code example
+  below, "3" means that the type tag associated with the function's third
+  argument should agree with the pointee type of the pointer argument specified
+  by ``ptr_idx``.
+
 For example:
 
 .. code-block:: c++
 
+  typedef int MPI_Datatype;
   int MPI_Send(void *buf, int count, MPI_Datatype datatype /*, other args omitted */)
       __attribute__(( pointer_with_type_tag(mpi,1,3) ));
+  // The function's 3rd argument will be a type tag; this type tag will
+  // determine the expected pointee type of the function's 1st argument.
   }];
 }
 

 def TypeTagForDatatypeDocs : Documentation {
   let Category = DocCatTypeSafety;
   let Content = [{
+When declaring a variable, use
+``__attribute__((type_tag_for_datatype(kind, type)))`` to create a type tag that
+is tied to the ``type`` argument given to the attribute.
+
+In the attribute prototype above:
+* ``kind`` is an identifier that should be used when annotating all applicable
+  type tags.
+* ``type`` indicates the name of the type.
+
 Clang supports annotating type tags of two forms.
 
-* **Type tag that is an expression containing a reference to some declared
-  identifier.** Use ``__attribute__((type_tag_for_datatype(kind, type)))`` on a
-  declaration with that identifier:
+* **Type tag that is a reference to a declared identifier.**
+  Use ``__attribute__((type_tag_for_datatype(kind, type)))`` when declaring that
+  identifier:
 
   .. code-block:: c++
 
+    typedef int MPI_Datatype;
     extern struct mpi_datatype mpi_datatype_int
         __attribute__(( type_tag_for_datatype(mpi,int) ));
     #define MPI_INT ((MPI_Datatype) &mpi_datatype_int)
+    // &mpi_datatype_int is a type tag. It is tied to type "int".
 
-* **Type tag that is an integral literal.** Introduce a ``static const``
-  variable with a corresponding initializer value and attach
-  ``__attribute__((type_tag_for_datatype(kind, type)))`` on that declaration,
-  for example:
+* **Type tag that is an integral literal.**
+  Declare a ``static const`` variable with an initializer value and attach
+  ``__attribute__((type_tag_for_datatype(kind, type)))`` on that declaration:
 
   .. code-block:: c++
 
+    typedef int MPI_Datatype;
+    static const MPI_Datatype mpi_datatype_int
+        __attribute__(( type_tag_for_datatype(mpi,int) )) = 42;
     #define MPI_INT ((MPI_Datatype) 42)
-    static const MPI_Datatype mpi_datatype_int
-        __attribute__(( type_tag_for_datatype(mpi,int) )) = 42
+    // The number 42 is a type tag. It is tied to type "int".
 
-The attribute also accepts an optional third argument that determines how the
-expression is compared to the type tag.  There are two supported flags:
 
+The ``type_tag_for_datatype`` attribute also accepts an optional third argument
+that determines how the type of the function argument specified by either
+``arg_idx`` or ``ptr_idx`` is compared against the type associated with the type
+tag. (Recall that for the ``argument_with_type_tag`` attribute, the type of the
+function argument specified by ``arg_idx`` is compared against the type
+associated with the type tag. Also recall that for the ``pointer_with_type_tag``
+attribute, the pointee type of the function argument specified by ``ptr_idx`` is
+compared against the type associated with the type tag.) There are two supported
+values for this optional third argument:
+
 * ``layout_compatible`` will cause types to be compared according to
-  layout-compatibility rules (C++11 [class.mem] p 17, 18).  This is
-  implemented to support annotating types like ``MPI_DOUBLE_INT``.
+  layout-compatibility rules (In C++11 [class.mem] p 17, 18, see the
+  layout-compatibility rules for two standard-layout struct types and for two
+  standard-layout union types). This is useful when creating a type tag
+  associated with a struct or union type. For example:
 
-  For example:
-
   .. code-block:: c++
 
     /* In mpi.h */
+    typedef int MPI_Datatype;
     struct internal_mpi_double_int { double d; int i; };
     extern struct mpi_datatype mpi_datatype_double_int
-        __attribute__(( type_tag_for_datatype(mpi, struct internal_mpi_double_int, layout_compatible) ));
+        __attribute__(( type_tag_for_datatype(mpi,
+                        struct internal_mpi_double_int, layout_compatible) ));
 
     #define MPI_DOUBLE_INT ((MPI_Datatype) &mpi_datatype_double_int)
 
+    int MPI_Send(void *buf, int count, MPI_Datatype datatype, ...)
+        __attribute__(( pointer_with_type_tag(mpi,1,3) ));
+
     /* In user code */
     struct my_pair { double a; int b; };
     struct my_pair *buffer;
-    MPI_Send(buffer, 1, MPI_DOUBLE_INT /*, ...  */); // no warning
+    MPI_Send(buffer, 1, MPI_DOUBLE_INT /*, ...  */); // no warning because the
+                                                     // layout of my_pair is
+                                                     // compatible with that of
+                                                     // internal_mpi_double_int
 
     struct my_int_pair { int a; int b; }
     struct my_int_pair *buffer2;
-    MPI_Send(buffer2, 1, MPI_DOUBLE_INT /*, ...  */); // warning: actual buffer element
-                                                      // type 'struct my_int_pair'
-                                                      // doesn't match specified MPI_Datatype
+    MPI_Send(buffer2, 1, MPI_DOUBLE_INT /*, ...  */); // warning because the
+                                                      // layout of my_int_pair
+                                                      // does not match that of
+                                                      // internal_mpi_double_int
 
-* ``must_be_null`` specifies that the expression should be a null pointer
-  constant, for example:
+* ``must_be_null`` specifies that the function argument specified by either
+  ``arg_idx`` (for the ``argument_with_type_tag`` attribute) or ``ptr_idx`` (for
+  the ``pointer_with_type_tag`` attribute) should be a null pointer constant.
+  The second argument to the ``type_tag_for_datatype`` attribute is ignored. For
+  example:
 
   .. code-block:: c++
 
     /* In mpi.h */
+    typedef int MPI_Datatype;
     extern struct mpi_datatype mpi_datatype_null
         __attribute__(( type_tag_for_datatype(mpi, void, must_be_null) ));
 
     #define MPI_DATATYPE_NULL ((MPI_Datatype) &mpi_datatype_null)
+    int MPI_Send(void *buf, int count, MPI_Datatype datatype, ...)
+        __attribute__(( pointer_with_type_tag(mpi,1,3) ));
 
     /* In user code */
+    struct my_pair { double a; int b; };
+    struct my_pair *buffer;
     MPI_Send(buffer, 1, MPI_DATATYPE_NULL /*, ...  */); // warning: MPI_DATATYPE_NULL
                                                         // was specified but buffer
                                                         // is not a null pointer