Closed hsutter closed 3 months ago
... 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:
+#define CPP2_UFCS_EMPTY(...)
#define CPP2_UFCS_IDENTITY(...) __VA_ARGS__
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
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__)
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).
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 formsm.f()
andf(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.
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
currently gets an error like this:
and with this PR gets this nicer error:
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 thoserequires
failures that are the hard-to-read diagnostics. However, therequires
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 thatrequires
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...
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 casesrequires
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?