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
