[Adding P1371's authors to the CC list. The unabridged original of Mikhail's post to std-proposals is here.]

On Sun, Jan 26, 2020 at 9:07 AM Михаил Найденов via Std-Proposals <std-proposals@lists.isocpp.org> wrote:

Hello, I would like to share my core concerns with the current PM proposal.

1. I don't see the need for the dereference pattern.

This pattern introduces (extreme) noise and unfamiliar (to put it mildly) look into the code.

It is not needed, because 
A) in PM we can always have implicit test and dereference 
B) the user can use a extractor for the *! case

The "dereference pattern" (named that in the paper) is pattern-matching on the pointee of a pointer. The non-trivial example from the paper is:

enum Color { Red, Black };

template<class T>
struct Node {
    void balance();
    Color color;
    std::shared_ptr<Node> lhs;
    T value;
    std::shared_ptr<Node> rhs;
};

template<class T>
void Node<T>::balance() {
    *this = inspect (*this) {
        [case Black, (*?) [case Red, (*?) [case Red, a, x, b], y, c], z, d]
            => Node{Red, std::make_shared<Node>(Black, a, x, b),
                    y, std::make_shared<Node>(Black, c, z, d)},
        [case Black, (*?) [case Red, a, x, (*?) [case Red, b, y, c]], z, d] // left-right case
            => Node{Red, std::make_shared<Node>(Black, a, x, b),
                    y, std::make_shared<Node>(Black, c, z, d)},
        [case Black, a, x, (*?) [case Red, (*?) [case Red, b, y, c], z, d]] // right-left case
            => Node{Red, std::make_shared<Node>(Black, a, x, b),
                    y, std::make_shared<Node>(Black, c, z, d)},
        [case Black, a, x, (*?) [case Red, b, y, (*?) [case Red, c, z, d]]] // right-right case
            => Node{Red, std::make_shared<Node>(Black, a, x, b),
                    y, std::make_shared<Node>(Black, c, z, d)},
        self => self // do nothing
    };
}

If I understand your "extractor pattern" idea correctly, you're suggesting that it could be rewritten as follows:

enum Color { Red, Black };

template<class T>
struct Node {
    void balance();
    Color color;
    std::shared_ptr<Node> lhs;
    T value;
    std::shared_ptr<Node> rhs;
};

namespace std {
    struct Deref {
        template<class T>
        static const T& extract(const T *p) { return *p; }
        template<class T>
        static const T *try_extract(const T *p) { return p; }
    };
    inline constexpr Deref deref;
}

template<class T>
void Node<T>::balance() {
    *this = inspect (*this) {
        [case Black, (std::deref?) [case Red, (std::deref?) [case Red, a, x, b], y, c], z, d]
            => Node{Red, std::make_shared<Node>(Black, a, x, b),
                    y, std::make_shared<Node>(Black, c, z, d)},
        [case Black, (std::deref?) [case Red, a, x, (std::deref?) [case Red, b, y, c]], z, d] // left-right case
            => Node{Red, std::make_shared<Node>(Black, a, x, b),
                    y, std::make_shared<Node>(Black, c, z, d)},
        [case Black, a, x, (std::deref?) [case Red, (std::deref?) [case Red, b, y, c], z, d]] // right-left case
            => Node{Red, std::make_shared<Node>(Black, a, x, b),
                    y, std::make_shared<Node>(Black, c, z, d)},
        [case Black, a, x, (std::deref?) [case Red, b, y, (std::deref?) [case Red, c, z, d]]] // right-right case
            => Node{Red, std::make_shared<Node>(Black, a, x, b),
                    y, std::make_shared<Node>(Black, c, z, d)},
        self => self // do nothing
    };
}

On the one hand, this doesn't seem any more cryptic than the current proposal. The current proposal makes up its own novel mini-language, complete with un-C++ syntax (postfix `!`, novel use of parentheses) and new "not reserved but still magic" identifiers which must be hard-coded into the compiler (`extract`, `try_extract`).  But, for that same reason, it doesn't seem to offer much benefit. The compiler implementation gets marginally simpler (maybe save 100 lines of compiler code, out of the tens of thousands this feature would take?), the library implementation gets 6 lines longer, and all user code gets marginally longer (has to use `std::deref` instead of `*`).  I'm sure that this change in isolation would not be worth the time it would take to discuss it.  If you could find a way to eliminate all of the cryptic magic in this proposal — from the `*?` mini-language to the cumbersome "extractor" API — then that would be worth discussing.

