[PATCH] D13925: Implement __attribute__((internal_linkage))

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 3 16:37:57 PST 2015


On Tue, Nov 3, 2015 at 7:23 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> rsmith added inline comments.
>
> ================
> Comment at: include/clang/Basic/AttrDocs.td:1628
> @@ +1627,3 @@
> +The ``internal_linkage`` attribute changes the linkage type of the declaration to internal.
> +This is similar to C-style ``static``, but can be used on classes and class methods
> +This can be used to contain the ABI of a C++ library by excluding unwanted class methods from the export tables.
> ----------------
> Missing period at end of sentence.
>
> ================
> Comment at: include/clang/Basic/AttrDocs.td:1628
> @@ +1627,3 @@
> +The ``internal_linkage`` attribute changes the linkage type of the declaration to internal.
> +This is similar to C-style ``static``, but can be used on classes and class methods
> +This can be used to contain the ABI of a C++ library by excluding unwanted class methods from the export tables.
> ----------------
> rsmith wrote:
>> Missing period at end of sentence.
> Please also say what it means if the attribute is applied to a class.
>
> ================
> Comment at: lib/AST/Decl.cpp:635-641
> @@ -634,2 +634,9 @@
>    assert(!isa<FieldDecl>(D) && "Didn't expect a FieldDecl!");
>
> +  for (const DeclContext *DC = D->getDeclContext();
> +       !isa<TranslationUnitDecl>(DC); DC = DC->getParent()) {
> +    const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC);
> +    if (ND && ND->getAttr<InternalLinkageAttr>())
> +      return LinkageInfo::internal();
> +  }
> +
> ----------------
> Dead code?
>
> ================
> Comment at: lib/AST/Decl.cpp:1362-1367
> @@ -1346,4 +1361,8 @@
>      }
> -    assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage());
> +    // Linkages may also differ if one of the declarations has
> +    // InternalLinkageAttr.
> +    assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage() ||
> +           (Old->hasAttr<InternalLinkageAttr>() !=
> +            D->hasAttr<InternalLinkageAttr>()));
>  #endif
>
> ----------------
> We should not introduce another case where the linkage of an entity can change after its first declaration. It seems reasonable to require this attribute to be on the first declaration of the function.
>
> ================
> Comment at: lib/Sema/SemaDeclAttr.cpp:3391
> @@ +3390,3 @@
> +                                  unsigned AttrSpellingListIndex) {
> +  if (checkAttrMutualExclusion<InternalLinkageAttr>(*this, D, Range, Ident))
> +    return nullptr;
> ----------------
> Aaron, could we move the mutual exclusion functionality to TableGen? (Separately from this patch, of course.) It looks like there are now 6 attributes that could use the "simple attribute" codepath if we did so.

Yes, I think that would be a good idea. Ideally, I would like the
tablegen to look like:

class Attr {
  // ...
  list<Attr> MutuallyExclusive = [];
}

def Hot {
  // ...
  let MutuallyExclusive = [Cold];
}

def Cold {
  // ...
  let MutuallyExclusive = [Hot];
}

I'll put this on my list of refactorings to do.

~Aaron

>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D13925
>
>
>


More information about the cfe-commits mailing list