[lmi] libstdc++ in gcc-7.3.0: std::vector<incomplete_type> [Was: Zebras in Git]

[Greg Chicares]

On 2018-04-24 18:04, Greg Chicares wrote:
> On 2018-04-24 14:26, Vadim Zeitlin wrote:
[...]
>>  I can't help wondering why
>> exactly can't this class be moved in wx_table_generator.cpp?
> 
> I would very much like to move it there, but I tried doing exactly that,
> and the compiler complained. I interpreted its complaint as meaning that
> I couldn't have a std::vector<some_incomplete_type>.
Specifically (note that '-std=c++17' is specified):

i686-w64-mingw32-g++ -MMD -MP -MT group_quote_pdf_gen_wx.o -MF 
group_quote_pdf_gen_wx.d  -c -I /opt
/lmi/src/lmi -I /opt/lmi/src/lmi/tools/pete-2.1.1 -I 
/opt/lmi/local/lib/wx/include/i686-w64-mingw32
-msw-unicode-3.1 -I /opt/lmi/local/include/wx-3.1 -I 
/opt/lmi/third_party/include -I /opt/lmi/third
_party/src -I /opt/lmi/local/include -I /opt/lmi/local/include/libxml2 
-DLMI_WX_NEW_USE_SO  -DLIBXM
L_USE_DLL -DSTRICT    -D_FILE_OFFSET_BITS=64 -DWXUSINGDLL -D__WXMSW__ 
-DBOOST_NO_AUTO_PTR -DBOOST_S
TRICT_CONFIG -DBOOST_STATIC_ASSERT_HPP   -std=c++17 -frounding-math 
-pedantic-errors -Werror -Wall 
-Wcast-align -Wconversion -Wdeprecated-declarations -Wdisabled-optimization 
-Wextra -Wimport -Wmult
ichar -Wpacked -Wpointer-arith -Wredundant-decls -Wshadow -Wsign-compare 
-Wundef -Wunreachable-code
 -Wwrite-strings  -Wctor-dtor-privacy -Wdeprecated -Wnon-template-friend 
-Woverloaded-virtual -Wpmf
-conversions -Wsynth  -Wcast-qual  -Wno-conversion -Wno-parentheses   
-D'BOOST_STATIC_ASSERT(A)=sta
tic_assert((A))'   -ggdb -O2 -fno-omit-frame-pointer    
/opt/lmi/src/lmi/group_quote_pdf_gen_wx.cpp
 -ogroup_quote_pdf_gen_wx.o
In file included from 
/usr/lib/gcc/i686-w64-mingw32/7.3-win32/include/c++/vector:64:0,
                 from /opt/lmi/src/lmi/ledger_evaluator.hpp:32,
                 from /opt/lmi/src/lmi/ledger.hpp:27,
                 from /opt/lmi/src/lmi/group_quote_pdf_gen_wx.cpp:32:
/usr/lib/gcc/i686-w64-mingw32/7.3-win32/include/c++/bits/stl_vector.h: In 
instantiation of 'std::ve
ctor<_Tp, _Alloc>::size_type std::vector<_Tp, _Alloc>::size() const [with _Tp = 
wx_table_generator:
:column_info; _Alloc = std::allocator<wx_table_generator::column_info>; 
std::vector<_Tp, _Alloc>::s
ize_type = unsigned int]':
/opt/lmi/src/lmi/wx_table_generator.hpp:100:61:   required from here
/usr/lib/gcc/i686-w64-mingw32/7.3-win32/include/c++/bits/stl_vector.h:671:50: 
error: invalid use of
 incomplete type 'class wx_table_generator::column_info'
       { return size_type(this->_M_impl._M_finish - this->_M_impl._M_start); }
                          ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~

I would surmise that the libstdc++ provided with MinGW-w64 gcc-7.3.0 does not 
yet
permit std::vector<incomplete_type>.

In case you want to repeat the test now (unlikely), or we want to repeat it 
later
with a subsequent compiler version, here's the patch I used:

---------8<--------8<--------8<--------8<--------8<--------8<--------8<-------
diff --git a/wx_table_generator.cpp b/wx_table_generator.cpp
index ef630af7..c508d5a0 100644
--- a/wx_table_generator.cpp
+++ b/wx_table_generator.cpp
@@ -27,6 +27,196 @@
 #include "assert_lmi.hpp"
 #include "miscellany.hpp"               // count_newlines(), split_into_lines()
 
