Re: [lmi] C++ modernization [Was: Replacing boost with std C++11]

[Greg Chicares]

On 2017-01-10 04:46, Greg Chicares wrote:
> On 2017-01-09 16:49, Vadim Zeitlin wrote:
> [...]
>> Personally I'd prefer if you could have a look at the pending ones
>> (https://github.com/vadz/lmi/pull/46, 47 and 48), especially because the
>> likelihood of conflicts when doing such global changes is pretty high.

[snip pull/46 discussion]

Moving on to pull/47...

There's a problem with one file:

| The only significant change was the removal of the default ctor of
| database_impl in rate_table.cpp as it was confusing to have an empty ctor
| which wasn't really equivalent to the default implementation due to the use of
| const member field. Clarify the intention by leaving only the ctor taking a
| path and calling it explicitly with an empty path when necessary.

Running rate_table_test:
Expect 'Possibly unknown field...':
Possibly unknown field 'Bloordyblop' ignored at line 1.

** uncaught exception: std::exception: boost::filesystem::path: invalid name 
".ndx" in path: ".ndx"

**** returning with error code 200
**********  errors detected; see stdout for details  ***********
/opt/lmi/src/lmi/workhorse.make:1141: recipe for target 
'rate_table_test.exe-run' failed

I think I'd better ask you to resolve that.

Trivially, some parts of the patch needed to be applied manually;
I note them here in case you want to verify my work:

$patch --dry-run <a00/patches/47.patch | grep -B1 FAILED    
checking file actuarial_table.cpp
Hunk #1 FAILED at 95.
Hunk #2 FAILED at 190.
Hunk #3 FAILED at 515.
3 out of 3 hunks FAILED
--
checking file database_view.cpp
Hunk #1 FAILED at 107.
1 out of 2 hunks FAILED
--
checking file null_stream.cpp
Hunk #1 FAILED at 48.
1 out of 1 hunk FAILED
--
checking file tier_view.cpp
Hunk #1 FAILED at 69.
1 out of 2 hunks FAILED

Stray comments:

--- a/any_member_test.cpp
+++ b/any_member_test.cpp
@@ -35,7 +35,7 @@
 struct base_datum
 {
     base_datum() :sane(7) {}
-    virtual ~base_datum() {}            // Just to make it polymorphic.
+    virtual ~base_datum() = default;    // Just to make it polymorphic.
     virtual int virtual_function() = 0; // Just to make it abstract.
     bool base_function()

Thank you for preserving my two-dimensional alignment (so that the
comments all start in the same column). Someday the world will realize
I was right about this.

-    trammel_base& operator=(trammel_base const&) {return *this;}
+    trammel_base& operator=(trammel_base const&) = default;

At first, I did a double-take here. But the original code did the
right thing: shallow-copy every data member (of which there are none)
and return *this. Writing '= default' here makes it clearer.

While working on pull/46, I read this:
  http://en.cppreference.com/w/cpp/language/override
which allows only two syntactic forms:
| declarator virt-specifier-seq(optional) pure-specifier(optional) (1)
| declarator virt-specifier-seq(optional) function-body (2)
Then, when reviewing pull/47, I wondered whether this:
  ~dev_null_stream_buffer() override = default;
violates that grammar. It doesn't: the reason why cppreference.com is
correct is that [8.4.1/1] "= default;" is a function-body.

I'll push this presently, and hope savannah doesn't choke on it.