Skip to content

[QUESTION] how to express this code in CppFront (is it possible)? #1235

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
germandiagogomez opened this issue Aug 18, 2024 · 13 comments · Fixed by #1262
Closed

[QUESTION] how to express this code in CppFront (is it possible)? #1235

germandiagogomez opened this issue Aug 18, 2024 · 13 comments · Fixed by #1262
Labels
bug Something isn't working

Comments

@germandiagogomez
Copy link

Hello community,

I am trying to translate this piece of C++ code into Cppfront:

template<class AssocContainer, class Func = std::plus<>>
void
combine_maps(AssocContainer& map1, const AssocContainer& map2, Func func = {})
{
    for (const auto& [k, v] : map2)
        map1[k] = func(map1[k], v);
}

And I tried this:

combine_maps:<AssocContainer, Func: type = std::plus<>>(inout map1: AssocContainer, map2: AssocContainer, func: Func = ()) = {
    for map2 do(kv) {
        map1[kv.first] = func(map1[kv.first], kv.second);
    }
}

My questions are simple:

  1. is there a way to default the template parameter to std::plus<> as in plain C++?
  2. is there a way to bind key and value in Cpp2 in a similar way to a structured binding? Maybe with pattern matching of some kind?
  3. can I default func: Func = () parameter? It seems to emit code where there is const Func & func = <empty here>

Thank you for your time and support to everyone.

@germandiagogomez germandiagogomez added the bug Something isn't working label Aug 18, 2024
@MaxSagebaum
Copy link
Contributor

Default arguments are currently not supported in cppfront. But Herb plans on doing so. Until then one would currently write:

combine_maps:<AssocContainer>(inout map1: AssocContainer, map2: AssocContainer, func) = {
    for map2 do(kv) {
        map1[kv.first] = func(map1[kv.first], kv.second);
    }
}

combine_maps:<AssocContainer>(inout map1: AssocContainer, map2: AssocContainer) = combine_maps(map1, map2, std::plus<>());

I currently can not say anything about the structured binding. But this is probably also on the todo list.

@DyXel
Copy link
Contributor

DyXel commented Aug 19, 2024

Default arguments are currently not supported in cppfront. But Herb plans on doing so.

This was already done as part of: 873b760, so I think this is just a bug, although the C++ version is taking func by value, so maybe try copy passing style in Cpp2 version?

@germandiagogomez
Copy link
Author

germandiagogomez commented Aug 19, 2024

@MaxSagebaum another question and struggle I keep having about syntax (that you used):

Is the syntax below valid cpp2?

f(std::plus<>())

For what I read in the docs I would assume that no. To create a temporary (that is a function call in theory I assume and initialization and function calls seem to have disjoint syntax in cpp2), you should do the following:

f(:std::plus<> = ())

Is my assumption correct?

@MaxSagebaum
Copy link
Contributor

This was already done as part of: 873b760, so I think this is just a bug, although the C++ version is taking func by value, so maybe try copy passing style in Cpp2 version?

Ah ok, my bet.

Is the syntax below valid cpp2?

f(std::plus<>())

Yes you are just creating an object via the constructor call and then call the function f. What you have written would be kind of the definition syntax for f but here the placeholder for the variable is missing: f(_ : std::plus<> = ()) = { ... }. So you have an unnamed variable. which is of the type std::plus<>.

Does this answer your question?

@germandiagogomez
Copy link
Author

Yes you are just creating an object via the constructor call and then call the function f

In C++ yes, what I am not sure is if that is valid Cppfront syntax for temporary construction, as far as I understand, and assuming the grammar is context-free, std::plus<>() would be ambiguous from a context-free grammar point of view.

So correct me if I am wrong, but this is what I believe to know so far:

this_is_always_a_function_call();

// That would be a constructor in C++, but in CppFront a function call also.
// Otherwise, the grammar is not context-free anymore
std::plus<>(); 

// This is a constructor indeed, for a temporary
:std::plus<> = ();

I do not see anywhere (and had compile errors I recall) where you can use function syntax to initialize a variable via a constructor. But I might be wrong and have missed something. I read all the docs for CppFront twice.

@MaxSagebaum
Copy link
Contributor

