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.

In D127593#3597374, @sberg wrote:

You'd only see it when compiling with Clang against MSVC's <utility>, where std::pair has a deleted copy assignment op (something I approximated with the struct S above). https://godbolt.org/z/bbEqvosvP shows how the behavior changed in Clang trunk. (Interestingly, the behavior is different between MSVC and GCC, and Clang now changed from matching the MSVC behavior to matching the GCC one.)

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 now marked as trivial?

I think that's the root cause why a build of PDFium with clang-cl against the MSVC standard library started to fail for me now, effectively complaining that __is_trivially_assignable(std::pair<int,int> &, std::pair<int,int> const &) is false (as expected) while __has_trivial_assign(std::pair<int,int>) is (unexpectedly) true.

Fix the AST test on windows and rebase on HEAD.

The OpenMP tests passed on D127998 (I think just randomly?). I'm going to commit this tomorrow morning so I can revert if needed.

I started to get those errors on D126194 as well, definitely annoying.

Try to manually enable back-compat

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.

In D127593#3585073, @erichkeane wrote:

The OMP failures are still there though, which is a touch concerning.

Thank you! LGTM. I see the CI still has those OpenMP failures - could you try to rebase on main to get it green?

I don't think the OpenMP tests are failing because of this PR, they are a bit flaky sometimes.

This is great, thanks for the changes! I added two follow-up inline comments.

Hi Javier, thank you for submitting this patch!

Update the approach to use an AST property, and enable this for all language variants (not just CPP20).

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 cxx_status.html as well?

P0848 applies to all special member function. At best we could mark it partial but most of the work still need to be done.
I gave a shot to P0848 a few months ago, but my assesment is that clang might have to significantly refactor of special member functions to do that cleanly

Fix typo

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 if someone wants to. Sorry for all the mail spam from today...

Fix conventions

Move diagnostic into Sema, make it fire once for every class template definitions.

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 less errors.

A working version.

Code and added/modified tests LGTM!

Rebase after fix.

Thank you for the quick fix!

So, it seems like this broke one of basic_arg_format's private constructors.. Sorry for that.

Thanks @erichkeane! I will land this on Monday if there are no further comments and the CI looks good.

Rebase again (HEAD CI was broken)

Rebase

Split the 'can compare constraints' code into a static function.

Update for look inside FunctionParamTypesAreEqual to be index based and simpler.

Formatting + clarify docs a bit

Fixed missing doc for new argument

Rebase

Added release notes.

In D122083#3402445, @erichkeane wrote:

1 more test I'd like to see that doesn't seem covered (ref to ptr),

Added, good idea.

Add test for auto**& combination

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 whatever we do with those 'looks right'.

Remove the curly brackets from one line loop.

I noticed issue #53911 which is very similar so I fixed it as well.

In D121965#3391714, @gchatelet wrote:

Thx!

In D121646#3382243, @erichkeane wrote:

In D121646#3380902, @royjacobson wrote:

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, rather than have to create a SmallString everywhere. WDYT?

There are other places (outside SemaOverload) that use this function and don't need a space, though.
I thought about just putting the space in the diagnostic itself, but then sometimes you don't get template arguments and it fails.

I see. I was hoping that wasn't the case! OK, LGTM. Let me know if you need me to commit this for you.

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, rather than have to create a SmallString everywhere. WDYT?

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.

Thanks for the quick feedback :)

Slightly update the test command line (add -triple %itanium_abi_triple like in other similar tests).

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 (see -verify).

Also, please add all the context (as requested above!) with the -U9999 stuff.

Adding context.
(Sorry - didn't realize you were speaking about the diff itself, thought you meant adding general context to the PR message...)

Updated test so it checks actual instantiation and update the description.