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.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113518/new/
https://reviews.llvm.org/D113518
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
