[BUG] operator is should be constrained or answered statically

@JohelEGP

Title: operator is should be constrained or answered statically.

Description:

Constrained: operator is should be non-viable given a type-unsafe input.
Answered statically: See https://quuxplusone.github.io/blog/2022/01/04/test-constexpr-friendliness/.

This is to implement P2392's §4.2.
It's also to correctly resolve #433,
as split off from #491:

Comments moved from #491.

The case of this PR is:

The alternative's is-as-expression-target matches
but the statement is ill-formed for the inspected type.

P2392 doesn't say what should happen in this case.
If one extracted the wording from the design, it would be UB by omission

IIUC what P1371 says, then it would similarly be UB by omission:

When inspect is executed, its condition is evaluated and matched in order (first match semantics) against each
pattern. If a pattern successfully matches the value of the condition and the boolean expression in the guard
evaluates to true (or if there is no guard at all), then the value of the resulting expression is yielded or control is
passed to the compound statement, depending on whether the inspect yields a value. If the guard expression
evaluates to false, control flows to the subsequent pattern.

If no pattern matches, none of the expressions or compound statements specified are executed. In that case if
the inspect expression yields void, control is passed to the next statement. If the inspect expression does not
yield void, std::terminate will be called.

It also links to
https://github.com/solodon4/Mach7,
https://github.com/mpark/patterns, and
https://github.com/jbandela/simple_match.
But this corner case doesn't seem to be mentioned in their README.

Originally posted by @JohelEGP in #491 (comment)

How about this instead?

For a dependent inspect,
when the is-as-expression-target of an alternative is well-formed,
the corresponding statement should also be well-formed.

So we move
from a runtime-checked contract
to a compile-time assertion.

Originally posted by @JohelEGP in #491 (comment)

UB by omission

Actually,
the proposals are clear that when there's a match,
execution continues at the corresponding statement.

The proposals are also clear in that
an ill-formed condition means the whole branch is a discarded statement.

So it falls out from the existing rules
that a well-formed condition implies the corresponding statement is also instantiated.

The actual bug is
that the validity check happens for the statement
and not at the condition (and thus for the whole branch).

Another bug is that an is-expression is always be well-formed,
resulting in always-false when the equivalent without is would be ill-formed.

There is an example at P2392 §3.4.3
that mixes conditions that don't work for all intended types.
It can demonstrate the issue.
Here is a reduction: https://cpp2.godbolt.org/z/c5rsjP6a7.

in: (min, max) -> _ = :<T> (x: T) -> bool requires std::integral<T> = { return min$ <= x <= max$; };
f: <T> (x: T) -> _ = {
  return inspect x -> std::string {
    is std::string = "a string";
    is (in(1, 2)) = "1 or 2";
    is _ = "something else";
  };
}
main: () = {
  s: std::string = ();
  std::cout << "(f(s))$\n"             // prints "a string".
            << "(f(1))$\n"             // prints "1 or 2".
            << "(f(42))$\n"            // prints "something else".
            << "(s is (in(1, 2)))$\n"; // prints "0".
}

Notice how s is (in(1, 2)) is well-formed.
It should be ill-formed outside a dependent inspect.
As the condition of the alternative of a dependent inspect,
it should make the alternative discarded.

Originally posted by @JohelEGP in #491 (comment)

Consider this degenerate case:

A debug build.
An inspect of 100 alternatives:
- First, 99 alternatives with a condition of a predicate well-formed for integers only,
- then, the match-all is _.

Because a condition is always well-formed,
an always-false condition for a given input type isn't optimized out.

Given an input std::string,
the first 99 conditions are necessarily evaluated sequentially,
even though they're all always-false.

Originally posted by @JohelEGP in #491 (comment)

Let's take the same inspect again.

  return inspect x -> std::string {
    is std::string = "a string";
    is (in(1, 2)) = "1 or 2";
    is _ = "something else";
  };

For the given conditions,
the operator is overloads of std::variant<std::string, i32>
should be well-formed and necessarily runtime-evaluated.
The same for std::variant<std::monostate> should be ill-formed.
So std::variant::operator is should only work
at runtime and when the query can be forwarded to one of its variant alternatives.
Nothing changes the fact that if the condition is well-formed, the statement should be instantiated.

Originally posted by @JohelEGP in #491 (comment)

Once operator is is constrained or answered statically,
it becomes necessary to fix inspect
to omit alternatives whose conditions are non-viable.
It also becomes possible
to omit alternatives whose conditions always-false.

Note that there's a distinction between an operator is that is constrained and answered statically.
A constrained operator is has preconditions on the input type.
There are other contexts where we might want the semantics of an inspect's alternative's condition.

