<div dir="ltr">This belongs in ASTDeclReader::attachPreviousDecl[Impl]. That's where we propagate data that's supposed to be consistent across a redeclaration chain from earlier declarations to later ones.<div><br></div><div>There's another wrinkle here: we should only be propagating the type from a previous declaration that's declared in the same scope (in particular, we should skip over declarations declared as local extern declarations within a function). This may in some cases require walking backwards along the redeclaration chain to find the previous declaration that was not a local extern declaration. That'd be observable in a case like this:</div><div><br></div><div>modulemap:</div><div>module A { module A1 { header "a1.h" } module A2 { header "a2.h" } }</div><div>module B { header "b.h" }</div><div><br></div><div>a1.h:</div><div>int a[];</div><div><br></div><div>b.h:</div><div>void g(int(*)[], int);</div><div>void g(int(*)[1], int) = delete;</div><div>template<typename T> void f(T t) {</div><div> extern int a[];</div><div> g(&a, t);</div><div>}</div><div><br></div><div>a2.h:</div><div>int a[1];</div><div><br></div><div>x.cc:</div><div>#include "a1.h"</div><div>#include "b.h"</div><div>void h() { f(0); } // should not produce an error; type of 'a' within 'f' should not have been updated</div><div><br></div><div>That example actually reveals another problem: we really don't want the completed type to be visible unless there is a visible declaration with the completed type. That suggests that propagating the type across the redeclaration chain may be the wrong approach, and we should instead handle this by changing isPreferredLookupResult (in SemaLookup.cpp) to prefer a VarDecl with a complete type over one with an incomplete type.<br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 5, 2016 at 12:27 PM, Vassil Vassilev <span dir="ltr"><<a href="mailto:v.g.vassilev@gmail.com" target="_blank">v.g.vassilev@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<div>I am not sure where else to put this
logic if not in isSameEntity... Could you point me to a better
place?<span class="HOEnZb"><font color="#888888"><br>
--Vassil</font></span><div><div class="h5"><br>
On 05/02/16 19:56, Richard Smith wrote:<br>
</div></div></div><div><div class="h5">
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">On Fri, Feb 5, 2016 at 7:04 AM,
Vassil Vassilev <span dir="ltr"><<a href="mailto:v.g.vassilev@gmail.com" target="_blank"></a><a href="mailto:v.g.vassilev@gmail.com" target="_blank">v.g.vassilev@gmail.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<div>Good point. Do you have in mind calling
'clang::Sema::MergeVarDeclTypes' or we to just
"duplicate" the logic in
clang::ASTDeclReader::mergeRedeclarable?</div>
</div>
</blockquote>
<div><br>
</div>
<div>It's not safe to call into Sema while we're in a
partially-deserialized state. We know the only interesting
case here is complete versus incomplete array types, so we
don't need anything like the complexity of
MergeVarDeclTypes -- something as easy as "if (new type is
incomplete and old type is not) set new type to old type"
should work.</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<div>
<div>
<div> On 05/02/16 02:50, Richard Smith
via cfe-commits wrote:<br>
</div>
</div>
</div>
<div>
<div>
<blockquote type="cite">
<div dir="ltr">I suspect we'll need to do a little
more than this: when we rebuild the
redeclaration chain, we should probably
propagate the complete type to later
redeclarations in the same scope. Otherwise, if
we import a module that has a complete type and
one that has an incomplete type, the declaration
found by name lookup might be the one with the
incomplete type, possibly resulting in
rejects-valid on code like this:
<div><br>
</div>
<div>a.h:</div>
<div>extern int a[5];</div>
<div><br>
</div>
<div>b.h:</div>
<div>extern int a[];</div>
<div><br>
</div>
<div>x.cc:</div>
<div>#include "a.h"</div>
<div>#include "b.h"</div>
<div>int k = sizeof(a);</div>
<div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Fri, Jan 22,
2016 at 11:03 AM, Yaron Keren via
cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank"></a><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:
yrnkrn<br>
Date: Fri Jan 22 13:03:27 2016<br>
New Revision: 258524<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=258524&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=258524&view=rev</a><br>
Log:<br>
Merge templated static member variables,
fixes <a href="http://llvm.org/pr26179" target="_blank">http://llvm.org/pr26179</a>.<br>
<br>
Patch by Vassil Vassilev!<br>
Reviewed by Richard Smith.<br>
<br>
<br>
Added:<br>
cfe/trunk/test/Modules/Inputs/PR26179/<br>
cfe/trunk/test/Modules/Inputs/PR26179/A.h<br>
cfe/trunk/test/Modules/Inputs/PR26179/B.h<br>
cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h<br>
cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap<br>
cfe/trunk/test/Modules/pr26179.cpp<br>
Modified:<br>
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp<br>
<br>
Modified:
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=258524&r1=258523&r2=258524&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=258524&r1=258523&r2=258524&view=diff</a><br>
==============================================================================<br>
---
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
(original)<br>
+++
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
Fri Jan 22 13:03:27 2016<br>
@@ -2595,8 +2595,24 @@ static bool
isSameEntity(NamedDecl *X, N<br>
// Variables with the same type and
linkage match.<br>
if (VarDecl *VarX =
dyn_cast<VarDecl>(X)) {<br>
VarDecl *VarY =
cast<VarDecl>(Y);<br>
- return
(VarX->getLinkageInternal() ==
VarY->getLinkageInternal())
&&<br>
-
VarX->getASTContext().hasSameType(VarX->getType(),
VarY->getType());<br>
+ if (VarX->getLinkageInternal()
== VarY->getLinkageInternal()) {<br>
+ ASTContext &C =
VarX->getASTContext();<br>
+ if
(C.hasSameType(VarX->getType(),
VarY->getType()))<br>
+ return true;<br>
+<br>
+ // We can get decls with
different types on the redecl chain. Eg.<br>
+ // template <typename T>
struct S { static T Var[]; }; // #1<br>
+ // template <typename T> T
S<T>::Var[sizeof(T)]; // #2<br>
+ // Only? happens when completing
an incomplete array type. In this case<br>
+ // when comparing #1 and #2 we
should go through their elements types.<br>
+ const ArrayType *VarXTy =
C.getAsArrayType(VarX->getType());<br>
+ const ArrayType *VarYTy =
C.getAsArrayType(VarY->getType());<br>
+ if (!VarXTy || !VarYTy)<br>
+ return false;<br>
+ if
(VarXTy->isIncompleteArrayType() ||
VarYTy->isIncompleteArrayType())<br>
+ return
C.hasSameType(VarXTy->getElementType(),
VarYTy->getElementType());<br>
+ }<br>
+ return false;<br>
}<br>
<br>
// Namespaces with the same name and
inlinedness match.<br>
<br>
Added:
cfe/trunk/test/Modules/Inputs/PR26179/A.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/A.h?rev=258524&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/A.h?rev=258524&view=auto</a><br>
==============================================================================<br>
---
cfe/trunk/test/Modules/Inputs/PR26179/A.h
(added)<br>
+++
cfe/trunk/test/Modules/Inputs/PR26179/A.h
Fri Jan 22 13:03:27 2016<br>
@@ -0,0 +1,4 @@<br>
+#include "basic_string.h"<br>
+#include "B.h"<br>
+<br>
+int *p = a;<br>
<br>
Added:
cfe/trunk/test/Modules/Inputs/PR26179/B.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/B.h?rev=258524&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/B.h?rev=258524&view=auto</a><br>
==============================================================================<br>
---
cfe/trunk/test/Modules/Inputs/PR26179/B.h
(added)<br>
+++
cfe/trunk/test/Modules/Inputs/PR26179/B.h
Fri Jan 22 13:03:27 2016<br>
@@ -0,0 +1,2 @@<br>
+#include "basic_string.h"<br>
+extern int a[5];<br>
<br>
Added:
cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h?rev=258524&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h?rev=258524&view=auto</a><br>
==============================================================================<br>
---
cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h
(added)<br>
+++
cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h
Fri Jan 22 13:03:27 2016<br>
@@ -0,0 +1,14 @@<br>
+#ifndef _GLIBCXX_STRING<br>
+#define _GLIBCXX_STRING 1<br>
+<br>
+template<typename T><br>
+struct basic_string {<br>
+ static T _S_empty_rep_storage[];<br>
+};<br>
+<br>
+template<typename T><br>
+T
basic_string<T>::_S_empty_rep_storage[sizeof(T)];<br>
+<br>
+extern int a[];<br>
+<br>
+#endif<br>
<br>
Added:
cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap?rev=258524&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap?rev=258524&view=auto</a><br>
==============================================================================<br>
---
cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap
(added)<br>
+++
cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap
Fri Jan 22 13:03:27 2016<br>
@@ -0,0 +1,9 @@<br>
+module A {<br>
+ header "A.h"<br>
+ export *<br>
+}<br>
+<br>
+module B {<br>
+ header "B.h"<br>
+ export *<br>
+}<br>
<br>
Added:
cfe/trunk/test/Modules/pr26179.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/pr26179.cpp?rev=258524&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/pr26179.cpp?rev=258524&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/pr26179.cpp
(added)<br>
+++ cfe/trunk/test/Modules/pr26179.cpp
Fri Jan 22 13:03:27 2016<br>
@@ -0,0 +1,7 @@<br>
+// RUN: rm -rf %t<br>
+// RUN: %clang_cc1 -I%S/Inputs/PR26179
-verify %s<br>
+// RUN: %clang_cc1 -fmodules
-fmodule-map-file=%S/Inputs/PR26179/module.modulemap
-fmodules-cache-path=%t
-I%S/Inputs/PR26179 -verify %s<br>
+<br>
+#include "A.h"<br>
+<br>
+// expected-no-diagnostics<br>
<br>
<br>
_______________________________________________<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/mailman/listinfo/cfe-commits</a><br>
</blockquote>
</div>
<br>
</div>
</div>
</div>
<br>
<fieldset></fieldset>
<br>
<pre>_______________________________________________
cfe-commits mailing list
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a>
</pre>
</blockquote>
<br>
</div>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
<br>
</div></div></div>
</blockquote></div><br></div></div></div>