Page MenuHomePhabricator

[clang] Add cc1 option -fctor-dtor-return-this
ClosedPublic

Authored by scw on Feb 7 2022, 6:39 PM.

Details

Summary

This cc1-only option forces constructors and non-deleting destructors to
return return this pointer in C++ ABI (except for Microsoft ABI, on
which this flag has no effect).

This is similar to ARM32, Apple ARM64, or Fuchsia C++ ABI, but can be
applied to any target triple.

Diff Detail

Event Timeline

scw created this revision.Feb 7 2022, 6:39 PM
scw requested review of this revision.Feb 7 2022, 6:39 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 7 2022, 6:39 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
scw updated this revision to Diff 463722.Sep 28 2022, 5:15 PM
scw edited the summary of this revision. (Show Details)

Rebased and updated comments and option explain text to only affect Itanium C++ ABI.

Herald added a project: Restricted Project. · View Herald TranscriptSep 28 2022, 5:15 PM
Herald added a subscriber: MaskRay. · View Herald Transcript
scw added a reviewer: MaskRay.Sep 28 2022, 5:17 PM
MaskRay added inline comments.Sep 28 2022, 9:45 PM
clang/include/clang/Driver/Options.td
3475

One idea is to make this a CC1 only option (NoDriverOption) so that it is clear this is internal only and not recommended for general use.

For some ABI changing options we penalize the option name, e.g. -fexperimental-relative-c++-abi-vtables

clang/lib/CodeGen/CGCXXABI.h
131

delete braces

scw updated this revision to Diff 464793.Oct 3 2022, 1:43 PM
scw retitled this revision from Implement -mctor_dtor_return_this ABI option. to Implement -fctor_dtor_return_this ABI option..
scw edited the summary of this revision. (Show Details)
  • Made the option cc1-only (invoke through -Xclang).
  • Changed the name to -f... from -m...
  • Remove mentioning of Itanium ABI--it applies to all C++ ABIs but is overridden by the Microsoft one.

Implement -fctor_dtor_return_this ABI option.

Typo in the subject. I'd use: Add cc1 option -fctor-dtor-return-this

clang/include/clang/Driver/Options.td
5608

If you use Itanium C++ ABI, it's clear it doesn't change Microsoft ABI.

The convention for HelpText is to omit the trailing period.

clang/lib/Driver/ToolChains/Clang.cpp
6312 ↗(On Diff #464793)

Clang.cpp handles driver options. Since the option is no longer a driver option, this should be dropped.

scw updated this revision to Diff 464801.Oct 3 2022, 1:48 PM

Remove unnecessary curly braces.

scw updated this revision to Diff 464809.Oct 3 2022, 2:03 PM
scw marked 2 inline comments as done.
scw retitled this revision from Implement -fctor_dtor_return_this ABI option. to [clang] Add cc1 option -fctor-dtor-return-this.
scw marked an inline comment as done.Oct 3 2022, 2:06 PM
scw added inline comments.
clang/include/clang/Driver/Options.td
5608

I was gonna do that but realized the change actually apply to all C++ ABIs (I changed CGCXXABI). It's just Microsoft overrides the same method I changed making it a no-op.

MaskRay added inline comments.Oct 3 2022, 2:06 PM
clang/include/clang/Basic/CodeGenOptions.def
495

Modify Itanium C++ ABI and delete (No effect on Microsoft ABI.)

clang/include/clang/Driver/Options.td
5608

Not done?

MaskRay accepted this revision.Oct 3 2022, 2:07 PM
MaskRay added inline comments.
clang/include/clang/Driver/Options.td
5608

Thanks. This is fine.

This revision is now accepted and ready to land.Oct 3 2022, 2:07 PM
This revision was landed with ongoing or failed builds.Oct 3 2022, 2:31 PM
This revision was automatically updated to reflect the committed changes.

I was rather hoping we could have a more general "floating ABI" flag that we can lump in anything that we'd like to have otherwise but can't break ABI for - so users like Fuschia that know they're compiling everything with the same version of Clang can opt into the floating ABI & get all the benefits whenever they come up, like we would with any other non-ABI impacting optimization.