r314037 - DR1113: anonymous namespaces formally give their contents internal linkage.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 22 16:48:53 PDT 2017


r314047.

On 22 September 2017 at 16:36, Richard Smith <richard at metafoo.co.uk> wrote:

> Investigating...
>
>
> On 22 September 2017 at 15:58, Vitaly Buka via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> This breaks check-clang-tools
>> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-
>> scei-ps4-windows10pro-fast/builds/12310/steps/test/logs/stdio
>>
>> On Fri, Sep 22, 2017 at 3:21 PM, Richard Smith via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> Author: rsmith
>>> Date: Fri Sep 22 15:21:44 2017
>>> New Revision: 314037
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=314037&view=rev
>>> Log:
>>> DR1113: anonymous namespaces formally give their contents internal
>>> linkage.
>>>
>>> This doesn't affect our code generation in any material way -- we
>>> already give
>>> such declarations internal linkage from a codegen perspective -- but it
>>> has
>>> some subtle effects on code validity.
>>>
>>> We suppress the 'L' (internal linkage) marker for mangled names in
>>> anonymous
>>> namespaces, because it is redundant (the information is already carried
>>> by the
>>> namespace); this deviates from GCC's behavior if a variable or function
>>> in an
>>> anonymous namespace is redundantly declared 'static' (where GCC does
>>> include
>>> the 'L'), but GCC's behavior is incoherent because such a declaration
>>> can be
>>> validly declared with or without the 'static'.
>>>
>>> We still deviate from the standard in one regard here: extern "C"
>>> declarations
>>> in anonymous namespaces are still granted external linkage. Changing
>>> those does
>>> not appear to have been an intentional consequence of the standard
>>> change in
>>> DR1113.
>>>
>>> Added:
>>>     cfe/trunk/test/CXX/drs/dr11xx.cpp
>>> Modified:
>>>     cfe/trunk/lib/AST/Decl.cpp
>>>     cfe/trunk/lib/AST/ItaniumMangle.cpp
>>>     cfe/trunk/test/CXX/basic/basic.link/p8.cpp
>>>     cfe/trunk/test/CodeGenCXX/anonymous-namespaces.cpp
>>>     cfe/trunk/test/SemaCXX/linkage2.cpp
>>>     cfe/trunk/test/SemaCXX/undefined-internal.cpp
>>>
>>> Modified: cfe/trunk/lib/AST/Decl.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.c
>>> pp?rev=314037&r1=314036&r2=314037&view=diff
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/lib/AST/Decl.cpp (original)
>>> +++ cfe/trunk/lib/AST/Decl.cpp Fri Sep 22 15:21:44 2017
>>> @@ -619,16 +619,16 @@ LinkageComputer::getLVForNamespaceScopeD
>>>    if (D->isInAnonymousNamespace()) {
>>>      const auto *Var = dyn_cast<VarDecl>(D);
>>>      const auto *Func = dyn_cast<FunctionDecl>(D);
>>> -    // FIXME: In C++11 onwards, anonymous namespaces should give decls
>>> -    // within them (including those inside extern "C" contexts) internal
>>> -    // linkage, not unique external linkage:
>>> +    // FIXME: The check for extern "C" here is not justified by the
>>> standard
>>> +    // wording, but we retain it from the pre-DR1113 model to avoid
>>> breaking
>>> +    // code.
>>>      //
>>>      // C++11 [basic.link]p4:
>>>      //   An unnamed namespace or a namespace declared directly or
>>> indirectly
>>>      //   within an unnamed namespace has internal linkage.
>>>      if ((!Var || !isFirstInExternCContext(Var)) &&
>>>          (!Func || !isFirstInExternCContext(Func)))
>>> -      return LinkageInfo::uniqueExternal();
>>> +      return getInternalLinkageFor(D);
>>>    }
>>>
>>>    // Set up the defaults.
>>> @@ -1130,7 +1130,7 @@ LinkageInfo LinkageComputer::getLVForLoc
>>>    if (const auto *Function = dyn_cast<FunctionDecl>(D)) {
>>>      if (Function->isInAnonymousNamespace() &&
>>>          !Function->isInExternCContext())
>>> -      return LinkageInfo::uniqueExternal();
>>> +      return getInternalLinkageFor(Function);
>>>
>>>      // This is a "void f();" which got merged with a file static.
>>>      if (Function->getCanonicalDecl()->getStorageClass() == SC_Static)
>>> @@ -1153,7 +1153,7 @@ LinkageInfo LinkageComputer::getLVForLoc
>>>    if (const auto *Var = dyn_cast<VarDecl>(D)) {
>>>      if (Var->hasExternalStorage()) {
>>>        if (Var->isInAnonymousNamespace() && !Var->isInExternCContext())
>>> -        return LinkageInfo::uniqueExternal();
>>> +        return getInternalLinkageFor(Var);
>>>
>>>        LinkageInfo LV;
>>>        if (Var->getStorageClass() == SC_PrivateExtern)
>>>
>>> Modified: cfe/trunk/lib/AST/ItaniumMangle.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Itaniu
>>> mMangle.cpp?rev=314037&r1=314036&r2=314037&view=diff
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/lib/AST/ItaniumMangle.cpp (original)
>>> +++ cfe/trunk/lib/AST/ItaniumMangle.cpp Fri Sep 22 15:21:44 2017
>>> @@ -1287,9 +1287,15 @@ void CXXNameMangler::mangleUnqualifiedNa
>>>        //
>>>        //   void test() { extern void foo(); }
>>>        //   static void foo();
>>> +      //
>>> +      // Don't bother with the L marker for names in anonymous
>>> namespaces; the
>>> +      // 12_GLOBAL__N_1 mangling is quite sufficient there, and this
>>> better
>>> +      // matches GCC anyway, because GCC does not treat anonymous
>>> namespaces as
>>> +      // implying internal linkage.
>>>        if (ND && ND->getFormalLinkage() == InternalLinkage &&
>>>            !ND->isExternallyVisible() &&
>>> -          getEffectiveDeclContext(ND)->isFileContext())
>>> +          getEffectiveDeclContext(ND)->isFileContext() &&
>>> +          !ND->isInAnonymousNamespace())
>>>          Out << 'L';
>>>
>>>        auto *FD = dyn_cast<FunctionDecl>(ND);
>>>
>>> Modified: cfe/trunk/test/CXX/basic/basic.link/p8.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/basic
>>> /basic.link/p8.cpp?rev=314037&r1=314036&r2=314037&view=diff
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/test/CXX/basic/basic.link/p8.cpp (original)
>>> +++ cfe/trunk/test/CXX/basic/basic.link/p8.cpp Fri Sep 22 15:21:44 2017
>>> @@ -52,6 +52,13 @@ void use_visible_no_linkage() {
>>>    visible_no_linkage3(); // expected-note {{used here}}
>>>  }
>>>
>>> +namespace {
>>> +  struct InternalLinkage {};
>>> +}
>>> +InternalLinkage internal_linkage(); // expected-error {{used but not
>>> defined}}
>>> +void use_internal_linkage() {
>>> +  internal_linkage(); // expected-note {{used here}}
>>> +}
>>>
>>>  extern inline int not_defined; // expected-error {{not defined}}
>>>  extern inline int defined_after_use;
>>>
>>> Added: cfe/trunk/test/CXX/drs/dr11xx.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/drs/d
>>> r11xx.cpp?rev=314037&view=auto
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/test/CXX/drs/dr11xx.cpp (added)
>>> +++ cfe/trunk/test/CXX/drs/dr11xx.cpp Fri Sep 22 15:21:44 2017
>>> @@ -0,0 +1,30 @@
>>> +// RUN: %clang_cc1 -std=c++98 %s -verify -fexceptions -fcxx-exceptions
>>> -pedantic-errors
>>> +// RUN: %clang_cc1 -std=c++11 %s -verify -fexceptions -fcxx-exceptions
>>> -pedantic-errors
>>> +// RUN: %clang_cc1 -std=c++14 %s -verify -fexceptions -fcxx-exceptions
>>> -pedantic-errors
>>> +// RUN: %clang_cc1 -std=c++17 %s -verify -fexceptions -fcxx-exceptions
>>> -pedantic-errors
>>> +// RUN: %clang_cc1 -std=c++2a %s -verify -fexceptions -fcxx-exceptions
>>> -pedantic-errors
>>> +
>>> +namespace dr1113 { // dr1113: partial
>>> +  namespace named {
>>> +    extern int a; // expected-note {{previous}}
>>> +    static int a; // expected-error {{static declaration of 'a' follows
>>> non-static}}
>>> +  }
>>> +  namespace {
>>> +    extern int a;
>>> +    static int a; // ok, both declarations have internal linkage
>>> +    int b = a;
>>> +  }
>>> +
>>> +  // FIXME: Per DR1113 and DR4, this is ill-formed due to ambiguity:
>>> the second
>>> +  // 'f' has internal linkage, and so does not have C language linkage,
>>> so is
>>> +  // not a redeclaration of the first 'f'.
>>> +  //
>>> +  // To avoid a breaking change here, Clang ignores the "internal
>>> linkage" effect
>>> +  // of anonymous namespaces on declarations declared within an 'extern
>>> "C"'
>>> +  // linkage-specification.
>>> +  extern "C" void f();
>>> +  namespace {
>>> +    extern "C" void f();
>>> +  }
>>> +  void g() { f(); }
>>> +}
>>>
>>> Modified: cfe/trunk/test/CodeGenCXX/anonymous-namespaces.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCX
>>> X/anonymous-namespaces.cpp?rev=314037&r1=314036&r2=314037&view=diff
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/test/CodeGenCXX/anonymous-namespaces.cpp (original)
>>> +++ cfe/trunk/test/CodeGenCXX/anonymous-namespaces.cpp Fri Sep 22
>>> 15:21:44 2017
>>> @@ -6,7 +6,8 @@ int f();
>>>
>>>  namespace {
>>>    // CHECK-1: @_ZN12_GLOBAL__N_11bE = internal global i32 0
>>> -  // CHECK-1: @_ZN12_GLOBAL__N_1L1cE = internal global i32 0
>>> +  // CHECK-1: @_ZN12_GLOBAL__N_11cE = internal global i32 0
>>> +  // CHECK-1: @_ZN12_GLOBAL__N_12c2E = internal global i32 0
>>>    // CHECK-1: @_ZN12_GLOBAL__N_11D1dE = internal global i32 0
>>>    // CHECK-1: @_ZN12_GLOBAL__N_11aE = internal global i32 0
>>>    int a = 0;
>>> @@ -15,6 +16,12 @@ namespace {
>>>
>>>    static int c = f();
>>>
>>> +  // Note, we can't use an 'L' mangling for c or c2 (like GCC does)
>>> based on
>>> +  // the 'static' specifier, because the variable can be redeclared
>>> without it.
>>> +  extern int c2;
>>> +  int g() { return c2; }
>>> +  static int c2 = f();
>>> +
>>>    class D {
>>>      static int d;
>>>    };
>>>
>>> Modified: cfe/trunk/test/SemaCXX/linkage2.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/l
>>> inkage2.cpp?rev=314037&r1=314036&r2=314037&view=diff
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/test/SemaCXX/linkage2.cpp (original)
>>> +++ cfe/trunk/test/SemaCXX/linkage2.cpp Fri Sep 22 15:21:44 2017
>>> @@ -143,9 +143,10 @@ namespace test13 {
>>>  }
>>>
>>>  namespace test14 {
>>> +  // Anonymous namespace implies internal linkage, so 'static' has no
>>> effect.
>>>    namespace {
>>> -    void a(void); // expected-note {{previous declaration is here}}
>>> -    static void a(void) {} // expected-error {{static declaration of
>>> 'a' follows non-static declaration}}
>>> +    void a(void);
>>> +    static void a(void) {}
>>>    }
>>>  }
>>>
>>>
>>> Modified: cfe/trunk/test/SemaCXX/undefined-internal.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/u
>>> ndefined-internal.cpp?rev=314037&r1=314036&r2=314037&view=diff
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/test/SemaCXX/undefined-internal.cpp (original)
>>> +++ cfe/trunk/test/SemaCXX/undefined-internal.cpp Fri Sep 22 15:21:44
>>> 2017
>>> @@ -187,6 +187,10 @@ namespace OverloadUse {
>>>      void f();
>>>      void f(int); // expected-warning {{function
>>> 'OverloadUse::(anonymous namespace)::f' has internal linkage but is not
>>> defined}}
>>>      void f(int, int); // expected-warning {{function
>>> 'OverloadUse::(anonymous namespace)::f' has internal linkage but is not
>>> defined}}
>>> +#if __cplusplus < 201103L
>>> +    // expected-note at -3 {{here}}
>>> +    // expected-note at -3 {{here}}
>>> +#endif
>>>    }
>>>    template<void x()> void t() { x(); }
>>>    template<void x(int)> void t(int*) { x(10); }
>>> @@ -194,6 +198,10 @@ namespace OverloadUse {
>>>    void g(int n) {
>>>      t<f>(&n); // expected-note {{used here}}
>>>      t<f>(&n, &n); // expected-note {{used here}}
>>> +#if __cplusplus < 201103L
>>> +    // expected-warning at -3 {{non-type template argument referring to
>>> function 'f' with internal linkage}}
>>> +    // expected-warning at -3 {{non-type template argument referring to
>>> function 'f' with internal linkage}}
>>> +#endif
>>>    }
>>>  }
>>>
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170922/9e27cd34/attachment-0001.html>


More information about the cfe-commits mailing list