- 
                Notifications
    You must be signed in to change notification settings 
- Fork 15k
[win][clang] Align scalar deleting destructors with MSABI #139566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 19 commits
1b0b624
              d945d0e
              2e07469
              2c51545
              9493c05
              ef899c2
              94f43ae
              83a8f06
              c9efdf8
              f5e7a45
              9d3c96c
              d71d3d4
              c670d74
              7147603
              057ac9f
              05c386a
              d07da86
              92bbda6
              83665e2
              c214b69
              bc55453
              7b238c9
              6150d04
              c0a863f
              d74fc8a
              a852a3e
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|  | @@ -8523,10 +8523,12 @@ class Sema final : public SemaBase { | |||
| bool Diagnose = true); | ||||
| FunctionDecl *FindUsualDeallocationFunction(SourceLocation StartLoc, | ||||
| ImplicitDeallocationParameters, | ||||
| DeclarationName Name); | ||||
| FunctionDecl *FindDeallocationFunctionForDestructor(SourceLocation StartLoc, | ||||
| CXXRecordDecl *RD, | ||||
| bool Diagnose = true); | ||||
| DeclarationName Name, | ||||
| bool Diagnose = true); | ||||
| FunctionDecl * | ||||
| FindDeallocationFunctionForDestructor(SourceLocation StartLoc, | ||||
| CXXRecordDecl *RD, bool Diagnose = true, | ||||
| bool LookForGlobal = false); | ||||
|          | ||||
| bool LookForGlobal); | 
Did you also mean to not add defaulted arguments to FindUsualDeallocationFunction? It has only one. Should I push a change to remove it too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind; I was looking at the wrong version of the patch somehow.
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -894,12 +894,19 @@ void MicrosoftCXXABI::emitVirtualObjectDelete(CodeGenFunction &CGF, | |
| const CXXDestructorDecl *Dtor) { | ||
| // FIXME: Provide a source location here even though there's no | ||
| // CXXMemberCallExpr for dtor call. | ||
|         
                  Fznamznon marked this conversation as resolved.
              Show resolved
            Hide resolved | ||
| bool UseGlobalDelete = DE->isGlobalDelete(); | ||
| CXXDtorType DtorType = UseGlobalDelete ? Dtor_Complete : Dtor_Deleting; | ||
| llvm::Value *MDThis = EmitVirtualDestructorCall(CGF, Dtor, DtorType, Ptr, DE, | ||
| /*CallOrInvoke=*/nullptr); | ||
| if (UseGlobalDelete) | ||
| CGF.EmitDeleteCall(DE->getOperatorDelete(), MDThis, ElementType); | ||
| if (!getContext().getTargetInfo().callGlobalDeleteInDeletingDtor( | ||
| getContext().getLangOpts())) { | ||
| bool UseGlobalDelete = DE->isGlobalDelete(); | ||
| CXXDtorType DtorType = UseGlobalDelete ? Dtor_Complete : Dtor_Deleting; | ||
| llvm::Value *MDThis = | ||
| EmitVirtualDestructorCall(CGF, Dtor, DtorType, Ptr, DE, | ||
| /*CallOrInvoke=*/nullptr); | ||
| if (UseGlobalDelete) | ||
| CGF.EmitDeleteCall(DE->getOperatorDelete(), MDThis, ElementType); | ||
| } else { | ||
| EmitVirtualDestructorCall(CGF, Dtor, Dtor_Deleting, Ptr, DE, | ||
| /*CallOrInvoke=*/nullptr); | ||
| } | ||
| } | ||
|  | ||
| void MicrosoftCXXABI::emitRethrow(CodeGenFunction &CGF, bool isNoReturn) { | ||
|  | @@ -2014,7 +2021,10 @@ llvm::Value *MicrosoftCXXABI::EmitVirtualDestructorCall( | |
| ASTContext &Context = getContext(); | ||
| llvm::Value *ImplicitParam = llvm::ConstantInt::get( | ||
| llvm::IntegerType::getInt32Ty(CGF.getLLVMContext()), | ||
| DtorType == Dtor_Deleting); | ||
| (DtorType == Dtor_Deleting) | | ||
| 4 * (D && D->isGlobalDelete() && | ||
|          | ||
| Context.getTargetInfo().callGlobalDeleteInDeletingDtor( | ||
| Context.getLangOpts()))); | ||
|  | ||
| QualType ThisTy; | ||
| if (CE) { | ||
|  | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The release notes should IMO mention
-fclang-abi-compat=20as a work-around for this divergence.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem, added, thanks.