COIN-OR::LEMON - Graph Library

Opened 2 years ago

Last modified 10 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 12 months ago.
Mostly clang-format (sorry!), added AllocatorTraits::construct and destroy
93c983122692.patch (8.4 KB) - added by Alpar Juttner 10 months ago.
docker.patch (3.3 KB) - added by David Torres Sanchez 10 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 21 months ago by Alpar Juttner

Will that work with older C++ standards?

comment:2 Changed 12 months ago by David Torres Sanchez

This doesn't work on C++20 as the construct method has also been removed. I get all sorts of other breaking changes from the maps_test on MSVC.

Will submit a patch for this.

Version 1, edited 12 months ago by David Torres Sanchez (previous) (next) (diff)

Changed 12 months ago by David Torres Sanchez

Attachment: ticket631.diff added

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

Changed 10 months ago by Alpar Juttner

Attachment: 93c983122692.patch added

comment:3 Changed 10 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 10 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 10 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 10 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 10 months ago by David Torres Sanchez

Attachment: docker.patch added

Add docker files with some linux distros

comment:7 in reply to:  6 Changed 10 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 10 months ago by David Torres Sanchez (previous) (diff)
Note: See TracTickets for help on using tickets.