Re: [lmi] Detecting whether move semantics actually take place

[Greg Chicares]

On 7/23/22 15:41, Vadim Zeitlin wrote:
> On Thu, 21 Jul 2022 00:53:01 +0000 Greg Chicares <gchicares@sbcglobal.net> 
> wrote:
> 
> GC> On 7/18/22 16:19, Vadim Zeitlin wrote:
> GC> > On Mon, 18 Jul 2022 15:48:14 +0000 Greg Chicares 
> <gchicares@sbcglobal.net> wrote:
> GC> [...]
> GC> > GC> Let me pose a hypothetical question first: does this line
> GC> > GC>      db = std::move(da);   // D& operator=(D&&)
> GC> > GC> actually perform a move and not a copy, in a context where
> GC> > GC> class D defaults all special member functions?
[...]
> GC> The question I really want to ask is: is it possible in C++20 to answer
> GC> that question for a different class E of which we know nothing?
> 
>  I thought it would be, and now you've conclusively proved that it can, by
> actually doing it, but I'm still not sure about how are you going to use
> it.

I hope that we can meaningfully augment this:

https://lists.nongnu.org/archive/html/lmi/2022-07/msg00104.html
|   #include "crtp_base.hpp"
| 
| ...and only once:
|
|   class A : private lmi::polymorphic_base<A> {};
|   static_assert( std::is_polymorphic_v           <A>);
|   static_assert( std::is_default_constructible_v <A>);
|   static_assert( std::is_destructible_v          <A>);
|   static_assert( std::is_copy_constructible_v    <A>);
|   static_assert( std::is_move_constructible_v    <A>);
|   static_assert( std::is_copy_assignable_v       <A>);
|   static_assert( std::is_move_assignable_v       <A>);

thus:

  static_assert( lmi::is_effectively_copyable <A>);
  static_assert( lmi::is_effectively_moveable <A>);

in order to handle this concern:

| >  One benefit that I hadn't mentioned yet, I think, is that if you ever
| > define a special member in your class, e.g. decide to do something in the
| > default ctor, it would silently inhibit moving. This should, arguably,
| > happen only rarely, but I guarantee that if it does happen, it will remain
| > unnoticed for a long time.
|
| Unless we guarantee that it cannot happen, e.g., with "concept" assertions.

so that, if we add an explicit dtor to class A, causing the move functions
to become undeclared, then we'll get a compile-time assertion failure.

Thus, instead of adding five lines of special-member-function boilerplate
to every class, we add one line at the end of each class definition, e.g.:

  class A : private lmi::polymorphic_base<A> {};
  static_assert(std::semiregular<A> && is_effectively_copyable_and_moveable<A>);

making the Rule of Zero safe.

>  I.e. now we know that we can have is_effectively_moveable<T> (for
> simplicity, I'm not distinguishing between move-constructible and
> move-assignable neither here nor below), but how are you going to use it,
> exactly? You could static_assert that it's true, but where would doing this
> be useful?

When we want to use the Rule of Zero and make sure it doesn't invisibly
break if we later add an explicit declaration. It's the invisibility
that's a problem. In-your-face breakage--a static_assert() failure--is
okay because we're forced to handle it immediately, before the breakage
can spread.

> GC> Here it's important to explain why I'm asking this question.
> GC> 
> GC> The number of classes in lmi is very approximately five hundred:
> GC>   $git grep '^\<class\>' *.?pp |wc -l
> GC>   527
> 
> <completely gratuitous aside>
> 
>  To make this slightly more precise we could at least avoid counting the
> forward declarations:
> 
>       % git grep '^class [^;]+$' *.?pp | wc -l
>       346

Good, thanks.

> BTW, while looking at this I've only tried using "git grep --count" and
> while this didn't work for giving the total number of matches, I've noticed
> a pretty unusual distribution of number of classes defined per file:
> 
>       % git grep --count '^class [^;]+$' *.?pp | sort -k2 -nr -t: | head -n10
>       pdf_command_wx.cpp:31
>       census_view.cpp:14
>       tn_range_type_trammels.hpp:9
[...]
> i.e. my pdf_command_wx.cpp is truly an outlier and only 26 files have more
> than 2 classes in it.

Yes, 'pdf_command_wx.cpp' and 'census_view.cpp' in particular use
polymorphism for inversion of control, whereas most of lmi uses
an imperative style.

> </aside>
> 
> GC> Suppose lmi copies an object, and I want to consider moving it instead.
> GC> I can write the move operation, but will move semantics actually take
> GC> place, or will the compiler silently substitute a copy to fulfill my
> GC> move request? I don't simply want to make the request; I want to know
> GC> how it's fulfilled. If moving is inherently impossible for some class,
> GC> I'd like to know that before I waste time trying to move it.
> 
>  I think the simplest way of answering this question is to declare the move
> constructor as default and rely on clang -Wdefaulted-function-deleted to
> tell you if it turns out that your move ctor is implicitly deleted. Isn't
> it?

