Skip to content

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

Closed
#506
Closed
[SUGGESTION] UFCS error message could be less misleading#439
#506
Labels
suggestion

Description

@realgdman
realgdman
opened on May 8, 2023
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");

Activity

  1. realgdman
    added
    suggestion
    on May 8, 2023

  2. JohelEGP commented on May 8, 2023

    @JohelEGP
    JohelEGP
    on May 8, 2023 · Hidden as resolved

  3. JohelEGP commented on May 8, 2023

    @JohelEGP
    JohelEGP
    on May 8, 2023 · Hidden as resolved

  4. JohelEGP commented on May 8, 2023

    @JohelEGP
    JohelEGP
    on May 8, 2023 · Hidden as resolved

  5. JohelEGP commented on May 8, 2023

    @JohelEGP
    JohelEGP
    on May 8, 2023
    Contributor

    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");
      |  ^~~~~~~~~

  6. JohelEGP commented on May 8, 2023

    @JohelEGP
    JohelEGP
    on May 8, 2023
    Contributor

    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.

  7. JohelEGP commented on May 8, 2023

    @JohelEGP
    JohelEGP
    on May 8, 2023 · Hidden as resolved

  8. JohelEGP commented on May 8, 2023

    @JohelEGP
    JohelEGP
    on May 8, 2023 · Hidden as resolved

  9. JohelEGP commented on May 8, 2023

    @JohelEGP
    JohelEGP
    on May 8, 2023 · Hidden as resolved

  10. realgdman commented on May 9, 2023

    @realgdman
    realgdman
    on May 9, 2023
    Author

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

  11. hsutter
    closed this as completedin #506on Dec 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    suggestion

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions