Re: [lmi] New CI breakage due to gcc 12 updates

[Vadim Zeitlin]

On Sun, 24 Jul 2022 01:28:36 +0200 I wrote:

Me>  The third problem is the most mysterious one. I'm not going to paste the
Me> full error message here because it's too long but the relevant excerpts
Me> are:
Me> 
Me> ---------------------------------- >8 --------------------------------------
Me> In constructor 'constexpr std::__cxx11::basic_string<_CharT, _Traits, 
_Alloc>::_Alloc_hider::_Alloc_hider(std::__cxx11::basic_string<_CharT, _Traits, 
_Alloc>::pointer, _Alloc&&) [with _CharT = char; _Traits = 
std::char_traits<char>; _Alloc = std::allocator<char>]',
Me>     inlined from 'constexpr std::__cxx11::basic_string<_CharT, _Traits, 
_Alloc>::basic_string(const std::__cxx11::basic_string<_CharT, _Traits, 
_Alloc>&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = 
std::allocator<char>]' at /usr/include/c++/12/bits/basic_string.h:540:9,
Me> ...
Me> inlined from 'void CensusView::update_class_names()' at 
/__w/lmi/lmi/census_view.cpp:1170:21:
Me> /usr/include/c++/12/bits/basic_string.h:204:43: error: null pointer 
dereference [-Werror=null-dereference]
Me>   204 |         : allocator_type(std::move(__a)), _M_p(__dat) { }
Me>       |                                           ^~~~~~~~~~~
Me> ---------------------------------- >8 --------------------------------------
Me> 
Me> which doesn't make sense to me because we are not using any null pointers
Me> here.

 It seems to be a compiler bug because I can reproduce it in a simple
example by just compiling it (it won't link, of course, due to the missing
function) with "-O2 -Wnull-dereference":

---------------------------------- >8 --------------------------------------
#include <algorithm>
#include <iterator>
#include <string>
#include <vector>

std::vector<std::string> const& cell_parms();

int main() {
    std::vector<std::string> all;

    for(auto const& i : cell_parms()) {
        all.push_back(i);
    }

    std::vector<std::string> unique;
    std::insert_iterator iin(unique, unique.begin());
    std::sort(all.begin(), all.end());
    std::unique_copy(all.begin(), all.end(), iin);
}
---------------------------------- >8 --------------------------------------

 However trying to further minimize this example (which I couldn't really
do), I've accidentally found a simple workaround: moving std::sort() before
insert_iterator creation somehow avoids the warning. I wouldn't usually
propose such a change just to work around a bug, but in this case it
actually makes sense to me: I think it's more natural to sort the vector
immediately after initializing it, rather than at some later time. And the
following simple diff

---------------------------------- >8 --------------------------------------
diff --git a/census_view.cpp b/census_view.cpp
index 66635eabf..58e77db5e 100644
--- a/census_view.cpp
+++ b/census_view.cpp
@@ -1159,6 +1159,7 @@ void CensusView::update_class_names()
         {
         all_class_names.push_back(i["EmployeeClass"].str());
         }
+    std::sort(all_class_names.begin(), all_class_names.end());

     std::vector<std::string> unique_class_names;

@@ -1166,7 +1167,6 @@ void CensusView::update_class_names()
         (unique_class_names
         ,unique_class_names.begin()
         );
-    std::sort(all_class_names.begin(), all_class_names.end());
     std::unique_copy(all_class_names.begin(), all_class_names.end(), iin);

     // Rebuild vector of class parameters so that it contains
---------------------------------- >8 --------------------------------------

is enough to avoid the warning in lmi itself too, while fixing it in some
other way is going to be ugly (I'm afraid we're going to need to disable it
with pragmas as I don't see what else can be done about it), so maybe we
could apply this instead?

 Please let me know what do you think of it and whether I should include
this or pragma-based workaround into my patch.

 Thanks,
VZ

P.S. Also, just FYI, I've still reported this to gcc developers:

        https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106434

     but there are already quite a few open -Wnull-dereference-related
     bugs, so I'm not sure if this is going to be looked into or not.

pgpqHZLNuVRiM.pgp
Description: PGP signature