[PATCH] D121646: [Concepts] Fix an assertion failure while diagnosing constrained function candidates

2022-03-14 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson created this revision. royjacobson added a reviewer: erichkeane. Herald added a project: All. royjacobson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. See: https://github.com/llvm/llvm-project/issues/54379 I tried to see if

[PATCH] D121646: [Concepts] Fix an assertion failure while diagnosing constrained function candidates

2022-03-14 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 415251. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121646/new/ https://reviews.llvm.org/D121646 Files: clang/lib/Sema/SemaOverload.cpp clang/test/SemaTemplate/concepts.cpp Index:

[PATCH] D121646: [Concepts] Fix an assertion failure while diagnosing constrained function candidates

2022-03-14 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. In D121646#3380893 , @erichkeane wrote: > This seems acceptable to me. Though, I wonder if > `getTemplateArgumentBindingsText` should produce the leading space, that way > we could just pass the result of it directly,

[PATCH] D121646: [Concepts] Fix an assertion failure while diagnosing constrained function candidates

2022-03-15 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. In D121646#3382243 , @erichkeane wrote: > In D121646#3380902 , @royjacobson > wrote: > >> In D121646#3380893 , @erichkeane >> wrote: >>

[PATCH] D121965: Add validation for number of arguments of __builtin_memcpy_inline

2022-03-17 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson created this revision. Herald added a project: All. royjacobson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. __builtin_memcpy_inline doesn't use the usual builtin argument validation code, so it crashed when receiving wrong

[PATCH] D121965: Add validation for number of arguments of __builtin_memcpy_inline

2022-03-18 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. @gchatelet In D121965#3391714 , @gchatelet wrote: > Thx! Thanks, will you be able to commit this for me as "Roy Jacobson "? I don't have write access :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D120255: [Concepts] Check constraints for explicit template instantiations

2022-03-01 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. In D120255#3352755 , @erichkeane wrote: > Hmm... doing FileCheck in a Sema test is highly irregular. I would expect us > to be able to test this in the type system in some way. Something like > `A<3>::f()` is invalid

[PATCH] D120255: [Concepts] Check constraints for explicit template instantiations

2022-03-01 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 412229. royjacobson edited the summary of this revision. royjacobson set the repository for this revision to rG LLVM Github Monorepo. royjacobson added a comment. Herald added projects: clang, All. Updated test so it checks actual instantiation and update

[PATCH] D120255: [Concepts] Check constraints for explicit template instantiations

2022-03-01 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 412252. royjacobson added a comment. Adding context. (Sorry - didn't realize you were speaking about the diff itself, thought you meant adding general context to the PR message...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D120255: [Concepts] Check constraints for explicit template instantiations

2022-03-01 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 412262. royjacobson added a comment. Slightly update the test command line (add -triple %itanium_abi_triple like in other similar tests). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120255/new/

[PATCH] D120255: [Concepts] Check constraints for explicit template instantiations

2022-03-02 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added inline comments. Comment at: clang/test/SemaTemplate/constraints-instantiation.cpp:3-8 +// void PR46029::A<1>::f() +// CHECK: define {{.*}} @_ZN7PR460291AILi1EE1fEv +// void PR46029::A<2>::f() +// CHECK: define {{.*}} @_ZN7PR460291AILi2EE1fEv +// void

[PATCH] D120255: [Concepts] Check constraints for explicit template instantiations

2022-03-02 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 412557. royjacobson added a comment. Thanks for the quick feedback :) I looked again at the AST and found out that there is a difference between instantiated and non-instantiated functions I missed - the instantiated functions have actual code AST. I

[PATCH] D120255: [Concepts] Check constraints for explicit template instantiations

2022-03-03 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. In D120255#3357007 , @erichkeane wrote: > Do you have push rights, or do you need someone to push for you? If you need > someone to push for you, please post the name and email address you'd like > the commit under.

