[PATCH] Initial support for Attribute subjects

[Aaron Ballman]

On Wed, Jul 10, 2013 at 3:53 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> +      else if ((*I)->isSubClassOf("SubsetSubject")) {
> +        std::string FN, TN;
> +        bool IsDecl = writeAppertainsToSubjectSubsetDefinition(OS, *I, FN, TN);
> +        std::string C = FN + "(cast<" + TN + ">(P))";
>
> This cast<> doesn't look right: we can get here if the potential
> subject is not a TN, can't we?

Good catch!  I assume the preference is for dyn_cast instead of
dyn_cast_or_null?  I will also update the appertainsTo*** functions
check for null input in the case dyn_cast fails.

> How are you intending that process of creating and adding attributes
> will work once this lands? Right now, the usual flow of handleFooAttr
> is:
>
> For each entity where we want to apply the attribute:
>  * Check attribute appertains to entity
>  * Perform other semantic checks on the attribute and on whether it
> can be attached to the entity
>  * Create FooAttr instance and add it
>
> With appertainsTo being a method on FooAttr, that won't work. Perhaps
> we can flip this around:
>
>  * Perform semantic checks on the attribute by itself
>  * Create FooAttr from the AttributeList
>  * For each entity where we want to apply the attribute:
>     * Check whether it appertains to the entity
>     * Perform semantic checks on the (attribute, entity) pair
>     * Add it
>
> There are a couple of cases where we need to modify the attribute once
> we see what it's being applied to; those don't look too hard to
> support but are something to keep in mind.

You are right, this would be slightly awkward since it requires the
attribute instance itself.  But I'm not certain the attribute instance
is strictly required -- what if these were static functions instead?
So you could do:

if (FooAttr::appertainsTo(something)) {

}

This would allow you to do:

* Check appertainsTo on a specific attribute
* Perform other semantic checks
* Create FooAttr instance and add it

The downside to this would be that you would no longer be able to do
someAttrInst->appertainsTo(something), but that could be something
easily added as an instance method if we felt it was beneficial at
some point.

Thoughts (and thanks for the review!)?

~Aaron