[SUGGESTION] UFCS error message could be less misleading

[realgdman]

main: () = {
	vi: std::vector<int> = ();
	vi.push_back("oops");
}

Gives cpp1 diagnostic

error: use of undeclared identifier 'push_back'
CPP2_UFCS(push_back, std::move(vi), "oops");
vector.cpp2:6:2: note: in instantiation of function template specialization 'main()::(anonymous class)::operator()<std::vector, const char (&)[5]>' requested here
CPP2_UFCS(push_back, std::move(vi), "oops");

[JohelEGP]

See https://cpp2.godbolt.org/z/r7hPnWj6M.
With

diff

+template<class> constexpr bool false_ = false;
 #define CPP2_UFCS(FUNCNAME,PARAM1,...) \
 [&](auto&& obj, auto&& ...params) CPP2_FORCE_INLINE_LAMBDA -> decltype(auto) { \
     if constexpr (requires{ CPP2_FORWARD(obj).FUNCNAME(CPP2_FORWARD(params)...); }) { \
         return CPP2_FORWARD(obj).FUNCNAME(CPP2_FORWARD(params)...); \
-    } else { \
+    } else if constexpr (requires{ FUNCNAME(CPP2_FORWARD(obj), CPP2_FORWARD(params)...); }) { \
         return FUNCNAME(CPP2_FORWARD(obj), CPP2_FORWARD(params)...); \
+    } else { \
+        static_assert(false_<decltype(obj)>, "No viable member or non-member candidate " #FUNCNAME); \
     } \
 }(PARAM1, __VA_ARGS__)

Clang outputs

build/_cppfront/main.cpp:23:2: error: static assertion failed due to requirement 'false_<std::vector<int, std::allocator<int>> &&>': No viable member or non-member candidate push_back
 CPP2_UFCS(push_back, std::move(vi), "oops");
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
build/_cppfront/main.cpp:15:23: note: expanded from macro 'CPP2_UFCS'
        static_assert(false_<decltype(obj)>, "No viable member or non-member candidate " #FUNCNAME); \
                      ^~~~~~~~~~~~~~~~~~~~~
build/_cppfront/main.cpp:23:2: note: in instantiation of function template specialization 'main()::(anonymous class)::operator()<std::vector<int>, const char (&)[5]>' requested here
build/_cppfront/main.cpp:17:2: note: expanded from macro 'CPP2_UFCS'
}(PARAM1, __VA_ARGS__)
 ^
1 error generated.

GCC outputs

build/_cppfront/main.cpp: In instantiation of 'main()::<lambda(auto:85&&, auto:86&& ...)> [with auto:85 = std::vector<int>; auto:86 = {const char (&)[5]}]':
build/_cppfront/main.cpp:23:2:   required from here
build/_cppfront/main.cpp:15:23: error: static assertion failed: No viable member or non-member candidate push_back
   15 |         static_assert(false_<decltype(obj)>, "No viable member or non-member candidate " #FUNCNAME); \
      |                       ^~~~~~~~~~~~~~~~~~~~~
build/_cppfront/main.cpp:23:2: note: in expansion of macro 'CPP2_UFCS'
   23 |  CPP2_UFCS(push_back, std::move(vi), "oops");
      |  ^~~~~~~~~
build/_cppfront/main.cpp:15:23: note: 'false_<std::vector<int>&&>' evaluates to false
   15 |         static_assert(false_<decltype(obj)>, "No viable member or non-member candidate " #FUNCNAME); \
      |                       ^~~~~~~~~~~~~~~~~~~~~
build/_cppfront/main.cpp:23:2: note: in expansion of macro 'CPP2_UFCS'
   23 |  CPP2_UFCS(push_back, std::move(vi), "oops");
      |  ^~~~~~~~~

[JohelEGP]

Another alternative: https://cpp2.godbolt.org/z/x15G7bPTv.

Clang outputs

build/_cppfront/main.cpp:26:2: error: static assertion failed due to requirement 'ufcs_invocable<push_back, std::vector<int, std::allocator<int>>, const char (&)[5]>': No UFCS for push_back
 CPP2_UFCS(push_back, std::move(vi), "oops");
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
build/_cppfront/main.cpp:18:23: note: expanded from macro 'CPP2_UFCS'
        static_assert(ufcs_invocable<FUNCNAME, Obj, Param...>, "No UFCS for " #FUNCNAME); \
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
build/_cppfront/main.cpp:26:2: note: in instantiation of function template specialization 'main()::(anonymous class)::operator()<std::vector<int>, const char (&)[5]>' requested here
build/_cppfront/main.cpp:20:2: note: expanded from macro 'CPP2_UFCS'
}(PARAM1, __VA_ARGS__)
 ^
1 error generated.

GCC outputs

build/_cppfront/main.cpp: In instantiation of 'main()::<lambda(Obj&&, Param&& ...)> [with Obj = std::vector<int>; Param = {const char (&)[5]}]':
build/_cppfront/main.cpp:26:2:   required from here
build/_cppfront/main.cpp:18:23: error: static assertion failed: No UFCS for push_back
   18 |         static_assert(ufcs_invocable<FUNCNAME, Obj, Param...>, "No UFCS for " #FUNCNAME); \
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
build/_cppfront/main.cpp:26:2: note: in expansion of macro 'CPP2_UFCS'
   26 |  CPP2_UFCS(push_back, std::move(vi), "oops");
      |  ^~~~~~~~~
build/_cppfront/main.cpp:18:23: note: 'ufcs_invocable<push_back, std::vector<int>, const char (&)[5]>' evaluates to false
   18 |         static_assert(ufcs_invocable<FUNCNAME, Obj, Param...>, "No UFCS for " #FUNCNAME); \
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
build/_cppfront/main.cpp:26:2: note: in expansion of macro 'CPP2_UFCS'
   26 |  CPP2_UFCS(push_back, std::move(vi), "oops");
      |  ^~~~~~~~~
make[2]: *** [CMakeFi

[JohelEGP]

Can't get better than this, other than reducing the macro stack-trace noise: https://cpp2.godbolt.org/z/4n4GM7sEn.

Clang outputs

build/_cppfront/main.cpp:24:2: error: static assertion failed due to requirement 'ufcs_invocable(push_back, std::move(vi), "oops")': 'push_back' is not UFCS invocable with object=='std::move(vi)', arguments=='"oops"'.
 CPP2_UFCS(push_back, std::move(vi), "oops");
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
build/_cppfront/main.cpp:16:23: note: expanded from macro 'CPP2_UFCS'
        static_assert(ufcs_invocable(FUNCNAME, PARAM1, __VA_ARGS__), "'" #FUNCNAME "' is not UFCS invocable with object=='" #PARAM1 "', arguments=='" #__VA_ARGS__ "'."); \
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
build/_cppfront/main.cpp:24:2: note: in instantiation of function template specialization 'main()::(anonymous class)::operator()<std::vector<int>, const char (&)[5]>' requested here
build/_cppfront/main.cpp:18:2: note: expanded from macro 'CPP2_UFCS'
}(PARAM1, __VA_ARGS__)
 ^
1 error generated.

GCC outputs

build/_cppfront/main.cpp: In instantiation of 'main()::<lambda(Obj&&, Param&& ...)> [with Obj = std::vector<int>; Param = {const char (&)[5]}]':
build/_cppfront/main.cpp:24:2:   required from here
build/_cppfront/main.cpp:16:37: error: static assertion failed: 'push_back' is not UFCS invocable with object=='std::move(vi)', arguments=='"oops"'.
   16 |         static_assert(ufcs_invocable(FUNCNAME, PARAM1, __VA_ARGS__), "'" #FUNCNAME "' is not UFCS invocable with object=='" #PARAM1 "', arguments=='" #__VA_ARGS__ "'."); \
      |                       ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
build/_cppfront/main.cpp:24:2: note: in expansion of macro 'CPP2_UFCS'
   24 |  CPP2_UFCS(push_back, std::move(vi), "oops");
      |  ^~~~~~~~~
build/_cppfront/main.cpp:16:37: note: 'false' evaluates to false
   16 |         static_assert(ufcs_invocable(FUNCNAME, PARAM1, __VA_ARGS__), "'" #FUNCNAME "' is not UFCS invocable with object=='" #PARAM1 "', arguments=='" #__VA_ARGS__ "'."); \
      |                       ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
build/_cppfront/main.cpp:24:2: note: in expansion of macro 'CPP2_UFCS'
   24 |  CPP2_UFCS(push_back, std::move(vi), "oops");
      |  ^~~~~~~~~

[JohelEGP]

Actually, the best might be to call both forms, let the compiler spew errors, and the user choose what's wrong.
That gives the most useful information, rather than just "you can't UFCS call 'f' with 'x' and 'y'".

Better implementation in the comment below. Old one here.

See https://cpp2.godbolt.org/z/48bP4nj6c.

Clang outputs

build/_cppfront/main.cpp:23:12: error: no matching member function for call to 'push_back'
 CPP2_UFCS(push_back, std::move(vi), "oops");
 ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
build/_cppfront/main.cpp:14:27: note: expanded from macro 'CPP2_UFCS'
        CPP2_FORWARD(obj).FUNCNAME(CPP2_FORWARD(params)...); \
        ~~~~~~~~~~~~~~~~~~^~~~~~~~
build/_cppfront/main.cpp:23:2: note: in instantiation of function template specialization 'main()::(anonymous class)::operator()<std::vector<int>, const char (&)[5]>' requested here
 CPP2_UFCS(push_back, std::move(vi), "oops");
 ^
build/_cppfront/main.cpp:17:2: note: expanded from macro 'CPP2_UFCS'
}(PARAM1, __VA_ARGS__)
 ^
/opt/compiler-explorer/clang-trunk-20230508/bin/../include/c++/v1/vector:595:62: note: candidate function not viable: no known conversion from 'const char[5]' to 'const value_type' (aka 'const int') for 1st argument
    _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void push_back(const_reference __x);
                                                             ^
/opt/compiler-explorer/clang-trunk-20230508/bin/../include/c++/v1/vector:597:62: note: candidate function not viable: no known conversion from 'const char[5]' to 'value_type' (aka 'int') for 1st argument
    _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void push_back(value_type&& __x);
                                                             ^
build/_cppfront/main.cpp:23:12: error: use of undeclared identifier 'push_back'
 CPP2_UFCS(push_back, std::move(vi), "oops");
           ^
2 errors generated.

GCC outputs

build/_cppfront/main.cpp: In instantiation of 'main()::<lambda(Obj&&, Param&& ...)> [with Obj = std::vector<int>; Param = {const char (&)[5]}]':
build/_cppfront/main.cpp:23:2:   required from here
build/_cppfront/main.cpp:14:35: error: no matching function for call to 'push_back(const char [5])'
   14 |         CPP2_FORWARD(obj).FUNCNAME(CPP2_FORWARD(params)...); \
      |                                   ^
build/_cppfront/main.cpp:23:2: note: in expansion of macro 'CPP2_UFCS'
   23 |  CPP2_UFCS(push_back, std::move(vi), "oops");
      |  ^~~~~~~~~
In file included from /opt/compiler-explorer/gcc-trunk-20230508/include/c++/14.0.0/vector:66,
                 from /app/cpp2util.h:206,
                 from /app/build/_cppfront/main.cpp:3:
/opt/compiler-explorer/gcc-trunk-20230508/include/c++/14.0.0/bits/stl_vector.h:1278:7: note: candidate: 'constexpr void std::vector<_Tp, _Alloc>::push_back(const value_type&) [with _Tp = int; _Alloc = std::allocator<int>; value_type = int]' (near match)
 1278 |       push_back(const value_type& __x)
      |       ^~~~~~~~~
/opt/compiler-explorer/gcc-trunk-20230508/include/c++/14.0.0/bits/stl_vector.h:1278:7: note:   conversion of argument 1 would be ill-formed:
/opt/compiler-explorer/gcc-trunk-20230508/include/c++/14.0.0/bits/stl_vector.h:1278:7: error: invalid conversion from 'const char*' to 'std::vector<int>::value_type' {aka 'int'} [-fpermissive]
 1278 |       push_back(const value_type& __x)
      |       ^~~~~~~~~
      |       |
      |       const char*
/opt/compiler-explorer/gcc-trunk-20230508/include/c++/14.0.0/bits/stl_vector.h:1295:7: note: candidate: 'constexpr void std::vector<_Tp, _Alloc>::push_back(value_type&&) [with _Tp = int; _Alloc = std::allocator<int>; value_type = int]' (near match)
 1295 |       push_back(value_type&& __x)
      |       ^~~~~~~~~
/opt/compiler-explorer/gcc-trunk-20230508/include/c++/14.0.0/bits/stl_vector.h:1295:7: note:   conversion of argument 1 would be ill-formed:
/opt/compiler-explorer/gcc-trunk-20230508/include/c++/14.0.0/bits/stl_vector.h:1295:7: error: invalid conversion from 'const char*' to 'std::vector<int>::value_type' {aka 'int'} [-fpermissive]
 1295 |       push_back(value_type&& __x)
      |       ^~~~~~~~~
      |       |
      |       const char*
build/_cppfront/main.cpp:15:17: error: 'push_back' was not declared in this scope
   15 |         FUNCNAME(CPP2_FORWARD(obj), CPP2_FORWARD(params)...); \
      |                 ^
build/_cppfront/main.cpp:23:2: note: in expansion of macro 'CPP2_UFCS'
   23 |  CPP2_UFCS(push_back, std::move(vi), "oops");
      |  ^~~~~~~~~

[JohelEGP]

It's possible to do better.

A static_assert can be added to explain that
UFCS failed because both writes are invalid,
and that both writes will be called unconditionally to facilitate diagnosing the problem.

Combining with the solution of #439 (comment), it's possible to error with (https://cpp2.godbolt.org/z/z8ahaqzsP):

error: static assertion failed: 'push_back' is not UFCS invocable with object=='std::move(vi)', arguments=='"oops"'. Neither 'std::move(vi).push_back("oops")' nor 'push_back(std::move(vi), "oops")' is valid. Calling both unconditionally. Please, review their diagnostics.

The above is good enough. Resolved stuff from here on.

However, that prints Cpp1, and exposes implementation details (including lowering to Cpp1).
The macro and cppfront's generation can be enhanced to add the Cpp2 code fragments to the output.
Then it'd be possible to error with:

error: static assertion failed: UFCS failed. 'vi.push_back("oops")' and rewritten 'push_back(vi, "oops")' are invalid. Calling both unconditionally. Please, review their diagnostics.

[JohelEGP]

The macro and cppfront's generation can be enhanced to add the Cpp2 code fragments to the output.

That, and writing code to the error message, might be undesirable.

The generated code, if it matters, would change from

CPP2_UFCS(push_back, std::move(vi), "oops")

to

CPP2_UFCS(push_back, std::move(vi), "oops", vi, "oops")

which I think is undesirable,
whether the extra metadata is visible on the call or not.

The code can get long, and trying to keep it under control can get complex.
For example, choosing to rely on source location for a function expression:

CPP2_UFCS(push_back, std::move(vi), 𝘶𝘨𝘭𝘺 𝘭𝘢𝘮𝘣𝘥𝘢 𝘨𝘦𝘯𝘦𝘳𝘢𝘵𝘦𝘥 𝘧𝘳𝘰𝘮 𝘤𝘰𝘮𝘱𝘭𝘦𝘹 𝘍𝘌, vi, (function expression at main.cpp2:3:15))

A static_assert can be added to explain that
UFCS failed because both writes are invalid,
and that both writes will be called unconditionally to facilitate diagnosing the problem.

This is the important part.

[JohelEGP]

That, and writing code to the error message, might be undesirable.

Still, it's worth considering.

The extra generation for better error messages could be gated behind a flag like -clean-cpp1.
Personally, I'd prefer an extra flag to have better error messages and clean code when reporting bugs.

[JohelEGP]

whether the extra metadata is visible on the call or not.

The only way for cppfront to pass metadata around without affecting call sites would be with macro magic.

However, that prints Cpp1, and exposes implementation details (including lowering to Cpp1).

Many error messages will still happen when compiling Cpp1,
so perhaps it doesn't matter that much.
And in fact, it may be relevant what the generated code is when diagnosing the problem
(like #414, #396, #231).

[realgdman]

Thanks for research, I don't expect those with macro is easy.