By the way, here's how you'd write the red-black rebalancing example in present-day C++14:

template<class T>
void Node<T>::balance() {
    auto is_red = [](auto&& p) { return p && p->color == Red; };
    auto new_node = [](auto&& a, auto&& x, auto&& b, auto&& y, auto&& c, auto&& z, auto&& d) {
        return Node(Red, std::make_shared<Node>(Black, a, x, b),
                    y, std::make_shared<Node>(Black, c, z, d));
    };
    if (color == Black && is_red(lhs) && is_red(lhs->lhs)) {
        *this = new_node(lhs->lhs->lhs, lhs->lhs->value, lhs->lhs->rhs, lhs->value, lhs->rhs, value, rhs);
    } else if (color == Black && is_red(lhs) && is_red(lhs->rhs)) {
        *this = new_node(lhs->lhs, lhs->value, lhs->rhs->lhs, lhs->rhs->value, lhs->rhs->rhs, value, rhs);
    } else if (color == Black && is_red(rhs) && is_red(rhs->lhs)) {
        *this = new_node(lhs, value, rhs->lhs->lhs, rhs->lhs->value, rhs->lhs->rhs, rhs->value, rhs->rhs);
    } else if (color == Black && is_red(rhs) && is_red(rhs->rhs)) {
        *this = new_node(lhs, value, rhs->lhs, rhs->value, rhs->rhs->lhs, rhs->rhs->value, rhs->rhs->rhs);
    } else {
        // do nothing
    }
}

I don't claim that this code is better than the pattern-matching version, but I don't think it's worse.
By introducing names for the subtrees (a,b,c,d), the pattern-matching code makes it easier to see what's happening in the algorithm. By eschewing arcane syntax and using C++ semantics (e.g. `auto&&` to indicate that we're definitely not making an unnecessary copy), the C++14 code makes it easier to see what's happening in the code.

[...]

4.  The lack of logical operators is a severe limitation, which I don't believe is justified.

 Considering we have two ways to write logical operators, we could reserve one for combining patterns and let the other for expressions

inspect(v) {
 (a || b) or 5 => ...
    ^^^ expression OR 
           ^^^ pattern OR 
}

Yes, this will complicate parsing (inside `inspect` rules will change a bit) and
Yes, the user is forced into syntax style (and new rules who to read code), but both are well worth the gain.
Using "ma[t]chers" for such basic tasks will make C++ look bad.

But in point 1 you were arguing in favor of library code and against weird core-language syntax! Now you've flipped around and want to see a novel mini-language operator instead of a standard library matcher `std::any_of`?

Now, admittedly, the proposal doesn't seem to have gotten as far as thinking about the library side of things. The only mentions of `namespace std` in the paper are where the sample code reopens `namespace std` to add user code to it (a bad habit that I'd like to see discouraged).  So when section 9.3 talks about the matcher `any_of`, it's not necessarily talking about adding that matcher to the library. They might still be thinking that every industry codebase would reimplement their own `any_of` (or `AnyOf`) matcher.

The other problem with adding `any_of` to the standard library is that `std::any_of` already exists — it's an STL algorithm! So they'll have to pick a new name for it if they want to put it into `namespace std`.

Sure, if we add novel built-in core-language syntax for everything we can think of, then we don't need to worry about library design... but it just results in bad language design.

I also strongly object to your proposed core-language syntax. You ask for `||` and `or` to mean subtly different things! Presumably you would also ask for a subtle difference between `&&` and `and`, and between `!` and `not`?  Right now, from C++03 all the way to C++17, these tokens have always meant exactly the same thing — to such a great extent that the compiler interprets `void f(T and x)` as a synonym for `void f(T&& x)`.  The current state of affairs is a little silly, but it is very very teachable, because it is consistent.  "`or` means `||`." Done.  You're proposing to greatly complicate the current state of affairs, by changing the explanation of `or` from 2 characters into several sentences (and those sentences will have to involve a very obscure and arcane feature with lots more crufty corner cases).

(Also note that C++2a Concepts changes the meaning of `||` and `&&` so that they mean subtly different things in requires-clauses from what they mean anywhere else in the language. I consider this a big mistake in terms of language design. So this year I may be overly sensitive to additional requests to change the meanings of logical operators in subtle ways.)

–Arthur