[cfe-commits] [patch] Add support for weakref

[Daniel Dunbar]

Hi Rafael,

Some comments:
--
> diff --git a/include/clang/Parse/AttributeList.h b/include/clang/Parse/AttributeList.h
> index ecaa6ae..930dcdf 100644
> --- a/include/clang/Parse/AttributeList.h
> +++ b/include/clang/Parse/AttributeList.h
> @@ -106,6 +106,7 @@ public:
>      AT_visibility,
>      AT_warn_unused_result,
>      AT_weak,
> +    AT_weakref,
>      AT_weak_import,
>      AT_reqd_wg_size,
>      IgnoredAttribute,
> diff --git a/lib/Parse/AttributeList.cpp b/lib/Parse/AttributeList.cpp
> index df48e3a..15ca53b 100644
> --- a/lib/Parse/AttributeList.cpp
> +++ b/lib/Parse/AttributeList.cpp
> @@ -57,6 +57,7 @@ AttributeList::Kind AttributeList::getKind(const IdentifierInfo *Name) {
>    // FIXME: Hand generating this is neither smart nor efficient.
>    return llvm::StringSwitch<AttributeList::Kind>(AttrName)
>      .Case("weak", AT_weak)
> +    .Case("weakref", AT_weakref)
>      .Case("pure", AT_pure)
>      .Case("mode", AT_mode)
>      .Case("used", AT_used)
> diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
> index cba1e9e..e35e3d3 100644
> --- a/lib/Sema/SemaDeclAttr.cpp
> +++ b/lib/Sema/SemaDeclAttr.cpp
> @@ -310,6 +310,32 @@ static void HandleNonNullAttr(Decl *d, const AttributeList &Attr, Sema &S) {
>    d->addAttr(::new (S.Context) NonNullAttr(S.Context, start, size));
>  }
>
> +static void HandleWeakRefAttr(Decl *d, const AttributeList &Attr, Sema &S) {
> +  // check the attribute arguments.

check -> Check

> +  if (Attr.getNumArgs() > 1) {
> +    S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 1;
> +    return;
> +  }
> +
> +  if (Attr.getNumArgs() > 0) {

 == 1 seems a bit more obvious to me.

> +    Expr *Arg = static_cast<Expr*>(Attr.getArg(0));
> +    Arg = Arg->IgnoreParenCasts();
> +    StringLiteral *Str = dyn_cast<StringLiteral>(Arg);
> +
> +    // FIXME: check if target symbol exists in current file
> +    if (Str == 0 || Str->isWide()) {
> +      S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_not_string)
> +          << "weakref" << 1;
> +      return;
> +    }
> +    d->addAttr(::new (S.Context) AliasAttr(S.Context, Str->getString()));
> +  }
> +
> +  // FIXME: In the case of Attr.getNumArgs() == 0, check that
> +  // there is also an alias attribute
> +  d->addAttr(::new (S.Context) WeakAttr());

Can you go ahead and nail this FIXME, so we don't accept invalid?

The GCC docs seem to indicate this is only valid (or only
implemented?) on static decls, should we restrict to that?

> +}
> +
>  static void HandleAliasAttr(Decl *d, const AttributeList &Attr, Sema &S) {
>    // check the attribute arguments.
>    if (Attr.getNumArgs() != 1) {
> @@ -1854,6 +1880,7 @@ static void ProcessDeclAttribute(Scope *scope, Decl *D,
>    case AttributeList::AT_warn_unused_result: HandleWarnUnusedResult(D,Attr,S);
>      break;
>    case AttributeList::AT_weak:        HandleWeakAttr        (D, Attr, S); break;
> +  case AttributeList::AT_weakref:     HandleWeakRefAttr     (D, Attr, S); break;
>    case AttributeList::AT_weak_import: HandleWeakImportAttr  (D, Attr, S); break;
>    case AttributeList::AT_transparent_union:
>      HandleTransparentUnionAttr(D, Attr, S);
> diff --git a/test/CodeGen/attributes.c b/test/CodeGen/attributes.c
> index 770ce76..4fdf1a5 100644
> --- a/test/CodeGen/attributes.c
> +++ b/test/CodeGen/attributes.c
> @@ -30,6 +30,12 @@ int t12 __attribute__((section("SECT")));
>  void __t8() {}
>  void t9() __attribute__((weak, alias("__t8")));
>
> +static void t22(void) __attribute__((weakref("t8")));
> +// CHECK: @t22 = alias weak void ()* @t8
> +
> +static void t23(void) __attribute__((weakref, alias("t8")));
> +// CHECK: @t23 = alias weak void ()* @t8
> +
>  // CHECK: declare extern_weak i32 @t15()
>  int __attribute__((weak_import)) t15(void);
>  int t17() {
--

I don't understand enough about weakref to evaluate the semantics of
this, but from the docs it seems to not be just equivalent to weak +
alias?

 - Daniel

On Tue, Feb 16, 2010 at 2:20 PM, Rafael Espindola <espindola at google.com> wrote:
> The attached patch adds supports to weakref and fixes PR5621.
>
> Cheers,
> --
> Rafael Ávila de Espíndola
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>