rnk marked 2 inline comments as done.
rnk added a comment.
Thanks for the feedback, I'm going to investigate if we can use the `used`
destructor bit to do this.
================
Comment at: clang/include/clang/AST/DeclCXX.h:959-963
+ /// Indicates if the complete destructor has been implicitly declared
+ /// yet. Only relevant in the Microsoft C++.
+ bool definedImplicitCompleteDestructor() const {
+ return data().DefinedImplicitCompleteDestructor;
+ }
----------------
rsmith wrote:
> "declared" in comment but "defined" in function name. Which is it?
>
> I wonder if perhaps the right answer is neither: if we had separate `Decl`s
> for the complete and base destructors, this would be tracked by the "used"
> bit on the complete object destructor. So perhaps `isCompleteDestructorUsed`
> and `setCompleteDestructorUsed` would make sense? (We could consider tracking
> this on the destructor instead of here, but I suppose tracking it here gives
> us the serialization and merging logic for free.)
Actually, can we just reuse the `used` bit on the destructor? I don't think
there is any way for the user to "use" the base destructor without using the
complete destructor. The only case I can think of that calls the base
destructor (ignoring aliasing optimizations) is the complete destructor of a
derived class. Such a class will also reference all the virtual bases of the
current class, so the semantic checks we are doing here are redundant, not
wrong.
Calling a pseudo-destructor for example uses the complete destructor, I think.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:16008-16013
+ // In the MS ABI, the complete destructor is implicitly defined,
+ // even if the base destructor is user defined.
+ CXXRecordDecl *Parent = Destructor->getParent();
+ if (Parent->getNumVBases() > 0 &&
+ !Parent->definedImplicitCompleteDestructor())
+ DefineImplicitCompleteDestructor(Loc, Destructor);
----------------
rsmith wrote:
> Can we avoid doing this when we know the call is to a base subobject
> destructor or uses virtual dispatch? Should we? (I mean I guess ideally we'd
> change this function to take a `GlobalDecl` instead of a `FunctionDecl*`, but
> that seems like a lot of work.)
>
> How does MSVC behave?
I think we can skip this if virtual dispatch is used, but I'm not sure what
kinds of devirtualization might break my assumptions.
For example, uncommenting `final` here causes MSVC to diagnose the error, but
it otherwise it compiles:
```
struct NoDtor { ~NoDtor() = delete; };
struct DefaultedDtor : NoDtor { ~DefaultedDtor() = default; };
struct HasVBases /*final*/ : virtual DefaultedDtor {
virtual ~HasVBases();
};
void deleteIt1(HasVBases *p) { delete p; }
```
If the NoDtor destructor is undeleted and the class is made final, then both
deleting and complete dtors are emitted for HasVBases. Clang would need to mark
DefaultedDtor referenced in this case.
I think it's OK to "overdiagnose" in this case. It's not possible to emit a
vtable here without diagnosing the error.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77081/new/
https://reviews.llvm.org/D77081
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
