Open SSoelvsten opened 2 years ago
On Windows, tests 124 - external_stack_io
seems to fail randomly. More problematic is 138 - memory_basic
that always in Release exits with code 0xc0000374, i.e. has some heap corruption, and in Debug always loops.
Are we sure this actually is not some bug on Windows that the unit test discovers?
As I am force pushing, it seems the Ubuntu version has been updated from 20 LTS to 22 LTS. Yet, 22 LTS has only newer versions of GCC and Clang available with sudo apt
. I'll update them accordingly.
Should I also have an older ubuntu version just to test with an older version of GCC? Yet, the Mac version still checks with GCC 7, so it is not entirely uncovered.
@SSoelvsten Ubuntu 22.04 is fine - you don't have to test on 20.04.
About the commit: Fix missing * operator in tpie::packed_array::iter_base
- can you please remove all the reformatting changes and other unneeded changes from this commit, so that it's easy to verify that the change is minimal? It's important to me to be sure that we're not breaking anything in the packed_array, and although I could go in and check the diff line-by-line to find the changes that aren't related to formatting, I'd rather have a revision history where such a commit doesn't combine compilation error fixing with code reformatting.
That is a very fair point. I have uncleaned the packed array, such that the actual bug fix is easy to identify.
Based on my latest comments above, I have fixed the Microsoft issue once more by making the const tpie::packed_array::iterator
follow a T* const
semantics rather than a const T* const
one.
The initial version of commit Ensure job completes with a checkmark even if skipped was done improperly for Linux and Mac, thereby breaking that part of the CI (quite silently). I have rebased a fix into said commit and now all three platforms have CI that is running.
This still leaves the unit test 138 - memory_basic
to expose a major problem on Windows.
I got an answer back on issue https://github.com/ilammy/msvc-dev-cmd/issues/68 with how to change the MSVC version. @Mortal , should I set it back to something before the breaking change on std::sort(...)
or should we keep the fix I added instead?
Follow-up on pull request #247 now that #255 added commit 27e7da14b3a614701ece45f6e6312e9c1dbe1880. This adds building on Windows using MSVC 14.3 (this is the newest version of Microsoft's compiler, as far as I can tell) and then running all unit tests. Essentially this boils down to
std::sort
.This turns out to also fix all of the issues with the Mac build. :partying_face: Below is a full list of all compiler errors that were fixed.
tpie\pipelining\merge_sorter.cpp(325)
:tpie\pipelining\merge_sorter.cpp(325)
:test\speed_regression\btree.cpp(65)
:test\speed_regression\btree.cpp(84)
:tpie\test\unit/common.h(41)
:test\unit\test_close_file.cpp(47)
:[x]
tpie\test\unit\test_array.cpp
The following errors seem to happen due to thestd::sort
on line 233 .include\algorithm(7305,1)
: error C2678: binary '*': no operator found which takes a left-hand operand of type 'const _BidIt' (or there is no acceptable conversion)include\algorithm(7305,17)
: error C2100: illegal indirectioninclude\algorithm(7307,1)
: error C2678: binary '*': no operator found which takes a left-hand operand of type 'const _BidIt' (or there is no acceptable conversion)include\algorithm(7307,17)
: error C2100: illegal indirectionThe 'stack trace' inside of the STL is:
void std::sort<_RanIt,std::less<void>>(const _RanIt,const _RanIt,_Pr)
void std::_Sort_unchecked<_RanIt,_Fn>(_RanIt,_RanIt,__int64,_Pr)
_BidIt std::_Insertion_sort_unchecked<_RanIt,_Pr>(const _BidIt,const _BidIt,_Pr)
This might be an issue with the
const
. The iterators probably are missing a const option for its * operator...?https://github.com/microsoft/STL/blob/main/stl/inc/algorithm#L7146 https://github.com/microsoft/STL/blob/main/stl/inc/algorithm#L1487
unit\test_btree.cpp(...)
:unit\test_freespac_collection.cpp(...)
:tpie\test\unit\test_array.cpp(223)
(closes #269):