[Review, -cxx-abi microsoft] Implement proper handling of virtual destructors (single-inheritance case)

Timur Iskhodzhanov timurrrr at google.com
Wed Feb 13 00:41:56 PST 2013


Done.

I've also fixed a couple of 80 char limit violations in my patch and
fixed a few compiler warnings I introduced in MicrosoftMangle.cpp
(interestingly, I haven't seen them on Win and Mac - only on Linux)
and committed r175045.

Thanks for the review!
Now I'll go bake the second half of the patch.

2013/2/13 John McCall <rjmccall at apple.com>:
> On Feb 12, 2013, at 5:28 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
>> I've renamed the -constructors test to -structors in r174966,
>> updating the patch accoridngly.
>
> +void MicrosoftCXXNameMangler::mangleCXXDtorType(CXXDtorType T) {
> +  switch (T) {
> +  case Dtor_Deleting:
> +    Out << "?_G";
> +    break;
> +  case Dtor_Base:
> +    // FIXME: We should be asked to mangle base dtors.
> +    // However, fixing this would require larger changes to the CodeGenModule.
> +    // Please put llvm_unreachable here when CGM is changed.
> +    // For now, just mangle a base dtor the same way as a complete dtor...
> +  case Dtor_Complete:
> +    Out << "?1";
> +    break;
> +  default:
> +    llvm_unreachable("Unsupported dtor type?");
> +  }
>
> Please have the concrete cases return, remove the default case, and
> put the unreachable after the switch.
>
> -  ImplicitParamDecl *&getVTTDecl(CodeGenFunction &CGF) {
> -    return CGF.CXXVTTDecl;
> +  ImplicitParamDecl *&getStructorImplicitParamDecl(CodeGenFunction &CGF) {
> +    return CGF.CXXStructorImplicitParamDecl;
>    }
> -  llvm::Value *&getVTTValue(CodeGenFunction &CGF) {
> -    return CGF.CXXVTTValue;
> +  llvm::Value *&getStructorImplicitParamValue(CodeGenFunction &CGF) {
> +    return CGF.CXXStructorImplicitParamValue;
>    }
>
> Adding the new functions makes sense, but for now, please leave the old
> functions in-place.  Every place that calls getVTT{Decl,Value} is something
> that needs to be abstracted properly.
>
> Otherwise, looks fine.
>
> John.



More information about the cfe-commits mailing list