[PATCH] D122083: [Concepts] Fix placeholder constraints when references are involved

2022-03-19 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson created this revision. Herald added a project: All. royjacobson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Placeholder types were not checked for constraint satisfaction when modified by references. GitHub issue #54443

[PATCH] D122083: [Concepts] Fix placeholder constraints when references are involved

2022-03-23 Thread Roy Jacobson 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 rG94fd00f41ebd: [Concepts] Fix placeholder constraints when references are involved (authored by royjacobson). Repository: rG LLVM Github Monorepo

[PATCH] D122083: [Concepts] Fix placeholder constraints when references are involved

2022-03-23 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 417618. royjacobson added a comment. Added release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122083/new/ https://reviews.llvm.org/D122083 Files: clang/docs/ReleaseNotes.rst

[PATCH] D122083: [Concepts] Fix placeholder constraints when references are involved

2022-03-23 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 417602. royjacobson added a comment. Add test for auto**& combination Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122083/new/ https://reviews.llvm.org/D122083 Files:

[PATCH] D122083: [Concepts] Fix placeholder constraints when references are involved

2022-03-23 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. In D122083#3402445 , @erichkeane wrote: > 1 more test I'd like to see that doesn't seem covered (ref to ptr), Added, good idea. > AND according to @aaron.ballman we need "Release Notes" for this. Otherwise > LGTM. Do

[PATCH] D122083: [Concepts] Fix placeholder constraints when references are involved

2022-03-23 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 417591. royjacobson added a comment. Remove the curly brackets from one line loop. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122083/new/ https://reviews.llvm.org/D122083 Files:

[PATCH] D122083: [Concepts] Fix placeholder constraints when references are involved

2022-03-23 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson marked an inline comment as done. royjacobson added a comment. In D122083#3402289 , @erichkeane wrote: > Can you add some tests for the OTHER forms of 'auto' as well? We have > `decltype(auto)` and `auto_type`, and I want to make sure

[PATCH] D122083: [Concepts] Fix placeholder constraints when references are involved

2022-03-23 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 417624. royjacobson added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122083/new/ https://reviews.llvm.org/D122083 Files: clang/docs/ReleaseNotes.rst

[PATCH] D122083: [Concepts] Fix placeholder constraints when references are involved

2022-03-20 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 416803. royjacobson edited the summary of this revision. royjacobson added a comment. I noticed issue #53911 which is very similar so I fixed it as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D123182: [Concepts] Fix issue #53640

2022-04-05 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson created this revision. Herald added a project: All. royjacobson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Constraints were used for overload resolution tie breaking even when it was not allowed because the functions had

[PATCH] D123182: [Concepts] Fix issue #53640

2022-04-05 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 420708. royjacobson added a comment. Fixed missing doc for new argument Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123182/new/ https://reviews.llvm.org/D123182 Files: clang/docs/ReleaseNotes.rst

[PATCH] D123182: [Concepts] Fix issue #53640

2022-04-11 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 422030. royjacobson added a comment. Formatting + clarify docs a bit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123182/new/ https://reviews.llvm.org/D123182 Files: clang/docs/ReleaseNotes.rst

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

2022-04-11 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:2958 /// ArgPos will have the parameter index of the first different parameter. +/// If `Reversed` is true, exactly one of FT1 and FT2 is an overload +/// candidate with a reversed parameter order.

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

