Skip to content

[SUGGESTION] Generalize UFCS x.f(args) to try f<x>(args) #307

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

Closed
JohelEGP opened this issue Mar 29, 2023 · 18 comments
Closed

[SUGGESTION] Generalize UFCS x.f(args) to try f<x>(args) #307

JohelEGP opened this issue Mar 29, 2023 · 18 comments

Comments

@JohelEGP
Copy link
Contributor

JohelEGP commented Mar 29, 2023

will C++2 support User-defined Literals?

For authoring UDLs: I hadn't decided whether or not to support authoring them, and this thread convinces me we should try using UFCS for that first. It's desirable to solve the problem using a more general language feature, if it works well also for this use case, rather than adding a special-purpose language feature.
[...]
-- #284 (comment)

Something that can't be done in Cpp2 is implement https://github.com/hanickadot/compile-time-regular-expressions's second syntax, which is valid C++20:

ctre::match<"REGEX">(subject); // C++20
"REGEX"_ctre.match(subject); // C++17 + N3599 extension

"REGEX".ctre::match(subject) wouldn't try ctre::match<"REGEX">(subject).

It's desirable to solve the problem using a more general language feature

It's great that x.f() works regardless of x's type or whether x is a literal or not. Which is unlike C++1's UDL, which requires long double or long long parameter for a numeric UDL, and a literal argument for the nicer syntax.

What do you think about adding the above attempt to the UFCS feature?

Will your feature suggestion eliminate X% of security vulnerabilities of a given kind in current C++ code? No (my guess).

Will your feature suggestion automate or eliminate X% of current C++ guidance literature? No (my guess)

Describe alternatives you've considered.
Supporting UDLs, like C++1.

@filipsajdak
Copy link
Contributor

I can prepare a POC to have some tests on that.

I am happy that you bring CRTE - to be honest, one of my first contributions to the cppfront was is/as implementation. I did one experiment with CRTE and provided is overload for CRTE regex:

template <ctll::fixed_string pattern, typename X >
    requires std::is_convertible_v<X, std::string_view>
auto is( X const& x ) -> bool {
    return ctre::match<pattern>(x); 
}

That allows you to test something against regex in the following manner:

if email is "[\\-_a-zA-Z0-9.+!%]*@[\\-_a-zA-Z0-9.]*"  {
    std::cout << "correct email" << std::endl;
} else {
    std::cout << "incorrect email" << std::endl;
}

I did not propose that, as Herb is unwilling to include external dependencies, and I treat it as an experiment.

@JohelEGP
Copy link
Contributor Author

I noticed that the README doesn't mention a UFCS proposal. IIRC, Herb and Bjarne have worked on them together. I wanted to see what it said about this issue.

CRTE

You have the R and T backwards. I wonder if that has to do with the repository's renaming from CTRE. I haven't been following it.

I did not propose that, as Herb is unwilling to include external dependencies, and I treat it as an experiment.

That's good. is is supposed to be overloadable. The question would be, is overloading currently supported?

@filipsajdak
Copy link
Contributor

So... doing a quick and dirty feasibility check, I have modified the CPP2_UFCS macro to check what you asked it looks like this:

#define CPP2_UFCS(FUNCNAME,PARAM1,...) \
[&](auto&& obj, auto&& ...params) CPP2_FORCE_INLINE_LAMBDA -> decltype(auto) { \
    if constexpr (requires{ FUNCNAME<PARAM1>(std::forward<decltype(params)>(params)...); }) { \
        return FUNCNAME<PARAM1>(std::forward<decltype(params)>(params)...); \
    } \
    else if constexpr (requires{ std::forward<decltype(obj)>(obj).FUNCNAME(std::forward<decltype(params)>(params)...); }) { \
        return std::forward<decltype(obj)>(obj).FUNCNAME(std::forward<decltype(params)>(params)...); \
    } else { \
        return FUNCNAME(std::forward<decltype(obj)>(obj), std::forward<decltype(params)>(params)...); \
    } \
}(PARAM1, __VA_ARGS__)

And I have tried to compile the following code:

#include "ctre.hpp"

main: () = {

    "REGEX".ctre::match("Is this REGEX?");

}

And end up with an error:

../tests/experiment_crte.cpp2:5:21: error: use of variable template 'match' requires template arguments
    CPP2_UFCS(ctre::match, "REGEX", "Is this REGEX?");
                    ^
../tests/ctre.hpp:5503:92: note: template is declared here
template <CTRE_REGEX_INPUT_TYPE input, typename... Modifiers> static constexpr inline auto match = regular_expression<typename regex_builder<input>::type, match_method, ctll::list<singleline, Modifiers...>>();
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                              ^
1 error generated.

The issue is that the compiler is evaluating the last else part:

        return FUNCNAME(std::forward<decltype(obj)>(obj), std::forward<decltype(params)>(params)...); \

