[lmi] Enabling '-Weffc++' by default

[Greg Chicares]

I had always thought that building with '-Weffc++' was sure to produce
reams of output, mostly pointing to issues in third-party libraries
beyond my control. However, apparently thanks again to '-isystem', it
seems almost within our grasp for lmi, and wouldn't that be glorious?
Specifically, if I do this in 'workhorse.make':

 gcc_common_extra_warnings := \
   -Wcast-qual \
+  -Weffc++ \

and then
  make clobber && make install
I still get reams of output, but almost all of it just says:
  'foo' should be initialized in the member initialization list
and it seems reasonable to suppose that all 5157 of them can and
should be fixed. Then we'll have only the four '-Weffc++' issues
listed below:

/opt/lmi/src/lmi/dbdict.hpp:37:14: error: base class ‘class 
cache_file_reads<DBDictionary>’ has accessible non-virtual destructor 
[-Werror=non-virtual-dtor]

/opt/lmi/src/lmi/html.hpp:137:7: error: ‘class html::attribute’ has pointer 
data members [-Werror=effc++]
/opt/lmi/src/lmi/html.hpp:137:7: error:   but does not override 
‘html::attribute(const html::attribute&)’ [-Werror=effc++]
/opt/lmi/src/lmi/html.hpp:137:7: error:   or ‘operator=(const 
html::attribute&)’ [-Werror=effc++]

/opt/lmi/src/lmi/html.hpp:166:14: error: ‘class html::detail::any_element’ has 
pointer data members [-Werror=effc++]
/opt/lmi/src/lmi/html.hpp:166:14: error:   but does not override 
‘html::detail::any_element(const html::detail::any_element&)’ [-Werror=effc++]
/opt/lmi/src/lmi/html.hpp:166:14: error:   or ‘operator=(const 
html::detail::any_element&)’ [-Werror=effc++]

/opt/lmi/src/lmi/pdf_command_wx.cpp:1321:7: error: ‘class 
{anonymous}::standard_page’ has pointer data members [-Werror=effc++]
/opt/lmi/src/lmi/pdf_command_wx.cpp:1321:7: error:   but does not override 
‘{anonymous}::standard_page(const {anonymous}::standard_page&)’ [-Werror=effc++]
/opt/lmi/src/lmi/pdf_command_wx.cpp:1321:7: error:   or ‘operator=(const 
{anonymous}::standard_page&)’ [-Werror=effc++]

The first one (dbdict.hpp:37) is weird: it appears only with
'-Weffc++', but is flagged "[-Werror=non-virtual-dtor]", so
perhaps '-Weffc++' strengthens '-Wnon-virtual-dtor' checks?
Anyway, the class consists of just one typedef and one static
member function, so AFAICS is shouldn't even have a (nonempty)
vtable, and the warning is overly aggressive. But adding
+    virtual ~cache_file_reads() = default;
shuts it up, and seems harmless.

The last one (pdf_command_wx.cpp:1321) seems easy to fix by
doing away with the pointer data member:
-    char const* const                    page_template_name_;
+    std::string const                    page_template_name_;
     std::unique_ptr<wxHtmlContainerCell> page_body_cell_;
     std::unique_ptr<wxHtmlContainerCell> header_cell_;
and I think that's correct. Vadim, was there some special
reason for using a C string in this particular case?

And would you please share your thoughts on the other two,
(html.hpp:137) and (html.hpp:166)? I hesitate to replace
"char const*" with string here, because I think you preferred
C strings for efficiency--and, unlike the (pdf_command_wx.cpp:1321)
case above, superficial changes wouldn't suffice here. Here is a
speculative patch for all four issues, and it treats the middle
two brutally, although the other changes might be okay:

----8<----8<----8<----8<----
diff --git a/cache_file_reads.hpp b/cache_file_reads.hpp
index 64b0cfe8..59322912 100644
--- a/cache_file_reads.hpp
+++ b/cache_file_reads.hpp
@@ -101,11 +101,11 @@ class file_cache
 
     struct record
     {
-        retrieved_type data;
-        std::time_t    write_time;
+        retrieved_type data       {};
+        std::time_t    write_time {};
     };
 
-    std::map<std::string,record> cache_;
+    std::map<std::string,record> cache_ {};
 };
 } // namespace detail
 
@@ -128,6 +128,8 @@ class cache_file_reads
         {
         return detail::file_cache<T>::instance().retrieve_or_reload(filename);
         }
+
+    virtual ~cache_file_reads() = default;
 };
 
 #endif // cache_file_reads_hpp
