Add UFCS failure case error diagnostics

[hsutter]

Partly addresses #1004

Context: this comment

The purpose of this PR is to improve the diagnostic when UFCS calls don't resolve. The main problem case I've encountered is when the UFCS is a definite last use and the function that would be called does not accept rvalues. So I've added diagnostics for those cases. For example, this code

f: (inout _) = { }

main: () = {
    x := 42;
    x.f();
}

currently gets an error like this:

demo.cpp2(5): error C3889: call to object of class type 'main::<lambda_1>': no matching call operator found
demo.cpp2(5): note: could be 'decltype(auto) main::<lambda_1>::operator ()(Obj &&,Params &&...) noexcept(<expr>) const'
demo.cpp2(5): note: the associated constraints are not satisfied
demo.cpp2(5): note: the constraint was not satisfied

and with this PR gets this nicer error:

static_assert failed: 'this function call syntax tries 'obj.func(...)', then 'func(obj,...);' - both failed, but the second would have succeeded if obj were an lvalue - the likely problem is that this is a definite last use of the object, and the function cannot accept an rvalue'

But to make that work I also had to comment out the requires clause that constrains the argument, so that we can enter the lambda and reach the new diagnostics, and it's those requires failures that are the hard-to-read diagnostics. However, the requires clause seems to be there to make UFCS calls possible in SFINAE contexts.

And all regressions pass... except the SFINAE cases (see the 3 changed .output files). Because those cases want that requires to fail, right?

Summarizing the problem

If I'm understanding this right, it seems there's an inherent tension here for UFCS calls that won't succeed...

• use requires so as not to enter the body, and be SFINAE-ABLE but also get the current hard-to-understand diagnostics in move-from-last-use-and-pass-to-something-that-can't-take-an-rvalue cases
• don't use requires and enter the body, and get the new nicer diagnostic for those cases

... and I don't yet see how to achieve both at the same time.

@JohelEGP since you crafted the current UFCS version, what do you think?

[JohelEGP]

... and I don't yet see how to achieve both at the same time.

This might be easily achievable.
The _NONLOCAL-suffixed UFCS macros should happen in an "immediate context".
In this PR, those are the only ones that need the requires for SFINAE.

Maybe these changes can achieve that:

• Add the macro to ignore a paste:

+#define CPP2_UFCS_EMPTY(...)
 #define CPP2_UFCS_IDENTITY(...)  __VA_ARGS__

• Effectively parameterize the requires:

-#define CPP2_UFCS_(LAMBDADEFCAPT,MVFWD,QUALID,TEMPKW,...) \
+#define CPP2_UFCS_(LAMBDADEFCAPT,SFINAE,MVFWD,QUALID,TEMPKW,...) \
 [LAMBDADEFCAPT]< \
     typename Obj, typename... Params \
     CPP2_UFCS_IS_NOTHROW_PARAM(MVFWD,QUALID,TEMPKW,__VA_ARGS__) \
     CPP2_UFCS_CONSTRAINT_PARAM(MVFWD,QUALID,TEMPKW,__VA_ARGS__) \
   > \
   CPP2_LAMBDA_NO_DISCARD (Obj&& obj, Params&& ...params) CPP2_FORCE_INLINE_LAMBDA_CLANG \
   noexcept(CPP2_UFCS_IS_NOTHROW_ARG(MVFWD,QUALID,TEMPKW,__VA_ARGS__)) CPP2_FORCE_INLINE_LAMBDA -> decltype(auto) \
-    requires CPP2_UFCS_CONSTRAINT_ARG(MVFWD,QUALID,TEMPKW,__VA_ARGS__) { \
+    SFINAE(requires CPP2_UFCS_CONSTRAINT_ARG(MVFWD,QUALID,TEMPKW,__VA_ARGS__)) { \

• Paste the requires depending on _NONLOCAL.

+#define CPP2_UFCS(...)                                    CPP2_UFCS_(&,CPP2_UFCS_EMPTY,CPP2_UFCS_IDENTITY,(),,__VA_ARGS__)
+#define CPP2_UFCS_MOVE(...)                               CPP2_UFCS_(&,CPP2_UFCS_EMPTY,std::move,(),,__VA_ARGS__)
+#define CPP2_UFCS_FORWARD(...)                            CPP2_UFCS_(&,CPP2_UFCS_EMPTY,CPP2_FORWARD,(),,__VA_ARGS__)
+#define CPP2_UFCS_TEMPLATE(...)                           CPP2_UFCS_(&,CPP2_UFCS_EMPTY,CPP2_UFCS_IDENTITY,(),template,__VA_ARGS__)
+#define CPP2_UFCS_QUALIFIED_TEMPLATE(QUALID,...)          CPP2_UFCS_(&,CPP2_UFCS_EMPTY,CPP2_UFCS_IDENTITY,QUALID,template,__VA_ARGS__)
+#define CPP2_UFCS_NONLOCAL(...)                           CPP2_UFCS_(,CPP2_UFCS_IDENTITY,CPP2_UFCS_IDENTITY,(),,__VA_ARGS__)
+#define CPP2_UFCS_TEMPLATE_NONLOCAL(...)                  CPP2_UFCS_(,CPP2_UFCS_IDENTITY,CPP2_UFCS_IDENTITY,(),template,__VA_ARGS__)
+#define CPP2_UFCS_QUALIFIED_TEMPLATE_NONLOCAL(QUALID,...) CPP2_UFCS_(,CPP2_UFCS_IDENTITY,CPP2_UFCS_IDENTITY,QUALID,template,__VA_ARGS__)

[JohelEGP]

I actually did this in #490, before realizing the need for SFINAE.
That solution is worth looking into.
In particular, after the static_assert, in the same branch, I omit both forms m.f() and f(m).
This is so that the compiler has a chance to generate its error messages, which might be useful.

An important piece of information not visible at the call site are the overload sets.
Just the error message from the static_assert is not enough to decide what's wrong,
whereas having the compiler's error message can make debugging easier.
For example, consider how a novice might attempt to solve the following without the clue from the compiler's error message (https://cpp1.godbolt.org/z/dbEKf144f):

#include <vector>
int main() {
  std::vector<int> x;
  std::vector<long> y;
  x.swap(y);
}

Just having the static_assert can obscure the parameter types.
If this was in CI, and the parameter types weren't visible just above the call site,
one might have to launch an IDE just to find out the parameters' types
(or working around the UFCS somehow, which does against the UX this PR aims to fix).

[hsutter]

That solution is worth looking into.

🎉 Indeed, that was exactly what was needed -- thanks!

In particular, after the static_assert, in the same branch, I omit both forms m.f() and f(m).

Wonderful, yes it does make the errors better... using the example above, now I get a message that includes:

void f<Obj>(_T0 &)': cannot convert argument 1 from 'Obj' to '_T0 &'
        with
        [
            Obj=int,
            _T0=int
        ]

Very nice. Thanks again.

[JohelEGP]

Maybe CPP2_UFCS_EMPTY wasn't necessary, and passing CPP2_UFCS_IDENTITY instead would have done the job.

[hsutter]

But then all the cases would be the same again, and we'd have the original problem in non-SFINAE cases?

[JohelEGP]

Ah, you're right.