[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb1c960fc6dc2: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions) (authored by royjacobson). Repository: rG LLVM Github

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson updated this revision to Diff 455721. royjacobson added a comment. Formatting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/ https://reviews.llvm.org/D128619 Files: clang/docs/ReleaseNotes.rst clang/lib/AST/DeclCXX.cpp

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson updated this revision to Diff 455696. royjacobson added a comment. We call LookupSpecialMember before we compute eligibility, so the assert fails. Remove it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson updated this revision to Diff 455689. royjacobson added a comment. Rename file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/ https://reviews.llvm.org/D128619 Files: clang/docs/ReleaseNotes.rst clang/lib/AST/DeclCXX.cpp

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

cor3ntin added inline comments. Comment at: clang/test/AST/conditionally-trivial-smfs-1.cpp:1 +// RUN: %clang_cc1 -std=c++20 -triple x86_64-pc-linux -ast-dump=json %s | FileCheck %s --check-prefixes=CHECK,LIN +// RUN: %clang_cc1 -std=c++20 -triple x86_64-pc-win32 -ast-dump=json

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson added a comment. I added handling of FunctionTemplateDecls in SemaDecl because they can be default constructors. This caused errors with std::pair for example - it has two enable_ifs constructors to control explicit/implicit construction. I will try committing again if the

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson updated this revision to Diff 455686. royjacobson added a comment. Default constructors can be template functions. Update patch to handle them correctly, and add asserts to LookupSpecialMember that can catch similar problems. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

haowei added subscribers: haowei, phosek, mcgrathr. haowei added a comment. Hi, We are seeing build failures on Fuchsia build bots after this change was landed. and we suspect this patch causes clang to miscompile certain code. I filed bug https://github.com/llvm/llvm-project/issues/57351 and

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG717161509914: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions) (authored by royjacobson). Repository: rG LLVM Github

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson updated this revision to Diff 454913. royjacobson added a comment. Rebase on main. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/ https://reviews.llvm.org/D128619 Files: clang/docs/ReleaseNotes.rst

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM, thank you for this!! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/ https://reviews.llvm.org/D128619 ___ cfe-commits

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

cor3ntin accepted this revision. cor3ntin added a comment. This revision is now accepted and ready to land. I would wait for @aaron.ballman or @erichkeane to sign off on it, but this looks good to me. Thanks again for working on this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson updated this revision to Diff 454577. royjacobson added a comment. Clarify ambiguous doc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/ https://reviews.llvm.org/D128619 Files: clang/docs/ReleaseNotes.rst

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

aaron.ballman added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L"); -Builder.defineMacro("__cpp_concepts", "201907L"); +Builder.defineMacro("__cpp_concepts", "202002L");

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L"); -Builder.defineMacro("__cpp_concepts", "201907L"); +Builder.defineMacro("__cpp_concepts", "202002L");

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson updated this revision to Diff 454567. royjacobson added a comment. Remove the __cpp_concepts version bump. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/ https://reviews.llvm.org/D128619 Files: clang/docs/ReleaseNotes.rst

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

erichkeane added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L"); -Builder.defineMacro("__cpp_concepts", "201907L"); +Builder.defineMacro("__cpp_concepts", "202002L");

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L"); -Builder.defineMacro("__cpp_concepts", "201907L"); +Builder.defineMacro("__cpp_concepts", "202002L");

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

erichkeane added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L"); -Builder.defineMacro("__cpp_concepts", "201907L"); +Builder.defineMacro("__cpp_concepts", "202002L");

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

aaron.ballman added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L"); -Builder.defineMacro("__cpp_concepts", "201907L"); +Builder.defineMacro("__cpp_concepts", "202002L");

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

cor3ntin added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L"); -Builder.defineMacro("__cpp_concepts", "201907L"); +Builder.defineMacro("__cpp_concepts", "202002L");

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson marked 11 inline comments as done. royjacobson added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L"); -Builder.defineMacro("__cpp_concepts", "201907L"); +

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson updated this revision to Diff 454338. royjacobson added a comment. Address Aaron's comments, add the consteval test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/ https://reviews.llvm.org/D128619 Files:

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

