Re: [lmi] depr.impldec

[Greg Chicares]

On 7/10/22 12:29, Vadim Zeitlin wrote:
> On Sat, 9 Jul 2022 22:58:19 +0000 Greg Chicares <gchicares@sbcglobal.net> 
> wrote:
[...]
> GC> I was tempted to remove "virtual" from dtors for classes that don't
> GC> manage resources
> 
>  Sorry again, but this seems completely contrary to what I would do, is
> there a negation missing in the sentence above?

I was thinking faster than I was typing, so let me try afresh.

I propose that we follow Sutter's suggestion here:
  http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-dtor-virtual
| A base class destructor should be either public and virtual,
| or protected and non-virtual

and, where it makes sense, prefer the "protected" path because
it's stricter.

> Because normally classes
> holding resources, such as e.g. std::unique_ptr<>, would _not_ have a
> virtual dtor, but other classes, including all those with virtual
> functions, would.

I meant to say raw resources like pointers or file handles for
which our dtor would want to clean up. And I meant to say "base"
classes. But ignore all of that, and let me simply suggest that
we apply the principle above where it makes sense.

> GC>  - when writing a non-virtual dtor for a polymorphic class,
> 
>  And this seems almost like a contradiction in terms: polymorphic classes
> should _always_ have a virtual dtor.

That depends on what we mean by "polymorphic". I meant to say
polymorphic in the formal sense only--having at least one virtual
function--but not in the common sense of dispatching actions
through a base class.

> In theory it is possible to use a
> polymorphic classes without ever deleting it via a pointer to the base
> class (which would result in undefined behaviour if the dtor is not
> virtual), but in practice it's impossible to ensure this, which is why all
> compilers will warn you about not using virtual dtor for a polymorphic
> class and, warning aside, it's really a very bad idea to not make the dtor
> virtual in such classes.

Unless we use "protected" to make those problems impossible.

But instead of elaborating theoretically and creating confusion
through imprecision, let me propose a ready-to commit change
(for which I don't apply the Rule of Six text-template proposed
in my last message, only because this way the diff is smaller
and easier to read):

--8<----8<----8<----8<----8<----8<----8<----8<--
diff --git a/datum_base.hpp b/datum_base.hpp
index 8bebc63fc..c57aca38e 100644
--- a/datum_base.hpp
+++ b/datum_base.hpp
@@ -31,20 +31,20 @@
 class LMI_SO datum_base
 {
   public:
-    datum_base() = default;
+    void enable(bool);
+    bool is_enabled() const;
 
+    virtual std::istream& read (std::istream&)       = 0;
+    virtual std::ostream& write(std::ostream&) const = 0;
+
+  protected:
+    datum_base() = default;
     datum_base(datum_base const&) = default;
     datum_base(datum_base&&) = default;
     datum_base& operator=(datum_base const&) = default;
     datum_base& operator=(datum_base&&) = default;
     virtual ~datum_base() = default;
 
-    void enable(bool);
-    bool is_enabled() const;
-
-    virtual std::istream& read (std::istream&)       = 0;
-    virtual std::ostream& write(std::ostream&) const = 0;
-
   private:
     bool enabled_ {true};
 };

--8<----8<----8<----8<----8<----8<----8<----8<--

We never instantiate a datum_base, and we never want to,
so all special member functions can be "protected".
We can still form a pointer to a datum_base, but we
can't delete through it because there's no public
dtor that would permit that. All our compilers accept
this; UBSan doesn't complain about it, either.

What do you think?