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

fix: emit optimizing inspect #529

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

fix: emit optimizing inspect #529

wants to merge 7 commits into from

Conversation

JohelEGP
Copy link
Contributor

@JohelEGP JohelEGP commented Jun 29, 2023

Resolves #492.
Resolves #433.
Closes #491.

Testing summary:

100% tests passed, 0 tests failed out of 676

Total Test time (real) =  98.81 sec

Acknowledgements:

JohelEGP added 7 commits June 28, 2023 21:16
This Cpp2 statement:
```Cpp2
    return inspect v -> std::string {
        is std::vector  = "std::vector";
        is std::array   = "std::array";
        is std::variant = "std::variant";
        is my_type      = "my_type";
        is _ = "unknown";
    };
```

Changes from being lowered to this Cpp1 statement:
```C++
    return [&] () -> std::string { auto&& __expr = v;
        if (cpp2::is<std::vector>(__expr)) { if constexpr( requires{"std::vector";} ) if constexpr( std::is_convertible_v<CPP2_TYPEOF(("std::vector")),std::string> ) return "std::vector"; else return std::string{}; else return std::string{}; }
        else if (cpp2::is<std::array>(__expr)) { if constexpr( requires{"std::array";} ) if constexpr( std::is_convertible_v<CPP2_TYPEOF(("std::array")),std::string> ) return "std::array"; else return std::string{}; else return std::string{}; }
        else if (cpp2::is<std::variant>(__expr)) { if constexpr( requires{"std::variant";} ) if constexpr( std::is_convertible_v<CPP2_TYPEOF(("std::variant")),std::string> ) return "std::variant"; else return std::string{}; else return std::string{}; }
        else if (cpp2::is<my_type>(__expr)) { if constexpr( requires{"my_type";} ) if constexpr( std::is_convertible_v<CPP2_TYPEOF(("my_type")),std::string> ) return "my_type"; else return std::string{}; else return std::string{}; }
        return "unknown"; }
    ();
```

To being lowered to this Cpp1 statement:
```C++
    return [&] () -> std::string { auto&& __expr = v;
        if constexpr (requires { cpp2::is<std::vector>(__expr); }) { if constexpr (!std::is_same_v<decltype(cpp2::is<std::vector>(__expr)), std::false_type>) { if constexpr (std::is_same_v<decltype(cpp2::is<std::vector>(__expr)), std::true_type>) { return "std::vector"; } else { if (cpp2::is<std::vector>(__expr)) { return "std::vector"; } } } }
        if constexpr (requires { cpp2::is<std::array>(__expr); }) { if constexpr (!std::is_same_v<decltype(cpp2::is<std::array>(__expr)), std::false_type>) { if constexpr (std::is_same_v<decltype(cpp2::is<std::array>(__expr)), std::true_type>) { return "std::array"; } else { if (cpp2::is<std::array>(__expr)) { return "std::array"; } } } }
        if constexpr (requires { cpp2::is<std::variant>(__expr); }) { if constexpr (!std::is_same_v<decltype(cpp2::is<std::variant>(__expr)), std::false_type>) { if constexpr (std::is_same_v<decltype(cpp2::is<std::variant>(__expr)), std::true_type>) { return "std::variant";  } else { if (cpp2::is<std::variant>(__expr)) { return "std::variant";  } } } }
        if constexpr (requires { cpp2::is<my_type>(__expr); }) { if constexpr (!std::is_same_v<decltype(cpp2::is<my_type>(__expr)), std::false_type>) { if constexpr (std::is_same_v<decltype(cpp2::is<my_type>(__expr)), std::true_type>) { return "my_type"; } else { if (cpp2::is<my_type>(__expr)) { return "my_type"; } } } }
        return "unknown";
    }();
```

This is the first lowered alternative after formatting:
```C++
  if constexpr (requires { cpp2::is<std::vector>(__expr); }) {
    if constexpr (!std::is_same_v<decltype(cpp2::is<std::vector>(__expr)),
                                  std::false_type>) {
      if constexpr (std::is_same_v<decltype(cpp2::is<std::vector>(__expr)),
                                   std::true_type>) {
        return "std::vector";
      } else {
        if (cpp2::is<std::vector>(__expr)) {
          return "std::vector";
        }
      }
    }
  }
```
@JohelEGP JohelEGP marked this pull request as ready for review June 29, 2023 23:04
// The alternative is elided due to the `is` being ambiguous.
// Like in P2392, the cases of the built-in `is` should be a chain of conditions.
// Using overloads to implement that is tedious and error-prone.
is std::ranges::view_interface<std::ranges::subrange<std::add_pointer_t<i32>>> = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not a [BUG].
This is not a [SUGGESTION] (but could be taken as one).

I have thought of having the function expression's signature be <T> (x: T)
so that I could use T::view_interface in the line I'm commenting on.
But it doesn't work because it's missing the lowered Cpp1 typename in front of it.

Cppfront already recognizes a template function's template parameters in its function parameter list to omit wrapping a function parameter's type in cpp2::in.
So do you think it'd make sense to extend Cpp2 to not require typename in such type-only contexts?

I'm also wondering how you plan to support such disambiguating typename in Cpp2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The more I think about it,
the more I'm convinced that
"yes, this should be a thing!"

This is basically Cpp1's optional typename,
but for Cpp2's type-only contexts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opened #531 for this.

@JohelEGP

This comment was marked as off-topic.

@JohelEGP JohelEGP marked this pull request as ready for review November 20, 2023 20:34
@hsutter
Copy link
Owner

hsutter commented Oct 31, 2024

Hi! Sorry it took me so long to get to this one... this one looks like it might be out of date, should I close it for now and you can reopen this or create a new PR when it's ready? Again, my apologies for not keeping up.

@hsutter hsutter added the question - further information requested Further information is requested label Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question - further information requested Further information is requested
Projects
None yet
2 participants