And fails as FUNCNAME is a template. When I remove the last else section, it compiles fine. So, the following implementation of CPP2_UFCS:

#define CPP2_UFCS(FUNCNAME,PARAM1,...) \
[&](auto&& obj, auto&& ...params) CPP2_FORCE_INLINE_LAMBDA -> decltype(auto) { \
    if constexpr (requires{ FUNCNAME<PARAM1>(std::forward<decltype(params)>(params)...); }) { \
        return FUNCNAME<PARAM1>(std::forward<decltype(params)>(params)...); \
    } \
    else if constexpr (requires{ std::forward<decltype(obj)>(obj).FUNCNAME(std::forward<decltype(params)>(params)...); }) { \
        return std::forward<decltype(obj)>(obj).FUNCNAME(std::forward<decltype(params)>(params)...); \
    } \
}(PARAM1, __VA_ARGS__)

It works, but we lost a match to the free function. We need to solve this issue if you want to proceed with this suggestion.

PS: sorry for misspelling CTRE with CRTE

@JohelEGP
Copy link
Contributor Author

That's weird. I feel like I've seen this before (was it here?). What if you wrapped the inner if-else in the else of the first if?

if (…) {…}
else {
  if (…) {…}
  else {…}
}

I also see that you rightly use PARAM1 rather than forwarding obj. It's a shame that PARAM1 has to be materialized even though it may remain unused, in this form of CPP2_UFCS. (This means that you may want to mark obj as [[maybe_unused]]).

@filipsajdak
Copy link
Contributor

I will try that when I get back to my PC.

@filipsajdak
Copy link
Contributor

filipsajdak commented Mar 29, 2023

This does not work (of course, I have used constexpr versions of ifs):

if (…) {…}
else {
  if (…) {…}
  else {…}
}

If you are interested in running that, I can spend some time finding a solution.

On the other hand, it is just syntactic sugar - I would like to focus more on more exciting topics.

You can call this with the following code:

#include "ctre.hpp"

main: () = {
    m := ctre::match<"(?<chars>[a-z]+)([0-9]+)">("abc123");

    if m {
        std::cout << m.get<"chars">() << std::endl;
        std::cout << m.get<2>() << std::endl;
    }
}

That generates:

auto main() -> int{

    auto m {ctre::match<"(?<chars>[a-z]+)([0-9]+)">("abc123")}; 

    if (std::move(m)) {
        std::cout << CPP2_UFCS_TEMPLATE_0(get, (<"chars">), m) << std::endl;
        std::cout << CPP2_UFCS_TEMPLATE_0(get, (<2>), std::move(m)) << std::endl;
    }
}

That compiles fine and returns:

../tests/experiment_crte.cpp2... ok (mixed Cpp1/Cpp2, Cpp2 code passes safety checks)

abc
123

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Mar 29, 2023

GCC also rejects the middle branch: https://compiler-explorer.com/z/MGqM3Mz5q.
With CPP2_UFCS_TEMPLATE: https://compiler-explorer.com/z/W1odG3hYb.

If you are interested in running that, I can spend some time finding a solution.

I'm interested, too. Not particularly about CTRE. I just used it as an example I knew would be familiar.

On the other hand, it is just syntactic sugar - I would like to focus more on more exciting topics.

Yeah. One that pulls it weight over UDLs. Simpler cases already work, so let's not dismiss this just yet: https://compiler-explorer.com/z/fbcT33W8W.

template<int> void f(int*);

struct X {
    constexpr X(const char(&)[2]) { }
};

template<X> void g(int);

int main() {
    CPP2_UFCS(f, 1, nullptr);
    CPP2_UFCS(g, "2", 3);
}

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Mar 30, 2023

Considering the inconsistent diagnosis between Clang and GCC (MSVC accepts), not using the macro-pasted template name as a template in a discarded statement must be IFNDR.

Forward declaring a function with the template name works for GCC in the simpler cases: https://compiler-explorer.com/z/nrK66xzvP; Clang diagnoses redefinition of '<variable template>' as different kind of symbol. Interestingly, GCC gives the same diagnosis as Clang when forward declaring as void FUNCNAME(...);. MSVC starts rejecting MF UFCS if FUNCNAME isn't a function at global scope, defining that FUNCNAME at global scope as any other kind of entity breaks all compilers. Despite f and h being function templates, it seems that h being constrained makes it not work without the forward declaration. And forward declaring doesn't help with CTRE: https://compiler-explorer.com/z/T9Kx7fGvW.

Another thing that came to mind as possible points against this suggestion are reasons for the rejection of "constexpr argument" proposals. The points I remember are f(x) not being obvious that it does one instantiation per value of x, and/or the compile-time costs of that. The same would apply to UFCS on x.f() performing f<x>().

@JohelEGP
Copy link
Contributor Author

MSVC is available, so I updated ^.