g: <T> (x: T) = {
  // Continues working.
  if constexpr T is std::string { }

  // The following can work given C++23's P2280 (C++20 DR).

  if constexpr x is std::string { }
  // Will be `true` for `T == int` (using the built-in `operator is`).
  // For a `T` that is a specialization of `std::variant`,
  // it would be ill-formed,
  // but can be made to work
  // if given the semantics of the condition of an `inspect`'s alternative
  // (i.e., always-`false`).

  if constexpr x is 0 { }
  // Can be `true` for `T == std::integral_constant<i32, 0>` if overloaded.
}

Minimal reproducer (https://cpp2.godbolt.org/z/87hjsccYd):

in: (min, max) -> _ = :<T> (x: T) -> bool requires std::integral<T> = { return min$ <= x <= max$; };
f: <T> (x: T) -> _ = {
  return inspect x -> std::string {
    is std::string = :(x) -> _ = { return "a string"; }();
    is (in(1, 2)) = "1 or 2";
    is _ = "something else";
  };
}
main: () = {
  s: std::string = ();
  std::cout        //
    << "(f(s))$\n" // prints ``.
    << "(f(1))$\n" // prints `1 or 2`.
    ;
  std::cout                   //
    << "(s is (in(1, 2)))$\n" // prints `0`.
    ;
}

Commands:

cppfront -clean-cpp1 main.cpp2
clang++17 -std=c++23 -stdlib=libc++ -lc++abi -pedantic-errors -Wall -Wextra -Wconversion -I . main.cpp

Expected result:

f(s): An error when instantiating :(x) -> _ = { return "a string"; }().
f(1): Alternative with is std::string to be statically elided.
s is (in(1, 2)): An error, just like in(1, 2)(s).

Actual result and error:

f(s): Unconditionally returns std::string() because the matched alternative's statement is ill-formed.
f(1): Unconditionally evaluates is std::string that will always be false.
s is (in(1, 2)): Unconditionally results in false, even though in(1, 2)(s) is ill-formed.

Cpp2 lowered to Cpp1.

#include "cpp2util.h"


[[nodiscard]] auto in(auto const& min, auto const& max) -> auto;
template<typename T> [[nodiscard]] auto f(T const& x) -> auto;


auto main() -> int;

[[nodiscard]] auto in(auto const& min, auto const& max) -> auto { return [_0 = min, _1 = max]<typename T>(T const& x) -> bool
requires (std::integral<T>)
{return [_0 = _0, _1 = x, _2 = _1]{ return cpp2::cmp_less_eq(_0,_1) && cpp2::cmp_less_eq(_1,_2); }(); };  }
template<typename T> [[nodiscard]] auto f(T const& x) -> auto{
  return [&] () -> std::string { auto&& __expr = x;
    if (cpp2::is<std::string>(__expr)) { if constexpr( requires{[](auto const& x) -> auto{return "a string"; }();} ) if constexpr( std::is_convertible_v<CPP2_TYPEOF(([](auto const& x) -> auto{return "a string"; }())),std::string> ) return [](auto const& x) -> auto{return "a string"; }(); else return std::string{}; else return std::string{}; }
    else if (cpp2::is(__expr, (in(1, 2)))) { if constexpr( requires{"1 or 2";} ) if constexpr( std::is_convertible_v<CPP2_TYPEOF(("1 or 2")),std::string> ) return "1 or 2"; else return std::string{}; else return std::string{}; }
    else return "something else"; }
  ();
}
auto main() -> int{
  std::string s {};
  std::cout        //
    << cpp2::to_string(f(s)) + "\n" // prints ``.
    << cpp2::to_string(f(1)) + "\n";// prints `1 or 2`.

  std::cout                   //
    << cpp2::to_string(cpp2::is(std::move(s), (in(1, 2)))) + "\n";// prints `0`.

}

Output.

Program returned: 0

1 or 2
0

There are other contexts where we might want the semantics of an inspect's alternative's condition.

See also P2392's §2.1.2, §4.1.8.

A: It's a better spelling for a chain of `if constexpr` `else`.

Given a resolution for this issue,
what's the use case for inspect constexpr?
From §3.1:

inspect constexpr requires expr and all alternative conditions to be compile time constant expressions. All
non-selected alternative results are discarded, and their return statements do not participate in function return
type deduction.

as returning cpp2::nonesuch needs to be similarly reviewed.

It also becomes possible
to omit alternatives whose conditions always-false.

It's not OK, it's necessary.
To omit them.
Otherwise the following would be ill-formed.

f: <T> (x: T) -> _ = {
  return inspect x -> std::string {
    is std::string = x.substr(0);
    is _ = "something else";
  };
}
main: () = {
  std::cout        //
    << "(f(1))$\n" // error: `1.substr(0)`.
    ;
}

All compilers agree that this can be made to work if I just add constexpr to relevant is overloads: https://compiler-explorer.com/z/j5q9co99c.

I agree that constexpr is necessary for the pure type matching. This also becomes critical if inspect is used in the context of a template parameter.

Did you create a test file where all common cases are evaluated?

Did you create a test file where all common cases are evaluated?

The one in the OP works,
thought you have to inspect the generated code to see they are:

f(1): Unconditionally evaluates is std::string that will always be false.

All compilers agree that this can be made to work if I just add constexpr to relevant is overloads: https://compiler-explorer.com/z/j5q9co99c.

But not to the point necessary to optimize inspect.
It definitely doesn't work in dependent contexts: https://compiler-explorer.com/z/3jsE8oz4z.
Seems like support for P2280 will be necessary.

It should still be possible
to fix these bugs
and continue supporting currently supported compilers
(so support for P2280 is not needed).

Have operator is either
return bool,
return std::bool_constant<B> for some B, or
be non-viable (for type-unsafe inputs).

Then, we can optimize inspect
by inspecting each alternative's condition
in the condition of a generated constexpr if that guards the alternative.

If the chosen operator is overload returns std::false_type, or
if there is no viable operator is,
the alternative is statically elided.
If the chosen operator is overload returns std::true_type,
the alternative's statement is unconditionally evaluated.
If the chosen operator is overload returns bool,
the alternative's statement is conditionally evaluated on it.

This is how it should look: https://cpp2.godbolt.org/z/s17G457aP.

#include <functional>
operator_is: <T, U> (_: U) -> _ = std::bool_constant<std::is_same_v<T, U>>();
operator_is: <T, F> (v: T, f: F) -> bool requires std::predicate<decltype((f)), decltype((v))> = { return std::invoke(f, v); }
template <class T, class U> concept has_type_operator_is = requires(U u) { operator_is<T>(u); };
template <class T, class U> concept has_value_operator_is = requires(T t, U u) { operator_is(t, u); };
in: (min, max) -> _ = :<T> (x: T) -> bool requires std::integral<T> = { return min$ <= x <= max$; };
f: <T> (x: T) -> _ = {
  // return inspect x -> std::string {
  //   is std::string = :(x) -> _ = { return "a string"; }();
  //   is (in(1, 2)) = "1 or 2";
  //   is _ = "something else";
  // };
  if constexpr has_type_operator_is<std::string, decltype((x))> {
    if constexpr !std::is_same_v<decltype(operator_is<std::string>(x)), std::false_type> {
      if constexpr std::is_same_v<decltype(operator_is<std::string>(x)), std::true_type> {
        return :(x) -> _ = { return "a string"; }();
      } else {
        if operator_is<std::string>(x) {
          return :(x) -> _ = { return "a string"; }();
        }
      }
    }
  }
  else
  if constexpr has_value_operator_is<decltype((x)), decltype(in(1, 2))> {
    if constexpr !std::is_same_v<decltype(operator_is(x, in(1, 2))), std::false_type> {
      if constexpr std::is_same_v<decltype(operator_is(x, in(1, 2))), std::true_type> {
        return "1 or 2";
      } else {
        if operator_is(x, in(1, 2)) {
          return "1 or 2";
        }
      }
    }
  }
  return "something else";
}
main: () = {
  s: std::string = ();
  std::cout        //
    // << "(f(s))$\n" // Now errors during instantiation.
    << "(f(1))$\n" // Now statically elides `is std::string`-alternative, but doesn't return `"1 or 2"`, why?
    ;
  // std::cout                   //
  //   << "(operator_is(s, in(1, 2)))$\n" // Still fails at link-time rather than compile-time, why?
  //   ;
}

I would guess it is the wrong forward declaration of operator_is:

template<typename T, typename F> [[nodiscard]] auto operator_is(T const& v, F const& f) -> bool;

The requirement is missing. This makes the compiler think that there is an alternative definition which leads to the linker error.

On a quick check, I could not find if the requires clause can be defined in a forward declaration.

Oh, right. That's #323 (comment).

Support more unary and binary fold-expressions

It should still be possible
to fix these bugs
and continue supporting currently supported compilers
(so support for P2280 is not needed).

Have operator is either
return bool,
return std::bool_constant<B> for some B, or
be non-viable (for type-unsafe inputs).

This is actually necessary since an operator is isn't necessarily constexpr.

I will take a look on your cases and will check if they are fixed or might be fixed in #701

It certainly is an improvement.

Navigation Menu

Repository navigation

[BUG] `operator is` should be constrained or answered statically #492

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

[BUG] operator is should be constrained or answered statically #492

Description

Activity

JohelEGP commented on Jun 4, 2023

JohelEGP commented on Jun 7, 2023

JohelEGP commented on Jun 20, 2023

JohelEGP commented on Jun 20, 2023

MaxSagebaum commented on Jun 20, 2023

JohelEGP commented on Jun 20, 2023

JohelEGP commented on Jun 20, 2023

JohelEGP commented on Jun 20, 2023

JohelEGP commented on Jun 20, 2023

MaxSagebaum commented on Jun 21, 2023

JohelEGP commented on Jun 21, 2023

JohelEGP commented on Dec 3, 2023

filipsajdak commented on Dec 4, 2023

JohelEGP commented on Dec 4, 2023

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

[BUG] `operator is` should be constrained or answered statically #492