You might be right that this is per se not context free. But it is currently working. The current disambiguation rules I do not know by heart but I think cppfront uses the following:

  • Matching < and > parse as template. If there is an intermediate < or > throw an error and remark about the disambiguation by using brackets e.g. my_template<(a>b)> instead of my_templateb>`.
  • A non matching < or > is parsed as the operator.

@hsutter: Is this correct. Maybe we should add it to the documentation.

@germandiagogomez
Copy link
Author

@MaxSagebaum not sure if you are following what I meant, actually it was not about the template parameters ambiguity. What is ambiguous is this syntax (in case it is allowed in CppFront the first thing):

MyType()
// versus
MyFunction()

In order to know that the first is a constructor call and not a function call, I need to know that MyType is a type and not a function, so I need to access the symbols table. My understanding is that this is why Cppfront has this syntax for temporary objects (and different from functions syntax):

:MyType = ();

That is unambiguously initializing an object. As it can be seen it does not overlap the call syntax (it has an equals sign in-between the parameter list and the type) and you do not need to know if your symbol is a type or a function, so you do not need to disambiguate. You know by the syntax only it initializes a type.

I wanted to confirm if your std::plus<>() syntax created an object in Cppfront as per the spec (actually it could work by accident). If this is the case, then the grammar is not context-free.

But I do not really care if it is context-free or not, just if it is legal creating an object without = in yhe middle as if it looked like a function call.

@DyXel
Copy link
Contributor

DyXel commented Aug 20, 2024

The reason that std::plus<>(); (without the :) is allowed, is merely a limitation of cppfront not being able to tell between a type and a function. The lowered output to C++ happens to work because C++ compilers are smart enough to disambiguate the 2 cases. In a stand-alone Cpp2 compiler (not transpiler) I believe std::plus<>(); like this would error out.

@germandiagogomez
Copy link
Author

The reason that std::plus<>(); (without the :) is allowed, is merely a limitation of cppfront not being able to tell between a type and a function. The lowered output to C++ happens to work because C++ compilers are smart enough to disambiguate the 2 cases. In a stand-alone Cpp2 compiler (not transpiler) I believe std::plus<>(); like this would error out.

Ok, this was my thesis also. So the correct syntax is the unambiguous.

hsutter added a commit that referenced this issue Aug 27, 2024
There should now be enough test cases to exercise all the places you can put default arguments

Also, only emit Cpp1 lambdas as 'mutable' if there are captures (when it matters) so that pure function expressions are not 'mutable'

Closes #1235
@hsutter
Copy link
Owner

hsutter commented Aug 27, 2024

Thanks! I see there were still some bugs here, and I have a PR ready that will fix those.


Re questions 1 and 3: With the above-linked PR, you can write this:

combine_maps:
    < AssocContainer, Func: type = std::plus<> >
    ( inout map1: AssocContainer, map2: AssocContainer, func: Func = () )
= {
    for map2 do(kv) {
        map1[kv.first] = func(map1[kv.first], kv.second);
    }
}

main: () = {
    m1: std::map<int, int> = ();
    m1[1] = 11;

    m2: std::map<int, int> = ();
    m2[1] = 22;

    combine_maps( m1, m2, :(x,y) x+y+33 );

    std::cout << "(m1.size())$, (m2.size())$, (m1[1])$\n";  // prints: 1, 1, 66
}

I just haven't committed PR #1262 directly yet because I want to keep these changes out of the way of re-rebasing #1250. Once #1250 is merged, I'll merge #1262 too and your example will work out of the box. Thanks for pointing this out!


Re question 2: No, I haven't implemented decomposition / structured-bindings yet. It's on the list!


Re the question about expression-scope function and object syntax:

You can always write expression-scope functions and objects using the same syntax as always, but omit the name. For example, if T is a type, you can write the expression f( :T=value ).

The syntax f( X(value) ) also works when X is the name of an ordinary function or a type (and therefore a constructor name since a constructor is a function). X(value) is just an expression. If X is a type, the only difference between the expressions X(value) and :X=value is that they lower to Cpp1 () and {} initialization respectively.

@hsutter
Copy link
Owner

hsutter commented Aug 27, 2024

The above code now works in main. Thanks for the bug reports!

@germandiagogomez
Copy link
Author

Thanks! I see there were still some bugs here, and I have a PR ready that will fix those.

Re questions 1 and 3: With the above-linked PR, you can write this:

combine_maps:
    < AssocContainer, Func: type = std::plus<> >
    ( inout map1: AssocContainer, map2: AssocContainer, func: Func = () )
= {
    for map2 do(kv) {
        map1[kv.first] = func(map1[kv.first], kv.second);
    }
}

main: () = {
    m1: std::map<int, int> = ();
    m1[1] = 11;

    m2: std::map<int, int> = ();
    m2[1] = 22;

    combine_maps( m1, m2, :(x,y) x+y+33 );

    std::cout << "(m1.size())$, (m2.size())$, (m1[1])$\n";  // prints: 1, 1, 66
}

Great! That is exactly what I wanted signature-wise. Thanks for your hard work.

I just haven't committed PR #1262 directly yet because I want to keep these changes out of the way of re-rebasing #1250. Once #1250 is merged, I'll merge #1262 too and your example will work out of the box. Thanks for pointing this out!

You are welcome.

Re question 2: No, I haven't implemented decomposition / structured-bindings yet. It's on the list!

The syntax f( X(value) ) also works when X is the name of an ordinary function or a type (and therefore a constructor name since a constructor is a function).X(value) is just an expression. If X is a type, the only difference between the expressions X(value) and :X=value is that they lower to Cpp1 () and {} initialization respectively.

Please confirm this so that I write proper Cpp2 then... my understanding is that std::plus<>() and :std::plus<> = () are both legal because a constructor is a function expression. This means that it is intentional and not an accident when lowering the code to C++, and both syntaxes are correct. Is that the case?

@hsutter
Copy link
Owner

hsutter commented Aug 31, 2024

Please confirm this so that I write proper Cpp2 then... my understanding is that std::plus<>() and :std::plus<> = () are both legal because a constructor is a function expression. This means that it is intentional and not an accident when lowering the code to C++, and both syntaxes are correct. Is that the case?

Correct, the only difference is () vs {} intialization in the Cpp1 code.

This Cpp2 code:

f( std::plus<>() );
f( :std::plus<> = () );

Lowers to this Cpp1 code:

f(std::plus<>());  // note: ( )
f(std::plus<>{});  // note: { }

Note: This particular example did expose a bug where I was checking to suppress redundant {} on expression lists everywhere except when they were empty for a default initializer... d'oh! Fixed, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants