[PATCH] D113518: [clang][Sema] Create delegating constructors even in templates

Fabian Wolff via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Dec 4 09:52:38 PST 2021

fwolff marked 2 inline comments as done.
fwolff added inline comments.

Comment at: clang/lib/Sema/SemaDeclCXX.cpp:4524-4525
   if (!Dependent) {
-    if (Context.hasSameUnqualifiedType(QualType(ClassDecl->getTypeForDecl(),0),
-                                       BaseType))
+    if (Delegating)
       return BuildDelegatingInitializer(BaseTInfo, Init, ClassDecl);
aaron.ballman wrote:
> Looking at the source for `BuildDelegatingInitializer()`, it looks like we should still be able to call this in a dependent context. In fact, it has a fixme comment specifically about that:
> ```
>     // 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.
> ```
> I'm wondering if the better fix here is to hoist the delegation check out of the `if (!Dependent)`. Did you try that and run into issues?
Yes, I have tried this, and it leads to a crash [[ https://github.com/llvm/llvm-project/blob/ca2f53897a2f2a60d8cb1538d5fcf930d814e9f5/clang/lib/Sema/SemaDeclCXX.cpp#L4444 | here ]] (because the cast fails). Apparently, this code path in `BuildDelegatingInitializer()` is never exercised for dependent contexts, despite the comment, and therefore doesn't work.

Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5071
-  if (CXXDestructorDecl *Dtor = LookupDestructor(Constructor->getParent())) {
-    MarkFunctionReferenced(Initializer->getSourceLocation(), Dtor);
-    DiagnoseUseOfDecl(Dtor, Initializer->getSourceLocation());
-  }
+  if (!Constructor->isDependentContext()) {
+    if (CXXDestructorDecl *Dtor = LookupDestructor(Constructor->getParent())) {
aaron.ballman wrote:
> Can you explain why this change was needed?
Without this, clang crashes because `LookupDestructor()` calls `LookupSpecialMember()`, which then runs into [[ https://github.com/llvm/llvm-project/blob/ca2f53897a2f2a60d8cb1538d5fcf930d814e9f5/clang/lib/Sema/SemaLookup.cpp#L3064-L3065 | this assertion ]]:
clang-14: /[...]/llvm-project/clang/lib/Sema/SemaLookup.cpp:3064: clang::Sema::SpecialMemberOverloadResult clang::Sema::LookupSpecialMember(clang::CXXRecordDecl*, clang::Sema::CXXSpecialMember, bool, bool, bool, bool, bool): Assertion `CanDeclareSpecialMemberFunction(RD) && "doing special member lookup into record that isn't fully complete"' failed.
Previously, this wasn't necessary because no delegating constructors would be generated for templates, and so this function wasn't ever called in a dependent context.



More information about the cfe-commits mailing list