ping...
On 22/02/16 21:51, Vassil Vassilev wrote:
On 02/02/16 02:52, Richard Smith via cfe-commits wrote:
On Thu, Jan 28, 2016 at 8:23 AM, Vassil Vassilev <vvasi...@cern.ch>
wrote:
Would this patch be more reasonable? It follows what
RegisterTemplateSpecialization (introduced in r245779) does. AFAICT
this
adds an update record far less often.
It's still adding redundant update records. We'll write the
appropriate update record as part of serializing the declaration
itself, so we only need to ensure that the declaration is emitted, not
actually emit an update record for it. Perhaps you could store a list
of such declarations on the ASTWriter, and call GetDeclRef on each of
them once we start emitting the AST file (or maybe just push them into
UpdatingVisibleDecls). Please also restrict this to the template
friend corner case.
The attached patch fixes the issues more or less in the direction you
suggest. I am not sure if I expressed well enough the conditions of
the template friend corner case. Could you have a look?
--Vassil
On 12/12/15 16:13, Vassil Vassilev wrote:
I couldn't find GetDecl routine in the ASTWriter. Could you elaborate?
Assuming you meant ASTWriter::GetDeclRef(D): It seems that the
conditions
when calling GetDeclRef differ from the conditions of
AddedCXXTemplateSpecialization. Eg:
ASTWriter::AddedCXXTemplateSpecialization {
assert(!WritingAST && "Already writing the AST!");
...
}
ASTWriter::GetDeclRef {
assert(WritingAST && "Cannot request a declaration ID before AST
writing");
..
}
IIUC this particular instantiation happens *after* module B was
built, thus
it needs to be retrospectively added to the serialized namespace. It
looks
like even avoiding somehow the asserts of GetDeclRef it wouldn't
help much.
Alternatively I could try to reduce the redundant update records by
narrowing down to instantiations coming in the context of friends.
--Vassil
On 12/12/15 01:07, Richard Smith wrote:
Instead of adding an update record directly in this case (which will
emit
far more update records than necessary), how about just calling
GetDecl(D)
from AddedCXXTemplateSpecialization to ensure that it gets emitted?
On Fri, Dec 4, 2015 at 7:46 AM, Vassil Vassilev <vvasi...@cern.ch>
wrote:
Hi,
Could you review my fix please.
Many thanks,
Vassil
On 08/10/15 15:53, Vassil Vassilev wrote:
Hi Richard,
I started working on https://llvm.org/bugs/show_bug.cgi?id=24954
IIUC r228485 introduces an abstraction to deal with
not-really-anonymous friend decls
(serialization::needsAnonymousDeclarationNumber in ASTCommon.cpp).
A comment explicitly says:
"// This doesn't apply to friend tag decls; Sema makes those
available
to name
// lookup in the surrounding context."
In the bug reproducer, the friend function (wrt __iom_t10) is
forward
declared in the same namespace, where Sema makes the friend
available for a
name lookup.
It seems that the friend operator<< in __iom_t10 (sorry about
the names
they come from libcxx) doesn't get registered in the ASTWriter's
DeclIDs but
it gets registered in outer namespace's lookup table. Thus, assert is
triggered when finalizing module A, since it rebuilds the lookups
of the
updated contexts.
The issue only appears when building module A deserializes/uses
module
B.
Currently I was assume that something wrong happens in either
needsAnonymousDeclarationNumber or I hit a predicted issue
ASTWriterDecl.cpp:1602
// FIXME: This is not correct; when we reach an imported
declaration
we
// won't emit its previous declaration.
(void)Writer.GetDeclRef(D->getPreviousDecl());
(void)Writer.GetDeclRef(MostRecent);
The issue seems a fairly complex one and I am a bit stuck.
Any hints are very very welcome ;)
Many thanks,
Vassil
_______________________________________________
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
