[RFC] __attribute__((internal_linkage))

Hi,

this is a proposal to add a new function attribute to Clang. The
attribute would be called "internal_linkage" (or any other name) and
it would, as the name implies, set the function linkage to "internal".

The attribute would let us apply the same behavior as of C "static"
keyword, but on C++ class methods. AFAIK, there is no way to do this
currently.

The first use case would be libc++.

To ensure ABI stability, libc++ attempts to tightly control the set of
symbols exported from libc++.so. This is achieved with a "trick" of
marking all functions and class methods defined in the library headers
on Linux/MacOSX with::

  __attribute__((always_inline, visibility("hidden")))

All direct calls to functions marked with the always_inline attribute
are inlined and are never emitted as an external symbol
reference. Hidden visibility removes the symbol from the shared
library exports list.

This approach has a number of drawbacks.

* always_inline functions are not always inlined. Inlining is an
  optimization, and there are multiple cases when a compiler can
  decide against it.

  * Clang can sometimes emit an **unreachable**
    call to an always_inline function as an external symbol reference.
  * Inlining may be impossible between functions with incompatible
    attributes. For example, ``__attribute__((no_sanitize_address))``,
    which suppresses AddressSanitizer instrumentation in a function,
    prevents inlining of functions that do not have the same
    attribute.

* At -O0 optimization level indiscriminate inlining results in very
  large stack frames. As a consequence, removing these attributes
  speeds up libc++ test suite by 10%.

In this case, what libc++ really needs is internal linkage. Inlining
is just an undesired side effect. The always_inline attribute could be
replaced with the proposed internal_linkage attribute for all the
benefits and no drawbacks.

large stack frames. As a consequence, removing these attributes

Dropping howard.hinnant@apple.com for Howard’s correct email.

1. What would happen if the user attempts to take the address of a
function marked
`__attribute__((internal_linkage))`? Wouldn't this case required external
linkage?
For libc++ it's sufficient if it causes a compile error because users
aren't allowed
to take the address of functions in the standard library.

Addresses of such functions would just compare not equal across TUs. I
think that's pretty reasonable, and probably conformant. There are many
ways that STL functions can be called indirectly (std::function's vtable),
so I don't think we want to try to forbid that.

2. It would be great if we could apply the attribute to a namespace, that
in turn applies it to
   all of the symbols within. Would this be possible?

The Clang attribute machinery *appears* to support attributes on namespace,
but there doesn't seem to be a single test for it in the test suite. I
don't think we can do that yet. Anyway, this feels like a separate feature
request to me.

C++ attributes are supported on namespaces, but I'm not certain about
GNU attributes on them. I'm not aware of any GNU attributes that
appertain to a namespace, so I'm not surprised about a lack of test
coverage.

As for the feature itself, I think it's possibly a related feature
instead of a separate one, but I don't feel strongly. Given the use
case is "let's remove a bunch of not-really-working" libc++ code, I
can see the namespace being the better approach for those needs, with
a declaration attribute that happens to be the underlying
implementation of the namespace attribute.

~Aaron

Do we really want to apply the attribute to the entire namespace? We'd
need to skip at least extern template methods (like
char_traits<char>).

Do we really want to apply the attribute to the entire namespace? We'd
need to skip at least extern template methods (like
char_traits<char>).

Eric was asking for it specifically, so I am assuming that was
something he might want to do. I would certainly be curious what the
semantics would be for it.

I mostly would like to avoid replacing one solution that clutters
libc++ with another one that clutters equally, if we can avoid it.

~Aaron

I've put together an implementation of this attribute:
http://reviews.llvm.org/D13925

Sorry the namespace question was a bit of a distraction. I was curious
because I want to add a `__libcpp_internal` namespace that I can put
implementation details into without having to worry about them being
visible. However I don't want that to infect this discussion.

Evgenii thanks for all the work. I noticed that your tests rule out
applying the new attribute to a class declaration. However libc++ currently
applies the the visibility attributes to classes. I've always assumed that
this simply propagated the visibility to each member function but please
let me know if I'm wrong. I think this attribute needs to work the same way.

/Eric

Take any random libstdc++ header, after macro expansion:

namespace std __attribute__(__visibility__("default"))

Annoyingly, using GNU-style attributes is also the only portable way
of putting an attribute on a namespace: g++ only accepts C++-style
namespace attributes in that same position (before the opening brace)
while clang only accepts it using a standards-compliant standalone
declaration inside the namespace.

Hmm, at least I thought it was? can't seem to find it anymore in the spec...

Do we really want to apply the attribute to the entire namespace? We'd
need to skip at least extern template methods (like
char_traits<char>).

Eric was asking for it specifically, so I am assuming that was
something he might want to do. I would certainly be curious what the
semantics would be for it.

I mostly would like to avoid replacing one solution that clutters
libc++ with another one that clutters equally, if we can avoid it.

I've had some time to think on this a lot more lately, and I have some
questions that may reverse my original opinion. What would the
behavior be for something like this:

namespace [[clang::internal_linkage]] n {
  // Some decls...
}

namespace n {
  // Other decls
}

Would it be an error if the namespace were re-opened without the
attribute? What happens if the order of the namespace declarations
were reversed? What if the namespaces were in separate TUs?

~Aaron

Do we really want to apply the attribute to the entire namespace? We'd
need to skip at least extern template methods (like
char_traits<char>).

Eric was asking for it specifically, so I am assuming that was
something he might want to do. I would certainly be curious what the
semantics would be for it.

I mostly would like to avoid replacing one solution that clutters
libc++ with another one that clutters equally, if we can avoid it.

I've had some time to think on this a lot more lately, and I have some
questions that may reverse my original opinion. What would the
behavior be for something like this:

namespace [[clang::internal_linkage]] n {
  // Some decls...
}

namespace n {
  // Other decls
}

Would it be an error if the namespace were re-opened without the
attribute? What happens if the order of the namespace declarations
were reversed? What if the namespaces were in separate TUs?

The current behavior is to apply internal linkage to "Some decls" but
not to "Other decls". I kinda like it.

...
namespace [[clang::internal_linkage]] n {
  // More decls...
}

And "More decls" are internal again.

Do we really want to apply the attribute to the entire namespace? We'd
need to skip at least extern template methods (like
char_traits<char>).

Eric was asking for it specifically, so I am assuming that was
something he might want to do. I would certainly be curious what the
semantics would be for it.

I mostly would like to avoid replacing one solution that clutters
libc++ with another one that clutters equally, if we can avoid it.

I've had some time to think on this a lot more lately, and I have some
questions that may reverse my original opinion. What would the
behavior be for something like this:

namespace [[clang::internal_linkage]] n {
  // Some decls...
}

namespace n {
  // Other decls
}

Would it be an error if the namespace were re-opened without the
attribute? What happens if the order of the namespace declarations
were reversed? What if the namespaces were in separate TUs?

The current behavior is to apply internal linkage to "Some decls" but
not to "Other decls". I kinda like it.

...
namespace [[clang::internal_linkage]] n {
  // More decls...
}

And "More decls" are internal again.

That behavior conflicts with how namespaces work in general. When you
open a namespace, you start adding declarations to it. When you reopen
the same namespace, you are adding declarations to the same namespace
as the original declarations. The behavior that you find useful is
something I would find confusing.

I don't have an issue with the non-namespace subjects for this
attribute, but I think we should be cautious about adding attributes
to namespaces; we can always add that feature later if it turns out to
be as desirable as I once thought it was. :wink:

~Aaron

OK, I've removed namespaces from the patch.
I've also added tests for forward declarations of classes and
functions, everything seems to work as expected.