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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to