Investigating...

On 22 September 2017 at 15:58, Vitaly Buka via cfe-commits <
cfe-commits@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@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.
>> cpp?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/
>> dr11xx.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/
>> linkage2.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/
>> undefined-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@-3 {{here}}
>> +    // expected-note@-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@-3 {{non-type template argument referring to
>> function 'f' with internal linkage}}
>> +    // expected-warning@-3 {{non-type template argument referring to
>> function 'f' with internal linkage}}
>> +#endif
>>    }
>>  }
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to