hokein accepted this revision.
hokein added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:4475
// name that denotes that base class type.
- bool Dependent = BaseType->isDependentType() || Init->isTypeDependent();
+ bool Dependent = CurContext->isDependentContext() &&
+ (BaseType->isDependentType() || Init->isTypeDependent());
----------------
sammccall wrote:
> hokein wrote:
> > it is unclear to me why we need a `isDependentContext()` here? looks like
> > this would special-case RecoveryExpr, the following dependent code-path
> > would not be executed for RecoveryExpr case?
> Right, if the init expr is type-dependent because it contains errors, but
> we're not actually in a template, we don't want to take the dependent path.
>
> The reason is that the dependent path produces an "as-written" form of the
> CXXCtorInitializer, which has weaker invariants, because we know we're not
> going to examine it much until template instantiation time.
> e.g. an initializer that is actually delegating is stored as a base
> initializer instead. (Thus avoiding the impossible requirement to check
> whether two dependent types are the same).
> Later on in Sema::SetCtorInitializers, we check
> `Constructor->isDependentContext()` to decide whether to propagate the
> "as-written" initializers without checking, but of course this is false. So
> then we go and shuffle the initializers into the correct order, build default
> initializers for the uninitialized members etc, and the delegating
> initializer is just... lost in the shuffle. We think it's a base init, so
> it's put in a map of type->init, and then it never gets consumed from there
> because the type is not actually a base class.
>
> I'll try to add a comment that hints at this, without getting into the
> concrete details...
I see, fair enough, thanks for the classifications.
================
Comment at: clang/test/AST/ast-dump-recovery.cpp:305
+ S s;
+ MemberInit() : x(invalid), y(invalid, invalid), s(1,2) {}
+ // CHECK: CXXConstructorDecl {{.*}} MemberInit 'void ()'
----------------
For completeness, I would add case 'x(undefine())', even though it already
works without this patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101641/new/
https://reviews.llvm.org/D101641
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