Suppose the chronology is:
 1. I write a class that's effectively-movable, but don't actually move it yet.
 2. I change the class in a way that spoils its movability.
 3. A decade later, I try to move it, and find that I can't.
Doesn't clang give that warning only at the third step, where I want it
at the second?

> GC> Similarly,
> GC> I'd like to know whether moving is only incidentally impossible: e.g.,
> GC>  - because I've made the move functions inaccessible; or
> GC>  - because I've deleted them for no good reason, just because I didn't
> GC>      want to spend time considering them; or
> GC>  - because I made a base class unmoveable for some such reason; or
> GC>  - because I casually added a 'const&' data member for no strong
> GC>      reason, rendering the class unassignable;
> GC> because then I might want to consider removing that incidental cause.

I'm confused here:

>  Note that is_effectively_moveable<T> doesn't actually help answering these

It doesn't help?

> questions. But both the clang warning and is_effectively_moveable concept
> (if you defined it, see below) do

Yet it does help?

I think you're saying that it becomes helpful if we make it constexpr
and use it in a static assertion.

> because they provide additional context
> for the error they generate. E.g., just for the illustration purposes, here
> is an example of a clang warning in my own simplified test case
> 
> ---------------------------------- >8 --------------------------------------
> effectively_movable.cpp:44:5: warning: explicitly defaulted move constructor 
> is implicitly deleted [-Wdefaulted-function-deleted]
>     movable_by_copy(movable_by_copy&&) = default;
>     ^
> effectively_movable.cpp:41:26: note: move constructor of 'movable_by_copy' is 
> implicitly deleted because base class 'not_movable' has a deleted move 
> constructor
> struct movable_by_copy : not_movable {
>                          ^
> effectively_movable.cpp:38:5: note: 'not_movable' has been explicitly marked 
> deleted here
>     not_movable(not_movable&&) = delete;
>     ^
> ---------------------------------- >8 --------------------------------------

Here's a test case that gives no '-Wdefaulted-function-deleted'
warning--in fact, if we comment out the assertion, clang compiles
it successfully.

Copy the relevant parts from branch odd/move_semantics and add
"constexpr" to equiplausible_construct(), then add:

  class E : public no_can_move {};
  static_assert(equiplausible_construct<E>());

I think the difference between our test cases shows that clang
diagnoses only the deletion of explicitly-declared special
member functions. But E's SMFs are implicit: the compiler
declares them, but AIUI does not define them because they
aren't actually used; and, not having defined them, it doesn't
need to delete them, so there's no warning.

And of course clang shouldn't routinely warn in cases like this,
because for many classes in many projects the warning would be
frequent and unwelcome. But for certain classes in lmi, I want
to be warned, and the static assertion above does what I want.

> GC> If I could write
> GC>   static_assert(move_will_actually_use_move_semantics<T>);
> GC> then I could save that waste of time (or consider remediating the
> GC> reason why such an assertion fails).
> 
>  So now we know that you can do it. But does this still seem like a better
> solution than just checking clang warning to you?

Yes, because I get it at step "2." above rather than step "3.".

> GC> We have
> GC>   std::is_move_constructible
> GC>   std::is_move_assignable
> GC> but they don't perform this job: they say that I can request a move,
> GC> but not that I can actually get move semantics.
> GC> 
> GC> Searching the web for an answer leads me to believe that most people
> GC> think the 'trait' I want cannot be implemented. I've tried to do it
> GC> anyway:
> GC>   https://git.savannah.nongnu.org/cgit/lmi.git/log/?h=odd/move_semantics
[...]
> GC> The technique uses a template class:
> GC> 
> GC>   template<typename T>
> GC>   struct ambiguator
> GC>   {
> GC>       operator T const&();
> GC>       operator T&&();
> GC>   };
> GC> 
> GC> such that
> GC>   T t {ambiguator<T>{}};
> GC> would be a compile-time error if both copy and move constructors are
> GC> candidates in overload resolution.
> 
>  This is an ingenuous solution, I don't know if I would have found it if
> you hadn't given it to me first. And, again, I can't find anything wrong
> with it.

The ingenuity isn't mine:
  
https://stackoverflow.com/questions/51901837/how-to-get-if-a-type-is-truly-move-constructible/51912859#51912859
In olden days I would have credited the author. But this isn't usenet,
and I can't determine who "C.M." is.

My implementation is an original work that expresses the idea of
ambiguating copy and move construction by means of a class that
defines both lvalue- and rvalue-reference conversion operators.
Thanks, "C.M.", whoever you are, for that idea; if you want to
give your name, I'll be glad to give you explicit credit.

>  Just to record this here: I had my doubts about whether this worked with
> move-only classes initially, but it does, because even if the copy ctor is
> deleted, it still participates in the overload resolution, and so still
> makes constructing T from ambiguator<T> ambiguous. And copy ctor can't be
> "implicitly deleted" by just not providing it, unlike the move one.
> 
> GC> The tests on that branch attempt to establish that the longed-for
> GC>   move_will_actually_use_move_semantics<T>
> GC> trait can be implemented as
> GC>   equiplausibly_constructible<T> && std::is_move_constructible<T>
> GC> and similarly for move assignment. Or have I only found fool's gold?
> 
>  I think you did manage to find the solution to the original problem, but
> I'm just not sure if it's the right problem to solve. If you think it is,

I do, for the reasons above.

> I'd like to mention a couple of improvements:
> 
>  First, and trivially, you should sprinkle magic constexpr dust all over
> your code, i.e. declare ambiguous conversion operators and all the
> functions (equiplausible_construct, equiplausible_assign, c_moveable,
> a_moveable) as constexpr, as this is needed to allow using static_assert<>
> with them.

Agreed. I plan to bring this out of the "sandbox" into a new lmi header
with thoughtful unit tests. As "C.M." remarks, tests should consider
special members with various cv-qualifiers and access-specifiers.

> You could also make sandbox_test a compile-only test if you
> replaced all LMI_TEST() in it with static_assert<>s.

Agreed. 'crtp_base_test.cpp' follows that suggestion.

>  Second, I think this would work even better with concepts rather than
> simple template functions, both because the former are slightly more
> readable (even if the functions here are pretty clear too) and because they
> result in much better error messages, similar to the clang warning above,
> except that this also works with gcc, if the requirement fails. Here is my
> simple concept-based test:
> 
> ---------------------------------- >8 --------------------------------------
> #include <concepts>
> 
> template<typename T>
> struct ambiguator {
>     constexpr operator T const&();
>     constexpr operator T&&();
> };

Okay, you've added "constexpr"...

> template <typename T>
> concept is_effectively_moveable =
>     std::move_constructible<T> && !std::constructible_from<T, ambiguator<T>>;

...and "concept"...

>  Please let me know if you'd like to have a patch changing sandbox_test.cpp
> to use concepts, it should be pretty simple to do

I think I can do that myself. But first I want to ask a question or two
about "concepts", which I've already been toying with recently.

AFAICT, to enforce a "concept" for a non-template class, we use
static_assert() as in this example, adapted from:

https://www.foonathan.net/2021/07/concepts-structural-nominal/

   struct vec2 { … };
// concept equality_comparable for vec2;     // we think this way...
   static_assert(equality_comparable<vec2>); // ...but write this way

In this limited case (non-template classes), <concepts> gives us very
little AFAICT, as there's little difference between these examples:

  #include <concepts>
  struct vec2 { … };
  static_assert(std::destructible<vec2>);

  #include <type_traits>
  struct vec2 { … };
  static_assert(std::is_destructible_v<vec2>);

It's a different story for class templates, but for non-template classes,
is there really nothing more to it than this?

Is a patch like the following an appropriate start? Notes:
 - it's accidentally a reversed patch, sorry
 - semiregular seems to imply movable and copyable, so we don't
     need to assert those individually
 - the class becomes regular, as opposed to semiregular, only
     when its comparison operators are seen

--8<----8<----8<----8<----8<----8<----8<----8<----8<----8<--
diff --git a/opt/lmi/stash/currency.hpp b/./currency.hpp
index 8c5e18631..d0338185b 100644
--- a/opt/lmi/stash/currency.hpp
+++ b/./currency.hpp
@@ -25,7 +25,6 @@
 #include "config.hpp"
 
 #include <cmath>                        // rint()
-#include <concepts>
 #include <limits>
 #include <ostream>
 #include <stdexcept>                    // runtime_error
@@ -69,9 +68,6 @@ class currency
 
     data_type m_ = {};
 };
-static_assert(std::copyable<currency>);
-static_assert(std::movable<currency>);
-static_assert(std::semiregular<currency>);
 
 consteval currency operator""_cents(unsigned long long int cents)
 {
@@ -97,9 +93,6 @@ inline bool operator<=(currency lhs, currency rhs)
 inline bool operator>=(currency lhs, currency rhs)
     {return !operator< (lhs, rhs);}
 
-static_assert(std::equality_comparable<currency>);
-static_assert(std::regular<currency>);
-
 inline currency operator+(currency lhs, currency rhs)
     {return currency {lhs} += rhs;}
 inline currency operator-(currency lhs, currency rhs)
--8<----8<----8<----8<----8<----8<----8<----8<----8<----8<--