General summary:

  • It works fine for function templates.
  • When FUNCTION names a variable template, Clang fails.
  • The added branch makes MSVC fail UFCS when FUNCTION is a MF.
  • GCC and Clang fail with CTRE's variable template. MSVC works.

Support for x.f() translating to f<x>() when f is not a function template would require C++1 reflection.

@JohelEGP
Copy link
Contributor Author

PS: sorry for misspelling CTRE with CRTE

No problem. I committed the same mistake with regularity at the beginning. Now I remember it was due to CRTP.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Mar 31, 2023

Your generated code from #307 (comment) inserts a if (std::move(m)) without it being its last use.

@filipsajdak
Copy link
Contributor

Your generated code from #307 (comment) inserts a if (std::move(m)) without it being its last use.

Yes, that is true. It is a bug.

@filipsajdak
Copy link
Contributor

filipsajdak commented Mar 31, 2023

It looks like an issue with if. It also manifests in a simple case:

main: () = {
    b : bool = true;

    if b {
        std::cout << b << std::endl;
    }
}

Generates:

auto main() -> int{
    bool b {true}; 

    if (std::move(b)) {
        std::cout << std::move(b) << std::endl;
    }
}

I will create a bug report.

@filipsajdak
Copy link
Contributor

Reported here: #311

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Apr 2, 2023

CTRE works for all compilers with the forward declaration void FUNCNAME(…); and using namespace ctre;: https://compiler-explorer.com/z/1nePoxMvd.

The j problem on MSVC is worked around by inserting the new branch for UFCS in the middle: https://compiler-explorer.com/z/K9T8zrhfP.

Actually asserting the test cases shows that forward declaring FUNCTION doesn't help. Remove it, and h and i are diagnosed consistently on all compilers: https://compiler-explorer.com/z/b3YKqM8vd.

template<std::same_as<char>> constexpr char h(int) { return 'h'; }
template<int> constexpr auto i = [](std::same_as<bool> auto) { return 'i'; };
    static_assert('h' == CPP2_UFCS(h, '4', 5));
    static_assert('i' == CPP2_UFCS(i, 4, true));
GCC  : <source>:11:24: error: no matching function for call to 'h(char, int)'
Clang: <source>:35:36: error: no matching function for call to 'h'
MSVC : <source>(35): error C2672: 'h': no matching overloaded function found
GCC  : <source>:11:24: error: missing template arguments before '(' token
Clang: <source>:36:36: error: use of variable template 'i' requires template arguments
MSVC : <source>(36): error C3245: 'i': use of a variable template requires template argument list

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Apr 2, 2023

CTRE works for all compilers with the forward declaration void FUNCNAME(…); and using namespace ctre;: https://compiler-explorer.com/z/1nePoxMvd.

using namespace ctre; makes sense. UFCS shouldn't work on a qualified name (although only GCC seems to reject that case). Using the _ctre UDL also needs a using declaration.


h and i are diagnosed

Turns out I forgot to append auto to h's template parameter to make it an NTTP. It was a type-constraint. So it works now: https://compiler-explorer.com/z/8Mf9bonnE.

-template<std::same_as<char>> constexpr char h(int) { return 'h'; }
+template<std::same_as<char> auto> constexpr char h(int) { return 'h'; }

I'm torn on this issue. I still think it'd be great if UFCS could subsume UDL's use cases with more generality. But without C++1 reflection, it seems it'd only be possible to support function templates. Even then, with the new branch in the macro, a variable named after a member function breaks UFCS, an overload for the new template branch breaks the discarded free function branch (commented lines at https://compiler-explorer.com/z/rT9cP3oMn give consistent diagnostics between all compilers), and there's probably more unknowns. And then there's:

Another thing that came to mind as possible points against this suggestion are reasons for the rejection of "constexpr argument" proposals. The points I remember are f(x) not being obvious that it does one instantiation per value of x, and/or the compile-time costs of that. The same would apply to UFCS on x.f() performing f<x>().
-- #307 (comment)

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Apr 4, 2023

an overload for the new template branch breaks the discarded free function branch

h<'4'>(5) was ambiguous, so it wasn't discarded. Redone: https://compiler-explorer.com/z/7zWcM9PbK.

@hsutter
Copy link
Owner

hsutter commented Apr 9, 2023

Thanks everyone for this discussion. It sounds to me like this would still be hard/fragile to implement. But on a more principled level, right now we have a clear distinction where the member function call syntax x.f() does UFCS which has as a major benefit that the argument comes first (for easier Intellisense etc.), and the ordinary function call syntax f(x) doesn't do UFCS. I worry that f<x>() would be blurring that line by pushing UFCS into the other ordinary function call syntax, and also not get the Intellisense benefits. So I'll close this one for now, but if you think I'm missing something please feel free to reopen. Thanks for understanding, and for the insights above!

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

No branches or pull requests

3 participants