2022-04-15 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:2967 + "parameters!"); + for (FunctionProtoType::param_type_iterator + O = OldType->param_type_begin(), erichkeane wrote: > Thanks for the clarification on

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

2022-04-15 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson marked 2 inline comments as done. royjacobson added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:9829 + bool CanCompareConstraints = false; + if (Cand1.Function && Cand2.Function && Cand1.Function->hasPrototype() && +

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

2022-04-15 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 423116. royjacobson marked an inline comment as not done. royjacobson added a comment. Update for look inside FunctionParamTypesAreEqual to be index based and simpler. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

2022-04-15 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 423120. royjacobson added a comment. Split the 'can compare constraints' code into a static function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123182/new/ https://reviews.llvm.org/D123182 Files:

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

2022-04-15 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 423169. royjacobson marked an inline comment as done. royjacobson added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123182/new/ https://reviews.llvm.org/D123182 Files:

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

2022-04-16 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. Thanks @erichkeane! I will land this on Monday if there are no further comments and the CI looks good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123182/new/ https://reviews.llvm.org/D123182

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

2022-04-16 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 423229. royjacobson added a comment. Rebase again (HEAD CI was broken) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123182/new/ https://reviews.llvm.org/D123182 Files: clang/docs/ReleaseNotes.rst

[PATCH] D150075: Fix PR#62594 : static lambda call operator is not convertible to function pointer on win32

2023-09-15 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. @shafik @aaron.ballman does any of you want to commandeer this diff? seems simple enough of a change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150075/new/ https://reviews.llvm.org/D150075

[PATCH] D154893: [Clang] Fix some triviality computations

2023-09-15 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. In D154893#4646405 , @cor3ntin wrote: > @royjacobson ^ I did not have a lot of time for Clang the last few months unfortunately. I might take a look at this again next month when I'll have a bit more time. Repository:

[PATCH] D154893: [Clang] Fix some triviality computations

2023-10-08 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. I, uh, got preoccupied by some recent events. Don't think I'll have the time for this unfortunately. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154893/new/ https://reviews.llvm.org/D154893

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

2022-04-19 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a subscriber: Mordante. royjacobson added a comment. So, it seems like this broke one of `basic_arg_format`'s private constructors.. Sorry for that. //format_arg.h:167 explicit basic_format_arg(_Tp __v) noexcept requires(same_as<_Tp, char_type> ||

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

2022-04-19 Thread Roy Jacobson 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 rG454d1df9423c: [Concepts] Fix overload resolution bug with constrained candidates (authored by royjacobson). Repository: rG LLVM Github Monorepo

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

2022-04-23 Thread Roy Jacobson 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 rG807e418413a0: [Concepts] Fix overload resolution bug with constrained candidates (authored by royjacobson). Repository: rG LLVM Github Monorepo

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

2022-04-23 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 424747. royjacobson added a comment. Rebase after fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123182/new/ https://reviews.llvm.org/D123182 Files: clang/docs/ReleaseNotes.rst

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

2022-05-30 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson planned changes to this revision. royjacobson added inline comments. Comment at: clang/lib/AST/DeclCXX.cpp:1901 + for (auto* Decl : R) { +auto* DD = dyn_cast(Decl); +if (DD && DD->isEligibleOrSelected()) erichkeane wrote: > What cases happen

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

2022-05-23 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2330 + /// To achieve that, we remove all non-selected destructors from the AST, + /// which is a bit unusual. We can't let those declarations be in AST and rely + /// on

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

2022-05-23 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. In D126194#3531280 , @cor3ntin wrote: > In D126194#3531267 , @erichkeane > wrote: > >> How much of P0848 is missing after this? If nothing/not much, should we >> update

[PATCH] D126160: [Concepts] Add an error for unsupported P0848 (destructor overloading) code

2022-05-22 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 431251. royjacobson added a comment. Move diagnostic into Sema, make it fire once for every class template definitions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126160/new/

[PATCH] D126160: [Concepts] Add an error for unsupported P0848 (destructor overloading) code

2022-05-22 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 431263. royjacobson added a comment. Fix conventions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126160/new/ https://reviews.llvm.org/D126160 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D126160: [Concepts] Add an error for unsupported P0848 (destructor overloading) code

2022-05-22 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson abandoned this revision. royjacobson added a comment. I continued hacking a bit at this and I think I got a working approach to implementing P0848 (down to 6 failing tests, will probably post at least a draft tomorrow). So I'll close for now. Maybe it's still useful for backporting

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

2022-05-23 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson created this revision. Herald added a project: All. royjacobson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This patch implements a necessary part of P0848, the overload resolution for destructors. It is now possible to

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

2022-05-23 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 431333. royjacobson added a comment. Fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126194/new/ https://reviews.llvm.org/D126194 Files: clang/docs/ReleaseNotes.rst

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

2022-05-23 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 431589. royjacobson added a comment. Update the approach to use an AST property, and enable this for all language variants (not just CPP20). There's still one test failing because OpenCL have got their own destructor thing going, I'll try to see what I

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

2022-06-21 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added subscribers: thakis, rsmith. royjacobson added a comment. In D127593#3598234 , @sberg wrote: > In general, there's still a handful of `__has_trivial_*` calls across > Abseil's recent master branch `absl/meta/type_traits.h`. Breaking

[PATCH] D129170: [Sema] Add deprecation warnings for some compiler provided __has_* type traits

2022-07-07 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson marked 3 inline comments as done. royjacobson added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:5400-5401 +SourceLocation KWLoc) { + if (!S.getLangOpts().CPlusPlus11) +return; +

[PATCH] D129170: [Sema] Add deprecation warnings for some compiler provided __has_* type traits

2022-07-08 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:5400-5401 +SourceLocation KWLoc) { + if (!S.getLangOpts().CPlusPlus11) +return; + aaron.ballman wrote: > erichkeane wrote: > >

[PATCH] D128750: [c++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

2022-07-03 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson requested changes to this revision. royjacobson added a comment. This revision now requires changes to proceed. Thanks for working on this! I like the approach, but two comments: I would like more tests for the different forms of template argument deduction. Some suggestions for

[PATCH] D129170: [Sema] Add deprecation warnings for some compiler provided __has_* type traits

2022-07-11 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 443742. royjacobson added a comment. Warn on <=C++03 as well, warn on __has_trivial_destructor. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129170/new/ https://reviews.llvm.org/D129170 Files:

[PATCH] D129170: [Sema] Add deprecation warnings for some compiler provided __has_* type traits

2022-07-11 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:5400-5401 +SourceLocation KWLoc) { + if (!S.getLangOpts().CPlusPlus11) +return; + aaron.ballman wrote: > royjacobson wrote: > >

[PATCH] D129170: [Sema] Add deprecation warnings for some compiler provided __has_* type traits

2022-07-12 Thread Roy Jacobson 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 rG0b89d1d59f82: [Sema] Add deprecation warnings for some compiler provided __has_* type traits (authored by royjacobson). Repository: rG LLVM

[PATCH] D129170: [Sema] Add deprecation warnings for some compiler provided __has_* type traits

2022-07-05 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson created this revision. Herald added a project: All. royjacobson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Some compiler provided type traits like __has_trivial_constructor have been documented as deprecated for quite

[PATCH] D129170: [Sema] Add deprecation warnings for some compiler provided __has_* type traits

2022-07-06 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson marked 9 inline comments as done and 2 inline comments as done. royjacobson added inline comments. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:190 : DiagGroup<"deprecated-dynamic-exception-spec">; +def DeprecatedHasBuiltins :

[PATCH] D129170: [Sema] Add deprecation warnings for some compiler provided __has_* type traits

2022-07-06 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 442680. royjacobson added a comment. Address CR comments. Make the switch case slightly more concise. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129170/new/ https://reviews.llvm.org/D129170 Files:

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

2022-06-10 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:352 + [this](const Expr *AtomicExpr) -> ExprResult { +// We only do this to immitate lvalue-to-rvalue conversion. +return PerformContextuallyConvertToBool(const_cast(AtomicExpr));

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

2022-06-13 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. I don't think the OpenMP tests are failing because of this PR, they are a bit flaky sometimes. @erichkeane - Since we haven't branched clang15 yet, I think it's Ok not to add the Ver15 value to the ABI options? Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

2022-06-13 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. This is great, thanks for the changes! I added two follow-up inline comments. Another two other small details: Please add documentation about this to LangOptions.h (We try to document there the changes for each ABI version), and please add a release note about this

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

2022-06-15 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. In D127593#3585073 , @erichkeane wrote: > The OMP failures are still there though, which is a touch concerning. Yeah, I rerun the CI and it still fails. I couldn't reproduce the failures locally, though, and the CI logs

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

2022-06-18 Thread Roy Jacobson 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 rG21eb1af469c3: [Concepts] Implement overload resolution for destructors (P0848) (authored by royjacobson). Repository: rG LLVM Github Monorepo

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

2022-06-16 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 437656. royjacobson added a comment. Update the PR to now handle triviality, which turned out to require a different approach. On the bright side it is now easier to see how to implemenet the rest of P0848. Added an AST dump test that checks the

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

2022-06-16 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. I started to get those errors on D126194 as well, definitely annoying. They both do spooky things with type triviality so maybe this is somehow related? I'm not sure. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

2022-06-16 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. The OpenMP tests passed on D127998 (I think just randomly?). I'm going to commit this tomorrow morning so I can revert if needed. Javier, thanks again for the patch and for the patience! Repository: rG LLVM Github Monorepo

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

2022-06-17 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added inline comments. Comment at: clang/docs/ReleaseNotes.rst:434 +- As per "Conditionally Trivial Special Member Functions" (P0848), it is + now possible to overload destructors using concepts. Note that the rest erichkeane wrote: > Do we have

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

2022-06-17 Thread Roy Jacobson 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 rG5ea341d7c4f9: [clang] Fix trivially copyable for copy constructor and copy assignment operator (authored by Javier-varez, committed by

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

2022-06-17 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 437940. royjacobson added a comment. Fix the AST test on windows and rebase on HEAD. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126194/new/ https://reviews.llvm.org/D126194 Files:

[PATCH] D127998: [CI test] D127593

2022-06-16 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson created this revision. Herald added a project: All. royjacobson requested review of this revision. Herald added projects: clang, clang-tools-extra. Herald added a subscriber: cfe-commits. Testing D127593 by Javier varez to see if the CI failures are

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

2022-06-12 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson requested changes to this revision. royjacobson added a reviewer: clang-language-wg. royjacobson added a comment. This revision now requires changes to proceed. Hi Javier, thank you for submitting this patch! As far as I could tell, this patch implements the CWG2171 defect report

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

2022-06-14 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson accepted this revision. royjacobson added a comment. This revision is now accepted and ready to land. Thank you! LGTM. I see the CI still has those OpenMP failures - could you try to rebase on main to get it green? @erichkeane is this good to go? Should I ping some ABI people about

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

2022-06-20 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. In D127593#3596601 , @sberg wrote: > Is it intended that a deleted copy assignment op as in `struct S { void > operator =(S &) = delete; };` (though, somewhat oddly, not in `struct S { > void operator =(S) = delete; };`) is

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

2022-06-21 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. In D127593#3597374 , @sberg wrote: > You'd only see it when compiling with Clang against MSVC's ``, where > `std::pair` has a deleted copy assignment op (something I approximated with > the `struct S` above).

[PATCH] D126160: [Concepts] Add an error for unsupported P0848 (destructor overloading) code

2022-05-22 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson planned changes to this revision. royjacobson added a comment. I've thought about this a bit more and I want to move the diagnostic into SemaTemplateInstantiateDecl or somewhere close. It makes more sense because it's closer to where P0848 needs to be implemented and it will spam

[PATCH] D126160: [Concepts] Add an error for unsupported P0848 (destructor overloading) code

2022-05-22 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 431229. royjacobson added a comment. A working version. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126160/new/ https://reviews.llvm.org/D126160 Files: clang/include/clang/Basic/DiagnosticASTKinds.td

[PATCH] D126160: [Concepts] Add an error for unsupported P0848 (destructor overloading) code

2022-05-22 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson created this revision. Herald added a project: All. royjacobson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We have received numeroud bug reports over P0848 not being implemented for destructors. We currently allow

[PATCH] D129583: [DOC] Add DR1734 and DR1496 Clang's cxx_dr_status as not implemented

2022-07-12 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson created this revision. Herald added a project: All. royjacobson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Those two DRs about the (copy) triviality of types with deleted special member functions are not implemented in

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

2022-07-20 Thread Roy Jacobson via Phabricator via cfe-commits
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)

2022-07-20 Thread Roy Jacobson via Phabricator via cfe-commits
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)

2022-07-20 Thread Roy Jacobson via Phabricator via cfe-commits
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)

2022-07-20 Thread Roy Jacobson via Phabricator via cfe-commits
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)

2022-07-20 Thread Roy Jacobson via Phabricator via cfe-commits
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)

2022-07-20 Thread Roy Jacobson via Phabricator via cfe-commits
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)