usaxena95 added a comment. Can you please also add `consteval` related test case (mentioned in https://github.com/llvm/llvm-project/issues/57046#issuecomment-1211665079) to cxx2a-consteval.cpp

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

aaron.ballman added inline comments. Comment at: clang/lib/AST/DeclCXX.cpp:1443-1447 +if (D->isTrivial()) { + data().HasTrivialSpecialMembers |= SMKind; +} +else + data().DeclaredNonTrivialSpecialMembers |= SMKind;

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

cor3ntin added a comment. @aaron.ballman I think this looks good, you want to double check? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/ https://reviews.llvm.org/D128619 ___ cfe-commits

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson added inline comments. Comment at: clang/docs/ReleaseNotes.rst:494-497 +- Implemented "Conditionally Trivial Special Member Functions" (`P0848 `_). + Note: The handling of deleted functions is not yet compliant, as Clang + does not

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson updated this revision to Diff 453712. royjacobson added a comment. Add disclaimer to cxx_status about the standing DRs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/ https://reviews.llvm.org/D128619 Files:

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

cor3ntin added a comment. This looks good to me. I *think* landing things without doing silent sfinae for incomplete types is okay-ish, but I'd like to here from other folks. @royjacobson Have you looked into that? Do you know how involved would it be? Either way, we should make sure that if

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

usaxena95 added a comment. I would like to see this land. This also fixes https://github.com/llvm/llvm-project/issues/57046. Are there any pending concerns of the reviewers which we need to address ? Happy to help in any way to move this patch forward. CC: @erichkeane @cor3ntin Repository:

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson added inline comments. Comment at: clang/docs/ReleaseNotes.rst:494-497 +- Implemented "Conditionally Trivial Special Member Functions" (`P0848 `_). + Note: The handling of deleted functions is not yet compliant, as Clang + does not

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson updated this revision to Diff 453411. royjacobson added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/ https://reviews.llvm.org/D128619 Files: clang/docs/ReleaseNotes.rst clang/lib/AST/DeclCXX.cpp

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson updated this revision to Diff 450072. royjacobson added a comment. Rebase on main and re-target LLVM16 in cxx_status Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/ https://reviews.llvm.org/D128619 Files:

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson added a comment. @erichkeane @cor3ntin If you have time for review right now, I think it's reasonably ready and I would still like to merge it this week so it can land in Clang15. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson updated this revision to Diff 447425. royjacobson marked an inline comment as done. royjacobson added a comment. Rebase on main and slightly update docs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:17875 +return true; + if (!Context.hasSameType(M1->getParamDecl(0)->getType(), + M2->getParamDecl(0)->getType())) shafik wrote: > What happens if we have

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

shafik added inline comments. Comment at: clang/lib/AST/DeclCXX.cpp:899 + // triviality properties of the class until selecting a destructor and + // computing the eligibility of SMFs. This is because those member + // functions may have constraints that we need

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson added inline comments. Comment at: clang/test/SemaCXX/constrained-special-member-functions.cpp:103 +// FIXME: DR1734. +static_assert(__is_trivially_copyable(CopyAssignmentChecker<1>)); +static_assert(!__is_trivially_copyable(CopyAssignmentChecker<2>));

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

cor3ntin added inline comments. Comment at: clang/test/SemaCXX/constrained-special-member-functions.cpp:103 +// FIXME: DR1734. +static_assert(__is_trivially_copyable(CopyAssignmentChecker<1>)); +static_assert(!__is_trivially_copyable(CopyAssignmentChecker<2>));

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:17899 + ConstraintSatisfaction Satisfaction; + if (S.CheckFunctionConstraints(Method, Satisfaction)) +SatisfactionStatus.push_back(false); cor3ntin wrote: > erichkeane

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson updated this revision to Diff 446264. royjacobson added a comment. Add a test for the CRTP constraints timings Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/ https://reviews.llvm.org/D128619 Files:

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:17899 + ConstraintSatisfaction Satisfaction; + if (S.CheckFunctionConstraints(Method, Satisfaction)) +SatisfactionStatus.push_back(false); erichkeane wrote: > royjacobson

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:17899 + ConstraintSatisfaction Satisfaction; + if (S.CheckFunctionConstraints(Method, Satisfaction)) +SatisfactionStatus.push_back(false); royjacobson wrote: > erichkeane

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:17899 + ConstraintSatisfaction Satisfaction; + if (S.CheckFunctionConstraints(Method, Satisfaction)) +SatisfactionStatus.push_back(false); erichkeane wrote: >

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:17899 + ConstraintSatisfaction Satisfaction; + if (S.CheckFunctionConstraints(Method, Satisfaction)) +SatisfactionStatus.push_back(false); royjacobson wrote: > erichkeane

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:17899 + ConstraintSatisfaction Satisfaction; + if (S.CheckFunctionConstraints(Method, Satisfaction)) +SatisfactionStatus.push_back(false); erichkeane wrote: > cor3ntin

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:17899 + ConstraintSatisfaction Satisfaction; + if (S.CheckFunctionConstraints(Method, Satisfaction)) +SatisfactionStatus.push_back(false); cor3ntin wrote: > erichkeane

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:17899 + ConstraintSatisfaction Satisfaction; + if (S.CheckFunctionConstraints(Method, Satisfaction)) +SatisfactionStatus.push_back(false); erichkeane wrote: > cor3ntin

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:17899 + ConstraintSatisfaction Satisfaction; + if (S.CheckFunctionConstraints(Method, Satisfaction)) +SatisfactionStatus.push_back(false); cor3ntin wrote: > royjacobson

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:17899 + ConstraintSatisfaction Satisfaction; + if (S.CheckFunctionConstraints(Method, Satisfaction)) +SatisfactionStatus.push_back(false); royjacobson wrote: > erichkeane

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson updated this revision to Diff 446172. royjacobson added a comment. Fixed the tests, fixed the type comparison checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/ https://reviews.llvm.org/D128619 Files:

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:17870 +/// parameter type and the same cv-qualifiers and ref-qualifier, if any. +bool AreSpecialMemberFunctionsSameKind(CXXMethodDecl *M1, CXXMethodDecl *M2, + Sema::CXXSpecialMember

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

erichkeane added a comment. I believe the calculation of the constraints here means this isn't conforming with concepts. I believe this means we have to delay calculating the constraint differences until this property is actually queried, and cannot do it early like this has.

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

cor3ntin added a comment. @aaron.ballman @erichkeane I think this should be on your radar, it would be great to have it in 15. I did review it and I think it looks good but there are enough dragons here that i'd rather have more eye balls on it. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:17870 +/// parameter type and the same cv-qualifiers and ref-qualifier, if any. +bool AreSpecialMemberFunctionsSameKind(CXXMethodDecl *M1, CXXMethodDecl *M2, + Sema::CXXSpecialMember

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

h-vetinari added inline comments. Comment at: clang/docs/ReleaseNotes.rst:494-497 +- Implemented "Conditionally Trivial Special Member Functions" (`P0848 `_). + Note: The handling of deleted functions is not yet compliant, as Clang + does not

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson updated this revision to Diff 445350. royjacobson marked an inline comment as not done. royjacobson added a comment. Add a test case with more subsumption Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson marked an inline comment as done. royjacobson added inline comments. Comment at: clang/test/AST/conditionally-trivial-smfs.cpp:39 + +template struct DefaultConstructorCheck<2>; +// CHECK: "kind": "ClassTemplateSpecializationDecl", BRevzin

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

BRevzin added inline comments. Comment at: clang/test/AST/conditionally-trivial-smfs.cpp:39 + +template struct DefaultConstructorCheck<2>; +// CHECK: "kind": "ClassTemplateSpecializationDecl", royjacobson wrote: > BRevzin wrote: > > It's possible

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson marked 4 inline comments as done and an inline comment as not done. royjacobson added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:17960-17961 +} +if (AnotherMethodIsMoreConstrained) + break; + } cor3ntin

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson updated this revision to Diff 445044. royjacobson edited the summary of this revision. royjacobson added a comment. Address Corentin's feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/ https://reviews.llvm.org/D128619

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

BRevzin added inline comments. Comment at: clang/test/AST/conditionally-trivial-smfs.cpp:39 + +template struct DefaultConstructorCheck<2>; +// CHECK: "kind": "ClassTemplateSpecializationDecl", It's possible that I just don't understand what these

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

cor3ntin added a comment. Thanks for working on that! The patch looks very good overall Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/ https://reviews.llvm.org/D128619 ___ cfe-commits mailing

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:17863-17869 +/// Assuming the suggested solution to CWG2595 gets accepted: +/// [class.mem.special]p6: +/// An eligible special member function is a special member function for which: +/// - the function is

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

royjacobson created this revision. Herald added a project: All. royjacobson updated this revision to Diff 444173. royjacobson added a comment. royjacobson updated this revision to Diff 19. royjacobson retitled this revision from "[WIP][Clang] Implement P0848 (Conditionally Trivial Special