diff --git a/html.cpp b/html.cpp
index 41b01fa4..ce885ad4 100644
--- a/html.cpp
+++ b/html.cpp
@@ -58,7 +58,7 @@ extern element      const tr        ("tr");
 
 std::string attribute::as_string() const
 {
-    std::string s(name_);
+    std::string s(name_.c_str());
     if(!value_.empty())
         {
         s += "=";
@@ -75,8 +75,8 @@ std::string any_element::get_start() const
 {
     std::string s("<");
     // Extra +1 for the space before attributes, even if it's not needed.
-    s.reserve(1 + std::strlen(name_) + 1 + attributes_.length() + 1);
-    s += name_;
+    s.reserve(1 + std::strlen(name_.c_str()) + 1 + attributes_.length() + 1);
+    s += name_.c_str();
     if(!attributes_.empty())
         {
         s += " ";
@@ -116,10 +116,10 @@ void element::update_contents(std::string&& contents)
 element::operator text() const
 {
     std::string s(get_start());
-    s.reserve(s.length() + contents_.length() + 2 + std::strlen(name_) + 1);
+    s.reserve(s.length() + contents_.length() + 2 + std::strlen(name_.c_str()) 
+ 1);
     s += contents_;
     s += "</";
-    s += name_;
+    s += name_.c_str();
     s += ">";
 
     return text::from_html(std::move(s));
diff --git a/html.hpp b/html.hpp
index 2e561fd3..bf4acb18 100644
--- a/html.hpp
+++ b/html.hpp
@@ -150,13 +150,13 @@ class attribute
     std::string as_string() const;
 
   private:
-    attribute(char const* name, std::string&& value)
+    attribute(std::string const& name, std::string&& value)
         :name_  {name}
         ,value_ {std::move(value)}
     {
     }
 
-    char const* const name_;
+    std::string const name_;
     std::string const value_;
 };
 
@@ -179,7 +179,7 @@ class LMI_SO any_element
     // Add the given attribute to our attributes string.
     void update_attributes(attribute const& attr);
 
-    char const* const name_;
+    std::string const name_;
 
   private:
     std::string       attributes_;
diff --git a/input_sequence_parser.cpp b/input_sequence_parser.cpp
index 9100f2c5..91b68129 100644
--- a/input_sequence_parser.cpp
+++ b/input_sequence_parser.cpp
@@ -42,6 +42,7 @@ SequenceParser::SequenceParser
     ,bool                            a_keywords_only
     )
     :input_stream_                  {input_expression}
+    ,diagnostics_                   {}
     ,years_to_maturity_             {a_years_to_maturity}
     ,issue_age_                     {a_issue_age}
     ,retirement_age_                {a_retirement_age}
diff --git a/input_sequence_parser.hpp b/input_sequence_parser.hpp
index b9086b4d..95712bf7 100644
--- a/input_sequence_parser.hpp
+++ b/input_sequence_parser.hpp
@@ -93,8 +93,8 @@ class SequenceParser final
     void mark_diagnostic_context();
 
     // Parser products.
-    std::string diagnostic_messages_;
-    std::vector<ValueInterval> intervals_;
+    std::string diagnostic_messages_             {};
+    std::vector<ValueInterval> intervals_        {};
 
     // Streams for parser input and diagnostic messages.
     std::istringstream input_stream_;
diff --git a/pdf_command_wx.cpp b/pdf_command_wx.cpp
index 83be5241..117a1da3 100644
--- a/pdf_command_wx.cpp
+++ b/pdf_command_wx.cpp
@@ -1402,7 +1402,7 @@ class standard_page : public numbered_page
         if(!page_body_cell_)
             {
             throw std::runtime_error
-                ("failed to parse template '" + 
std::string{page_template_name_} + "'"
+                ("failed to parse template '" + page_template_name_ + "'"
                 );
             }
 
@@ -1435,7 +1435,7 @@ class standard_page : public numbered_page
         return header_cell_.get();
     }
 
-    char const* const                    page_template_name_;
+    std::string const                    page_template_name_;
     std::unique_ptr<wxHtmlContainerCell> page_body_cell_;
     std::unique_ptr<wxHtmlContainerCell> header_cell_;
     std::vector<int>                     page_break_positions_;
diff --git a/workhorse.make b/workhorse.make
index 8c3a276e..8543dcca 100644
--- a/workhorse.make
+++ b/workhorse.make
@@ -541,6 +541,7 @@ postponed_gcc_cxx_warnings := \
 
 gcc_common_extra_warnings := \
   -Wcast-qual \
+  -Weffc++ \
 
 bourn_cast_test.o: gcc_common_extra_warnings += \
   -Wno-double-promotion \
---->8---->8---->8---->8----