+//   is_centered_ is a member variable, initialized in the ctor
+//   is_hidden() is a member function, whose return value is dynamic
+// Should these really be implemented in those two different ways?
+// Wouldn't it be better to treat is_hidden() the same as is_centered_?
+//
+// Is this a struct only because we want its members to be publicly
+// accessible? But their values can also be changed by clients, and
+// isn't that undesirable?
+//
+// In wx_table_generator::do_output_values():
+//   if(align_right_)
+//   if(ci.is_centered_)
+// it seems that right-alignment is a quasi-global, while
+// center-alignment is a column_info data member. Historically, this
+// evolved because right-alignment was recently added to support
+// illustrations, while center-alignment was already used for group
+// quotes. But when code is complex for "historical reasons", it's
+// natural to ask whether it ought to be refactored for uniformity.
+
+// Under what circumstances might columns be hidden, centered, or clipped?
+//
+// General answer:
+//  In principle, all of those are independent. In practice, fixed width
+// columns are centered and variable width columns are clipped and only the
+// former can be hidden. But I don't think this low level class should impose
+// such high level constraints on its use, which is why it doesn't do it.
+//
+// - is_hidden()
+//
+// Apparently used only for group premium quotes, e.g.:
+//
+//             case e_col_total_face_amount:
+//                 if(!has_suppl_amount)
+//                     // Leave the header empty to hide this column.
+//                     break;
+//                 // Fall through
+//             ...
+//                 header = cd.header_;
+//
+// Some columns are conditionally hidden by should_show_column():
+//
+//     // May be overridden to return false if the given column shouldn't be 
shown
+//     // for the specific ledger values (currently used to exclude individual
+//     // columns from composite illustrations).
+//     virtual bool should_show_column(Ledger const& ledger, int column) const
+//
+// but that technique seems to be orthogonal to is_hidden() and used
+// only for illustration PDFs.
+// --No, it's not orthogonal, should_show_column() is used to decide whether
+// the column label should be left empty, making the column hidden.
+//
+//  - is_centered()
+//
+// This seems to be used only in one place:
+//
+//     if(ci.is_centered())
+//         {
+//         // Centre the text for the columns configured to do it.
+//         x_text += (width - dc_.GetTextExtent(s).x) / 2;
+//         }
+//
+// What exactly does it mean for a column to be "centered"? I think this
+// is a different concept than using "center" alignment for cells in a
+// spreadsheet column, which would give, e.g.:
+//     1
+//   11111
+// --No, it's exactly the same concept.
+// In spreadsheet terminology, almost all our columns are numeric, and our
+// numeric columns are right-aligned. But the function is documented thus:
+//
+//     // Return true if this column should be centered, rather than
+//     // left-aligned. Notice that this is ignored for globally right-aligned
+//     // tables.
+//
+// Is it then the case that:
+//  - for illustration PDFs, all columns are right-aligned, and
+//  - is_centered is used only for group quotes, where it really does
+//    mean the same thing as "center" alignment in a spreadsheet
+// ?
+// Answer: yes.
+//  This is indeed not as lucid as I'd like, but the alternative would to
+// modify the PDF quotes code to align all the columns explicitly, which I
+// preferred not to do as part of illustrations work. Maybe now, that this is
+// merged, it's indeed worth changing this.
+//  OTOH, unlike a spreadsheet, this class doesn't have any notion of numeric
+// or text values, so its align_right() method is still useful to globally
+// configure all columns to be right-aligned. Perhaps we could just add a
+// similar align_centre() method and call it from the group PDF quotes code
+// and continue to handle the variable width columns specially by
+// left-aligning them in any case?
+//
+// Apparently is_centered() always returns true (but is ignored)
+// for illustrations, and this comment inside the function body
+// applies to group quotes only:
+//
+//     // Fixed width columns are centered by default, variable width ones
+//     // are not as long strings look better with the default left
+//     // alignment.
+//
+// What sort of columns are not centered?
+// Answer: Variable width ones (only used by PDF quotes).
+//
+// - needs_clipping()
+//
+// And what sort of columns need to be clipped? As currently implemented,
+// this function is the logical negation of is_centered(), so only columns
+// that are not centered need clipping--but what columns are those? Only
+// the "Participant" column on group quotes?
+// Answer: Currently, yes, as it's the only variable width column.
+//
+// Does this all boil down to...
+//  - left-align and clip the group-quote "Participant" column
+//  - center all other group-quote columns
+//  - ignore all these accessors for illustration PDFs
+// ?
+// Answer: yes.
+//
+// And what does 'is_variable_width_' mean? As implemented, it means
+// that the constructor's 'width' argument was zero at the time of
+// construction. Is that in effect just another way of identifying
+// the "Participant" column?
+// Answer:
+//  No, as with "centered" above, it really means what it says: a variable
+// width column is one whose width is not fixed, i.e. not defined by the
+// widest string that can appear in it, but takes all the available space
+// remaining from the other, fixed width columns (in principle, there can be
+// more than one variable width column, even if currently this is not the
+// case).
+// The fundamental distinction is really
+// between fixed and variable width columns: the latter ones are always
+// left-aligned and need to be clipped, while the former ones are either
+// centered or right-aligned (if align_right() was called) and not clipped.
+// And I think things are reasonably simple seen from this point of view and
+// this is how you're supposed to see them, because it's how this class is
+// used, while the various accessors discussed above are just its
+// implementation details.
+
+class wx_table_generator::column_info
+{
+  public:
+    column_info(std::string const& header, int width)
+        :header_(header)
+        ,width_(width)
+        ,is_variable_width_(width == 0)
+        {
+        }
+
+    // A column with empty header is considered to be suppressed and
+    // doesn't appear in the output at all.
+    bool is_hidden() const { return header_.empty(); }
+
+    // Return true if this column should be centered, rather than
+    // left-aligned. Notice that this is ignored for globally right-aligned
+    // tables.
+    bool is_centered() const
+    {
+        // Fixed width columns are centered by default, variable width ones
+        // are not as long strings look better with the default left
+        // alignment.
+        return !is_variable_width_;
+    }
+
+    bool is_variable_width() const
+    {
+        return is_variable_width_;
+    }
+
+    // Return true if the contents of this column needs to be clipped when
+    // outputting it.
+    bool needs_clipping() const
+    {
+        // Variable width columns can have practically unlimited length and
+        // hence overflow into the next column or even beyond and must be
+        // clipped to prevent this from happening. Fixed width columns are
+        // not supposed to overflow anyhow, so clipping them is unnecessary.
+        return is_variable_width_;
+    }
+
+    std::string const header_;
+
+    // Width in pixels. Because the wxPdfDC uses wxMM_POINTS, each
+    // pixel is one point = 1/72 inch.
+    //
+    // Modified directly by wx_table_generator code, hence not const.
+    int width_;
+
+  private:
+    bool const is_variable_width_;
+};
+
 namespace
 {
 
diff --git a/wx_table_generator.hpp b/wx_table_generator.hpp
index 8f912301..5455fe7e 100644
--- a/wx_table_generator.hpp
+++ b/wx_table_generator.hpp
@@ -185,194 +185,4 @@ class wx_table_generator
     bool align_right_ = false;
 };
 