2022-07-25 Thread Roy Jacobson via Phabricator via cfe-commits
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)

2022-07-25 Thread Roy Jacobson via Phabricator via cfe-commits
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)

2022-07-14 Thread Roy Jacobson via Phabricator via cfe-commits
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

[PATCH] D129583: [DOC] Add DR1734 and DR1496 Clang's cxx_dr_status as not implemented

2022-07-13 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 22. royjacobson added a comment. Update comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129583/new/ https://reviews.llvm.org/D129583 Files: clang/test/CXX/drs/dr14xx.cpp

[PATCH] D129583: [DOC] Add DR1734 and DR1496 Clang's cxx_dr_status as not implemented

2022-07-13 Thread Roy Jacobson 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 rG202b327f5d29: [DOC] Add DR1734 and DR1496 Clangs cxx_dr_status as not implemented (authored by royjacobson). Repository: rG LLVM Github Monorepo

[PATCH] D129583: [DOC] Add DR1734 and DR1496 Clang's cxx_dr_status as not implemented

2022-07-13 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. Thanks for the suggestions and for the quick review! @aaron.ballman Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129583/new/ https://reviews.llvm.org/D129583 ___

[PATCH] D128750: [c++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

2022-07-26 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. In D128750#3663161 , @ychen wrote: > In D128750#3662898 , @royjacobson > wrote: > >> but in your example with T/U/V we have 3 template parameters that we could >> reorder in 6

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

2022-07-22 Thread Roy Jacobson via Phabricator via cfe-commits
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)

2022-07-23 Thread Roy Jacobson via Phabricator via cfe-commits
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] D130058: [Clang] Diagnose ill-formed constant expression when setting a non fixed enum to a value outside the range of the enumeration values

2022-07-27 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added inline comments. Comment at: clang/test/SemaCXX/constant-expression-cxx11.cpp:2420 + constexpr E1 x2 = static_cast(8); // expected-error {{must be initialized by a constant expression}} + // expected-note@-1 {{integer value 8 is outside the valid range of

[PATCH] D129170: [Sema] Add deprecation warnings for some compiler provided __has_* type traits

2022-07-13 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. In D129170#3650618 , @glandium wrote: > From the commit message: > >> This patch adds deprecation warnings for the usage of those builtins, except >> for __has_trivial_destructor which doesn't have a GCC alternative. > > The

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

2022-07-17 Thread Roy Jacobson via Phabricator via cfe-commits
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)

2022-07-15 Thread Roy Jacobson via Phabricator via cfe-commits
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)

2022-07-15 Thread Roy Jacobson via Phabricator via cfe-commits
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)

2022-07-15 Thread Roy Jacobson via Phabricator via cfe-commits
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

  1   2   3   4   >