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

feat: support function types #526

Closed
wants to merge 5 commits into from
Closed

Conversation

JohelEGP
Copy link
Contributor

@JohelEGP JohelEGP commented Jun 23, 2023

Resolves #525.
Resolves #502.
Resolves #343. Resolves #711. Resolves #718.
Partially addresses #387 (parameter-declaration-list).

Testing summary:

100% tests passed, 0 tests failed out of 776

Total Test time (real) =  39.42 sec

Acknowledgements:

@JohelEGP JohelEGP marked this pull request as ready for review June 24, 2023 02:43
@JohelEGP JohelEGP marked this pull request as draft June 24, 2023 21:56
@JohelEGP JohelEGP changed the title feat: accept function-type after type-id feat: support function types Jun 27, 2023
@JohelEGP JohelEGP marked this pull request as ready for review June 27, 2023 22:31
@@ -5783,7 +5895,7 @@ class cppfront
}
}
printer.preempt_position_push(n.position());
emit( *type );
emit( *type, false, true );
printer.preempt_position_pop();
// one pointer is enough for now, pointer-to-function fun can be later
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this about?
Is it an outdated comment now?

@JohelEGP JohelEGP marked this pull request as draft June 28, 2023 01:11
@JohelEGP JohelEGP marked this pull request as ready for review June 28, 2023 19:54
@JohelEGP
Copy link
Contributor Author

JohelEGP commented Jul 1, 2023

Title: Observable difference in template function's type.

Description:

This is due to the fact that
a template function's function parameter
with the type of a template parameter
doesn't use cpp2::in.

Minimal reproducer:

f: <T> (x: T) = { g: * (_: T) throws = f; }
main: ()      = { _ = f(:i32 = 0); }
Commands:
cppfront main.cpp2
clang++18 -std=c++23 -stdlib=libc++ -lc++abi -pedantic-errors -Wall -Wextra -Wconversion -I . main.cpp

Expected result:

I'm not sure.

Attempting to extent the exception in the description can only get you so far.
If you did it for literal * (_: T) throws,
the same wouldn't apply for h<T> which itself aliases * (_: T) throws.
If you did it for all other uses of T,
we lose out on the optimization opportunities brought by cpp2::in.

Actual result and error:

Cpp2 lowered to Cpp1:
#define CPP2_USE_MODULES         Yes

//=== Cpp2 type declarations ====================================================


#include "cpp2util.h"



//=== Cpp2 type definitions and function declarations ===========================

#line 1 "pure2-function-type-id.cpp2"
template<typename T> auto f(T const& x) -> void;
auto main() -> int;


//=== Cpp2 function definitions =================================================

#line 1 "pure2-function-type-id.cpp2"
template<typename T> auto f(T const& x) -> void{cpp2::fn_t<void(cpp2::in<T>)>* g {f}; }
auto main() -> int{(void) f(cpp2::i32{0});}

Output:
regression-tests/pure2-function-type-id/pure2-function-type-id.cpp2:1:83: error: address of overloaded function 'f' does not match required type 'void (int)'
    1 | template<typename T> auto f(T const& x) -> void{cpp2::fn_t<void(cpp2::in<T>)>* g {f}; }
      |                                                                                   ^
regression-tests/pure2-function-type-id/pure2-function-type-id.cpp2:2:27: note: in instantiation of function template specialization 'f<int>' requested here
    2 | auto main() -> int{(void) f(cpp2::i32{0});}
      |                           ^
regression-tests/pure2-function-type-id/pure2-function-type-id.cpp2:1:27: note: candidate template ignored: could not match 'const T &' against 'int'
    1 | template<typename T> auto f(T const& x) -> void{cpp2::fn_t<void(cpp2::in<T>)>* g {f}; }
      |                           ^
1 error generated.

@MaxSagebaum
Copy link
Contributor

LGTM.

In your current commits, there are always some white-space fixes. These clutter the commit a little bit. Maybe we can have a pull request, that fixes all the white-spaces once and for all.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Jul 3, 2023

Yeah.
I wanted to reduce having to manually adjust whitespace, even if just a little.
So I set my related IDE settings back on when working on Cppfront.
I suggest you set this when reviewing:
1688390206

@MaxSagebaum
Copy link
Contributor

Thanks for the hint, I did not know that this can be enabled in the github view.

Add _function-type_ as an alternative production of _type-id_.
A parameter's identifier must be named `_`.
`!throws` needs to be specified for Cpp1-`noexcept` function types.

Some examples:
```Cpp2
main: () = {
  f0: * () = :()      = {};
  f7: * (move _: i32) = :(move x: i32) = {};
  [[assert: inspect f0 -> bool {
    is () !throws = (std::terminate(), false);
    is * ()       = true;
    is _          = false;
  }]]
}

// Test case from hsutter#343.
f2: () -> std::function<(_: std::string) -> std::string> = {
  return :(s: std::string) -> std::string = { return s + " World!"; };
}

// Adapted from <https://github.com/hsutter/cppfront/wiki/Design-note%3A-Postfix-operators>.
            f:   (x: i32) -> * (_: i32) -> std::string = :(x: i32) -> std::string = "";
postfix_operators: () = {
  [[assert: f is (_: i32) -> * (_: i32) -> std::string]]
  //             /           |  |
  //            /            |  |
  [[assert: f(42)     is     * (_: i32) -> std::string]]
  //               _________/   |
  //              /             |
  [[assert: f(42)*     is      (_: i32) -> std::string]]
  //                   ________/
  //                  /
  [[assert: (f(42)*)(1)         is         std::string]]
}
`error C3537: you cannot cast to a type that contains 'auto'`.
See <https://compiler-explorer.com/z/Tfbj795s3> not working
and <https://compiler-explorer.com/z/31osnhrGo> working.
hsutter added a commit that referenced this pull request Jul 23, 2024
Hand-merged #526 into current main code

Some basic examples compile, but `reflect.h2` doesn't yet, needs investigation

I've commented out some error message emission on what appeared to be trial parses(?), because the presence of the error message stopped further processing
@hsutter
Copy link
Owner

hsutter commented Jul 23, 2024

Thanks again for this good PR, and deep apologies for the delay as I haven't kept up with PRs.

Because main has moved along since this PR, I took a pass at hand-merging this PR to rebase it to the current main and integrate it with some other features added since then. The result is #1177 ...

@hsutter
Copy link
Owner

hsutter commented Jul 27, 2024

Although I was not able to get a manually-rebased version of this working in #1177, I did try my own implementation in #1183 and was able to get that working, so I'll commit that in favor of #526 and #1177. But I still appreciate this, thanks again!

@JohelEGP JohelEGP deleted the function_type_id branch July 27, 2024 21:15
JohelEGP referenced this pull request Sep 22, 2024
…in <> lists (#1183)

* Starting over on function types - initial support for function types in <> lists

Should generally support std::function<> style uses

Aside: I'm happy to find that C++ allows parameter names in function types, thank you WG21! I hadn't seen those used in examples so I had been expecting that cppfront would have to suppress those, but std::function< int ( std::string& param_name ) > is legal code. Sweet.

This commit does not yet support pointer-to-function local variables...

* Add support for pointer-to-function variables

* Add docs examples for function typeids with std::function & *pfn

* Add support for pointer to function parameter types

* Support function type ids in type aliases
@JohelEGP JohelEGP mentioned this pull request Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment