Author: rsmith Date: Fri Dec 18 16:19:11 2015 New Revision: 256045 URL: http://llvm.org/viewvc/llvm-project?rev=256045&view=rev Log: [modules] Don't try to use the definition of a class if RequireCompleteType(..., 0) says we're not permitted to do so. The definition might not be visible, even though we know what it is.
Added: cfe/trunk/test/Modules/hidden-definition.cpp Modified: cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/Sema/SemaLookup.cpp cfe/trunk/lib/Sema/SemaOverload.cpp cfe/trunk/lib/Sema/SemaType.cpp cfe/trunk/test/Modules/cxx-templates.cpp cfe/trunk/test/Modules/submodules-merge-defs.cpp cfe/trunk/test/SemaTemplate/ms-lookup-template-base-classes.cpp Modified: cfe/trunk/include/clang/Sema/Sema.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=256045&r1=256044&r2=256045&view=diff ============================================================================== --- cfe/trunk/include/clang/Sema/Sema.h (original) +++ cfe/trunk/include/clang/Sema/Sema.h Fri Dec 18 16:19:11 2015 @@ -1348,7 +1348,7 @@ public: void emit(const SemaDiagnosticBuilder &DB, llvm::index_sequence<Is...>) const { // Apply all tuple elements to the builder in order. - bool Dummy[] = {(DB << getPrintable(std::get<Is>(Args)))...}; + bool Dummy[] = {false, (DB << getPrintable(std::get<Is>(Args)))...}; (void)Dummy; } @@ -1425,6 +1425,7 @@ public: return RequireCompleteType(Loc, T, Diagnoser); } + void completeExprArrayBound(Expr *E); bool RequireCompleteExprType(Expr *E, TypeDiagnoser &Diagnoser); bool RequireCompleteExprType(Expr *E, unsigned DiagID); Modified: cfe/trunk/lib/Sema/SemaLookup.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=256045&r1=256044&r2=256045&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaLookup.cpp (original) +++ cfe/trunk/lib/Sema/SemaLookup.cpp Fri Dec 18 16:19:11 2015 @@ -2428,7 +2428,8 @@ addAssociatedClassesAndNamespaces(Associ } // Only recurse into base classes for complete types. - if (!Class->hasDefinition()) + if (Result.S.RequireCompleteType(Result.InstantiationLoc, + Result.S.Context.getRecordType(Class), 0)) return; // Add direct and indirect base classes along with their associated @@ -2521,10 +2522,8 @@ addAssociatedClassesAndNamespaces(Associ // classes. Its associated namespaces are the namespaces in // which its associated classes are defined. case Type::Record: { - Result.S.RequireCompleteType(Result.InstantiationLoc, QualType(T, 0), - /*no diagnostic*/ 0); - CXXRecordDecl *Class - = cast<CXXRecordDecl>(cast<RecordType>(T)->getDecl()); + CXXRecordDecl *Class = + cast<CXXRecordDecl>(cast<RecordType>(T)->getDecl()); addAssociatedClassesAndNamespaces(Result, Class); break; } @@ -3208,7 +3207,8 @@ void Sema::ArgumentDependentLookup(Decla for (Decl *DI = D; DI; DI = DI->getPreviousDecl()) { DeclContext *LexDC = DI->getLexicalDeclContext(); if (isa<CXXRecordDecl>(LexDC) && - AssociatedClasses.count(cast<CXXRecordDecl>(LexDC))) { + AssociatedClasses.count(cast<CXXRecordDecl>(LexDC)) && + isVisible(cast<NamedDecl>(DI))) { DeclaredInAssociatedClass = true; break; } Modified: cfe/trunk/lib/Sema/SemaOverload.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=256045&r1=256044&r2=256045&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaOverload.cpp (original) +++ cfe/trunk/lib/Sema/SemaOverload.cpp Fri Dec 18 16:19:11 2015 @@ -3085,11 +3085,7 @@ IsUserDefinedConversion(Sema &S, Expr *F S.IsDerivedFrom(From->getLocStart(), From->getType(), ToType))) ConstructorsOnly = true; - S.RequireCompleteType(From->getExprLoc(), ToType, 0); - // RequireCompleteType may have returned true due to some invalid decl - // during template instantiation, but ToType may be complete enough now - // to try to recover. - if (ToType->isIncompleteType()) { + if (S.RequireCompleteType(From->getExprLoc(), ToType, 0)) { // We're not going to find any constructors. } else if (CXXRecordDecl *ToRecordDecl = dyn_cast<CXXRecordDecl>(ToRecordType->getDecl())) { @@ -6685,7 +6681,8 @@ void Sema::AddMemberOperatorCandidates(O // the set of member candidates is empty. if (const RecordType *T1Rec = T1->getAs<RecordType>()) { // Complete the type if it can be completed. - RequireCompleteType(OpLoc, T1, 0); + if (RequireCompleteType(OpLoc, T1, 0) && !T1Rec->isBeingDefined()) + return; // If the type is neither complete nor being defined, bail out now. if (!T1Rec->getDecl()->getDefinition()) return; Modified: cfe/trunk/lib/Sema/SemaType.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=256045&r1=256044&r2=256045&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaType.cpp (original) +++ cfe/trunk/lib/Sema/SemaType.cpp Fri Dec 18 16:19:11 2015 @@ -6366,6 +6366,50 @@ static void processTypeAttrs(TypeProcess } } +void Sema::completeExprArrayBound(Expr *E) { + if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E->IgnoreParens())) { + if (VarDecl *Var = dyn_cast<VarDecl>(DRE->getDecl())) { + if (isTemplateInstantiation(Var->getTemplateSpecializationKind())) { + SourceLocation PointOfInstantiation = E->getExprLoc(); + + if (MemberSpecializationInfo *MSInfo = + Var->getMemberSpecializationInfo()) { + // If we don't already have a point of instantiation, this is it. + if (MSInfo->getPointOfInstantiation().isInvalid()) { + MSInfo->setPointOfInstantiation(PointOfInstantiation); + + // This is a modification of an existing AST node. Notify + // listeners. + if (ASTMutationListener *L = getASTMutationListener()) + L->StaticDataMemberInstantiated(Var); + } + } else { + VarTemplateSpecializationDecl *VarSpec = + cast<VarTemplateSpecializationDecl>(Var); + if (VarSpec->getPointOfInstantiation().isInvalid()) + VarSpec->setPointOfInstantiation(PointOfInstantiation); + } + + InstantiateVariableDefinition(PointOfInstantiation, Var); + + // Update the type to the newly instantiated definition's type both + // here and within the expression. + if (VarDecl *Def = Var->getDefinition()) { + DRE->setDecl(Def); + QualType T = Def->getType(); + DRE->setType(T); + // FIXME: Update the type on all intervening expressions. + E->setType(T); + } + + // We still go on to try to complete the type independently, as it + // may also require instantiations or diagnostics if it remains + // incomplete. + } + } + } +} + /// \brief Ensure that the type of the given expression is complete. /// /// This routine checks whether the expression \p E has a complete type. If the @@ -6380,87 +6424,26 @@ static void processTypeAttrs(TypeProcess /// /// \returns \c true if the type of \p E is incomplete and diagnosed, \c false /// otherwise. -bool Sema::RequireCompleteExprType(Expr *E, TypeDiagnoser &Diagnoser){ +bool Sema::RequireCompleteExprType(Expr *E, TypeDiagnoser &Diagnoser) { QualType T = E->getType(); - // Fast path the case where the type is already complete. - if (!T->isIncompleteType()) - // FIXME: The definition might not be visible. - return false; - // Incomplete array types may be completed by the initializer attached to // their definitions. For static data members of class templates and for // variable templates, we need to instantiate the definition to get this // initializer and complete the type. if (T->isIncompleteArrayType()) { - if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E->IgnoreParens())) { - if (VarDecl *Var = dyn_cast<VarDecl>(DRE->getDecl())) { - if (isTemplateInstantiation(Var->getTemplateSpecializationKind())) { - SourceLocation PointOfInstantiation = E->getExprLoc(); - - if (MemberSpecializationInfo *MSInfo = - Var->getMemberSpecializationInfo()) { - // If we don't already have a point of instantiation, this is it. - if (MSInfo->getPointOfInstantiation().isInvalid()) { - MSInfo->setPointOfInstantiation(PointOfInstantiation); - - // This is a modification of an existing AST node. Notify - // listeners. - if (ASTMutationListener *L = getASTMutationListener()) - L->StaticDataMemberInstantiated(Var); - } - } else { - VarTemplateSpecializationDecl *VarSpec = - cast<VarTemplateSpecializationDecl>(Var); - if (VarSpec->getPointOfInstantiation().isInvalid()) - VarSpec->setPointOfInstantiation(PointOfInstantiation); - } - - InstantiateVariableDefinition(PointOfInstantiation, Var); - - // Update the type to the newly instantiated definition's type both - // here and within the expression. - if (VarDecl *Def = Var->getDefinition()) { - DRE->setDecl(Def); - T = Def->getType(); - DRE->setType(T); - E->setType(T); - } - - // We still go on to try to complete the type independently, as it - // may also require instantiations or diagnostics if it remains - // incomplete. - } - } - } + completeExprArrayBound(E); + T = E->getType(); } // FIXME: Are there other cases which require instantiating something other // than the type to complete the type of an expression? - // Look through reference types and complete the referred type. - if (const ReferenceType *Ref = T->getAs<ReferenceType>()) - T = Ref->getPointeeType(); - return RequireCompleteType(E->getExprLoc(), T, Diagnoser); } -namespace { - struct TypeDiagnoserDiag : Sema::TypeDiagnoser { - unsigned DiagID; - - TypeDiagnoserDiag(unsigned DiagID) - : Sema::TypeDiagnoser(DiagID == 0), DiagID(DiagID) {} - - void diagnose(Sema &S, SourceLocation Loc, QualType T) override { - if (Suppressed) return; - S.Diag(Loc, DiagID) << T; - } - }; -} - bool Sema::RequireCompleteExprType(Expr *E, unsigned DiagID) { - TypeDiagnoserDiag Diagnoser(DiagID); + BoundTypeDiagnoser<> Diagnoser(DiagID); return RequireCompleteExprType(E, Diagnoser); } @@ -6612,9 +6595,16 @@ bool Sema::RequireCompleteTypeImpl(Sourc if (!T->isIncompleteType(&Def)) { // If we know about the definition but it is not visible, complain. NamedDecl *SuggestedDef = nullptr; - if (!Diagnoser.Suppressed && Def && - !hasVisibleDefinition(Def, &SuggestedDef, /*OnlyNeedComplete*/true)) - diagnoseMissingImport(Loc, SuggestedDef, /*NeedDefinition*/true); + if (Def && + !hasVisibleDefinition(Def, &SuggestedDef, /*OnlyNeedComplete*/true)) { + // If the user is going to see an error here, recover by making the + // definition visible. + bool TreatAsComplete = !Diagnoser.Suppressed && !isSFINAEContext(); + if (!Diagnoser.Suppressed) + diagnoseMissingImport(Loc, SuggestedDef, /*NeedDefinition*/true, + /*Recover*/TreatAsComplete); + return !TreatAsComplete; + } return false; } @@ -6630,6 +6620,9 @@ bool Sema::RequireCompleteTypeImpl(Sourc // chain for a declaration that can be accessed through a mechanism other // than name lookup (eg, referenced in a template, or a variable whose type // could be completed by the module)? + // + // FIXME: Should we map through to the base array element type before + // checking for a tag type? if (Tag || IFace) { NamedDecl *D = Tag ? static_cast<NamedDecl *>(Tag->getDecl()) : IFace->getDecl(); @@ -6660,12 +6653,16 @@ bool Sema::RequireCompleteTypeImpl(Sourc = Context.getAsConstantArrayType(MaybeTemplate)) MaybeTemplate = Array->getElementType(); if (const RecordType *Record = MaybeTemplate->getAs<RecordType>()) { + bool Instantiated = false; + bool Diagnosed = false; if (ClassTemplateSpecializationDecl *ClassTemplateSpec = dyn_cast<ClassTemplateSpecializationDecl>(Record->getDecl())) { - if (ClassTemplateSpec->getSpecializationKind() == TSK_Undeclared) - return InstantiateClassTemplateSpecialization(Loc, ClassTemplateSpec, - TSK_ImplicitInstantiation, - /*Complain=*/!Diagnoser.Suppressed); + if (ClassTemplateSpec->getSpecializationKind() == TSK_Undeclared) { + Diagnosed = InstantiateClassTemplateSpecialization( + Loc, ClassTemplateSpec, TSK_ImplicitInstantiation, + /*Complain=*/!Diagnoser.Suppressed); + Instantiated = true; + } } else if (CXXRecordDecl *Rec = dyn_cast<CXXRecordDecl>(Record->getDecl())) { CXXRecordDecl *Pattern = Rec->getInstantiatedFromMemberClass(); @@ -6673,13 +6670,28 @@ bool Sema::RequireCompleteTypeImpl(Sourc MemberSpecializationInfo *MSI = Rec->getMemberSpecializationInfo(); assert(MSI && "Missing member specialization information?"); // This record was instantiated from a class within a template. - if (MSI->getTemplateSpecializationKind() != TSK_ExplicitSpecialization) - return InstantiateClass(Loc, Rec, Pattern, - getTemplateInstantiationArgs(Rec), - TSK_ImplicitInstantiation, - /*Complain=*/!Diagnoser.Suppressed); + if (MSI->getTemplateSpecializationKind() != + TSK_ExplicitSpecialization) { + Diagnosed = InstantiateClass(Loc, Rec, Pattern, + getTemplateInstantiationArgs(Rec), + TSK_ImplicitInstantiation, + /*Complain=*/!Diagnoser.Suppressed); + Instantiated = true; + } } } + + if (Instantiated) { + // Instantiate* might have already complained that the template is not + // defined, if we asked it to. + if (!Diagnoser.Suppressed && Diagnosed) + return true; + // If we instantiated a definition, check that it's usable, even if + // instantiation produced an error, so that repeated calls to this + // function give consistent answers. + if (!T->isIncompleteType()) + return RequireCompleteTypeImpl(Loc, T, Diagnoser); + } } if (Diagnoser.Suppressed) @@ -6716,7 +6728,7 @@ bool Sema::RequireCompleteTypeImpl(Sourc bool Sema::RequireCompleteType(SourceLocation Loc, QualType T, unsigned DiagID) { - TypeDiagnoserDiag Diagnoser(DiagID); + BoundTypeDiagnoser<> Diagnoser(DiagID); return RequireCompleteType(Loc, T, Diagnoser); } @@ -6757,14 +6769,10 @@ bool Sema::RequireLiteralType(SourceLoca assert(!T->isDependentType() && "type should not be dependent"); QualType ElemType = Context.getBaseElementType(T); - RequireCompleteType(Loc, ElemType, 0); - - if (T->isLiteralType(Context)) + if ((!RequireCompleteType(Loc, ElemType, 0) || ElemType->isVoidType()) && + T->isLiteralType(Context)) return false; - if (Diagnoser.Suppressed) - return true; - Diagnoser.diagnose(*this, Loc, T); if (T->isVariableArrayType()) @@ -6779,10 +6787,8 @@ bool Sema::RequireLiteralType(SourceLoca // A partially-defined class type can't be a literal type, because a literal // class type must have a trivial destructor (which can't be checked until // the class definition is complete). - if (!RD->isCompleteDefinition()) { - RequireCompleteType(Loc, ElemType, diag::note_non_literal_incomplete, T); + if (RequireCompleteType(Loc, ElemType, diag::note_non_literal_incomplete, T)) return true; - } // If the class has virtual base classes, then it's not an aggregate, and // cannot have any constexpr constructors or a trivial default constructor, @@ -6834,7 +6840,7 @@ bool Sema::RequireLiteralType(SourceLoca } bool Sema::RequireLiteralType(SourceLocation Loc, QualType T, unsigned DiagID) { - TypeDiagnoserDiag Diagnoser(DiagID); + BoundTypeDiagnoser<> Diagnoser(DiagID); return RequireLiteralType(Loc, T, Diagnoser); } Modified: cfe/trunk/test/Modules/cxx-templates.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cxx-templates.cpp?rev=256045&r1=256044&r2=256045&view=diff ============================================================================== --- cfe/trunk/test/Modules/cxx-templates.cpp (original) +++ cfe/trunk/test/Modules/cxx-templates.cpp Fri Dec 18 16:19:11 2015 @@ -105,8 +105,8 @@ void g() { TemplateInstantiationVisibility<char[1]> tiv1; TemplateInstantiationVisibility<char[2]> tiv2; - TemplateInstantiationVisibility<char[3]> tiv3; // expected-error {{must be imported from module 'cxx_templates_b_impl'}} - // expected-note@cxx-templates-b-impl.h:10 {{previous definition is here}} + TemplateInstantiationVisibility<char[3]> tiv3; // expected-error 2{{must be imported from module 'cxx_templates_b_impl'}} + // expected-note@cxx-templates-b-impl.h:10 2{{previous definition is here}} TemplateInstantiationVisibility<char[4]> tiv4; int &p = WithPartialSpecializationUse().f(); Added: cfe/trunk/test/Modules/hidden-definition.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/hidden-definition.cpp?rev=256045&view=auto ============================================================================== --- cfe/trunk/test/Modules/hidden-definition.cpp (added) +++ cfe/trunk/test/Modules/hidden-definition.cpp Fri Dec 18 16:19:11 2015 @@ -0,0 +1,16 @@ +// RUN: rm -rf %t +// RUN: mkdir %t +// RUN: echo 'struct X {}; struct Y : X { friend int f(Y); };' > %t/a.h +// RUN: echo 'module a { header "a.h" }' > %t/map +// RUN: %clang_cc1 -fmodules -x c++ -emit-module -fmodule-name=a %t/map -o %t/a.pcm +// RUN: %clang_cc1 -fmodules -x c++ -verify -fmodule-file=%t/a.pcm %s -fno-modules-error-recovery + +struct X; +struct Y; + +// Ensure that we can't use the definitions of X and Y, since we've not imported module a. +Y *yp; +X *xp = yp; // expected-error {{cannot initialize}} +_Static_assert(!__is_convertible(Y*, X*), ""); +X &xr = *yp; // expected-error {{unrelated type}} +int g(Y &y) { f(y); } // expected-error {{undeclared identifier 'f'}} Modified: cfe/trunk/test/Modules/submodules-merge-defs.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/submodules-merge-defs.cpp?rev=256045&r1=256044&r2=256045&view=diff ============================================================================== --- cfe/trunk/test/Modules/submodules-merge-defs.cpp (original) +++ cfe/trunk/test/Modules/submodules-merge-defs.cpp Fri Dec 18 16:19:11 2015 @@ -22,7 +22,7 @@ A pre_a; #endif // expected-note@defs.h:1 +{{here}} extern class A pre_a2; -int pre_use_a = use_a(pre_a2); // expected-error {{'A' must be imported}} expected-error {{'use_a' must be imported}} +int pre_use_a = use_a(pre_a2); // expected-error 2{{'A' must be imported}} expected-error {{'use_a' must be imported}} // expected-note@defs.h:2 +{{here}} B::Inner2 pre_bi; // expected-error +{{must be imported}} @@ -48,7 +48,7 @@ int pre_e = E(0); // expected-error {{mu // expected-note@defs.h:32 +{{here}} int pre_ff = F<int>().f(); // expected-error +{{must be imported}} -int pre_fg = F<int>().g<int>(); // expected-error +{{must be imported}} +int pre_fg = F<int>().g<int>(); // expected-error +{{must be imported}} expected-error 2{{expected}} // expected-note@defs.h:34 +{{here}} G::A pre_ga // expected-error +{{must be imported}} Modified: cfe/trunk/test/SemaTemplate/ms-lookup-template-base-classes.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/ms-lookup-template-base-classes.cpp?rev=256045&r1=256044&r2=256045&view=diff ============================================================================== --- cfe/trunk/test/SemaTemplate/ms-lookup-template-base-classes.cpp (original) +++ cfe/trunk/test/SemaTemplate/ms-lookup-template-base-classes.cpp Fri Dec 18 16:19:11 2015 @@ -308,7 +308,7 @@ template <typename T> struct B { struct template <typename T> struct C : A<T>, B<T> { NameFromBase m; // expected-error {{member 'NameFromBase' found in multiple base classes of different types}} expected-warning {{use of identifier 'NameFromBase' found via unqualified lookup into dependent bases of class templates is a Microsoft extension}} }; -static_assert(sizeof(C<int>) == 4, ""); // expected-note {{in instantiation of template class 'two_types_in_base::C<int>' requested here}} +static_assert(sizeof(C<int>) != 0, ""); // expected-note {{in instantiation of template class 'two_types_in_base::C<int>' requested here}} } namespace type_and_decl_in_base { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits