From: John McCall Date: Sat, 10 Apr 2010 07:37:23 +0000 (+0000) Subject: Diagnose misordered initializers in constructor templates immediately instead of X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d6ca8da0f5a4115813055729faaa5128e994806d;p=clang Diagnose misordered initializers in constructor templates immediately instead of when they're instantiated. Merge the note into the -Wreorder warning; it doesn't really contribute much, and it was splitting a thought across diagnostics anyway. Don't crash in the parser when a constructor's initializers end in a comma and there's no body; the recovery here is still terrible, but anything's better than a crash. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@100922 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index c21726ba7b..859121d6d0 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -2471,14 +2471,10 @@ def err_mem_init_not_member_or_class : Error< "member initializer %0 does not name a non-static data member or base " "class">; -def warn_field_initialized : Warning< - "member '%0' will be initialized after">, +def warn_initializer_out_of_order : Warning< + "%select{field|base class}0 %1 will be initialized after " + "%select{field|base}2 %3">, InGroup, DefaultIgnore; -def warn_base_initialized : Warning< - "base class %0 will be initialized after">, - InGroup, DefaultIgnore; -def note_fieldorbase_initialized_here : Note< - "%select{field|base}0 %1">; def err_base_init_does_not_name_class : Error< "constructor initializer %0 does not name a class">; diff --git a/lib/Parse/ParseCXXInlineMethods.cpp b/lib/Parse/ParseCXXInlineMethods.cpp index 87e22fa9dc..5b5452402b 100644 --- a/lib/Parse/ParseCXXInlineMethods.cpp +++ b/lib/Parse/ParseCXXInlineMethods.cpp @@ -217,12 +217,17 @@ void Parser::ParseLexedMethodDefs(ParsingClass &Class) { "ParseFunctionTryBlock left tokens in the token stream!"); continue; } - if (Tok.is(tok::colon)) + if (Tok.is(tok::colon)) { ParseConstructorInitializer(LM.D); - else + + // Error recovery. + if (!Tok.is(tok::l_brace)) { + Actions.ActOnFinishFunctionBody(LM.D, Action::StmtArg(Actions)); + continue; + } + } else Actions.ActOnDefaultCtorInitializers(LM.D); - // FIXME: What if ParseConstructorInitializer doesn't leave us with a '{'?? ParseFunctionStatementBody(LM.D); assert(!PP.getSourceManager().isBeforeInTranslationUnit(origLoc, Tok.getLocation()) && diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp index 5c31c1d9a3..6dbb99e395 100644 --- a/lib/Parse/Parser.cpp +++ b/lib/Parse/Parser.cpp @@ -668,9 +668,15 @@ Parser::DeclPtrTy Parser::ParseFunctionDefinition(ParsingDeclarator &D, // If we have a colon, then we're probably parsing a C++ // ctor-initializer. - if (Tok.is(tok::colon)) + if (Tok.is(tok::colon)) { ParseConstructorInitializer(Res); - else + + // Recover from error. + if (!Tok.is(tok::l_brace)) { + Actions.ActOnFinishFunctionBody(Res, Action::StmtArg(Actions)); + return Res; + } + } else Actions.ActOnDefaultCtorInitializers(Res); return ParseFunctionStatementBody(Res); diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 3eb8713b77..bc8b7d6bc6 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -1433,7 +1433,7 @@ Sema::SetBaseOrMemberInitializers(CXXConstructorDecl *Constructor, CXXBaseOrMemberInitializer **Initializers, unsigned NumInitializers, bool AnyErrors) { - if (Constructor->isDependentContext()) { + if (Constructor->getDeclContext()->isDependentContext()) { // Just store the initializers as written, they will be checked during // instantiation. if (NumInitializers > 0) { @@ -1672,86 +1672,85 @@ static void *GetKeyForMember(ASTContext &Context, static void DiagnoseBaseOrMemInitializerOrder(Sema &SemaRef, const CXXConstructorDecl *Constructor, - CXXBaseOrMemberInitializer **MemInits, - unsigned NumMemInits) { - if (Constructor->isDependentContext()) + CXXBaseOrMemberInitializer **Inits, + unsigned NumInits) { + if (Constructor->getDeclContext()->isDependentContext()) return; - if (SemaRef.Diags.getDiagnosticLevel(diag::warn_base_initialized) == - Diagnostic::Ignored && - SemaRef.Diags.getDiagnosticLevel(diag::warn_field_initialized) == - Diagnostic::Ignored) + if (SemaRef.Diags.getDiagnosticLevel(diag::warn_initializer_out_of_order) + == Diagnostic::Ignored) return; - // Also issue warning if order of ctor-initializer list does not match order - // of 1) base class declarations and 2) order of non-static data members. - llvm::SmallVector AllBaseOrMembers; + // Build the list of bases and members in the order that they'll + // actually be initialized. The explicit initializers should be in + // this same order but may be missing things. + llvm::SmallVector IdealInitKeys; const CXXRecordDecl *ClassDecl = Constructor->getParent(); - // Push virtual bases before others. + // 1. Virtual bases. for (CXXRecordDecl::base_class_const_iterator VBase = ClassDecl->vbases_begin(), E = ClassDecl->vbases_end(); VBase != E; ++VBase) - AllBaseOrMembers.push_back(GetKeyForBase(SemaRef.Context, - VBase->getType())); + IdealInitKeys.push_back(GetKeyForBase(SemaRef.Context, VBase->getType())); + // 2. Non-virtual bases. for (CXXRecordDecl::base_class_const_iterator Base = ClassDecl->bases_begin(), E = ClassDecl->bases_end(); Base != E; ++Base) { - // Virtuals are alread in the virtual base list and are constructed - // first. if (Base->isVirtual()) continue; - AllBaseOrMembers.push_back(GetKeyForBase(SemaRef.Context, - Base->getType())); + IdealInitKeys.push_back(GetKeyForBase(SemaRef.Context, Base->getType())); } + // 3. Direct fields. for (CXXRecordDecl::field_iterator Field = ClassDecl->field_begin(), E = ClassDecl->field_end(); Field != E; ++Field) - AllBaseOrMembers.push_back(GetKeyForTopLevelField(*Field)); + IdealInitKeys.push_back(GetKeyForTopLevelField(*Field)); - int Last = AllBaseOrMembers.size(); - int curIndex = 0; - CXXBaseOrMemberInitializer *PrevMember = 0; - for (unsigned i = 0; i < NumMemInits; i++) { - CXXBaseOrMemberInitializer *Member = MemInits[i]; - void *MemberInCtorList = GetKeyForMember(SemaRef.Context, Member, true); + unsigned NumIdealInits = IdealInitKeys.size(); + unsigned IdealIndex = 0; - for (; curIndex < Last; curIndex++) - if (MemberInCtorList == AllBaseOrMembers[curIndex]) + CXXBaseOrMemberInitializer *PrevInit = 0; + for (unsigned InitIndex = 0; InitIndex != NumInits; ++InitIndex) { + CXXBaseOrMemberInitializer *Init = Inits[InitIndex]; + void *InitKey = GetKeyForMember(SemaRef.Context, Init, true); + + // Scan forward to try to find this initializer in the idealized + // initializers list. + for (; IdealIndex != NumIdealInits; ++IdealIndex) + if (InitKey == IdealInitKeys[IdealIndex]) break; - if (curIndex == Last) { - assert(PrevMember && "Member not in member list?!"); - // Initializer as specified in ctor-initializer list is out of order. - // Issue a warning diagnostic. - if (PrevMember->isBaseInitializer()) { - // Diagnostics is for an initialized base class. - Type *BaseClass = PrevMember->getBaseClass(); - SemaRef.Diag(PrevMember->getSourceLocation(), - diag::warn_base_initialized) - << QualType(BaseClass, 0); - } else { - FieldDecl *Field = PrevMember->getMember(); - SemaRef.Diag(PrevMember->getSourceLocation(), - diag::warn_field_initialized) - << Field->getNameAsString(); - } - // Also the note! - if (FieldDecl *Field = Member->getMember()) - SemaRef.Diag(Member->getSourceLocation(), - diag::note_fieldorbase_initialized_here) << 0 - << Field->getNameAsString(); - else { - Type *BaseClass = Member->getBaseClass(); - SemaRef.Diag(Member->getSourceLocation(), - diag::note_fieldorbase_initialized_here) << 1 - << QualType(BaseClass, 0); - } - for (curIndex = 0; curIndex < Last; curIndex++) - if (MemberInCtorList == AllBaseOrMembers[curIndex]) + + // If we didn't find this initializer, it must be because we + // scanned past it on a previous iteration. That can only + // happen if we're out of order; emit a warning. + if (IdealIndex == NumIdealInits) { + assert(PrevInit && "initializer not found in initializer list"); + + Sema::SemaDiagnosticBuilder D = + SemaRef.Diag(PrevInit->getSourceLocation(), + diag::warn_initializer_out_of_order); + + if (PrevInit->isMemberInitializer()) + D << 0 << PrevInit->getMember()->getDeclName(); + else + D << 1 << PrevInit->getBaseClassInfo()->getType(); + + if (Init->isMemberInitializer()) + D << 0 << Init->getMember()->getDeclName(); + else + D << 1 << Init->getBaseClassInfo()->getType(); + + // Move back to the initializer's location in the ideal list. + for (IdealIndex = 0; IdealIndex != NumIdealInits; ++IdealIndex) + if (InitKey == IdealInitKeys[IdealIndex]) break; + + assert(IdealIndex != NumIdealInits && + "initializer not found in initializer list"); } - PrevMember = Member; + + PrevInit = Init; } } diff --git a/test/SemaCXX/constructor-initializer.cpp b/test/SemaCXX/constructor-initializer.cpp index 8b23e13009..a22c417032 100644 --- a/test/SemaCXX/constructor-initializer.cpp +++ b/test/SemaCXX/constructor-initializer.cpp @@ -87,12 +87,11 @@ struct Derived : Base, Base1, virtual V { struct Current : Derived { int Derived; - Current() : Derived(1), ::Derived(), // expected-warning {{member 'Derived' will be initialized after}} \ - // expected-note {{base '::Derived'}} \ - // expected-warning {{base class '::Derived' will be initialized after}} + Current() : Derived(1), ::Derived(), // expected-warning {{field 'Derived' will be initialized after base '::Derived'}} \ + // expected-warning {{base class '::Derived' will be initialized after base 'Derived::V'}} ::Derived::Base(), // expected-error {{type '::Derived::Base' is not a direct or virtual base of 'Current'}} Derived::Base1(), // expected-error {{type 'Derived::Base1' is not a direct or virtual base of 'Current'}} - Derived::V(), // expected-note {{base 'Derived::V'}} + Derived::V(), ::NonExisting(), // expected-error {{member initializer 'NonExisting' does not name a non-static data member or}} INT::NonExisting() {} // expected-error {{expected a class or namespace}} \ // expected-error {{member initializer 'NonExisting' does not name a non-static data member or}} diff --git a/test/SemaCXX/warn-reorder-ctor-initialization.cpp b/test/SemaCXX/warn-reorder-ctor-initialization.cpp index f4191565df..15e0867b26 100644 --- a/test/SemaCXX/warn-reorder-ctor-initialization.cpp +++ b/test/SemaCXX/warn-reorder-ctor-initialization.cpp @@ -6,12 +6,13 @@ struct BB1 {}; class complex : public BB, BB1 { public: - complex() : s2(1), // expected-warning {{member 's2' will be initialized after}} - s1(1) , // expected-note {{field s1}} - s3(3), // expected-warning {{member 's3' will be initialized after}} - BB1(), // expected-note {{base 'BB1'}} \ - // expected-warning {{base class 'BB1' will be initialized after}} - BB() {} // expected-note {{base 'BB'}} + complex() + : s2(1), // expected-warning {{field 's2' will be initialized after field 's1'}} + s1(1), + s3(3), // expected-warning {{field 's3' will be initialized after base 'BB1'}} + BB1(), // expected-warning {{base class 'BB1' will be initialized after base 'BB'}} + BB() + {} int s1; int s2; int s3; @@ -44,14 +45,12 @@ struct C : public A, public B, private virtual V { struct D : public A, public B { - D() : A(), V() { } // expected-warning {{base class 'A' will be initialized after}} \ - // expected-note {{base 'V'}} + D() : A(), V() { } // expected-warning {{base class 'A' will be initialized after base 'V'}} }; struct E : public A, public B, private virtual V { - E() : A(), V() { } // expected-warning {{base class 'A' will be initialized after}} \ - // expected-note {{base 'V'}} + E() : A(), V() { } // expected-warning {{base class 'A' will be initialized after base 'V'}} }; @@ -64,13 +63,11 @@ struct B1 { }; struct F : public A1, public B1, private virtual V { - F() : A1(), V() { } // expected-warning {{base class 'A1' will be initialized after}} \ - // expected-note {{base 'V'}} + F() : A1(), V() { } // expected-warning {{base class 'A1' will be initialized after base 'V'}} }; struct X : public virtual A, virtual V, public virtual B { - X(): A(), V(), B() {} // expected-warning {{base class 'A' will be initialized after}} \ - // expected-note {{base 'V'}} + X(): A(), V(), B() {} // expected-warning {{base class 'A' will be initialized after base 'V'}} }; class Anon { @@ -80,8 +77,8 @@ class Anon { class Anon2 { int c; union {int a,b;}; int d; Anon2() : c(2), - d(10), // expected-warning {{member 'd' will be initialized after}} - b(1) {} // expected-note {{field b}} + d(10), // expected-warning {{field 'd' will be initialized after field 'b'}} + b(1) {} }; class Anon3 { union {int a,b;}; @@ -95,7 +92,19 @@ struct S2: virtual S1 { }; struct S3 { }; struct S4: virtual S3, S2 { - S4() : S2(), // expected-warning {{base class 'T1::S2' will be initialized after}} - S3() { }; // expected-note {{base 'T1::S3'}} + S4() : S2(), // expected-warning {{base class 'T1::S2' will be initialized after base 'T1::S3'}} + S3() { }; }; } + +namespace test2 { + struct Foo { Foo(); }; + class A { + template A(T *t) : + y(), // expected-warning {{field 'y' will be initialized after field 'x'}} + x() + {} + Foo x; + Foo y; + }; +} diff --git a/test/SemaTemplate/instantiate-member-initializers.cpp b/test/SemaTemplate/instantiate-member-initializers.cpp index ca94bef226..45503b38b3 100644 --- a/test/SemaTemplate/instantiate-member-initializers.cpp +++ b/test/SemaTemplate/instantiate-member-initializers.cpp @@ -10,8 +10,8 @@ A a0; A a1; // expected-note{{in instantiation of member function 'A::A' requested here}} template struct B { - B() : b(1), // expected-warning {{member 'b' will be initialized after}} - a(2) { } // expected-note {{field a}} + B() : b(1), // expected-warning {{field 'b' will be initialized after field 'a'}} + a(2) { } int a; int b;