-//   is_centered_ is a member variable, initialized in the ctor
-//   is_hidden() is a member function, whose return value is dynamic
-// Should these really be implemented in those two different ways?
-// Wouldn't it be better to treat is_hidden() the same as is_centered_?
-//
-// Is this a struct only because we want its members to be publicly
-// accessible? But their values can also be changed by clients, and
-// isn't that undesirable?
-//
-// In wx_table_generator::do_output_values():
-//   if(align_right_)
-//   if(ci.is_centered_)
-// it seems that right-alignment is a quasi-global, while
-// center-alignment is a column_info data member. Historically, this
-// evolved because right-alignment was recently added to support
-// illustrations, while center-alignment was already used for group
-// quotes. But when code is complex for "historical reasons", it's
-// natural to ask whether it ought to be refactored for uniformity.
-
-// Under what circumstances might columns be hidden, centered, or clipped?
-//
-// General answer:
-//  In principle, all of those are independent. In practice, fixed width
-// columns are centered and variable width columns are clipped and only the
-// former can be hidden. But I don't think this low level class should impose
-// such high level constraints on its use, which is why it doesn't do it.
-//
-// - is_hidden()
-//
-// Apparently used only for group premium quotes, e.g.:
-//
-//             case e_col_total_face_amount:
-//                 if(!has_suppl_amount)
-//                     // Leave the header empty to hide this column.
-//                     break;
-//                 // Fall through
-//             ...
-//                 header = cd.header_;
-//
-// Some columns are conditionally hidden by should_show_column():
-//
-//     // May be overridden to return false if the given column shouldn't be 
shown
-//     // for the specific ledger values (currently used to exclude individual
-//     // columns from composite illustrations).
-//     virtual bool should_show_column(Ledger const& ledger, int column) const
-//
-// but that technique seems to be orthogonal to is_hidden() and used
-// only for illustration PDFs.
-// --No, it's not orthogonal, should_show_column() is used to decide whether
-// the column label should be left empty, making the column hidden.
-//
-//  - is_centered()
-//
-// This seems to be used only in one place:
-//
-//     if(ci.is_centered())
-//         {
-//         // Centre the text for the columns configured to do it.
-//         x_text += (width - dc_.GetTextExtent(s).x) / 2;
-//         }
-//
-// What exactly does it mean for a column to be "centered"? I think this
-// is a different concept than using "center" alignment for cells in a
-// spreadsheet column, which would give, e.g.:
-//     1
-//   11111
-// --No, it's exactly the same concept.
-// In spreadsheet terminology, almost all our columns are numeric, and our
-// numeric columns are right-aligned. But the function is documented thus:
-//
-//     // Return true if this column should be centered, rather than
-//     // left-aligned. Notice that this is ignored for globally right-aligned
-//     // tables.
-//
-// Is it then the case that:
-//  - for illustration PDFs, all columns are right-aligned, and
-//  - is_centered is used only for group quotes, where it really does
-//    mean the same thing as "center" alignment in a spreadsheet
-// ?
-// Answer: yes.
-//  This is indeed not as lucid as I'd like, but the alternative would to
-// modify the PDF quotes code to align all the columns explicitly, which I
-// preferred not to do as part of illustrations work. Maybe now, that this is
-// merged, it's indeed worth changing this.
-//  OTOH, unlike a spreadsheet, this class doesn't have any notion of numeric
-// or text values, so its align_right() method is still useful to globally
-// configure all columns to be right-aligned. Perhaps we could just add a
-// similar align_centre() method and call it from the group PDF quotes code
-// and continue to handle the variable width columns specially by
-// left-aligning them in any case?
-//
-// Apparently is_centered() always returns true (but is ignored)
-// for illustrations, and this comment inside the function body
-// applies to group quotes only:
-//
-//     // Fixed width columns are centered by default, variable width ones
-//     // are not as long strings look better with the default left
-//     // alignment.
-//
-// What sort of columns are not centered?
-// Answer: Variable width ones (only used by PDF quotes).
-//
-// - needs_clipping()
-//
-// And what sort of columns need to be clipped? As currently implemented,
-// this function is the logical negation of is_centered(), so only columns
-// that are not centered need clipping--but what columns are those? Only
-// the "Participant" column on group quotes?
-// Answer: Currently, yes, as it's the only variable width column.
-//
-// Does this all boil down to...
-//  - left-align and clip the group-quote "Participant" column
-//  - center all other group-quote columns
-//  - ignore all these accessors for illustration PDFs
-// ?
-// Answer: yes.
-//
-// And what does 'is_variable_width_' mean? As implemented, it means
-// that the constructor's 'width' argument was zero at the time of
-// construction. Is that in effect just another way of identifying
-// the "Participant" column?
-// Answer:
-//  No, as with "centered" above, it really means what it says: a variable
-// width column is one whose width is not fixed, i.e. not defined by the
-// widest string that can appear in it, but takes all the available space
-// remaining from the other, fixed width columns (in principle, there can be
-// more than one variable width column, even if currently this is not the
-// case).
-// The fundamental distinction is really
-// between fixed and variable width columns: the latter ones are always
-// left-aligned and need to be clipped, while the former ones are either
-// centered or right-aligned (if align_right() was called) and not clipped.
-// And I think things are reasonably simple seen from this point of view and
-// this is how you're supposed to see them, because it's how this class is
-// used, while the various accessors discussed above are just its
-// implementation details.
-
-class wx_table_generator::column_info
-{
-  public:
-    column_info(std::string const& header, int width)
-        :header_(header)
-        ,width_(width)
-        ,is_variable_width_(width == 0)
-        {
-        }
-
-    // A column with empty header is considered to be suppressed and
-    // doesn't appear in the output at all.
-    bool is_hidden() const { return header_.empty(); }
-
-    // Return true if this column should be centered, rather than
-    // left-aligned. Notice that this is ignored for globally right-aligned
-    // tables.
-    bool is_centered() const
-    {
-        // Fixed width columns are centered by default, variable width ones
-        // are not as long strings look better with the default left
-        // alignment.
-        return !is_variable_width_;
-    }
-
-    bool is_variable_width() const
-    {
-        return is_variable_width_;
-    }
-
-    // Return true if the contents of this column needs to be clipped when
-    // outputting it.
-    bool needs_clipping() const
-    {
-        // Variable width columns can have practically unlimited length and
-        // hence overflow into the next column or even beyond and must be
-        // clipped to prevent this from happening. Fixed width columns are
-        // not supposed to overflow anyhow, so clipping them is unnecessary.
-        return is_variable_width_;
-    }
-
-    std::string const header_;
-
-    // Width in pixels. Because the wxPdfDC uses wxMM_POINTS, each
-    // pixel is one point = 1/72 inch.
-    //
-    // Modified directly by wx_table_generator code, hence not const.
-    int width_;
-
-  private:
-    bool const is_variable_width_;
-};
-
 #endif // wx_table_generator_hpp
--------->8-------->8-------->8-------->8-------->8-------->8-------->8-------