[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-27 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D119051#3888667 , @rnk wrote: > I realized I forgot some things I should've mentioned: > > - This probably deserves a release note, if it doesn't have one already that > I missed or forgot about e4ec6ce8a75c208b49b163c81cda9

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-27 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D119051#3888906 , @erichkeane wrote: > I noticed in a downstream that this changes how we emit structs to IR > sometimes (see https://godbolt.org/z/74xeq9rTj for an example, but the > 'no-opaque-pointers' isn't necessary to caus

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a reviewer: clang-vendors. aaron.ballman added a comment. Adding clang-vendors because of the potential for disruption. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-27 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. I noticed in a downstream that this changes how we emit structs to IR sometimes (see https://godbolt.org/z/74xeq9rTj for an example, but the 'no-opaque-pointers' isn't necessary to cause this, just anything that causes emission of the struct). Are we OK with that?

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-27 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I realized I forgot some things I should've mentioned: - This probably deserves a release note, if it doesn't have one already that I missed or forgot about - We should tag #clang-vendors , since this does change ABI. You've alre

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-26 Thread David Blaikie via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7846d590033e: Extend the C++03 definition of POD to include defaulted functions (authored by dblaikie). Changed prior to commit: https://reviews.l

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-25 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-c

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-24 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Ping (hoping to ensure enough progress is made on this that we can get it in before the LLVM 16 branch - since it's complicated the last couple of releases already due to not having all the fallout of the ABI fixes in together) Repository: rG LLVM Github Monorepo C

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-19 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-13 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Hmm - pinging this since my last comment didn't seem to send mail (at least not that appeared on llvm-commits archive) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 __

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-12 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments. Comment at: clang/include/clang/Basic/TargetInfo.h:1569 + virtual bool areDefaultedSMFStillPOD(const LangOptions&) const; + rnk wrote: > Please add a doc comment for this, with some history about how previously > explicitly def

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-11 Thread David Blaikie via Phabricator via cfe-commits
dblaikie updated this revision to Diff 467024. dblaikie added a comment. Add doc comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 Files: clang/include/clang/Basic/LangOptions.h clang/include/cl

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. +#clang-vendors for ABI changes Comment at: clang/include/clang/Basic/TargetInfo.h:1569 + virtual bool areDefaultedSMFStillPOD(const LangOptions&) const; + Please add a doc comment for this,

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-06 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments. Comment at: clang/lib/AST/DeclCXX.cpp:774-775 +if ((!Constructor->isDeleted() && !Constructor->isDefaulted()) || +(getLangOpts().getClangABICompat() <= + LangOptions::ClangABI::Ver15 || Target.isPS() || Target.isOSD

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-06 Thread David Blaikie via Phabricator via cfe-commits
dblaikie updated this revision to Diff 465956. dblaikie added a comment. Move condition to TargetInfo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 Files: clang/include/clang/Basic/LangOptions.h cla

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-05 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: clang/lib/AST/DeclCXX.cpp:774-775 +if ((!Constructor->isDeleted() && !Constructor->isDefaulted()) || +(getLangOpts().getClangABICompat() <= + LangOptions::ClangABI::Ver15 || Target.isPS() || Target.isOSDarwin

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-05 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D119051#3834985 , @rnk wrote: > Relatedly, if we put this POD behavior query on TargetInfo, I wonder if it > makes sense to move the tail padding predicate from TargetCXXABI to > TargetInfo: > https://github.com/llvm/llvm-pr

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-04 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Relatedly, if we put this POD behavior query on TargetInfo, I wonder if it makes sense to move the tail padding predicate from TargetCXXABI to TargetInfo: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/TargetCXXABI.h#L282 The switch there is basical

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-04 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D119051#3747673 , @rnk wrote: > In D119051#3747201 , @dblaikie > wrote: > >> So... my conclusion is that Clang's AArch64 appears to be correct for x86 >> as well, and we should just

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-04 Thread David Blaikie via Phabricator via cfe-commits
dblaikie updated this revision to Diff 465159. dblaikie added a comment. Remove Microsoft return ABI test, now that the Microsoft ABI implementation no longer depends on the AST isPOD property Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/ne

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-10-04 Thread David Blaikie via Phabricator via cfe-commits
dblaikie updated this revision to Diff 465158. dblaikie added a comment. rebase and fix an AST test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 Files: clang/include/clang/Basic/LangOptions.h clang

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-24 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a subscriber: ayzhao. rnk added a comment. In D119051#3747201 , @dblaikie wrote: > So... my conclusion is that Clang's AArch64 appears to be correct for x86 as > well, and we should just rename the function and use it unconditionally, > remov

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-24 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Each godbolt link shows MSVC 19.latest and clang, both in x86 and aarch64, with a reference pod and non-pod example at the start and end, and the variable test in the middle to see whether the codegen matches the pod or non-pod baselines above and below. > 1. checked

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-24 Thread David Blaikie via Phabricator via cfe-commits
dblaikie marked an inline comment as done. dblaikie added a comment. In D119051#3724121 , @rnk wrote: > Regarding the usage of isPOD in the MSVC ABI code, we should have a routine > similar to `isTrivialForAArch64MSVC` that works for the other MSVC > ar

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-15 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a subscriber: hans. rnk added a comment. Regarding the usage of isPOD in the MSVC ABI code, we should have a routine similar to `isTrivialForAArch64MSVC` that works for the other MSVC architectures that we support (x86, x64, and arm32). I don't think we should try to rely on definitio

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Do we need to gate this on use of `-fms-compatibility` as well? I'm not certain how cygwin factors in where it's sort of gcc and sort of msvc (perhaps the triple is sufficient for that?). Comment at: clang/lib/AST/DeclCXX.cpp:892 +if ((!

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-15 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: clang/lib/AST/DeclCXX.cpp:892 + LangOptions::ClangABI::Ver15)) { + // C++03 [class]p4: + // A POD-struct is an aggregate class that has [...] no user-defined dblaikie wrote: > erichkea

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-12 Thread David Blaikie via Phabricator via cfe-commits
dblaikie marked an inline comment as done and an inline comment as not done. dblaikie added inline comments. Comment at: clang/test/SemaCXX/class-layout.cpp:2-9 +// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-c++

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-12 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a subscriber: hansw. dblaikie added a comment. In D119051#3717934 , @dblaikie wrote: > I guess the other way to test for pod-for-purposes-of-ABI is IRgen. Looks > like MSVC isn't observable based on sizeof or alignof on these issues (the

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-12 Thread David Blaikie via Phabricator via cfe-commits
dblaikie updated this revision to Diff 452315. dblaikie added a comment. Add PS4/Darwin handling Remove unnecessary warning suppression Add MSVC return ABI testing Add tail-padding based testing as well as the alignment based testing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-12 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: clang/lib/AST/DeclCXX.cpp:892 + LangOptions::ClangABI::Ver15)) { + // C++03 [class]p4: + // A POD-struct is an aggregate class that has [...] no user-defined Does this language change

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. > In D119051#3715939 , @aaron.ballman > wrote: > >> In D119051#3714645 , @dblaikie >> wrote: >> >>> >> >> I would have thought use of `__is_pod` would tell us, but I'm not seeing t

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-11 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. I guess the other way to test for pod-for-purposes-of-ABI is IRgen. Looks like MSVC isn't observable based on sizeof or alignof on these issues (the previous godbolt shows MSVC's answer to alignment for the packed and size for the trailing packing don't change based on

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-11 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. > So I could test this other ways that actually impact layout - like whether > things can be packed in tail padding (can pack in tail padding for non-pod, > right?). Or we could ast dump and inspect the property directly? Maybe there > are some other options? Here's w

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-11 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. I guess the other packed behavior/ABI checking 277123376ce08c98b07c154bf83e4092a5d4d3c6 In D119051#3715939 , @aaron.ballman wrote: > In D119051#3714645

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-11 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. >> There could be more testing than only the indirect result of the packing >> problem that first inspired this patch. Any suggestions on what might be the >> most direct way to test whether the type's been considered pod in this sense? > > I would have thought use of

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D119051#3714645 , @dblaikie wrote: > Realized maybe we don't need a separate driver flag for this at all, and rely > only on the abi-compat flag? That seems to be how (at least some) other ABI > compat changes have been

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-10 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Still waiting for the pre-merge checks to complete, but hopefully this is clean now. Realized maybe we don't need a separate driver flag for this at all, and rely only on the abi-compat flag? That seems to be how (at least some) other ABI compat changes have been hand

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-10 Thread David Blaikie via Phabricator via cfe-commits
dblaikie updated this revision to Diff 451686. dblaikie added a comment. Remove driver flag (just rely on ABI compat) & tidy up testing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 Files: clang/inclu

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Oh wow that's an awful lot of pings without any response; I'm very sorry you had that experience, so thank you for tagging me to try to get this unstuck! The precommit CI test failures definitely look relevant and should be fixed up. Comment at:

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-09 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a subscriber: aaron.ballman. dblaikie added a comment. @aaron.ballman is this something you're comfortable reviewing or could recommend anyone else who might be suitable? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ htt

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-08 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-08-01 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-07-25 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-07-17 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-07-08 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-06-27 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-06-21 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-06-13 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-06-06 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-06-01 Thread David Blaikie via Phabricator via cfe-commits
dblaikie marked 5 inline comments as done. dblaikie added a comment. In D119051#3550653 , @probinson wrote: > I'm in the middle of upstreaming PS5 support and I still didn't think of > this... doh! Ah, right - done! Repository: rG LLVM Github Monore

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-06-01 Thread David Blaikie via Phabricator via cfe-commits
dblaikie updated this revision to Diff 433472. dblaikie added a comment. Generalize to handle PS5 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 Files: clang/include/clang/Basic/LangOptions.def clang

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-06-01 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. I'm in the middle of upstreaming PS5 support and I still didn't think of this... doh! Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5588 + bool DefaultedSMFArePOD = !RawTriple.isPS4() && !RawTriple.isOSDarwin(); + `isPS4()` =>

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-05-27 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596 + DefaultedSMFArePOD = false; + } + probinson wrote: > dblaikie wrote: > > probinson wrote: > > > dblaikie wrote: > > > > probinson wrote: > > > > > Does this mean `-fcla

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-05-26 Thread Paul Robinson via Phabricator via cfe-commits
probinson added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596 + DefaultedSMFArePOD = false; + } + dblaikie wrote: > probinson wrote: > > dblaikie wrote: > > > probinson wrote: > > > > Does this mean `-fclang-abi-compat` will overri

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-05-26 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596 + DefaultedSMFArePOD = false; + } + probinson wrote: > dblaikie wrote: > > probinson wrote: > > > Does this mean `-fclang-abi-compat` will override the PS4/Darwin special

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-05-25 Thread Paul Robinson via Phabricator via cfe-commits
probinson added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596 + DefaultedSMFArePOD = false; + } + dblaikie wrote: > probinson wrote: > > Does this mean `-fclang-abi-compat` will override the PS4/Darwin special > > case? I think w

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-05-25 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596 + DefaultedSMFArePOD = false; + } + probinson wrote: > Does this mean `-fclang-abi-compat` will override the PS4/Darwin special > case? I think we don't want to do that

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-05-25 Thread Paul Robinson via Phabricator via cfe-commits
probinson added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596 + DefaultedSMFArePOD = false; + } + Does this mean `-fclang-abi-compat` will override the PS4/Darwin special case? I think we don't want to do that. Repository: rG

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-05-24 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-05-24 Thread David Blaikie via Phabricator via cfe-commits
dblaikie updated this revision to Diff 431871. dblaikie added a comment. Herald added a subscriber: MaskRay. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 Files: clang/include/clang/Basic/LangO

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-03-07 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D119051#3350072 , @rnk wrote: > I would structure this as a LangOpt, and feed the target checks and ABI > compat checks into the default setting for it. It could be something like > `DefaultedSMFArePOD` / `-f[no-]defaulted-s

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-03-07 Thread David Blaikie via Phabricator via cfe-commits
dblaikie updated this revision to Diff 413641. dblaikie added a comment. Herald added subscribers: dexonsmith, dang. Herald added a project: All. Add a -f driver flag for defaulted-smf-are-pod Include the other cases from itanium ABI issue 66 (implicitly and explicitly deleted special members, an

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-02-28 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In D119051#3316026 , @dblaikie wrote: > Ah, looks like this is the existing > https://github.com/itanium-cxx-abi/cxx-abi/issues/66 If you're going to change the ABI, you might as well tackle the rest of the differences mention

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-02-28 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I would structure this as a LangOpt, and feed the target checks and ABI compat checks into the default setting for it. It could be something like `DefaultedSMFArePOD` / `-f[no-]defaulted-smf-are-pod` (smf being special member functions). The LangOpt default is true, and the

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-02-28 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-02-21 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-02-11 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D119051#3315747 , @rjmccall wrote: > And this should be raised as an Itanium issue. Ah, looks like this is the existing https://github.com/itanium-cxx-abi/cxx-abi/issues/66 Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-02-11 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Thanks for chiming in! In D119051#3315741 , @rjmccall wrote: > Changing the C++03 POD definition is going to be substantially ABI-breaking > at this point. Any logic to change it needs to be platform-specific. Even if it's on

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-02-11 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. And this should be raised as an Itanium issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-commits mailing list cfe-commit

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-02-11 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Changing the C++03 POD definition is going to be substantially ABI-breaking at this point. Any logic to change it needs to be platform-specific. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.l

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-02-11 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D119051#3314138 , @Bhramar.vatsa wrote: > Sorry, but I can only add a bit more confusion: > https://godbolt.org/z/fT19KTh34 > There are two cases, only differing in terms of user-defined constructor. > > Gcc and clang differ

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

2022-02-11 Thread Bhramar Vatsa via Phabricator via cfe-commits
Bhramar.vatsa added a comment. Sorry, but I can only add a bit more confusion: https://godbolt.org/z/dzYhhxbz4 There are two cases, only differing in terms of user-defined constructor. Gcc and clang differs in the two cases. Gcc at least packs the second case (without user defined constructor),