[cfe-commits] r106818 - in /cfe/trunk: include/clang/AST/Attr.h lib/Sema/Sema.h lib/Sema/SemaDeclAttr.cpp lib/Sema/SemaTemplateInstantiateDecl.cpp test/SemaTemplate/attributes.cpp

[Douglas Gregor]

On Jun 24, 2010, at 8:22 PM, Chandler Carruth wrote:

> Author: chandlerc
> Date: Thu Jun 24 22:22:07 2010
> New Revision: 106818
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=106818&view=rev
> Log:
> Implement dependent alignment attribute support. This is a bit gross given the
> current attribute system, but it is enough to handle class templates which
> specify parts of their alignment in terms of their template parameters.

Looks like this fixes PR6322.

> +/// \brief Atribute for specifying the alignment of a variable or type.
> +///
> +/// This node will either contain the precise Alignment (in bits, not bytes!)
> +/// or will contain the expression for the alignment attribute in the case of
> +/// a dependent expression within a class or function template. At template
> +/// instantiation time these are transformed into concrete attributes.
> class AlignedAttr : public Attr {
>   unsigned Alignment;
> +  Expr *AlignmentExpr;
> public:
>   AlignedAttr(unsigned alignment)
> -    : Attr(attr::Aligned), Alignment(alignment) {}
> +    : Attr(attr::Aligned), Alignment(alignment), AlignmentExpr(0) {}
> +  AlignedAttr(Expr *E)
> +    : Attr(attr::Aligned), Alignment(0), AlignmentExpr(E) {}
> +
> +  /// getAlignmentExpr - Get a dependent alignment expression if one is present.
> +  Expr *getAlignmentExpr() const {
> +    return AlignmentExpr;
> +  }
> +
> +  /// isDependent - Is the alignment a dependent expression
> +  bool isDependent() const {
> +    return getAlignmentExpr();
> +  }

Why not always store the Expr* for the expression describing the alignment, and then check getAlignmentExpr() && (getAlignmentExpr()->isValueDependent() || getAlignmentExpr()->isTypeDependent()) to determine when the attribute is dependent? Then the AST visitors, printers, etc., would be able to print the attribute as it was written in the source.

> Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=106818&r1=106817&r2=106818&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Thu Jun 24 22:22:07 2010
> @@ -143,14 +143,29 @@
>   return false;
> }
> 
> -// FIXME: Is this too simple?
> +// FIXME: Is this still too simple?
> void TemplateDeclInstantiator::InstantiateAttrs(Decl *Tmpl, Decl *New) {
> -  for (const Attr *TmplAttr = Tmpl->getAttrs(); TmplAttr; 
> +  for (const Attr *TmplAttr = Tmpl->getAttrs(); TmplAttr;
>        TmplAttr = TmplAttr->getNext()) {
> -    
> +    // FIXME: This should be generalized to more than just the AlignedAttr.
> +    if (const AlignedAttr *Aligned = dyn_cast<AlignedAttr>(TmplAttr)) {
> +      if (Aligned->isDependent()) {
> +        // The alignment expression is not potentially evaluated.
> +        EnterExpressionEvaluationContext Unevaluated(SemaRef,
> +                                                     Action::Unevaluated);
> +
> +        OwningExprResult Result = SemaRef.SubstExpr(Aligned->getAlignmentExpr(),
> +                                                    TemplateArgs);
> +        if (!Result.isInvalid())
> +          // FIXME: Is this the correct source location?
> +          SemaRef.AddAlignedAttr(Aligned->getAlignmentExpr()->getExprLoc(),
> +                                 New, Result.takeAs<Expr>());
> +        continue;
> +      }
> +    }
> +

Looks good.

	- Doug