[modules] PR24954
Vassil Vassilev via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 28 08:23:57 PST 2016
Would this patch be more reasonable? It follows what
RegisterTemplateSpecialization (introduced in r245779) does. AFAICT this
adds an update record far less often.
--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 <vvasilev at 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
>>
>>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160128/3069ac45/attachment-0001.html>
-------------- next part --------------
From 2c12f1539c36548f7558e67a91e702c9233c0007 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev at gmail.com>
Date: Sun, 27 Sep 2015 21:12:39 +0200
Subject: [PATCH] Fix adding a templated friend functions to a namespace from
another module.
This should be reproducible with chained PCHs too.
Fixes https://llvm.org/bugs/show_bug.cgi?id=24954
---
include/clang/Serialization/ASTWriter.h | 3 +++
lib/Serialization/ASTWriter.cpp | 20 +++++++++++++++++++
test/Modules/Inputs/PR24954/A.h | 10 ++++++++++
test/Modules/Inputs/PR24954/B.h | 30 ++++++++++++++++++++++++++++
test/Modules/Inputs/PR24954/module.modulemap | 9 +++++++++
test/Modules/pr24954.cpp | 7 +++++++
6 files changed, 79 insertions(+)
create mode 100644 test/Modules/Inputs/PR24954/A.h
create mode 100644 test/Modules/Inputs/PR24954/B.h
create mode 100644 test/Modules/Inputs/PR24954/module.modulemap
create mode 100644 test/Modules/pr24954.cpp
diff --git a/include/clang/Serialization/ASTWriter.h b/include/clang/Serialization/ASTWriter.h
index ed34547..0f2ce34 100644
--- a/include/clang/Serialization/ASTWriter.h
+++ b/include/clang/Serialization/ASTWriter.h
@@ -865,6 +865,9 @@ public:
void CompletedTagDefinition(const TagDecl *D) override;
void AddedVisibleDecl(const DeclContext *DC, const Decl *D) override;
void AddedCXXImplicitMember(const CXXRecordDecl *RD, const Decl *D) override;
+ using ASTMutationListener::AddedCXXTemplateSpecialization;
+ void AddedCXXTemplateSpecialization(const FunctionTemplateDecl *TD,
+ const FunctionDecl *D) override;
void ResolvedExceptionSpec(const FunctionDecl *FD) override;
void DeducedReturnType(const FunctionDecl *FD, QualType ReturnType) override;
void ResolvedOperatorDelete(const CXXDestructorDecl *DD,
diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp
index 128935c..05a5331 100644
--- a/lib/Serialization/ASTWriter.cpp
+++ b/lib/Serialization/ASTWriter.cpp
@@ -5710,6 +5710,26 @@ void ASTWriter::AddedCXXImplicitMember(const CXXRecordDecl *RD, const Decl *D) {
DeclUpdates[RD].push_back(DeclUpdate(UPD_CXX_ADDED_IMPLICIT_MEMBER, D));
}
+void ASTWriter::AddedCXXTemplateSpecialization(const FunctionTemplateDecl *TD,
+ const FunctionDecl *D) {
+ assert(!WritingAST && "Already writing the AST!");
+
+ // TODO: The checks are dups of RegisterTemplateSpecialization.
+ // Factor the checks out.
+
+ // A decl coming from PCH was modified.
+ if (!TD->isFromASTFile())
+ return;
+
+ // We only need to associate the first local declaration of the
+ // specialization. The other declarations will get pulled in by it.
+ if (getFirstLocalDecl(D) != D)
+ return;
+
+ DeclUpdates[TD].push_back(DeclUpdate(
+ UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION, D));
+}
+
void ASTWriter::ResolvedExceptionSpec(const FunctionDecl *FD) {
assert(!DoneWritingDeclsAndTypes && "Already done writing updates!");
if (!Chain) return;
diff --git a/test/Modules/Inputs/PR24954/A.h b/test/Modules/Inputs/PR24954/A.h
new file mode 100644
index 0000000..5e5d5bf
--- /dev/null
+++ b/test/Modules/Inputs/PR24954/A.h
@@ -0,0 +1,10 @@
+#include "B.h"
+
+template <class T>
+class Expr {
+public:
+ void print(B::basic_ostream<char>& os) {
+ os << B::setw(42);
+ os << B::endl;
+ }
+};
diff --git a/test/Modules/Inputs/PR24954/B.h b/test/Modules/Inputs/PR24954/B.h
new file mode 100644
index 0000000..a8ddc71
--- /dev/null
+++ b/test/Modules/Inputs/PR24954/B.h
@@ -0,0 +1,30 @@
+namespace B {
+
+ template <class _CharT>
+ struct basic_ostream {
+ basic_ostream& operator<<(basic_ostream& (*__pf)());
+ };
+
+
+ template <class _CharT> basic_ostream<_CharT>&
+ endl();
+
+ struct S1 {
+ template <class _CharT> friend void
+ operator<<(basic_ostream<_CharT>& __os, const S1& __x);
+ };
+
+ S1 setw(int __n);
+
+ template <class _CharT> class S2;
+
+ template <class _CharT> void
+ operator<<(basic_ostream<_CharT>& __os, const S2<_CharT>& __x);
+
+ template <class _CharT>
+ struct S2 {
+ template <class _Cp> friend void
+ operator<<(basic_ostream<_Cp>& __os, const S2<_Cp>& __x);
+ };
+
+}
diff --git a/test/Modules/Inputs/PR24954/module.modulemap b/test/Modules/Inputs/PR24954/module.modulemap
new file mode 100644
index 0000000..4937418
--- /dev/null
+++ b/test/Modules/Inputs/PR24954/module.modulemap
@@ -0,0 +1,9 @@
+module A {
+ header "A.h"
+ export *
+}
+
+module B {
+ header "B.h"
+ export *
+}
diff --git a/test/Modules/pr24954.cpp b/test/Modules/pr24954.cpp
new file mode 100644
index 0000000..407ee06
--- /dev/null
+++ b/test/Modules/pr24954.cpp
@@ -0,0 +1,7 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -I%S/Inputs/PR24954 -verify %s
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs/PR24954 -verify %s
+
+#include "A.h"
+
+// expected-no-diagnostics
--
1.9.1
More information about the cfe-commits
mailing list