<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">ping...<br>
On 07/02/16 22:33, Vassil Vassilev wrote:<br>
</div>
<blockquote cite="mid:56B7AA17.9090701@gmail.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
<div class="moz-cite-prefix">Improve a comment.<br>
--Vassil<br>
On 07/02/16 20:48, Vassil Vassilev wrote:<br>
</div>
<blockquote cite="mid:56B79F80.8030105@gmail.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Would this patch be any better?<br>
--Vassil<br>
On 05/02/16 21:49, Richard Smith wrote:<br>
</div>
<blockquote
cite="mid:CAOfiQq==e_F=1ZACRu4Z5Wgihouu5KDMTuV5mbeEw1dxzk4Thw@mail.gmail.com"
type="cite">
<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
moz-do-not-send="true"
class="moz-txt-link-abbreviated"
href="mailto:v.g.vassilev@gmail.com"><a class="moz-txt-link-abbreviated" href="mailto:v.g.vassilev@gmail.com">v.g.vassilev@gmail.com</a></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
moz-do-not-send="true"
class="moz-txt-link-abbreviated"
href="mailto:v.g.vassilev@gmail.com"><a class="moz-txt-link-abbreviated" href="mailto:v.g.vassilev@gmail.com">v.g.vassilev@gmail.com</a></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
moz-do-not-send="true"
class="moz-txt-link-abbreviated"
href="mailto:cfe-commits@lists.llvm.org"><a class="moz-txt-link-abbreviated" href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a></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
moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://llvm.org/viewvc/llvm-project?rev=258524&view=rev"><a class="moz-txt-link-freetext" href="http://llvm.org/viewvc/llvm-project?rev=258524&view=rev">http://llvm.org/viewvc/llvm-project?rev=258524&view=rev</a></a><br>
Log:<br>
Merge templated
static member
variables, fixes <a
moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://llvm.org/pr26179"><a class="moz-txt-link-freetext" href="http://llvm.org/pr26179">http://llvm.org/pr26179</a></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
moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=258524&r1=258523&r2=258524&view=diff"><a class="moz-txt-link-freetext" href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=258524&r1=258523&r2=258524&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=258524&r1=258523&r2=258524&view=diff</a></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
moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/A.h?rev=258524&view=auto"><a class="moz-txt-link-freetext" href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/A.h?rev=258524&view=auto">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/A.h?rev=258524&view=auto</a></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
moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/B.h?rev=258524&view=auto"><a class="moz-txt-link-freetext" href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/B.h?rev=258524&view=auto">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/B.h?rev=258524&view=auto</a></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
moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h?rev=258524&view=auto"><a class="moz-txt-link-freetext" href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h?rev=258524&view=auto">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h?rev=258524&view=auto</a></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
moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap?rev=258524&view=auto"><a class="moz-txt-link-freetext" href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap?rev=258524&view=auto">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap?rev=258524&view=auto</a></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
moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/pr26179.cpp?rev=258524&view=auto"><a class="moz-txt-link-freetext" href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/pr26179.cpp?rev=258524&view=auto">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/pr26179.cpp?rev=258524&view=auto</a></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
moz-do-not-send="true"
class="moz-txt-link-abbreviated"
href="mailto:cfe-commits@lists.llvm.org"><a class="moz-txt-link-abbreviated" href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a></a><br>
<a
moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits"><a class="moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a></a><br>
</blockquote>
</div>
<br>
</div>
</div>
</div>
<br>
<fieldset></fieldset>
<br>
<pre>_______________________________________________
cfe-commits mailing list
<a moz-do-not-send="true" href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>
<a moz-do-not-send="true" 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>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>