From 195d8ef452a0712105a88194848db736d415f3c2 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 29 May 2014 03:15:31 +0000 Subject: [PATCH] When merging functions across modules (and in particular, instantiations of member functions), ensure that the redecl chain never transitions from 'inline' to 'not inline', since that violates an AST invariant. llvm-svn: 209794 --- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 12 +++++- clang/lib/Serialization/ASTReaderDecl.cpp | 39 ++++++++++++++++++- clang/test/Modules/Inputs/templates-left.h | 4 ++ clang/test/Modules/Inputs/templates-right.h | 4 ++ clang/test/Modules/Inputs/templates-top.h | 9 +++++ clang/test/Modules/templates.mm | 14 +++++++ 6 files changed, 78 insertions(+), 4 deletions(-) diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index be2db41efd87..40177457c14a 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -3377,8 +3377,16 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation, !PatternDecl->getReturnType()->getContainedAutoType()) return; - if (PatternDecl->isInlined()) - Function->setImplicitlyInline(); + if (PatternDecl->isInlined()) { + // Function, and all later redeclarations of it (from imported modules, + // for instance), are now implicitly inline. + for (auto *D = Function->getMostRecentDecl(); /**/; + D = D->getPreviousDecl()) { + D->setImplicitlyInline(); + if (D == Function) + break; + } + } InstantiatingTemplate Inst(*this, PointOfInstantiation, Function); if (Inst.isInvalid()) diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 8a0849cb9092..7e652c471810 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -2491,6 +2491,32 @@ void ASTDeclReader::attachPreviousDecl(Decl *D, Decl *Previous) { D->IdentifierNamespace |= Previous->IdentifierNamespace & (Decl::IDNS_Ordinary | Decl::IDNS_Tag | Decl::IDNS_Type); + + // If the previous declaration is an inline function declaration, then this + // declaration is too. + if (auto *FD = dyn_cast(D)) { + if (cast(Previous)->IsInline != FD->IsInline) { + // FIXME: [dcl.fct.spec]p4: + // If a function with external linkage is declared inline in one + // translation unit, it shall be declared inline in all translation + // units in which it appears. + // + // Be careful of this case: + // + // module A: + // template struct X { void f(); }; + // template inline void X::f() {} + // + // module B instantiates the declaration of X::f + // module C instantiates the definition of X::f + // + // If module B and C are merged, we do not have a violation of this rule. + // + //if (!FD->IsInline || Previous->getOwningModule()) + // Diag(FD->getLocation(), diag::err_odr_differing_inline); + FD->IsInline = true; + } + } } template @@ -3162,8 +3188,17 @@ void ASTDeclReader::UpdateDecl(Decl *D, ModuleFile &ModuleFile, return; } - if (Record[Idx++]) - FD->setImplicitlyInline(); + if (Record[Idx++]) { + // Maintain AST consistency: any later redeclarations of this function + // are inline if this one is. (We might have merged another declaration + // into this one.) + for (auto *D = FD->getMostRecentDecl(); /**/; + D = D->getPreviousDecl()) { + D->setImplicitlyInline(); + if (D == FD) + break; + } + } FD->setInnerLocStart(Reader.ReadSourceLocation(ModuleFile, Record, Idx)); if (auto *CD = dyn_cast(FD)) std::tie(CD->CtorInitializers, CD->NumCtorInitializers) = diff --git a/clang/test/Modules/Inputs/templates-left.h b/clang/test/Modules/Inputs/templates-left.h index ae9d8bd8942c..2bd79be94584 100644 --- a/clang/test/Modules/Inputs/templates-left.h +++ b/clang/test/Modules/Inputs/templates-left.h @@ -56,3 +56,7 @@ template struct DelayUpdates; template<> struct DelayUpdates; template struct DelayUpdates; template void testDelayUpdates(DelayUpdates *p = 0) {} + +void outOfLineInlineUseLeftF(void (OutOfLineInline::*)() = &OutOfLineInline::f); +void outOfLineInlineUseLeftG(void (OutOfLineInline::*)() = &OutOfLineInline::g); +void outOfLineInlineUseLeftH(void (OutOfLineInline::*)() = &OutOfLineInline::h); diff --git a/clang/test/Modules/Inputs/templates-right.h b/clang/test/Modules/Inputs/templates-right.h index c8056d6539f5..5907cbca73ee 100644 --- a/clang/test/Modules/Inputs/templates-right.h +++ b/clang/test/Modules/Inputs/templates-right.h @@ -39,3 +39,7 @@ int defineListDoubleRight() { } template struct MergePatternDecl; + +void outOfLineInlineUseRightF(void (OutOfLineInline::*)() = &OutOfLineInline::f); +void outOfLineInlineUseRightG(void (OutOfLineInline::*)() = &OutOfLineInline::g); +void outOfLineInlineUseRightH(void (OutOfLineInline::*)() = &OutOfLineInline::h); diff --git a/clang/test/Modules/Inputs/templates-top.h b/clang/test/Modules/Inputs/templates-top.h index 168155b5b9c1..1216266f34fb 100644 --- a/clang/test/Modules/Inputs/templates-top.h +++ b/clang/test/Modules/Inputs/templates-top.h @@ -31,3 +31,12 @@ template struct ExplicitInstantiation { }; template struct DelayUpdates {}; + +template struct OutOfLineInline { + void f(); + void g(); + void h(); +}; +template inline void OutOfLineInline::f() {} +template inline void OutOfLineInline::g() {} +template inline void OutOfLineInline::h() {} diff --git a/clang/test/Modules/templates.mm b/clang/test/Modules/templates.mm index 350d88e9d72b..645e17161697 100644 --- a/clang/test/Modules/templates.mm +++ b/clang/test/Modules/templates.mm @@ -4,6 +4,12 @@ // expected-no-diagnostics @import templates_left; + +void testInlineRedeclEarly() { + // instantiate definition now, we'll add another declaration in _right. + OutOfLineInline().h(); +} + @import templates_right; // CHECK: @list_left = global { %{{.*}}*, i32, [4 x i8] } { %{{.*}}* null, i32 8, @@ -39,6 +45,14 @@ void testRedeclDefinition() { redeclDefinitionEmit(); } +void testInlineRedecl() { + outOfLineInlineUseLeftF(); + outOfLineInlineUseRightG(); + + outOfLineInlineUseRightF(); + outOfLineInlineUseLeftG(); +} + // CHECK-NOT: @_ZN21ExplicitInstantiationILb0ELb0EE1fEv( // CHECK: declare {{.*}}@_ZN21ExplicitInstantiationILb1ELb0EE1fEv( // CHECK: define {{.*}}@_ZN21ExplicitInstantiationILb1ELb1EE1fEv(