COIN-OR::LEMON - Graph Library

Opened 2 years ago

Last modified 8 months ago

#631 new defect

Lemon c++20 compatibility patch

Reported by: Alpar Juttner Owned by: Alpar Juttner
Priority: major Milestone: LEMON 1.4 release
Component: core Version: hg main
Keywords: Cc: tew@…
Revision id:

Attachments (4)

destory.diff (2.7 KB) - added by Alpar Juttner 2 years ago.
ticket631.diff (87.9 KB) - added by David Torres Sanchez 10 months ago.
Mostly clang-format (sorry!), added AllocatorTraits::construct and destroy
93c983122692.patch (8.4 KB) - added by Alpar Juttner 9 months ago.
docker.patch (3.3 KB) - added by David Torres Sanchez 8 months ago.
Add docker files with some linux distros

Download all attachments as: .zip

Change History (11)

Changed 2 years ago by Alpar Juttner

Attachment: destory.diff added

comment:1 Changed 19 months ago by Alpar Juttner

Will that work with older C++ standards?

comment:2 Changed 10 months ago by David Torres Sanchez

This doesn't work on C++20 as the construct method has also been removed. One can use the traits, in the same way as destroy, and it seems backwards compatible with older standards.

I get all sorts of other breaking changes from the maps_test on MSVC.

Last edited 10 months ago by David Torres Sanchez (previous) (diff)

Changed 10 months ago by David Torres Sanchez

Attachment: ticket631.diff added

Mostly clang-format (sorry!), added AllocatorTraits::construct and destroy

Changed 9 months ago by Alpar Juttner

Attachment: 93c983122692.patch added

comment:3 Changed 9 months ago by Alpar Juttner

The attached 93c983122692.patch is a reworked version of ticket631.diff. A basically removed the whitespace and reformatting changes. Now it can be seen what has actually changed.

One specific comment. I don't like the comment after the closing curly bracket like this

}; // end class StaticPath

In one hand, it is superfluous as most advanced editor can automatically show this information. On the other hand it is dangerous because it easily gets out of sync with the reality.

comment:4 Changed 9 months ago by Alpar Juttner

Could anyone tell me if these changes are compatible with the older standards (with C++17, C++11 or even with C++98)?

comment:5 Changed 9 months ago by David Torres Sanchez

Thanks Alpar!

From the C++ reference, it should be OK as far back as C++11 as it says in the docs for std::allocator_traits::construct. You can check it yourself in a clean build by changing the standard on lemon/CMakeLists.txt lines 76 and 81 (on your latest #658 patch).

comment:6 Changed 8 months ago by David Torres Sanchez

I've come across a strange issue which I think stems from this. Basically gcc/g++-9 test fails because of test/lgf_reader_writer_test.cc on ubuntu-20.04 but not for example debian or archlinux.

To reproduce this on an ubuntu machine do:

CC="/usr/bin/gcc-9" CXX="/usr/bin/g++-9" cmake -S . -Bbuild
cmake --build build --target all
cd build && ctest --verbose

Alternatively, use the patch I'm about to submit with docker images, run (with docker and docker-compose installed)

cd docker
docker-compose up --build lemon-ubuntu

ubuntu can be replaced with debian, alpine or archlinux.

In either case, for ubuntu, you should see the error:

24: Test command: /home/lib/build/test/lgf_reader_writer_test
24: Test timeout computed to be: 10000000
24: terminate called after throwing an instance of 'lemon::FormatError'
24:   what():  lemon:FormatError: Item not found
24/44 Test #24: lgf_reader_writer_test ...........Child aborted***Exception:   0.30 sec

Changed 8 months ago by David Torres Sanchez

Attachment: docker.patch added

Add docker files with some linux distros

comment:7 in reply to:  6 Changed 8 months ago by David Torres Sanchez

Alternatively, use the patch I'm about to submit with docker images, run (with docker and docker-compose installed)

docker.patch <- here it is.

Last edited 8 months ago by David Torres Sanchez (previous) (diff)
Note: See TracTickets for help on using tickets.