From 8823dbc552ec6946027c59ac53510404b98671b6 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 29 Aug 2019 22:49:32 +0000 Subject: [PATCH 1/6] Refactor InitListChecker to make it a bit clearer that hasError is only set to true in VerifyOnly mode in cases where it's also set to true when actually building the initializer list. Add FIXMEs for the two cases where that's not true. No functionality change intended. llvm-svn: 370417 --- clang/lib/Sema/SemaInit.cpp | 80 +++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 44 deletions(-) diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 83c475a5f1cd9..7c33e99803619 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -1093,49 +1093,32 @@ void InitListChecker::CheckExplicitInitList(const InitializedEntity &Entity, if (hadError) return; - if (Index < IList->getNumInits()) { + // Don't complain for incomplete types, since we'll get an error elsewhere. + if (Index < IList->getNumInits() && !T->isIncompleteType()) { // We have leftover initializers + bool ExtraInitsIsError = SemaRef.getLangOpts().CPlusPlus || + (SemaRef.getLangOpts().OpenCL && T->isVectorType()); + hadError = ExtraInitsIsError; if (VerifyOnly) { - if (SemaRef.getLangOpts().CPlusPlus || - (SemaRef.getLangOpts().OpenCL && - IList->getType()->isVectorType())) { - hadError = true; - } return; - } - - if (StructuredIndex == 1 && - IsStringInit(StructuredList->getInit(0), T, SemaRef.Context) == - SIF_None) { - unsigned DK = diag::ext_excess_initializers_in_char_array_initializer; - if (SemaRef.getLangOpts().CPlusPlus) { - DK = diag::err_excess_initializers_in_char_array_initializer; - hadError = true; - } - // Special-case + } else if (StructuredIndex == 1 && + IsStringInit(StructuredList->getInit(0), T, SemaRef.Context) == + SIF_None) { + unsigned DK = + ExtraInitsIsError + ? diag::err_excess_initializers_in_char_array_initializer + : diag::ext_excess_initializers_in_char_array_initializer; SemaRef.Diag(IList->getInit(Index)->getBeginLoc(), DK) << IList->getInit(Index)->getSourceRange(); - } else if (!T->isIncompleteType()) { - // Don't complain for incomplete types, since we'll get an error - // elsewhere - QualType CurrentObjectType = StructuredList->getType(); - int initKind = - CurrentObjectType->isArrayType()? 0 : - CurrentObjectType->isVectorType()? 1 : - CurrentObjectType->isScalarType()? 2 : - CurrentObjectType->isUnionType()? 3 : - 4; - - unsigned DK = diag::ext_excess_initializers; - if (SemaRef.getLangOpts().CPlusPlus) { - DK = diag::err_excess_initializers; - hadError = true; - } - if (SemaRef.getLangOpts().OpenCL && initKind == 1) { - DK = diag::err_excess_initializers; - hadError = true; - } - + } else { + int initKind = T->isArrayType() ? 0 : + T->isVectorType() ? 1 : + T->isScalarType() ? 2 : + T->isUnionType() ? 3 : + 4; + + unsigned DK = ExtraInitsIsError ? diag::err_excess_initializers + : diag::ext_excess_initializers; SemaRef.Diag(IList->getInit(Index)->getBeginLoc(), DK) << initKind << IList->getInit(Index)->getSourceRange(); } @@ -1363,8 +1346,8 @@ void InitListChecker::CheckSubElementType(const InitializedEntity &Entity, hadError = true; else { ExprRes = SemaRef.DefaultFunctionArrayLvalueConversion(ExprRes.get()); - if (ExprRes.isInvalid()) - hadError = true; + if (ExprRes.isInvalid()) + hadError = true; } UpdateStructuredListElement(StructuredList, StructuredIndex, ExprRes.getAs()); @@ -1389,10 +1372,15 @@ void InitListChecker::CheckSubElementType(const InitializedEntity &Entity, ++StructuredIndex; } else { if (!VerifyOnly) { - // We cannot initialize this element, so let - // PerformCopyInitialization produce the appropriate diagnostic. - SemaRef.PerformCopyInitialization(Entity, SourceLocation(), expr, - /*TopLevelOfInitList=*/true); + // We cannot initialize this element, so let PerformCopyInitialization + // produce the appropriate diagnostic. We already checked that this + // initialization will fail. + ExprResult Copy = + SemaRef.PerformCopyInitialization(Entity, SourceLocation(), expr, + /*TopLevelOfInitList=*/true); + (void)Copy; + assert(Copy.isInvalid() && + "expected non-aggregate initialization to fail"); } hadError = true; ++Index; @@ -1872,6 +1860,8 @@ void InitListChecker::CheckArrayType(const InitializedEntity &Entity, // enough elements, or if we're performing an array new with an unknown // bound. // FIXME: This needs to detect holes left by designated initializers too. + // FIXME: Doing this now is wrong; these holes can be filled by later + // designated initializers. if ((maxElementsKnown && elementIndex < maxElements) || Entity.isVariableLengthArrayNew()) CheckEmptyInitializable( @@ -2132,6 +2122,8 @@ void InitListChecker::CheckStructUnionTypes( if (VerifyOnly && Field != FieldEnd && !DeclType->isUnionType() && !Field->getType()->isIncompleteArrayType()) { // FIXME: Should check for holes left by designated initializers too. + // FIXME: Doing this now is wrong; these holes can be filled by later + // designated initializers. for (; Field != FieldEnd && !hadError; ++Field) { if (!Field->isUnnamedBitfield() && !Field->hasInClassInitializer()) CheckEmptyInitializable( From 33e9be6c8b5aad9c164e082c7e78cc08dd13cac1 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 29 Aug 2019 22:49:33 +0000 Subject: [PATCH 2/6] Refactor InitListChecker to check only a single (explicit) initializer list, rather than recursively checking multiple lists in C. This simplification is in preparation for making InitListChecker maintain more state that's specific to the explicit initializer list, particularly when handling designated initialization. llvm-svn: 370418 --- clang/lib/Sema/SemaInit.cpp | 57 +++++------------------ clang/test/Sema/designated-initializers.c | 6 +-- 2 files changed, 15 insertions(+), 48 deletions(-) diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 7c33e99803619..21e7f120f4a02 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -274,11 +274,10 @@ namespace { /// initialize the subobjects after the one designated. class InitListChecker { Sema &SemaRef; - bool hadError; + bool hadError = false; bool VerifyOnly; // no diagnostics, no structure building bool TreatUnavailableAsInvalid; // Used only in VerifyOnly mode. - llvm::DenseMap SyntacticToSemantic; - InitListExpr *FullyStructuredList; + InitListExpr *FullyStructuredList = nullptr; void CheckImplicitInitList(const InitializedEntity &Entity, InitListExpr *ParentIList, QualType T, @@ -842,18 +841,18 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity, } InitListChecker::InitListChecker(Sema &S, const InitializedEntity &Entity, - InitListExpr *IL, QualType &T, - bool VerifyOnly, + InitListExpr *IL, QualType &T, bool VerifyOnly, bool TreatUnavailableAsInvalid) : SemaRef(S), VerifyOnly(VerifyOnly), TreatUnavailableAsInvalid(TreatUnavailableAsInvalid) { // FIXME: Check that IL isn't already the semantic form of some other // InitListExpr. If it is, we'd create a broken AST. - hadError = false; - - FullyStructuredList = - getStructuredSubobjectInit(IL, 0, T, nullptr, 0, IL->getSourceRange()); + if (!VerifyOnly) { + FullyStructuredList = + getStructuredSubobjectInit(IL, 0, T, nullptr, 0, IL->getSourceRange()); + FullyStructuredList->setSyntacticForm(IL); + } CheckExplicitInitList(Entity, IL, T, FullyStructuredList, /*TopLevelObject=*/true); @@ -1075,11 +1074,6 @@ void InitListChecker::CheckExplicitInitList(const InitializedEntity &Entity, InitListExpr *IList, QualType &T, InitListExpr *StructuredList, bool TopLevelObject) { - if (!VerifyOnly) { - SyntacticToSemantic[IList] = StructuredList; - StructuredList->setSyntacticForm(IList); - } - unsigned Index = 0, StructuredIndex = 0; CheckListElementTypes(Entity, IList, T, /*SubobjectIsDesignatorContext=*/true, Index, StructuredList, StructuredIndex, TopLevelObject); @@ -1231,29 +1225,8 @@ void InitListChecker::CheckSubElementType(const InitializedEntity &Entity, IsStringInit(SubInitList->getInit(0), ElemType, SemaRef.Context) == SIF_None) { expr = SubInitList->getInit(0); - } else if (!SemaRef.getLangOpts().CPlusPlus) { - InitListExpr *InnerStructuredList - = getStructuredSubobjectInit(IList, Index, ElemType, - StructuredList, StructuredIndex, - SubInitList->getSourceRange(), true); - CheckExplicitInitList(Entity, SubInitList, ElemType, - InnerStructuredList); - - if (!hadError && !VerifyOnly) { - bool RequiresSecondPass = false; - FillInEmptyInitializations(Entity, InnerStructuredList, - RequiresSecondPass, StructuredList, - StructuredIndex); - if (RequiresSecondPass && !hadError) - FillInEmptyInitializations(Entity, InnerStructuredList, - RequiresSecondPass, StructuredList, - StructuredIndex); - } - ++StructuredIndex; - ++Index; - return; } - // C++ initialization is handled later. + // Nested aggregate initialization and C++ initialization are handled later. } else if (isa(expr)) { // This happens during template instantiation when we see an InitListExpr // that we've already checked once. @@ -1265,7 +1238,7 @@ void InitListChecker::CheckSubElementType(const InitializedEntity &Entity, return; } - if (SemaRef.getLangOpts().CPlusPlus) { + if (SemaRef.getLangOpts().CPlusPlus || isa(expr)) { // C++ [dcl.init.aggr]p2: // Each member is copy-initialized from the corresponding // initializer-clause. @@ -2316,7 +2289,7 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity, // Determine the structural initializer list that corresponds to the // current subobject. if (IsFirstDesignator) - StructuredList = SyntacticToSemantic.lookup(IList); + StructuredList = FullyStructuredList; else { Expr *ExistingInit = StructuredIndex < StructuredList->getNumInits() ? StructuredList->getInit(StructuredIndex) : nullptr; @@ -2833,9 +2806,7 @@ InitListChecker::getStructuredSubobjectInit(InitListExpr *IList, unsigned Index, if (VerifyOnly) return nullptr; // No structured list in verification-only mode. Expr *ExistingInit = nullptr; - if (!StructuredList) - ExistingInit = SyntacticToSemantic.lookup(IList); - else if (StructuredIndex < StructuredList->getNumInits()) + if (StructuredList && StructuredIndex < StructuredList->getNumInits()) ExistingInit = StructuredList->getInit(StructuredIndex); if (InitListExpr *Result = dyn_cast_or_null(ExistingInit)) @@ -2918,10 +2889,6 @@ InitListChecker::getStructuredSubobjectInit(InitListExpr *IList, unsigned Index, // lists. if (StructuredList) StructuredList->updateInit(SemaRef.Context, StructuredIndex, Result); - else { - Result->setSyntacticForm(IList); - SyntacticToSemantic[IList] = Result; - } return Result; } diff --git a/clang/test/Sema/designated-initializers.c b/clang/test/Sema/designated-initializers.c index 43f3318824d58..0a72e8ff3794c 100644 --- a/clang/test/Sema/designated-initializers.c +++ b/clang/test/Sema/designated-initializers.c @@ -46,7 +46,7 @@ struct point array[10] = { struct point array2[10] = { [10].x = 2.0, // expected-error{{array designator index (10) exceeds array bounds (10)}} [4 ... 5].y = 2.0, // expected-note 2 {{previous initialization is here}} - [4 ... 6] = { .x = 3, .y = 4.0 } // expected-warning 2 {{subobject initialization overrides initialization of other fields within its enclosing subobject}} + [4 ... 6] = { .x = 3, .y = 4.0 } // expected-warning 2 {{initializer overrides prior initialization of this subobject}} }; struct point array3[10] = { @@ -364,7 +364,7 @@ struct { }, .u1 = { - .a = 0, - .b = 0, + .a = 0, // expected-note {{previous initialization is here}} + .b = 0, // expected-warning {{initializer overrides prior initialization of this subobject}} }, }; From cd839ccf9985884c25e87cc34f658c02c5e1cd93 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 29 Aug 2019 22:49:34 +0000 Subject: [PATCH 3/6] Fix silent wrong-code bugs and crashes with designated initialization. We failed to correctly handle the 'holes' left behind by designated initializers in VerifyOnly mode. This would result in us thinking that a designated initialization would be valid, only to find that it is not actually valid when we come to build it. In a +Asserts build, that would assert, and in a -Asserts build, that would silently lose some part of the initialization or crash. With this change, when an InitListExpr contains any designators, we now always build a structured list so that we can track the locations of the 'holes' that we need to go back and fill in. We could in principle do better: we only need the structured form if there is a designator that jumps backwards (and can otherwise check for the holes as we progress through the initializer list), but dealing with that turns out to be rather complicated, so it's not done as part of this patch. llvm-svn: 370419 --- clang/include/clang/AST/Expr.h | 2 + clang/lib/Sema/SemaInit.cpp | 415 ++++++++++-------- .../test/SemaCXX/designated-initializers.cpp | 147 +++++++ 3 files changed, 388 insertions(+), 176 deletions(-) diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index e6b16455c33f0..3dff4df0a11ec 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -4495,6 +4495,8 @@ class InitListExpr : public Expr { // Explicit InitListExpr's originate from source code (and have valid source // locations). Implicit InitListExpr's are created by the semantic analyzer. + // FIXME: This is wrong; InitListExprs created by semantic analysis have + // valid source locations too! bool isExplicit() const { return LBraceLoc.isValid() && RBraceLoc.isValid(); } diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 21e7f120f4a02..0f83a71aff421 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -272,12 +272,23 @@ namespace { /// point. CheckDesignatedInitializer() recursively steps into the /// designated subobject and manages backing out the recursion to /// initialize the subobjects after the one designated. +/// +/// If an initializer list contains any designators, we build a placeholder +/// structured list even in 'verify only' mode, so that we can track which +/// elements need 'empty' initializtion. class InitListChecker { Sema &SemaRef; bool hadError = false; - bool VerifyOnly; // no diagnostics, no structure building + bool VerifyOnly; // No diagnostics. bool TreatUnavailableAsInvalid; // Used only in VerifyOnly mode. InitListExpr *FullyStructuredList = nullptr; + NoInitExpr *DummyExpr = nullptr; + + NoInitExpr *getDummyInit() { + if (!DummyExpr) + DummyExpr = new (SemaRef.Context) NoInitExpr(SemaRef.Context.VoidTy); + return DummyExpr; + } void CheckImplicitInitList(const InitializedEntity &Entity, InitListExpr *ParentIList, QualType T, @@ -352,14 +363,14 @@ class InitListChecker { void UpdateStructuredListElement(InitListExpr *StructuredList, unsigned &StructuredIndex, Expr *expr); + InitListExpr *createInitListExpr(QualType CurrentObjectType, + SourceRange InitRange, + unsigned ExpectedNumInits); int numArrayElements(QualType DeclType); int numStructUnionElements(QualType DeclType); - static ExprResult PerformEmptyInit(Sema &SemaRef, - SourceLocation Loc, - const InitializedEntity &Entity, - bool VerifyOnly, - bool TreatUnavailableAsInvalid); + ExprResult PerformEmptyInit(SourceLocation Loc, + const InitializedEntity &Entity); // Explanation on the "FillWithNoInit" mode: // @@ -411,11 +422,8 @@ class InitListChecker { } // end anonymous namespace -ExprResult InitListChecker::PerformEmptyInit(Sema &SemaRef, - SourceLocation Loc, - const InitializedEntity &Entity, - bool VerifyOnly, - bool TreatUnavailableAsInvalid) { +ExprResult InitListChecker::PerformEmptyInit(SourceLocation Loc, + const InitializedEntity &Entity) { InitializationKind Kind = InitializationKind::CreateValue(Loc, Loc, Loc, true); MultiExprArg SubInit; @@ -517,43 +525,44 @@ ExprResult InitListChecker::PerformEmptyInit(Sema &SemaRef, << Entity.getElementIndex(); } } + hadError = true; return ExprError(); } - return VerifyOnly ? ExprResult(static_cast(nullptr)) + return VerifyOnly ? ExprResult() : InitSeq.Perform(SemaRef, Entity, Kind, SubInit); } void InitListChecker::CheckEmptyInitializable(const InitializedEntity &Entity, SourceLocation Loc) { - assert(VerifyOnly && - "CheckEmptyInitializable is only inteded for verification mode."); - if (PerformEmptyInit(SemaRef, Loc, Entity, /*VerifyOnly*/true, - TreatUnavailableAsInvalid).isInvalid()) - hadError = true; + // If we're building a fully-structured list, we'll check this at the end + // once we know which elements are actually initialized. Otherwise, we know + // that there are no designators so we can just check now. + if (FullyStructuredList) + return; + PerformEmptyInit(Loc, Entity); } void InitListChecker::FillInEmptyInitForBase( unsigned Init, const CXXBaseSpecifier &Base, const InitializedEntity &ParentEntity, InitListExpr *ILE, bool &RequiresSecondPass, bool FillWithNoInit) { - assert(Init < ILE->getNumInits() && "should have been expanded"); - InitializedEntity BaseEntity = InitializedEntity::InitializeBase( SemaRef.Context, &Base, false, &ParentEntity); - if (!ILE->getInit(Init)) { - ExprResult BaseInit = - FillWithNoInit - ? new (SemaRef.Context) NoInitExpr(Base.getType()) - : PerformEmptyInit(SemaRef, ILE->getEndLoc(), BaseEntity, - /*VerifyOnly*/ false, TreatUnavailableAsInvalid); + if (Init >= ILE->getNumInits() || !ILE->getInit(Init)) { + ExprResult BaseInit = FillWithNoInit + ? new (SemaRef.Context) NoInitExpr(Base.getType()) + : PerformEmptyInit(ILE->getEndLoc(), BaseEntity); if (BaseInit.isInvalid()) { hadError = true; return; } - ILE->setInit(Init, BaseInit.getAs()); + if (!VerifyOnly) { + assert(Init < ILE->getNumInits() && "should have been expanded"); + ILE->setInit(Init, BaseInit.getAs()); + } } else if (InitListExpr *InnerILE = dyn_cast(ILE->getInit(Init))) { FillInEmptyInitializations(BaseEntity, InnerILE, RequiresSecondPass, @@ -576,12 +585,14 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field, InitializedEntity MemberEntity = InitializedEntity::InitializeMember(Field, &ParentEntity); - if (const RecordType *RType = ILE->getType()->getAs()) - if (!RType->getDecl()->isUnion()) - assert(Init < NumInits && "This ILE should have been expanded"); - if (Init >= NumInits || !ILE->getInit(Init)) { + if (const RecordType *RType = ILE->getType()->getAs()) + if (!RType->getDecl()->isUnion()) + assert((Init < NumInits || VerifyOnly) && + "This ILE should have been expanded"); + if (FillWithNoInit) { + assert(!VerifyOnly && "should not fill with no-init in verify-only mode"); Expr *Filler = new (SemaRef.Context) NoInitExpr(Field->getType()); if (Init < NumInits) ILE->setInit(Init, Filler); @@ -594,6 +605,9 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field, // members in the aggregate, then each member not explicitly initialized // shall be initialized from its brace-or-equal-initializer [...] if (Field->hasInClassInitializer()) { + if (VerifyOnly) + return; + ExprResult DIE = SemaRef.BuildCXXDefaultInitExpr(Loc, Field); if (DIE.isInvalid()) { hadError = true; @@ -610,28 +624,28 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field, } if (Field->getType()->isReferenceType()) { - // C++ [dcl.init.aggr]p9: - // If an incomplete or empty initializer-list leaves a - // member of reference type uninitialized, the program is - // ill-formed. - SemaRef.Diag(Loc, diag::err_init_reference_member_uninitialized) - << Field->getType() - << ILE->getSyntacticForm()->getSourceRange(); - SemaRef.Diag(Field->getLocation(), - diag::note_uninit_reference_member); + if (!VerifyOnly) { + // C++ [dcl.init.aggr]p9: + // If an incomplete or empty initializer-list leaves a + // member of reference type uninitialized, the program is + // ill-formed. + SemaRef.Diag(Loc, diag::err_init_reference_member_uninitialized) + << Field->getType() + << ILE->getSyntacticForm()->getSourceRange(); + SemaRef.Diag(Field->getLocation(), + diag::note_uninit_reference_member); + } hadError = true; return; } - ExprResult MemberInit = PerformEmptyInit(SemaRef, Loc, MemberEntity, - /*VerifyOnly*/false, - TreatUnavailableAsInvalid); + ExprResult MemberInit = PerformEmptyInit(Loc, MemberEntity); if (MemberInit.isInvalid()) { hadError = true; return; } - if (hadError) { + if (hadError || VerifyOnly) { // Do nothing } else if (Init < NumInits) { ILE->setInit(Init, MemberInit.getAs()); @@ -644,14 +658,15 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field, RequiresSecondPass = true; } } else if (InitListExpr *InnerILE - = dyn_cast(ILE->getInit(Init))) + = dyn_cast(ILE->getInit(Init))) { FillInEmptyInitializations(MemberEntity, InnerILE, RequiresSecondPass, ILE, Init, FillWithNoInit); - else if (DesignatedInitUpdateExpr *InnerDIUE - = dyn_cast(ILE->getInit(Init))) + } else if (DesignatedInitUpdateExpr *InnerDIUE = + dyn_cast(ILE->getInit(Init))) { FillInEmptyInitializations(MemberEntity, InnerDIUE->getUpdater(), RequiresSecondPass, ILE, Init, /*FillWithNoInit =*/true); + } } /// Recursively replaces NULL values within the given initializer list @@ -667,6 +682,11 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity, assert((ILE->getType() != SemaRef.Context.VoidTy) && "Should not have void type"); + // We don't need to do any checks when just filling NoInitExprs; that can't + // fail. + if (FillWithNoInit && VerifyOnly) + return; + // If this is a nested initializer list, we might have changed its contents // (and therefore some of its properties, such as instantiation-dependence) // while filling it in. Inform the outer initializer list so that its state @@ -709,7 +729,7 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity, unsigned NumElems = numStructUnionElements(ILE->getType()); if (RDecl->hasFlexibleArrayMember()) ++NumElems; - if (ILE->getNumInits() < NumElems) + if (!VerifyOnly && ILE->getNumInits() < NumElems) ILE->resizeInits(SemaRef.Context, NumElems); unsigned Init = 0; @@ -771,6 +791,7 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity, } else ElementType = ILE->getType(); + bool SkipEmptyInitChecks = false; for (unsigned Init = 0; Init != NumElements; ++Init) { if (hadError) return; @@ -779,21 +800,25 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity, ElementEntity.getKind() == InitializedEntity::EK_VectorElement) ElementEntity.setElementIndex(Init); - if (Init >= NumInits && ILE->hasArrayFiller()) + if (Init >= NumInits && (ILE->hasArrayFiller() || SkipEmptyInitChecks)) return; Expr *InitExpr = (Init < NumInits ? ILE->getInit(Init) : nullptr); if (!InitExpr && Init < NumInits && ILE->hasArrayFiller()) ILE->setInit(Init, ILE->getArrayFiller()); else if (!InitExpr && !ILE->hasArrayFiller()) { + // In VerifyOnly mode, there's no point performing empty initialization + // more than once. + if (SkipEmptyInitChecks) + continue; + Expr *Filler = nullptr; if (FillWithNoInit) Filler = new (SemaRef.Context) NoInitExpr(ElementType); else { ExprResult ElementInit = - PerformEmptyInit(SemaRef, ILE->getEndLoc(), ElementEntity, - /*VerifyOnly*/ false, TreatUnavailableAsInvalid); + PerformEmptyInit(ILE->getEndLoc(), ElementEntity); if (ElementInit.isInvalid()) { hadError = true; return; @@ -804,6 +829,8 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity, if (hadError) { // Do nothing + } else if (VerifyOnly) { + SkipEmptyInitChecks = true; } else if (Init < NumInits) { // For arrays, just set the expression used for value-initialization // of the "holes" in the array. @@ -829,34 +856,44 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity, } } } else if (InitListExpr *InnerILE - = dyn_cast_or_null(InitExpr)) + = dyn_cast_or_null(InitExpr)) { FillInEmptyInitializations(ElementEntity, InnerILE, RequiresSecondPass, ILE, Init, FillWithNoInit); - else if (DesignatedInitUpdateExpr *InnerDIUE - = dyn_cast_or_null(InitExpr)) + } else if (DesignatedInitUpdateExpr *InnerDIUE = + dyn_cast_or_null(InitExpr)) { FillInEmptyInitializations(ElementEntity, InnerDIUE->getUpdater(), RequiresSecondPass, ILE, Init, /*FillWithNoInit =*/true); + } } } +static bool hasAnyDesignatedInits(const InitListExpr *IL) { + for (const Stmt *Init : *IL) + if (Init && isa(Init)) + return true; + return false; +} + InitListChecker::InitListChecker(Sema &S, const InitializedEntity &Entity, InitListExpr *IL, QualType &T, bool VerifyOnly, bool TreatUnavailableAsInvalid) - : SemaRef(S), VerifyOnly(VerifyOnly), - TreatUnavailableAsInvalid(TreatUnavailableAsInvalid) { - // FIXME: Check that IL isn't already the semantic form of some other - // InitListExpr. If it is, we'd create a broken AST. - - if (!VerifyOnly) { + : SemaRef(S), VerifyOnly(VerifyOnly), + TreatUnavailableAsInvalid(TreatUnavailableAsInvalid) { + if (!VerifyOnly || hasAnyDesignatedInits(IL)) { FullyStructuredList = - getStructuredSubobjectInit(IL, 0, T, nullptr, 0, IL->getSourceRange()); - FullyStructuredList->setSyntacticForm(IL); + createInitListExpr(T, IL->getSourceRange(), IL->getNumInits()); + + // FIXME: Check that IL isn't already the semantic form of some other + // InitListExpr. If it is, we'd create a broken AST. + if (!VerifyOnly) + FullyStructuredList->setSyntacticForm(IL); } + CheckExplicitInitList(Entity, IL, T, FullyStructuredList, /*TopLevelObject=*/true); - if (!hadError && !VerifyOnly) { + if (!hadError && FullyStructuredList) { bool RequiresSecondPass = false; FillInEmptyInitializations(Entity, FullyStructuredList, RequiresSecondPass, /*OuterILE=*/nullptr, /*OuterIndex=*/0); @@ -963,7 +1000,7 @@ void InitListChecker::CheckImplicitInitList(const InitializedEntity &Entity, StructuredSubobjectInitList, StructuredSubobjectInitIndex); - if (!VerifyOnly) { + if (StructuredSubobjectInitList) { StructuredSubobjectInitList->setType(T); unsigned EndIndex = (Index == StartIndex? StartIndex : Index - 1); @@ -977,7 +1014,7 @@ void InitListChecker::CheckImplicitInitList(const InitializedEntity &Entity, } // Complain about missing braces. - if ((T->isArrayType() || T->isRecordType()) && + if (!VerifyOnly && (T->isArrayType() || T->isRecordType()) && !ParentIList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()) && !isIdiomaticBraceElisionEntity(Entity)) { SemaRef.Diag(StructuredSubobjectInitList->getBeginLoc(), @@ -993,7 +1030,7 @@ void InitListChecker::CheckImplicitInitList(const InitializedEntity &Entity, // Warn if this type won't be an aggregate in future versions of C++. auto *CXXRD = T->getAsCXXRecordDecl(); - if (CXXRD && CXXRD->hasUserDeclaredConstructor()) { + if (!VerifyOnly && CXXRD && CXXRD->hasUserDeclaredConstructor()) { SemaRef.Diag(StructuredSubobjectInitList->getBeginLoc(), diag::warn_cxx2a_compat_aggregate_init_with_ctors) << StructuredSubobjectInitList->getSourceRange() << T; @@ -1077,11 +1114,12 @@ void InitListChecker::CheckExplicitInitList(const InitializedEntity &Entity, unsigned Index = 0, StructuredIndex = 0; CheckListElementTypes(Entity, IList, T, /*SubobjectIsDesignatorContext=*/true, Index, StructuredList, StructuredIndex, TopLevelObject); - if (!VerifyOnly) { + if (StructuredList) { QualType ExprTy = T; if (!ExprTy->isArrayType()) ExprTy = ExprTy.getNonLValueExprType(SemaRef.Context); - IList->setType(ExprTy); + if (!VerifyOnly) + IList->setType(ExprTy); StructuredList->setType(ExprTy); } if (hadError) @@ -1224,6 +1262,8 @@ void InitListChecker::CheckSubElementType(const InitializedEntity &Entity, if (SubInitList->getNumInits() == 1 && IsStringInit(SubInitList->getInit(0), ElemType, SemaRef.Context) == SIF_None) { + // FIXME: It would be more faithful and no less correct to include an + // InitListExpr in the semantic form of the initializer list in this case. expr = SubInitList->getInit(0); } // Nested aggregate initialization and C++ initialization are handled later. @@ -1232,8 +1272,7 @@ void InitListChecker::CheckSubElementType(const InitializedEntity &Entity, // that we've already checked once. assert(SemaRef.Context.hasSameType(expr->getType(), ElemType) && "found implicit initialization for the wrong type"); - if (!VerifyOnly) - UpdateStructuredListElement(StructuredList, StructuredIndex, expr); + UpdateStructuredListElement(StructuredList, StructuredIndex, expr); ++Index; return; } @@ -1272,8 +1311,12 @@ void InitListChecker::CheckSubElementType(const InitializedEntity &Entity, UpdateStructuredListElement(StructuredList, StructuredIndex, Result.getAs()); - } else if (!Seq) + } else if (!Seq) { hadError = true; + } else if (StructuredList) { + UpdateStructuredListElement(StructuredList, StructuredIndex, + getDummyInit()); + } ++Index; return; } @@ -1290,10 +1333,11 @@ void InitListChecker::CheckSubElementType(const InitializedEntity &Entity, // type here, though. if (IsStringInit(expr, arrayType, SemaRef.Context) == SIF_None) { - if (!VerifyOnly) { + // FIXME: Should we do this checking in verify-only mode? + if (!VerifyOnly) CheckStringInit(expr, ElemType, arrayType, SemaRef); + if (StructuredList) UpdateStructuredListElement(StructuredList, StructuredIndex, expr); - } ++Index; return; } @@ -1437,17 +1481,18 @@ void InitListChecker::CheckScalarType(const InitializedEntity &Entity, return; } + ExprResult Result; if (VerifyOnly) { - if (!SemaRef.CanPerformCopyInitialization(Entity,expr)) - hadError = true; - ++Index; - return; + if (SemaRef.CanPerformCopyInitialization(Entity, expr)) + Result = getDummyInit(); + else + Result = ExprError(); + } else { + Result = + SemaRef.PerformCopyInitialization(Entity, expr->getBeginLoc(), expr, + /*TopLevelOfInitList=*/true); } - ExprResult Result = - SemaRef.PerformCopyInitialization(Entity, expr->getBeginLoc(), expr, - /*TopLevelOfInitList=*/true); - Expr *ResultExpr = nullptr; if (Result.isInvalid()) @@ -1455,8 +1500,9 @@ void InitListChecker::CheckScalarType(const InitializedEntity &Entity, else { ResultExpr = Result.getAs(); - if (ResultExpr != expr) { + if (ResultExpr != expr && !VerifyOnly) { // The type was promoted, update initializer list. + // FIXME: Why are we updating the syntactic init list? IList->setInit(Index, ResultExpr); } } @@ -1498,22 +1544,25 @@ void InitListChecker::CheckReferenceType(const InitializedEntity &Entity, return; } + ExprResult Result; if (VerifyOnly) { - if (!SemaRef.CanPerformCopyInitialization(Entity,expr)) - hadError = true; - ++Index; - return; + if (SemaRef.CanPerformCopyInitialization(Entity,expr)) + Result = getDummyInit(); + else + Result = ExprError(); + } else { + Result = + SemaRef.PerformCopyInitialization(Entity, expr->getBeginLoc(), expr, + /*TopLevelOfInitList=*/true); } - ExprResult Result = - SemaRef.PerformCopyInitialization(Entity, expr->getBeginLoc(), expr, - /*TopLevelOfInitList=*/true); - if (Result.isInvalid()) hadError = true; expr = Result.getAs(); - IList->setInit(Index, expr); + // FIXME: Why are we updating the syntactic init list? + if (!VerifyOnly) + IList->setInit(Index, expr); if (hadError) ++StructuredIndex; @@ -1534,10 +1583,9 @@ void InitListChecker::CheckVectorType(const InitializedEntity &Entity, if (Index >= IList->getNumInits()) { // Make sure the element type can be value-initialized. - if (VerifyOnly) - CheckEmptyInitializable( - InitializedEntity::InitializeElement(SemaRef.Context, 0, Entity), - IList->getEndLoc()); + CheckEmptyInitializable( + InitializedEntity::InitializeElement(SemaRef.Context, 0, Entity), + IList->getEndLoc()); return; } @@ -1546,25 +1594,27 @@ void InitListChecker::CheckVectorType(const InitializedEntity &Entity, // instead of breaking it apart (which is doomed to failure anyway). Expr *Init = IList->getInit(Index); if (!isa(Init) && Init->getType()->isVectorType()) { + ExprResult Result; if (VerifyOnly) { - if (!SemaRef.CanPerformCopyInitialization(Entity, Init)) - hadError = true; - ++Index; - return; + if (SemaRef.CanPerformCopyInitialization(Entity, Init)) + Result = getDummyInit(); + else + Result = ExprError(); + } else { + Result = + SemaRef.PerformCopyInitialization(Entity, Init->getBeginLoc(), Init, + /*TopLevelOfInitList=*/true); } - ExprResult Result = - SemaRef.PerformCopyInitialization(Entity, Init->getBeginLoc(), Init, - /*TopLevelOfInitList=*/true); - Expr *ResultExpr = nullptr; if (Result.isInvalid()) hadError = true; // types weren't compatible. else { ResultExpr = Result.getAs(); - if (ResultExpr != Init) { + if (ResultExpr != Init && !VerifyOnly) { // The type was promoted, update initializer list. + // FIXME: Why are we updating the syntactic init list? IList->setInit(Index, ResultExpr); } } @@ -1583,8 +1633,7 @@ void InitListChecker::CheckVectorType(const InitializedEntity &Entity, for (unsigned i = 0; i < maxElements; ++i, ++numEltsInit) { // Don't attempt to go past the end of the init list if (Index >= IList->getNumInits()) { - if (VerifyOnly) - CheckEmptyInitializable(ElementEntity, IList->getEndLoc()); + CheckEmptyInitializable(ElementEntity, IList->getEndLoc()); break; } @@ -1727,8 +1776,10 @@ void InitListChecker::CheckArrayType(const InitializedEntity &Entity, // of the structured initializer list doesn't match exactly, // because doing so would involve allocating one character // constant for each string. - if (!VerifyOnly) { + // FIXME: Should we do these checks in verify-only mode too? + if (!VerifyOnly) CheckStringInit(IList->getInit(Index), DeclType, arrayType, SemaRef); + if (StructuredList) { UpdateStructuredListElement(StructuredList, StructuredIndex, IList->getInit(Index)); StructuredList->resizeInits(SemaRef.Context, StructuredIndex); @@ -1827,14 +1878,11 @@ void InitListChecker::CheckArrayType(const InitializedEntity &Entity, DeclType = SemaRef.Context.getConstantArrayType(elementType, maxElements, ArrayType::Normal, 0); } - if (!hadError && VerifyOnly) { + if (!hadError) { // If there are any members of the array that get value-initialized, check // that is possible. That happens if we know the bound and don't have // enough elements, or if we're performing an array new with an unknown // bound. - // FIXME: This needs to detect holes left by designated initializers too. - // FIXME: Doing this now is wrong; these holes can be filled by later - // designated initializers. if ((maxElementsKnown && elementIndex < maxElements) || Entity.isVariableLengthArrayNew()) CheckEmptyInitializable( @@ -1912,7 +1960,7 @@ void InitListChecker::CheckStructUnionTypes( // If there's a default initializer, use it. if (isa(RD) && cast(RD)->hasInClassInitializer()) { - if (VerifyOnly) + if (!StructuredList) return; for (RecordDecl::field_iterator FieldEnd = RD->field_end(); Field != FieldEnd; ++Field) { @@ -1929,11 +1977,10 @@ void InitListChecker::CheckStructUnionTypes( for (RecordDecl::field_iterator FieldEnd = RD->field_end(); Field != FieldEnd; ++Field) { if (!Field->isUnnamedBitfield()) { - if (VerifyOnly) - CheckEmptyInitializable( - InitializedEntity::InitializeMember(*Field, &Entity), - IList->getEndLoc()); - else + CheckEmptyInitializable( + InitializedEntity::InitializeMember(*Field, &Entity), + IList->getEndLoc()); + if (StructuredList) StructuredList->setInitializedFieldInUnion(*Field); break; } @@ -1959,7 +2006,7 @@ void InitListChecker::CheckStructUnionTypes( CheckSubElementType(BaseEntity, IList, Base.getType(), Index, StructuredList, StructuredIndex); InitializedSomething = true; - } else if (VerifyOnly) { + } else { CheckEmptyInitializable(BaseEntity, InitLoc); } @@ -2067,7 +2114,7 @@ void InitListChecker::CheckStructUnionTypes( StructuredList, StructuredIndex); InitializedSomething = true; - if (DeclType->isUnionType() && !VerifyOnly) { + if (DeclType->isUnionType() && StructuredList) { // Initialize the first field within the union. StructuredList->setInitializedFieldInUnion(*Field); } @@ -2091,12 +2138,10 @@ void InitListChecker::CheckStructUnionTypes( } } - // Check that any remaining fields can be value-initialized. - if (VerifyOnly && Field != FieldEnd && !DeclType->isUnionType() && + // Check that any remaining fields can be value-initialized if we're not + // building a structured list. (If we are, we'll check this later.) + if (!StructuredList && Field != FieldEnd && !DeclType->isUnionType() && !Field->getType()->isIncompleteArrayType()) { - // FIXME: Should check for holes left by designated initializers too. - // FIXME: Doing this now is wrong; these holes can be filled by later - // designated initializers. for (; Field != FieldEnd && !hadError; ++Field) { if (!Field->isUnnamedBitfield() && !Field->hasInClassInitializer()) CheckEmptyInitializable( @@ -2282,10 +2327,7 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity, DesignatedInitExpr::Designator *D = DIE->getDesignator(DesigIdx); bool IsFirstDesignator = (DesigIdx == 0); - if (!VerifyOnly) { - assert((IsFirstDesignator || StructuredList) && - "Need a non-designated initializer list to start from"); - + if (IsFirstDesignator ? FullyStructuredList : StructuredList) { // Determine the structural initializer list that corresponds to the // current subobject. if (IsFirstDesignator) @@ -2302,7 +2344,7 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity, SourceRange(D->getBeginLoc(), DIE->getEndLoc())); else if (InitListExpr *Result = dyn_cast(ExistingInit)) StructuredList = Result; - else { + else if (!VerifyOnly) { if (DesignatedInitUpdateExpr *E = dyn_cast(ExistingInit)) StructuredList = E->getUpdater(); @@ -2331,9 +2373,9 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity, // struct X { int a, b; }; // struct X xs[] = { [0] = (struct X) { 1, 2 }, [0].b = 3 }; // - // Here, xs[0].a == 0 and xs[0].b == 3, since the second, - // designated initializer re-initializes the whole - // subobject [0], overwriting previous initializers. + // Here, xs[0].a == 1 and xs[0].b == 3, since the second, + // designated initializer re-initializes only its current object + // subobject [0].b. SemaRef.Diag(D->getBeginLoc(), diag::warn_subobject_initializer_overrides) << SourceRange(D->getBeginLoc(), DIE->getEndLoc()); @@ -2342,9 +2384,15 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity, diag::note_previous_initializer) << /*FIXME:has side effects=*/0 << ExistingInit->getSourceRange(); } + } else { + // We don't need to track the structured representation of a designated + // init update of an already-fully-initialized object in verify-only + // mode. The only reason we would need the structure is to determine + // where the uninitialized "holes" are, and in this case, we know there + // aren't any and we can't introduce any. + StructuredList = nullptr; } } - assert(StructuredList && "Expected a structured initializer list"); } if (D->isFieldDesignator()) { @@ -2449,14 +2497,14 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity, // the initializer list. if (RT->getDecl()->isUnion()) { FieldIndex = 0; - if (!VerifyOnly) { + if (StructuredList) { FieldDecl *CurrentField = StructuredList->getInitializedFieldInUnion(); if (CurrentField && !declaresSameEntity(CurrentField, *Field)) { assert(StructuredList->getNumInits() == 1 && "A union should never have more than one initializer!"); Expr *ExistingInit = StructuredList->getInit(0); - if (ExistingInit) { + if (ExistingInit && !VerifyOnly) { // We're about to throw away an initializer, emit warning. SemaRef.Diag(D->getFieldLoc(), diag::warn_initializer_overrides) @@ -2487,15 +2535,14 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity, return true; } - if (!VerifyOnly) { - // Update the designator with the field declaration. + // Update the designator with the field declaration. + if (!VerifyOnly) D->setField(*Field); - // Make sure that our non-designated initializer list has space - // for a subobject corresponding to this field. - if (FieldIndex >= StructuredList->getNumInits()) - StructuredList->resizeInits(SemaRef.Context, FieldIndex + 1); - } + // Make sure that our non-designated initializer list has space + // for a subobject corresponding to this field. + if (StructuredList && FieldIndex >= StructuredList->getNumInits()) + StructuredList->resizeInits(SemaRef.Context, FieldIndex + 1); // This designator names a flexible array member. if (Field->getType()->isIncompleteArrayType()) { @@ -2681,7 +2728,13 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity, DesignatedEndIndex.setIsUnsigned(true); } - if (!VerifyOnly && StructuredList->isStringLiteralInit()) { + bool IsStringLiteralInitUpdate = + StructuredList && StructuredList->isStringLiteralInit(); + if (IsStringLiteralInitUpdate && VerifyOnly) { + // We're just verifying an update to a string literal init. We don't need + // to split the string up into individual characters to do that. + StructuredList = nullptr; + } else if (IsStringLiteralInitUpdate) { // We're modifying a string literal init; we have to decompose the string // so we can modify the individual characters. ASTContext &Context = SemaRef.Context; @@ -2741,7 +2794,7 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity, // Make sure that our non-designated initializer list has space // for a subobject corresponding to this array element. - if (!VerifyOnly && + if (StructuredList && DesignatedEndIndex.getZExtValue() >= StructuredList->getNumInits()) StructuredList->resizeInits(SemaRef.Context, DesignatedEndIndex.getZExtValue() + 1); @@ -2803,10 +2856,11 @@ InitListChecker::getStructuredSubobjectInit(InitListExpr *IList, unsigned Index, unsigned StructuredIndex, SourceRange InitRange, bool IsFullyOverwritten) { - if (VerifyOnly) - return nullptr; // No structured list in verification-only mode. + if (!StructuredList) + return nullptr; + Expr *ExistingInit = nullptr; - if (StructuredList && StructuredIndex < StructuredList->getNumInits()) + if (StructuredIndex < StructuredList->getNumInits()) ExistingInit = StructuredList->getInit(StructuredIndex); if (InitListExpr *Result = dyn_cast_or_null(ExistingInit)) @@ -2821,18 +2875,26 @@ InitListChecker::getStructuredSubobjectInit(InitListExpr *IList, unsigned Index, if (!IsFullyOverwritten) return Result; - if (ExistingInit) { + if (ExistingInit && !VerifyOnly) { // We are creating an initializer list that initializes the // subobjects of the current object, but there was already an // initialization that completely initialized the current - // subobject, e.g., by a compound literal: + // subobject: // // struct X { int a, b; }; + // struct X xs[] = { [0] = { 1, 2 }, [0].b = 3 }; + // + // Here, xs[0].a == 1 and xs[0].b == 3, since the second, + // designated initializer overwrites the [0].b initializer + // from the prior initialization. + // + // When the existing initializer is an expression rather than an + // initializer list, we cannot decompose and update it in this way. + // For example: + // // struct X xs[] = { [0] = (struct X) { 1, 2 }, [0].b = 3 }; // - // Here, xs[0].a == 0 and xs[0].b == 3, since the second, - // designated initializer re-initializes the whole - // subobject [0], overwriting previous initializers. + // This case is handled by CheckDesignatedInitializer. SemaRef.Diag(InitRange.getBegin(), diag::warn_subobject_initializer_overrides) << InitRange; @@ -2840,6 +2902,27 @@ InitListChecker::getStructuredSubobjectInit(InitListExpr *IList, unsigned Index, << /*FIXME:has side effects=*/0 << ExistingInit->getSourceRange(); } + unsigned ExpectedNumInits = 0; + if (Index < IList->getNumInits()) { + if (auto *Init = dyn_cast_or_null(IList->getInit(Index))) + ExpectedNumInits = Init->getNumInits(); + else + ExpectedNumInits = IList->getNumInits() - Index; + } + + InitListExpr *Result = + createInitListExpr(CurrentObjectType, InitRange, ExpectedNumInits); + + // Link this new initializer list into the structured initializer + // lists. + StructuredList->updateInit(SemaRef.Context, StructuredIndex, Result); + return Result; +} + +InitListExpr * +InitListChecker::createInitListExpr(QualType CurrentObjectType, + SourceRange InitRange, + unsigned ExpectedNumInits) { InitListExpr *Result = new (SemaRef.Context) InitListExpr(SemaRef.Context, InitRange.getBegin(), None, @@ -2852,17 +2935,6 @@ InitListChecker::getStructuredSubobjectInit(InitListExpr *IList, unsigned Index, // Pre-allocate storage for the structured initializer list. unsigned NumElements = 0; - unsigned NumInits = 0; - bool GotNumInits = false; - if (!StructuredList) { - NumInits = IList->getNumInits(); - GotNumInits = true; - } else if (Index < IList->getNumInits()) { - if (InitListExpr *SubList = dyn_cast(IList->getInit(Index))) { - NumInits = SubList->getNumInits(); - GotNumInits = true; - } - } if (const ArrayType *AType = SemaRef.Context.getAsArrayType(CurrentObjectType)) { @@ -2870,26 +2942,17 @@ InitListChecker::getStructuredSubobjectInit(InitListExpr *IList, unsigned Index, NumElements = CAType->getSize().getZExtValue(); // Simple heuristic so that we don't allocate a very large // initializer with many empty entries at the end. - if (GotNumInits && NumElements > NumInits) + if (NumElements > ExpectedNumInits) NumElements = 0; } - } else if (const VectorType *VType = CurrentObjectType->getAs()) + } else if (const VectorType *VType = CurrentObjectType->getAs()) { NumElements = VType->getNumElements(); - else if (const RecordType *RType = CurrentObjectType->getAs()) { - RecordDecl *RDecl = RType->getDecl(); - if (RDecl->isUnion()) - NumElements = 1; - else - NumElements = std::distance(RDecl->field_begin(), RDecl->field_end()); + } else if (CurrentObjectType->isRecordType()) { + NumElements = numStructUnionElements(CurrentObjectType); } Result->reserveInits(SemaRef.Context, NumElements); - // Link this new initializer list into the structured initializer - // lists. - if (StructuredList) - StructuredList->updateInit(SemaRef.Context, StructuredIndex, Result); - return Result; } @@ -2911,7 +2974,7 @@ void InitListChecker::UpdateStructuredListElement(InitListExpr *StructuredList, // struct PP { struct P p } l = { { .a = 2 }, .p.b = 3 }; // There is an overwrite taking place because the first braced initializer // list "{ .a = 2 }' already provides value for .p.b (which is zero). - if (PrevInit->getSourceRange().isValid()) { + if (PrevInit->getSourceRange().isValid() && !VerifyOnly) { SemaRef.Diag(expr->getBeginLoc(), diag::warn_initializer_overrides) << expr->getSourceRange(); diff --git a/clang/test/SemaCXX/designated-initializers.cpp b/clang/test/SemaCXX/designated-initializers.cpp index 04002c0b6c11f..739817372ebd7 100644 --- a/clang/test/SemaCXX/designated-initializers.cpp +++ b/clang/test/SemaCXX/designated-initializers.cpp @@ -30,3 +30,150 @@ void bar() { Bar::Test(); // expected-note {{in instantiation of member function 'Bar::Test' requested here}} Bar::Test(); // expected-note {{in instantiation of member function 'Bar::Test' requested here}} } + +namespace Reorder { + struct X { + X(int n); + private: + int i; + }; + + struct foo { + X x; + X y; + }; + + foo n = {.y = 4, .x = 5}; + X arr[2] = {[1] = 1, [0] = 2}; +} + +namespace Reorder2 { + struct S { + S(); + S(const S &); + ~S(); + }; + + struct EF { + S s; + }; + + struct PN { + PN(const PN &); + }; + extern PN pn; + + struct FLN { + EF ef; + int it; + PN pn; + }; + + void f() { + FLN new_elem = { + .ef = EF(), + .pn = pn, + .it = 0, + }; + } +} + +namespace Reorder3 { + struct S { + int a, &b, &c; // expected-note 2{{here}} + }; + S s1 = { + .a = 1, .c = s1.a, .b = s1.a + }; + S s2 = { + .a = 1, .c = s2.a + }; // expected-error {{uninitialized}} + S s3 = { + .b = s3.a, .a = 1, + }; // expected-error {{uninitialized}} +} + +// Check that we don't even think about whether holes in a designated +// initializer are zero-initializable if the holes are filled later. +namespace NoCheckingFilledHoles { + template struct Error { using type = typename T::type; }; // expected-error 3{{'::'}} + + template + struct DefaultInitIsError { + DefaultInitIsError(Error = {}); // expected-note 3{{instantiation}} expected-note 3{{passing}} + DefaultInitIsError(int, int); + }; + + template + struct X { + int a; + DefaultInitIsError e; + int b; + }; + X<1> x1 = { + .b = 2, + .a = 1, + {4, 4} + }; + X<2> x2 = { + .e = {4, 4}, + .b = 2, + .a = 1 + }; + X<3> x3 = { + .b = 2, + .a = 1 + }; // expected-note {{default function argument}} + X<4> x4 = { + .a = 1, + .b = 2 + }; // expected-note {{default function argument}} + X<5> x5 = { + .e = {4, 4}, + .a = 1, + .b = 2 + }; + X<6> x6 = { + .a = 1, + .b = 2, + .e = {4, 4} + }; + + template struct Y { X x; }; + Y<7> y7 = { + .x = {.a = 1, .b = 2}, // expected-note {{default function argument}} + .x.e = {3, 4} + }; + Y<8> y8 = { + .x = {.e = {3, 4}}, + .x.a = 1, + .x.b = 2 + }; +} + +namespace LargeArrayDesignator { + struct X { + int arr[1000000000]; + }; + struct Y { + int arr[3]; + }; + void f(X x); + void f(Y y) = delete; + void g() { + f({.arr[4] = 1}); + } +} + +namespace ADL { + struct A {}; + void f(A, int); + + namespace X { + void f(A, int); + // OK. Would fail if checking {} against type A set the type of the + // initializer list to A, because ADL would find ADL::f, resulting in + // ambiguity. + void g() { f({}, {}); } + } +} From 3944c9638e56af25717407c583aaeae8a01d7629 Mon Sep 17 00:00:00 2001 From: Alex Lorenz Date: Thu, 29 Aug 2019 22:56:38 +0000 Subject: [PATCH 4/6] [clang-scan-deps] reuse the file manager across invocations of the dependency scanner on a single worker thread This behavior can be controlled using the new `-reuse-filemanager` clang-scan-deps option. By default the file manager is reused. The added test/ClangScanDeps/symlink.cpp is able to pass with the reused filemanager after the related FileEntryRef changes landed earlier. The test test/ClangScanDeps/subframework_header_dir_symlink.m still fails when the file manager is reused (I run the FileCheck with not to make it PASS). I will address this in a follow-up patch that improves the DirectoryEntry name modelling in the FileManager. llvm-svn: 370420 --- .../DependencyScanningService.h | 5 +++- .../DependencyScanningWorker.h | 3 +++ clang/include/clang/Tooling/Tooling.h | 5 +++- .../DependencyScanningService.cpp | 5 ++-- .../DependencyScanningWorker.cpp | 4 ++- clang/lib/Tooling/Tooling.cpp | 8 ++++-- .../subframework_header_dir_symlink_cdb.json | 12 +++++++++ .../ClangScanDeps/Inputs/symlink_cdb.json | 12 +++++++++ .../subframework_header_dir_symlink.m | 25 +++++++++++++++++++ clang/test/ClangScanDeps/symlink.cpp | 23 +++++++++++++++++ clang/tools/clang-scan-deps/ClangScanDeps.cpp | 7 +++++- 11 files changed, 101 insertions(+), 8 deletions(-) create mode 100644 clang/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json create mode 100644 clang/test/ClangScanDeps/Inputs/symlink_cdb.json create mode 100644 clang/test/ClangScanDeps/subframework_header_dir_symlink.m create mode 100644 clang/test/ClangScanDeps/symlink.cpp diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h index 0dde0ad35df20..c49f92d082c20 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h @@ -34,16 +34,19 @@ enum class ScanningMode { /// the invidual dependency scanning workers. class DependencyScanningService { public: - DependencyScanningService(ScanningMode Mode); + DependencyScanningService(ScanningMode Mode, bool ReuseFileManager = true); ScanningMode getMode() const { return Mode; } + bool canReuseFileManager() const { return ReuseFileManager; } + DependencyScanningFilesystemSharedCache &getSharedCache() { return SharedCache; } private: const ScanningMode Mode; + const bool ReuseFileManager; /// The global file system cache. DependencyScanningFilesystemSharedCache SharedCache; }; diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h index 79c652ef5a9c3..d56f5395da167 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h @@ -54,6 +54,9 @@ class DependencyScanningWorker { /// dependencies. This filesystem persists accross multiple compiler /// invocations. llvm::IntrusiveRefCntPtr DepFS; + /// The file manager that is reused accross multiple invocations by this + /// worker. If null, the file manager will not be reused. + llvm::IntrusiveRefCntPtr Files; }; } // end namespace dependencies diff --git a/clang/include/clang/Tooling/Tooling.h b/clang/include/clang/Tooling/Tooling.h index 5df816e671533..b8c7435c5e220 100644 --- a/clang/include/clang/Tooling/Tooling.h +++ b/clang/include/clang/Tooling/Tooling.h @@ -349,12 +349,15 @@ class ClangTool { /// clang modules. /// \param BaseFS VFS used for all underlying file accesses when running the /// tool. + /// \param Files The file manager to use for underlying file operations when + /// running the tool. ClangTool(const CompilationDatabase &Compilations, ArrayRef SourcePaths, std::shared_ptr PCHContainerOps = std::make_shared(), IntrusiveRefCntPtr BaseFS = - llvm::vfs::getRealFileSystem()); + llvm::vfs::getRealFileSystem(), + IntrusiveRefCntPtr Files = nullptr); ~ClangTool(); diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp index 48aa68218c3ea..6ddce0dcee85a 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp @@ -12,5 +12,6 @@ using namespace clang; using namespace tooling; using namespace dependencies; -DependencyScanningService::DependencyScanningService(ScanningMode Mode) - : Mode(Mode) {} +DependencyScanningService::DependencyScanningService(ScanningMode Mode, + bool ReuseFileManager) + : Mode(Mode), ReuseFileManager(ReuseFileManager) {} diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index c80a55645eb97..2d49e0d794af6 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -148,6 +148,8 @@ DependencyScanningWorker::DependencyScanningWorker( if (Service.getMode() == ScanningMode::MinimizedSourcePreprocessing) DepFS = new DependencyScanningWorkerFilesystem(Service.getSharedCache(), RealFS); + if (Service.canReuseFileManager()) + Files = new FileManager(FileSystemOptions(), RealFS); } llvm::Expected @@ -164,7 +166,7 @@ DependencyScanningWorker::getDependencyFile(const std::string &Input, /// Create the tool that uses the underlying file system to ensure that any /// file system requests that are made by the driver do not go through the /// dependency scanning filesystem. - tooling::ClangTool Tool(CDB, Input, PCHContainerOps, RealFS); + tooling::ClangTool Tool(CDB, Input, PCHContainerOps, RealFS, Files); Tool.clearArgumentsAdjusters(); Tool.setRestoreWorkingDir(false); Tool.setPrintErrorMessage(false); diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp index 8c0d13d243d18..472a3ab57c332 100644 --- a/clang/lib/Tooling/Tooling.cpp +++ b/clang/lib/Tooling/Tooling.cpp @@ -378,16 +378,20 @@ bool FrontendActionFactory::runInvocation( ClangTool::ClangTool(const CompilationDatabase &Compilations, ArrayRef SourcePaths, std::shared_ptr PCHContainerOps, - IntrusiveRefCntPtr BaseFS) + IntrusiveRefCntPtr BaseFS, + IntrusiveRefCntPtr Files) : Compilations(Compilations), SourcePaths(SourcePaths), PCHContainerOps(std::move(PCHContainerOps)), OverlayFileSystem(new llvm::vfs::OverlayFileSystem(std::move(BaseFS))), InMemoryFileSystem(new llvm::vfs::InMemoryFileSystem), - Files(new FileManager(FileSystemOptions(), OverlayFileSystem)) { + Files(Files ? Files + : new FileManager(FileSystemOptions(), OverlayFileSystem)) { OverlayFileSystem->pushOverlay(InMemoryFileSystem); appendArgumentsAdjuster(getClangStripOutputAdjuster()); appendArgumentsAdjuster(getClangSyntaxOnlyAdjuster()); appendArgumentsAdjuster(getClangStripDependencyFileAdjuster()); + if (Files) + Files->setVirtualFileSystem(OverlayFileSystem); } ClangTool::~ClangTool() = default; diff --git a/clang/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json b/clang/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json new file mode 100644 index 0000000000000..a405c6b3af137 --- /dev/null +++ b/clang/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json @@ -0,0 +1,12 @@ +[ +{ + "directory": "DIR", + "command": "clang -E DIR/subframework_header_dir_symlink.m -D EMPTY -iframework Inputs/frameworks", + "file": "DIR/subframework_header_dir_symlink.m" +}, +{ + "directory": "DIR", + "command": "clang -E DIR/subframework_header_dir_symlink2.m -FInputs/frameworks_symlink -iframework Inputs/frameworks", + "file": "DIR/subframework_header_dir_symlink2.m" +} +] diff --git a/clang/test/ClangScanDeps/Inputs/symlink_cdb.json b/clang/test/ClangScanDeps/Inputs/symlink_cdb.json new file mode 100644 index 0000000000000..43bb418adb19d --- /dev/null +++ b/clang/test/ClangScanDeps/Inputs/symlink_cdb.json @@ -0,0 +1,12 @@ +[ +{ + "directory": "DIR", + "command": "clang -E DIR/symlink.cpp -IInputs", + "file": "DIR/symlink.cpp" +}, +{ + "directory": "DIR", + "command": "clang -E DIR/symlink2.cpp -IInputs", + "file": "DIR/symlink2.cpp" +} +] diff --git a/clang/test/ClangScanDeps/subframework_header_dir_symlink.m b/clang/test/ClangScanDeps/subframework_header_dir_symlink.m new file mode 100644 index 0000000000000..5cc17851e7c08 --- /dev/null +++ b/clang/test/ClangScanDeps/subframework_header_dir_symlink.m @@ -0,0 +1,25 @@ +// REQUIRES: shell +// RUN: rm -rf %t.dir +// RUN: rm -rf %t.cdb +// RUN: mkdir -p %t.dir +// RUN: cp %s %t.dir/subframework_header_dir_symlink.m +// RUN: cp %s %t.dir/subframework_header_dir_symlink2.m +// RUN: mkdir %t.dir/Inputs +// RUN: cp -R %S/Inputs/frameworks %t.dir/Inputs/frameworks +// RUN: ln -s %t.dir/Inputs/frameworks %t.dir/Inputs/frameworks_symlink +// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/subframework_header_dir_symlink_cdb.json > %t.cdb +// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -reuse-filemanager=0 | \ +// RUN: FileCheck %s +// FIXME: Make this work when the filemanager is reused: +// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -reuse-filemanager=1 | \ +// RUN: not FileCheck %s + +#ifndef EMPTY +#include "Framework/Framework.h" +#endif + +// CHECK: clang-scan-deps dependency +// CHECK-NEXT: subframework_header_dir_symlink.m +// CHECK: clang-scan-deps dependency +// CHECK-NEXT: subframework_header_dir_symlink.m +// CHECK-NEXT: Inputs{{/|\\}}frameworks_symlink{{/|\\}}Framework.framework{{/|\\}}Headers{{/|\\}}Framework.h diff --git a/clang/test/ClangScanDeps/symlink.cpp b/clang/test/ClangScanDeps/symlink.cpp new file mode 100644 index 0000000000000..e1a6ac931aa80 --- /dev/null +++ b/clang/test/ClangScanDeps/symlink.cpp @@ -0,0 +1,23 @@ +// REQUIRES: shell +// RUN: rm -rf %t.dir +// RUN: rm -rf %t.cdb +// RUN: mkdir -p %t.dir +// RUN: cp %s %t.dir/symlink.cpp +// RUN: cp %s %t.dir/symlink2.cpp +// RUN: mkdir %t.dir/Inputs +// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header.h +// RUN: ln -s %t.dir/Inputs/header.h %t.dir/Inputs/symlink.h +// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/symlink_cdb.json > %t.cdb +// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -reuse-filemanager=0 | FileCheck %s +// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -reuse-filemanager=1 | FileCheck %s + +#include "symlink.h" +#include "header.h" + +// CHECK: symlink.cpp +// CHECK-NEXT: Inputs{{/|\\}}symlink.h +// CHECK-NEXT: Inputs{{/|\\}}header.h + +// CHECK: symlink2.cpp +// CHECK-NEXT: Inputs{{/|\\}}symlink.h +// CHECK-NEXT: Inputs{{/|\\}}header.h diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp index b838845c4484d..5e567fef9dae3 100644 --- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp +++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp @@ -107,6 +107,11 @@ llvm::cl::opt llvm::cl::desc("Compilation database"), llvm::cl::Required, llvm::cl::cat(DependencyScannerCategory)); +llvm::cl::opt ReuseFileManager( + "reuse-filemanager", + llvm::cl::desc("Reuse the file manager and its cache between invocations."), + llvm::cl::init(true), llvm::cl::cat(DependencyScannerCategory)); + } // end anonymous namespace int main(int argc, const char **argv) { @@ -153,7 +158,7 @@ int main(int argc, const char **argv) { // Print out the dependency results to STDOUT by default. SharedStream DependencyOS(llvm::outs()); - DependencyScanningService Service(ScanMode); + DependencyScanningService Service(ScanMode, ReuseFileManager); #if LLVM_ENABLE_THREADS unsigned NumWorkers = NumThreads == 0 ? llvm::hardware_concurrency() : NumThreads; From 864fd44d8f9ebf771d80cf49f59acb693a7978ea Mon Sep 17 00:00:00 2001 From: Petr Hosek Date: Thu, 29 Aug 2019 23:12:06 +0000 Subject: [PATCH 5/6] [CMake][Fuchsia] Enable experimental pass manager by default We plan on using experimental new pass manager for Fuchsia toolchain. Differential Revision: https://reviews.llvm.org/D58214 llvm-svn: 370421 --- clang/cmake/caches/Fuchsia-stage2.cmake | 1 + clang/cmake/caches/Fuchsia.cmake | 1 + 2 files changed, 2 insertions(+) diff --git a/clang/cmake/caches/Fuchsia-stage2.cmake b/clang/cmake/caches/Fuchsia-stage2.cmake index 1f8a9e78763fa..b7833c8ac036c 100644 --- a/clang/cmake/caches/Fuchsia-stage2.cmake +++ b/clang/cmake/caches/Fuchsia-stage2.cmake @@ -25,6 +25,7 @@ endif() set(CLANG_DEFAULT_RTLIB compiler-rt CACHE STRING "") set(CLANG_PLUGIN_SUPPORT OFF CACHE BOOL "") +set(ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER ON CACHE BOOL "") set(ENABLE_LINKER_BUILD_ID ON CACHE BOOL "") set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "") diff --git a/clang/cmake/caches/Fuchsia.cmake b/clang/cmake/caches/Fuchsia.cmake index 63bd62d1e93de..486d53b75f8e5 100644 --- a/clang/cmake/caches/Fuchsia.cmake +++ b/clang/cmake/caches/Fuchsia.cmake @@ -19,6 +19,7 @@ endif() set(CLANG_DEFAULT_RTLIB compiler-rt CACHE STRING "") set(CLANG_PLUGIN_SUPPORT OFF CACHE BOOL "") +set(ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER ON CACHE BOOL "") set(ENABLE_LINKER_BUILD_ID ON CACHE BOOL "") set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "") From 4625c18b5f4841c1f853d33cce0fa5b6abf74399 Mon Sep 17 00:00:00 2001 From: Bruno Cardoso Lopes Date: Thu, 29 Aug 2019 23:14:08 +0000 Subject: [PATCH 6/6] [Modules] Make ReadModuleMapFileBlock errors reliable This prevents a crash when an error should be emitted instead. During implicit module builds, there are cases where ReadASTCore is called with ImportedBy set to nullptr, which breaks expectations in ReadModuleMapFileBlock, leading to crashes. Fix this by improving ReadModuleMapFileBlock to handle ImportedBy correctly. This only happens non deterministically in the wild, when the underlying file system changes while concurrent compiler invocations use implicit modules, forcing rebuilds which see an inconsistent filesystem state. That said, there's no much to do w.r.t. writing tests here. rdar://problem/48828801 llvm-svn: 370422 --- .../clang/Basic/DiagnosticSerializationKinds.td | 4 ++-- clang/lib/Serialization/ASTReader.cpp | 16 +++++++++------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td index 43ba19b5853e2..0461d2f429a7b 100644 --- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td +++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td @@ -77,13 +77,13 @@ def remark_module_import : Remark< InGroup; def err_imported_module_not_found : Error< - "module '%0' in AST file '%1' (imported by AST file '%2') " + "module '%0' in AST file '%1' %select{(imported by AST file '%2') |}4" "is not defined in any loaded module map file; " "maybe you need to load '%3'?">, DefaultFatal; def note_imported_by_pch_module_not_found : Note< "consider adding '%0' to the header search path">; def err_imported_module_modmap_changed : Error< - "module '%0' imported by AST file '%1' found in a different module map file" + "module '%0' %select{in|imported by}4 AST file '%1' found in a different module map file" " (%2) than when the importing AST file was built (%3)">, DefaultFatal; def err_imported_module_relocated : Error< "module '%0' was built in directory '%1' but now resides in " diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 6a18af8a08223..aaa59fcf50621 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -3823,7 +3823,6 @@ ASTReader::ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F, const FileEntry *ModMap = M ? Map.getModuleMapFileForUniquing(M) : nullptr; // Don't emit module relocation error if we have -fno-validate-pch if (!PP.getPreprocessorOpts().DisablePCHValidation && !ModMap) { - assert(ImportedBy && "top-level import should be verified"); if ((ClientLoadCapabilities & ARR_OutOfDate) == 0) { if (auto *ASTFE = M ? M->getASTFile() : nullptr) { // This module was defined by an imported (explicit) module. @@ -3832,12 +3831,13 @@ ASTReader::ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F, } else { // This module was built with a different module map. Diag(diag::err_imported_module_not_found) - << F.ModuleName << F.FileName << ImportedBy->FileName - << F.ModuleMapPath; + << F.ModuleName << F.FileName + << (ImportedBy ? ImportedBy->FileName : "") << F.ModuleMapPath + << !ImportedBy; // In case it was imported by a PCH, there's a chance the user is // just missing to include the search path to the directory containing // the modulemap. - if (ImportedBy->Kind == MK_PCH) + if (ImportedBy && ImportedBy->Kind == MK_PCH) Diag(diag::note_imported_by_pch_module_not_found) << llvm::sys::path::parent_path(F.ModuleMapPath); } @@ -3851,11 +3851,13 @@ ASTReader::ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F, auto StoredModMap = FileMgr.getFile(F.ModuleMapPath); if (!StoredModMap || *StoredModMap != ModMap) { assert(ModMap && "found module is missing module map file"); - assert(ImportedBy && "top-level import should be verified"); + assert((ImportedBy || F.Kind == MK_ImplicitModule) && + "top-level import should be verified"); + bool NotImported = F.Kind == MK_ImplicitModule && !ImportedBy; if ((ClientLoadCapabilities & ARR_OutOfDate) == 0) Diag(diag::err_imported_module_modmap_changed) - << F.ModuleName << ImportedBy->FileName - << ModMap->getName() << F.ModuleMapPath; + << F.ModuleName << (NotImported ? F.FileName : ImportedBy->FileName) + << ModMap->getName() << F.ModuleMapPath << NotImported; return OutOfDate; }