OSchip
/
llvm-project
mirror of https://github.com/THU-DSP-LAB/llvm-project.git
13
0
Fork 2
Code Issues Packages Projects Releases Wiki Activity

[Sema] Preserve invalid CXXCtorInitializers using RecoveryExpr in initializer 

Before this patch, CXXCtorInitializers that don't typecheck get discarded in
most cases. In particular:

 - typos that can't be corrected don't turn into RecoveryExpr. The full expr
   disappears instead, and without an init expr we discard the node.
 - initializers that fail initialization (e.g. constructor overload resolution)
   are discarded too.

This patch addresses both these issues (a bit clunkily and repetitively, for
member/base/delegating initializers)

It does not preserve any AST nodes when the member/base can't be resolved or
other problems of that nature. That breaks invariants of CXXCtorInitializer
itself, and we don't have a "weak" RecoveryCtorInitializer like we do for Expr.

I believe the changes to diagnostics in existing tests are improvements.
(We're able to do some analysis on the non-broken parts of the initializer)

Differential Revision: https://reviews.llvm.org/D101641
This commit is contained in:
Sam McCall 2021-04-30 17:21:02 +02:00
parent bd63977ca9
commit 13a86c2bb4
4 changed files with 140 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

129
clang/lib/Sema/SemaDeclCXX.cpp
View File

@ -4162,7 +4162,8 @@ Sema::BuildMemInitializer(Decl *ConstructorD,
SourceLocation IdLoc,
Expr *Init,
SourceLocation EllipsisLoc) {
ExprResult Res = CorrectDelayedTyposInExpr(Init);
ExprResult Res = CorrectDelayedTyposInExpr(Init, /*InitDecl=*/nullptr,
/*RecoverUncorrectedTypos=*/true);
if (!Res.isUsable())
return true;
Init = Res.get();
@ -4375,18 +4376,25 @@ Sema::BuildMemberInitializer(ValueDecl *Member, Expr *Init,
InitializationSequence InitSeq(*this, MemberEntity, Kind, Args);
ExprResult MemberInit = InitSeq.Perform(*this, MemberEntity, Kind, Args,
nullptr);
if (MemberInit.isInvalid())
return true;
if (!MemberInit.isInvalid()) {
// C++11 [class.base.init]p7:
// The initialization of each base and member constitutes a
// full-expression.
MemberInit = ActOnFinishFullExpr(MemberInit.get(), InitRange.getBegin(),
/*DiscardedValue*/ false);
}
// C++11 [class.base.init]p7:
// The initialization of each base and member constitutes a
// full-expression.
MemberInit = ActOnFinishFullExpr(MemberInit.get(), InitRange.getBegin(),
/*DiscardedValue*/ false);
if (MemberInit.isInvalid())
return true;
Init = MemberInit.get();
if (MemberInit.isInvalid()) {
// Args were sensible expressions but we couldn't initialize the member
// from them. Preserve them in a RecoveryExpr instead.
Init = CreateRecoveryExpr(InitRange.getBegin(), InitRange.getEnd(), Args,
Member->getType())
.get();
if (!Init)
return true;
} else {
Init = MemberInit.get();
}
}
if (DirectMember) {
@ -4428,29 +4436,35 @@ Sema::BuildDelegatingInitializer(TypeSourceInfo *TInfo, Expr *Init,
InitializationSequence InitSeq(*this, DelegationEntity, Kind, Args);
ExprResult DelegationInit = InitSeq.Perform(*this, DelegationEntity, Kind,
Args, nullptr);
if (DelegationInit.isInvalid())
return true;
if (!DelegationInit.isInvalid()) {
assert(DelegationInit.get()->containsErrors() ||
cast<CXXConstructExpr>(DelegationInit.get())->getConstructor() &&
"Delegating constructor with no target?");
assert(cast<CXXConstructExpr>(DelegationInit.get())->getConstructor() &&
"Delegating constructor with no target?");
// C++11 [class.base.init]p7:
// The initialization of each base and member constitutes a
// full-expression.
DelegationInit = ActOnFinishFullExpr(
DelegationInit.get(), InitRange.getBegin(), /*DiscardedValue*/ false);
}
// C++11 [class.base.init]p7:
// The initialization of each base and member constitutes a
// full-expression.
DelegationInit = ActOnFinishFullExpr(
DelegationInit.get(), InitRange.getBegin(), /*DiscardedValue*/ false);
if (DelegationInit.isInvalid())
return true;
// If we are in a dependent context, template instantiation will
// perform this type-checking again. Just save the arguments that we
// received in a ParenListExpr.
// FIXME: This isn't quite ideal, since our ASTs don't capture all
// of the information that we have about the base
// initializer. However, deconstructing the ASTs is a dicey process,
// and this approach is far more likely to get the corner cases right.
if (CurContext->isDependentContext())
DelegationInit = Init;
if (DelegationInit.isInvalid()) {
DelegationInit =
CreateRecoveryExpr(InitRange.getBegin(), InitRange.getEnd(), Args,
QualType(ClassDecl->getTypeForDecl(), 0));
if (DelegationInit.isInvalid())
return true;
} else {
// If we are in a dependent context, template instantiation will
// perform this type-checking again. Just save the arguments that we
// received in a ParenListExpr.
// FIXME: This isn't quite ideal, since our ASTs don't capture all
// of the information that we have about the base
// initializer. However, deconstructing the ASTs is a dicey process,
// and this approach is far more likely to get the corner cases right.
if (CurContext->isDependentContext())
DelegationInit = Init;
}
return new (Context) CXXCtorInitializer(Context, TInfo, InitRange.getBegin(),
DelegationInit.getAs<Expr>(),
@ -4474,7 +4488,12 @@ Sema::BuildBaseInitializer(QualType BaseType, TypeSourceInfo *BaseTInfo,
// of that class, the mem-initializer is ill-formed. A
// mem-initializer-list can initialize a base class using any
// name that denotes that base class type.
bool Dependent = BaseType->isDependentType() || Init->isTypeDependent();
// We can store the initializers in "as-written" form and delay analysis until
// instantiation if the constructor is dependent. But not for dependent
// (broken) code in a non-template! SetCtorInitializers does not expect this.
bool Dependent = CurContext->isDependentContext() &&
(BaseType->isDependentType() || Init->isTypeDependent());
SourceRange InitRange = Init->getSourceRange();
if (EllipsisLoc.isValid()) {
@ -4561,26 +4580,30 @@ Sema::BuildBaseInitializer(QualType BaseType, TypeSourceInfo *BaseTInfo,
InitRange.getEnd());
InitializationSequence InitSeq(*this, BaseEntity, Kind, Args);
ExprResult BaseInit = InitSeq.Perform(*this, BaseEntity, Kind, Args, nullptr);
if (BaseInit.isInvalid())
return true;
if (!BaseInit.isInvalid()) {
// C++11 [class.base.init]p7:
// The initialization of each base and member constitutes a
// full-expression.
BaseInit = ActOnFinishFullExpr(BaseInit.get(), InitRange.getBegin(),
/*DiscardedValue*/ false);
}
// C++11 [class.base.init]p7:
// The initialization of each base and member constitutes a
// full-expression.
BaseInit = ActOnFinishFullExpr(BaseInit.get(), InitRange.getBegin(),
/*DiscardedValue*/ false);
if (BaseInit.isInvalid())
return true;
// If we are in a dependent context, template instantiation will
// perform this type-checking again. Just save the arguments that we
// received in a ParenListExpr.
// FIXME: This isn't quite ideal, since our ASTs don't capture all
// of the information that we have about the base
// initializer. However, deconstructing the ASTs is a dicey process,
// and this approach is far more likely to get the corner cases right.
if (CurContext->isDependentContext())
BaseInit = Init;
if (BaseInit.isInvalid()) {
BaseInit = CreateRecoveryExpr(InitRange.getBegin(), InitRange.getEnd(),
Args, BaseType);
if (BaseInit.isInvalid())
return true;
} else {
// If we are in a dependent context, template instantiation will
// perform this type-checking again. Just save the arguments that we
// received in a ParenListExpr.
// FIXME: This isn't quite ideal, since our ASTs don't capture all
// of the information that we have about the base
// initializer. However, deconstructing the ASTs is a dicey process,
// and this approach is far more likely to get the corner cases right.
if (CurContext->isDependentContext())
BaseInit = Init;
}
return new (Context) CXXCtorInitializer(Context, BaseTInfo,
BaseSpec->isVirtual(),

55
clang/test/AST/ast-dump-recovery.cpp
View File

@ -296,3 +296,58 @@ void InvalidCondition() {
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 2
invalid() ? 1 : 2;
}
void CtorInitializer() {
struct S{int m};
class MemberInit {
int x, y, z;
S s;
MemberInit() : x(invalid), y(invalid, invalid), z(invalid()), s(1,2) {}
// CHECK: CXXConstructorDecl {{.*}} MemberInit 'void ()'
// CHECK-NEXT: |-CXXCtorInitializer Field {{.*}} 'x' 'int'
// CHECK-NEXT: | `-ParenListExpr
// CHECK-NEXT: | `-RecoveryExpr {{.*}} '<dependent type>'
// CHECK-NEXT: |-CXXCtorInitializer Field {{.*}} 'y' 'int'
// CHECK-NEXT: | `-ParenListExpr
// CHECK-NEXT: | |-RecoveryExpr {{.*}} '<dependent type>'
// CHECK-NEXT: | `-RecoveryExpr {{.*}} '<dependent type>'
// CHECK-NEXT: |-CXXCtorInitializer Field {{.*}} 'z' 'int'
// CHECK-NEXT: | `-ParenListExpr
// CHECK-NEXT: | `-RecoveryExpr {{.*}} '<dependent type>'
// CHECK-NEXT: | `-UnresolvedLookupExpr {{.*}} '<overloaded function type>'
// CHECK-NEXT: |-CXXCtorInitializer Field {{.*}} 's' 'S'
// CHECK-NEXT: | `-RecoveryExpr {{.*}} 'S' contains-errors
// CHECK-NEXT: | |-IntegerLiteral {{.*}} 1
// CHECK-NEXT: | `-IntegerLiteral {{.*}} 2
};
class BaseInit : S {
BaseInit(float) : S("no match") {}
// CHECK: CXXConstructorDecl {{.*}} BaseInit 'void (float)'
// CHECK-NEXT: |-ParmVarDecl
// CHECK-NEXT: |-CXXCtorInitializer 'S'
// CHECK-NEXT: | `-RecoveryExpr {{.*}} 'S'
// CHECK-NEXT: | `-StringLiteral
BaseInit(double) : S(invalid) {}
// CHECK: CXXConstructorDecl {{.*}} BaseInit 'void (double)'
// CHECK-NEXT: |-ParmVarDecl
// CHECK-NEXT: |-CXXCtorInitializer 'S'
// CHECK-NEXT: | `-ParenListExpr
// CHECK-NEXT: | `-RecoveryExpr {{.*}} '<dependent type>'
};
class DelegatingInit {
DelegatingInit(float) : DelegatingInit("no match") {}
// CHECK: CXXConstructorDecl {{.*}} DelegatingInit 'void (float)'
// CHECK-NEXT: |-ParmVarDecl
// CHECK-NEXT: |-CXXCtorInitializer 'DelegatingInit'
// CHECK-NEXT: | `-RecoveryExpr {{.*}} 'DelegatingInit'
// CHECK-NEXT: | `-StringLiteral
DelegatingInit(double) : DelegatingInit(invalid) {}
// CHECK: CXXConstructorDecl {{.*}} DelegatingInit 'void (double)'
// CHECK-NEXT: |-ParmVarDecl
// CHECK-NEXT: |-CXXCtorInitializer 'DelegatingInit'
// CHECK-NEXT: | `-ParenListExpr
// CHECK-NEXT: | `-RecoveryExpr {{.*}} '<dependent type>'
};
}

4
clang/test/CXX/drs/dr6xx.cpp
View File

@ -606,11 +606,13 @@ namespace dr654 { // dr654: sup 1423
namespace dr655 { // dr655: yes
struct A { A(int); }; // expected-note 2-3{{not viable}}
// expected-note@-1 {{'dr655::A' declared here}}
struct B : A {
A a;
A a; // expected-note {{member is declared here}}
B();
B(int) : B() {} // expected-error 0-1 {{C++11}}
B(int*) : A() {} // expected-error {{no matching constructor}}
// expected-error@-1 {{must explicitly initialize the member 'a'}}
};
}

8
clang/test/CXX/temp/temp.decls/temp.variadic/p4.cpp
View File

@ -102,7 +102,9 @@ struct HasMixins : public Mixins... {
struct A { }; // expected-note{{candidate constructor (the implicit copy constructor) not viable: no known conversion from 'int' to 'const A' for 1st argument}} \
// expected-note{{candidate constructor (the implicit move constructor) not viable: no known conversion from 'int' to 'A' for 1st argument}} \
// expected-note{{candidate constructor (the implicit default constructor) not viable: requires 0 arguments, but 1 was provided}}
struct B { };
struct B { }; // expected-note{{candidate constructor (the implicit copy constructor) not viable: no known conversion from 'int' to 'const B' for 1st argument}} \
// expected-note{{candidate constructor (the implicit move constructor) not viable: no known conversion from 'int' to 'B' for 1st argument}} \
// expected-note{{candidate constructor (the implicit default constructor) not viable: requires 0 arguments, but 1 was provided}}
struct C { };
struct D { };
@ -123,7 +125,9 @@ template<typename ...Mixins>
HasMixins<Mixins...>::HasMixins(const HasMixins &other): Mixins(other)... { }
template<typename ...Mixins>
HasMixins<Mixins...>::HasMixins(int i): Mixins(i)... { } // expected-error{{no matching constructor for initialization of 'A'}}
HasMixins<Mixins...>::HasMixins(int i): Mixins(i)... { }
// expected-error@-1 {{no matching constructor for initialization of 'A'}}
// expected-error@-2 {{no matching constructor for initialization of 'B'}}
void test_has_mixins() {
HasMixins<A, B> ab;