[cfe-commits] [PATCH] Type safety attributes (was: Compile-time MPI_Datatype checking)

[Hal Finkel]

Dmitri,

Thank you for working on this, I think that this will be very useful.
I've lost count of how many bugs I've fixed that would have been caught
with this patch (along with properly-annotated system headers).

A few comments and questions:

layout_compatible - The documentation on this needs to be expanded.
I recommend an example or two (at least).

Do we need a new has_feature check so that we can properly guard the
new attributes in public headers?

Are C++ objects that are implicitly convertible to pointers of the
correct type handled correctly [this is not obvious to me from reading
the patch, but I'm not an expert here]?

> +  bool IsMPI = ArgumentKind->isStr("mpi");
> +
> +  if (SpecifiedType->isVoidType()) {
> +    // Type tag with matching void type requires a null pointer.
> +    if (!ArgumentExpr->isNullPointerConstant(Context,
> +
> Expr::NPC_ValueDependentIsNotNull)) {
> +      Diag(ArgumentExpr->getExprLoc(),
> +           IsMPI ? diag::warn_mpi_datatype_null_but_buffer_not_null
> +                 : diag::warn_type_safety_null_pointer_required)
> +          << ArgumentExpr->getSourceRange()
> +          << TypeTagExpr->getSourceRange();
> +    }
> +    return;
> +  }

Can we generalize this as well? Maybe we should call this
void_must_be_null, and make it like layout_compatible (an optional
parameter)? [As far as I can tell, this is the only MPI-specific logic
left, is there anything else?]

 -Hal

On Wed, 23 May 2012 21:10:09 +0300
Dmitri Gribenko <gribozavr at gmail.com> wrote:

> Hello,
> 
> Please find attached a patch that implements a generalization of my
> previous attempts at compile-time MPI_Datatype checking.  Please
> review.
> 
> I'm including an excerpt from the patch (docs/LanguageExtensions.html)
> with a description of proposed attributes and usage examples below.
> 
> <!--
> =======================================================================
> --> <h2 id="type_safety">Type Safety Checking</h2> <!--
> =======================================================================
> -->
> 
> <p>Clang supports additional attributes to enable checking type safety
> properties that can't be enforced by C type system.  Usecases include:
> <ul>
> <li>MPI library implementations, where these attributes enable
> checking that buffer type matches the passed
> <tt>MPI_Datatype</tt>;</li> <li>for HDF5 library there is a similar
> usecase as MPI;</li> <li>checking types of variadic functions'
> arguments for functions like <tt>fcntl()</tt> and
> <tt>ioctl()</tt>.</li> </ul>
> </p>
> 
> <h3
> id="argument_with_type_tag"><tt>argument_with_type_tag(...)</tt></h3>
> 
> <p>Use <tt>__attribute__((argument_with_type_tag(arg_kind, arg_idx,
> type_tag_idx)))</tt> on a function declaration to specify that the
> function accepts a type tag that determines the type of some other
> argument. <tt>arg_kind</tt> is an identifier that should be used when
> annotating all applicable type tags.</p>
> 
> <p>This attribute is primarily useful for checking arguments of
> variadic functions (<tt>pointer_with_type_tag</tt> can be used in
> most of non-variadic cases).</p>
> 
> <p>For example:</p>
> <blockquote>
> <pre>
> int fcntl(int fd, int cmd, ...)
>       __attribute__(( argument_with_type_tag(fcntl,3,2) ));
> </pre>
> </blockquote>
> 
> <h3
> id="pointer_with_type_tag"><tt>pointer_with_type_tag(...)</tt></h3>
> 
> <p>Use <tt>__attribute__((pointer_with_type_tag(ptr_kind, ptr_idx,
> type_tag_idx)))</tt> on a function declaration to specify that the
> function a type tag that determines the pointee type of some other
> pointer argument.</p>
> 
> <p>For example:</p>
> <blockquote>
> <pre>
> int MPI_Send(void *buf, int count, MPI_Datatype datatype /*, other
> args omitted */)
>     __attribute__(( pointer_with_type_tag(mpi,1,3) ));
> </pre>
> </blockquote>
> 
> <h3
> id="type_tag_for_datatype"><tt>type_tag_for_datatype(...)</tt></h3>
> 
> <p>Clang supports annotating type tags of two forms.</p>
> 
> <p><b>Type tag that is an expression containing a reference to some
> declared identifier.</b> Use
> <tt>__attribute__((type_tag_for_datatype(kind, type)))</tt> on a
> declaration with that identifier:</p>
> 
> <blockquote>
> <pre>
> extern struct mpi_datatype mpi_datatype_int
>     __attribute__(( type_tag_for_datatype(mpi,int) ));
> #define MPI_INT ((MPI_Datatype) &mpi_datatype_int)
> </pre>
> </blockquote>
> 
> <p><b>Type tag that is an integral literal.</b>  Introduce a
> <tt>static const</tt> variable with a corresponding initializer value
> and attach <tt>__attribute__((type_tag_for_datatype(kind,
> type)))</tt> on that declaration, for example</p>
> 
> <blockquote>
> <pre>
> #define MPI_INT ((MPI_Datatype) 42)
> static const MPI_Datatype mpi_datatype_int
>     __attribute__(( type_tag_for_datatype(mpi,int) )) = 42
> </pre>
> </blockquote>
> 
> <p>The attribute also accepts an optional third argument
> <tt>layout_compatible<tt>, that will cause types to be compared
> according to layout-compatibility rules.  This is implemented to
> support annotating types like <tt>MPI_DOUBLE_INT</tt>.</p>
> 
> Dmitri Gribenko
> 



-- 
Hal Finkel
Postdoctoral Appointee
Leadership Computing Facility
Argonne National Laboratory