[PATCH] D101641: [Sema] Preserve invalid CXXCtorInitializers using RecoveryExpr in initializer

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Doh, I forgot this never landed :-(



================
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());
----------------
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...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101641/new/

https://reviews.llvm.org/D101641