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