ping...
On 07/02/16 22:33, Vassil Vassilev wrote:
Improve a comment.
--Vassil
On 07/02/16 20:48, Vassil Vassilev wrote:
Would this patch be any better?
--Vassil
On 05/02/16 21:49, Richard Smith wrote:
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.
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:
modulemap:
module A { module A1 { header "a1.h" } module A2 { header "a2.h" } }
module B { header "b.h" }
a1.h:
int a[];
b.h:
void g(int(*)[], int);
void g(int(*)[1], int) = delete;
template<typename T> void f(T t) {
extern int a[];
g(&a, t);
}
a2.h:
int a[1];
x.cc:
#include "a1.h"
#include "b.h"
void h() { f(0); } // should not produce an error; type of 'a'
within 'f' should not have been updated
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.
On Fri, Feb 5, 2016 at 12:27 PM, Vassil Vassilev
<v.g.vassi...@gmail.com> wrote:
I am not sure where else to put this logic if not in
isSameEntity... Could you point me to a better place?
--Vassil
On 05/02/16 19:56, Richard Smith wrote:
On Fri, Feb 5, 2016 at 7:04 AM, Vassil Vassilev
<v.g.vassi...@gmail.com> wrote:
Good point. Do you have in mind calling
'clang::Sema::MergeVarDeclTypes' or we to just "duplicate"
the logic in clang::ASTDeclReader::mergeRedeclarable?
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.
On 05/02/16 02:50, Richard Smith via cfe-commits wrote:
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:
a.h:
extern int a[5];
b.h:
extern int a[];
x.cc:
#include "a.h"
#include "b.h"
int k = sizeof(a);
On Fri, Jan 22, 2016 at 11:03 AM, Yaron Keren via
cfe-commits <cfe-commits@lists.llvm.org> wrote:
Author: yrnkrn
Date: Fri Jan 22 13:03:27 2016
New Revision: 258524
URL:
http://llvm.org/viewvc/llvm-project?rev=258524&view=rev
Log:
Merge templated static member variables, fixes
http://llvm.org/pr26179.
Patch by Vassil Vassilev!
Reviewed by Richard Smith.
Added:
cfe/trunk/test/Modules/Inputs/PR26179/
cfe/trunk/test/Modules/Inputs/PR26179/A.h
cfe/trunk/test/Modules/Inputs/PR26179/B.h
cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h
cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap
cfe/trunk/test/Modules/pr26179.cpp
Modified:
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=258524&r1=258523&r2=258524&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
(original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri
Jan 22 13:03:27 2016
@@ -2595,8 +2595,24 @@ static bool
isSameEntity(NamedDecl *X, N
// Variables with the same type and linkage match.
if (VarDecl *VarX = dyn_cast<VarDecl>(X)) {
VarDecl *VarY = cast<VarDecl>(Y);
- return (VarX->getLinkageInternal() ==
VarY->getLinkageInternal()) &&
- VarX->getASTContext().hasSameType(VarX->getType(),
VarY->getType());
+ if (VarX->getLinkageInternal() ==
VarY->getLinkageInternal()) {
+ ASTContext &C = VarX->getASTContext();
+ if (C.hasSameType(VarX->getType(),
VarY->getType()))
+ return true;
+
+ // We can get decls with different types on the
redecl chain. Eg.
+ // template <typename T> struct S { static T
Var[]; }; // #1
+ // template <typename T> T
S<T>::Var[sizeof(T)]; // #2
+ // Only? happens when completing an incomplete
array type. In this case
+ // when comparing #1 and #2 we should go
through their elements types.
+ const ArrayType *VarXTy =
C.getAsArrayType(VarX->getType());
+ const ArrayType *VarYTy =
C.getAsArrayType(VarY->getType());
+ if (!VarXTy || !VarYTy)
+ return false;
+ if (VarXTy->isIncompleteArrayType() ||
VarYTy->isIncompleteArrayType())
+ return
C.hasSameType(VarXTy->getElementType(),
VarYTy->getElementType());
+ }
+ return false;
}
// Namespaces with the same name and inlinedness match.
Added: cfe/trunk/test/Modules/Inputs/PR26179/A.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/A.h?rev=258524&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/PR26179/A.h (added)
+++ cfe/trunk/test/Modules/Inputs/PR26179/A.h Fri Jan
22 13:03:27 2016
@@ -0,0 +1,4 @@
+#include "basic_string.h"
+#include "B.h"
+
+int *p = a;
Added: cfe/trunk/test/Modules/Inputs/PR26179/B.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/B.h?rev=258524&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/PR26179/B.h (added)
+++ cfe/trunk/test/Modules/Inputs/PR26179/B.h Fri Jan
22 13:03:27 2016
@@ -0,0 +1,2 @@
+#include "basic_string.h"
+extern int a[5];
Added:
cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h?rev=258524&view=auto
==============================================================================
---
cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h
(added)
+++
cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h
Fri Jan 22 13:03:27 2016
@@ -0,0 +1,14 @@
+#ifndef _GLIBCXX_STRING
+#define _GLIBCXX_STRING 1
+
+template<typename T>
+struct basic_string {
+ static T _S_empty_rep_storage[];
+};
+
+template<typename T>
+T basic_string<T>::_S_empty_rep_storage[sizeof(T)];
+
+extern int a[];
+
+#endif
Added:
cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap?rev=258524&view=auto
==============================================================================
---
cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap
(added)
+++
cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap
Fri Jan 22 13:03:27 2016
@@ -0,0 +1,9 @@
+module A {
+ header "A.h"
+ export *
+}
+
+module B {
+ header "B.h"
+ export *
+}
Added: cfe/trunk/test/Modules/pr26179.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/pr26179.cpp?rev=258524&view=auto
==============================================================================
--- cfe/trunk/test/Modules/pr26179.cpp (added)
+++ cfe/trunk/test/Modules/pr26179.cpp Fri Jan 22
13:03:27 2016
@@ -0,0 +1,7 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -I%S/Inputs/PR26179 -verify %s
+// RUN: %clang_cc1 -fmodules
-fmodule-map-file=%S/Inputs/PR26179/module.modulemap
-fmodules-cache-path=%t -I%S/Inputs/PR26179 -verify %s
+
+#include "A.h"
+
+// expected-no-diagnostics
_______________________________________________
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 <mailto: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
