[llvm-dev] [cfe-dev] [RFC] __attribute__((internal_linkage))

[Evgenii Stepanov via llvm-dev]

Hi,

it's been put on hold for other higher priority work. I still want to
get it done one day, but it's not clear when that would be.

On Tue, Jul 12, 2016 at 6:11 PM, Akira Hatanaka <ahatanak at gmail.com> wrote:
> Hi Evgenii,
>
> I was wondering what the status is of your work to attach "internal_linkage"
> to methods of standard library classes in libc++. The following piece of
> code doesn't link because a symbol (string::empty) is undefined and it
> sounds like your work might fix the linkage error I'm seeing.
>
> $ cat test1.cpp
> #include <string>
> #include <functional>
>
> int main() {
>   using namespace std::placeholders;
>   auto b = std::bind(&std::string::empty, _1);
> }
>
> On Mon, Nov 9, 2015 at 1:58 PM, Evgenii Stepanov via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>>
>> In fact, during the code review we've decided against allowing this
>> attribute on namespaces: http://reviews.llvEm.org/D13925
>>
>>
>> The intended use of this attribute in libc++ is on functions that
>> programs are not allowed to take address of, like all methods of
>> standard library classes. Visibility("hidden") attribute (which we are
>> replacing with internal_linkage) violates ODR the same way.
>>
>> Also, this attribute (for us) is not about locality or performance.
>> Its primary purpose is to reliably exclude a class method from the
>> export table. This is the way libc++ manages ABI stability: all class
>> methods that are not part of the library ABI are defined in the
>> headers with a set of attributes that ensure that they become internal
>> to the translation unit. Always_inline does that in almost all cases,
>> but almost is not good enough. Internal_linkage is a straighforward
>> way to achieve the same.
>>
>>
>> On Mon, Nov 9, 2015 at 1:34 PM, Martin J. O'Riordan
>> <martin.oriordan at movidius.com> wrote:
>> > With respect only to '__attribute__((internal_linkage))', not 'nodebug'
>> > and other parts of this topic; does hiding "some" members and not others not
>> > introduce a violation of the ODR because some members of the class as it
>> > appears in one translation unit are not the same actual definitions as the
>> > apparently "same" members of the class in another translation unit?  In
>> > particular the ODR requirement that would ensure that the address of a
>> > method was the same regardless of where the address was taken, including
>> > 'static' methods (no 'this' pointer)?  Or hidden things like the guard
>> > variables for local static objects, default argument initialisers (which are
>> > instanced), etc.?
>> >
>> > I must admit that having spent years trying to lock down the definition
>> > of the ODR, that this seems to fly in the opposite direction of the intent
>> > of that rule.
>> >
>> > I can (sort of) see why for particular reasons of locality you might
>> > want multiple actual definitions, even though they might exhibit the same
>> > behaviour, but maybe the answer for locality optimisation would be for the
>> > linker technology to evolve so that multiple copies of the same definition
>> > (sic) could be made; perhaps even at the Basic Block level [Block Level
>> > Linking].  But this too would lie under the auspices of the "as if" rule and
>> > respect the ODR.
>> >
>> > There is something I am missing in this that is not making me at all
>> > comfortable about the use of this attribute for namespaces (apart from the
>> > reopening issues raised by others), and in particular the application to
>> > parts of a class but not the whole class.  I just feel that this is leading
>> > to semantic trouble.
>> >
>> > Thanks,
>> >
>> >         MartinO
>> >
>> > -----Original Message-----
>> > From: Evgenii Stepanov [mailto:eugeni.stepanov at gmail.com]
>> > Sent: 09 November 2015 18:49
>> > To: Robinson, Paul <Paul_Robinson at playstation.sony.com>
>> > Cc: Chris Lattner <clattner at apple.com>;
>> > sstewartgallus00 at mylangara.bc.ca; llvm-dev at lists.llvm.org; Martin J.
>> > O'Riordan <martin.oriordan at movidius.com>
>> > Subject: Re: [llvm-dev] [cfe-dev] [RFC]
>> > __attribute__((internal_linkage))
>> >
>> > Sorry, I totally missed updates to this thread.
>> >
>> > Anonymous namespace does not work for libc++ because we need to hide
>> > some class members, and export the rest. Namespace would hide all of them.
>> > AFAIK, "static" is un-deprecated in C++11 for multiple reasons, including
>> > this one.
>> >
>> > I agree that nodebug should not affect codegen. It would also kill debug
>> > info (ex. all debug locations in libc++ headers).
>> >
>> >
>> > On Mon, Nov 2, 2015 at 11:57 AM, Robinson, Paul via llvm-dev
>> > <llvm-dev at lists.llvm.org> wrote:
>> >>
>> >>
>> >>> -----Original Message-----
>> >>> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of
>> >>> Chris Lattner via llvm-dev
>> >>> Sent: Saturday, October 31, 2015 2:58 PM
>> >>> To: sstewartgallus00 at mylangara.bc.ca
>> >>> Cc: LLVM Developers
>> >>> Subject: Re: [llvm-dev] [cfe-dev] [RFC]
>> >>> __attribute__((internal_linkage))
>> >>>
>> >>> Hi Stewart,
>> >>>
>> >>> I saw this get brought up at the LLVM devmtg in the context of
>> >>> improving the stack size of libc++ frames.
>> >>>
>> >>> Have you guys considered a different approach to solving this
>> >>> problem?  In the case of libc++, the _LIBCPP_INLINE_VISIBILITY macro
>> >>> currently expands out to "__attribute__ ((__always_inline__))”.  It
>> >>> seems reasonable to me to have it also add the “nodebug” attribute as
>> >>> well.  This should allow the allocas generated by inlining lots of
>> >>> frames to be promoted to registers (because there is no debug info to
>> >>> pessimize).
>> >>
>> >> Are you suggesting that 'nodebug' should affect code generation?
>> >> It most definitely should not...
>> >> --paulr
>> >>
>> >>>
>> >>> This would dramatically shrink the stack frames of code using libc++
>> >>> at - O0, and would also make it go a lot faster.
>> >>>
>> >>> -Chris
>> >>>
>> >>>
>> >>> > On Oct 29, 2015, at 6:35 AM, Martin J. O'Riordan via llvm-dev
>> >>> > <llvm-
>> >>> dev at lists.llvm.org> wrote:
>> >>> >
>> >>> > I haven't been able to figure out from this thread why this
>> >>> > attribute is
>> >>> necessary at all.
>> >>> >
>> >>> > Anonymous or unnamed namespaces were added to C++ for this very
>> >>> > purpose,
>> >>> but the ISO C++ Standard does not discuss "linkage" per-se because it
>> >>> is outside the scope of the language specification.  But it does
>> >>> describes it in terms of having a hidden name which is "unique" to
>> >>> the translation unit, and under the "as if" rule, internal linkage is
>> >>> a common convention for achieving this.
>> >>> >
>> >>> > This is just Standardese for dealing with what compiler
>> >>> > implementers
>> >>> typically handle as "internal linkage" anyway; at the same time, the
>> >>> use of 'static' for this purpose was deprecated.  This is
>> >>> specifically stated in C++98 section 7.3.1.1 and unnamed namespaces
>> >>> are still similarly defined in C++17 (Working Paper) although the
>> >>> specific reference to the deprecation of 'static' for this purpose is
>> >>> now gone.
>> >>> >
>> >>> > The closest the Standard gets to talking about linkage is with
>> >>> > Linkage
>> >>> Specifications, and even here it tries to avoid to avoid treading on
>> >>> the definitions things like internal versus external linkage.
>> >>> >
>> >>> > So I am curious, what does this proposed attribute on namespaces
>> >>> > achieve
>> >>> that cannot already be achieved with an anonymous or unnamed
>> >>> namespace?
>> >>> >
>> >>> > Thanks,
>> >>> >
>> >>> >     Martin O'Riordan - Movidius Ltd.
>> >>> >
>> >>> > -----Original Message-----
>> >>> > From: cfe-dev [mailto:cfe-dev-bounces at lists.llvm.org] On Behalf Of
>> >>> Evgenii Stepanov via cfe-dev
>> >>> > Sent: 23 October 2015 22:41
>> >>> > To: Steven Stewart-Gallus <sstewartgallus00 at mylangara.bc.ca>
>> >>> > Cc: Clang Dev <cfe-dev at lists.llvm.org>
>> >>> > Subject: Re: [cfe-dev] [RFC] __attribute__((internal_linkage))
>> >>> >
>> >>> > Sounds right. The proposed attribute is a rough equivalent of
>> >>> >
>> >>> > static void foo::do_method(void)
>> >>> > {
>> >>> >  // implementation here
>> >>> > }
>> >>> >
>> >>> > where "static" does not mean a static class member, but makes the
>> >>> declaration local to the translation unit. C-style "static".
>> >>> >
>> >>> > The patch in http://reviews.llvm.org/D13925 (waiting for review,
>> >>> > btw!)
>> >>> supports this attribute on classes and namespaces, with this syntax:
>> >>> >
>> >>> > namespace __attribute__((internal_linkage)) { }
>> >>> >
>> >>> >
>> >>> > On Fri, Oct 23, 2015 at 9:03 AM, Steven Stewart-Gallus via cfe-dev
>> >>> > <cfe-
>> >>> dev at lists.llvm.org> wrote:
>> >>> >> Hello,
>> >>> >>
>> >>> >> Can I ask for some clarification?
>> >>> >>
>> >>> >> Apparently, C++ doesn't allow to static class methods?
>> >>> >>
>> >>> >> While in C one might write inside a header file:
>> >>> >>
>> >>> >> static inline void mylib_foo_do_method(struct mylib_foo *foo) {
>> >>> >>   // implementation here
>> >>> >> }
>> >>> >>
>> >>> >> In C++ the typical style would be to write something like:
>> >>> >>
>> >>> >> namespace mylib {
>> >>> >>
>> >>> >> void foo::do_method(void)
>> >>> >> {
>> >>> >>  // implementation here
>> >>> >> }
>> >>> >> }
>> >>> >>
>> >>> >> Unfortunately, the C++ case then implies some linkage behaviour
>> >>> >> that some might not want.
>> >>> >>
>> >>> >> Apparently, one can't do things like:
>> >>> >>
>> >>> >> namespace mylib {
>> >>> >>
>> >>> >> static void foo::do_method(void)
>> >>> >> {
>> >>> >>  // implementation here
>> >>> >> }
>> >>> >> }
>> >>> >>
>> >>> >> Or,
>> >>> >>
>> >>> >> namespace {
>> >>> >> namespace mylib {
>> >>> >>
>> >>> >> static void foo::do_method(void)
>> >>> >> {
>> >>> >>  // implementation here
>> >>> >> }
>> >>> >> }
>> >>> >> }
>> >>> >>
>> >>> >> Also, if you wanted to an attribute to a whole namespace you
>> >>> >> should do something like the following I think:
>> >>> >>
>> >>> >> namespace mylib {
>> >>> >> [[clang::internal_linkage]];
>> >>> >>
>> >>> >> static void foo::do_method(void)
>> >>> >> {
>> >>> >>  // implementation here
>> >>> >> }
>> >>> >> }
>> >>> >>
>> >>> >>
>> >>> >> Thank you,
>> >>> >> Steven Stewart-Gallus
>> >>> >> _______________________________________________
>> >>> >> cfe-dev mailing list
>> >>> >> cfe-dev at lists.llvm.org
>> >>> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>> >>> > _______________________________________________
>> >>> > cfe-dev mailing list
>> >>> > cfe-dev at lists.llvm.org
>> >>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>> >>> >
>> >>> > _______________________________________________
>> >>> > LLVM Developers mailing list
>> >>> > llvm-dev at lists.llvm.org
>> >>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> >>>
>> >>> _______________________________________________
>> >>> LLVM Developers mailing list
>> >>> llvm-dev at lists.llvm.org
>> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> >> _______________________________________________
>> >> LLVM Developers mailing list
>> >> llvm-dev at lists.llvm.org
>> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> >
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>