Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SUGGESTION] UFCS error message could be less misleading #439

Closed
realgdman opened this issue May 8, 2023 · 9 comments · Fixed by #506
Closed

[SUGGESTION] UFCS error message could be less misleading #439

realgdman opened this issue May 8, 2023 · 9 comments · Fixed by #506

Comments

@realgdman
Copy link

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

This comment was marked as resolved.

@JohelEGP

This comment was marked as resolved.

@JohelEGP

This comment was marked as resolved.

@JohelEGP
Copy link
Contributor

JohelEGP commented May 8, 2023

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
Copy link
Contributor

JohelEGP commented May 8, 2023

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

This comment was marked as resolved.

@JohelEGP

This comment was marked as resolved.

@JohelEGP

This comment was marked as resolved.

@realgdman
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants