Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

coin-lemon: fixed C++17 and C++20 compilation warnings. #15516

Closed
wants to merge 6 commits into from
Closed

coin-lemon: fixed C++17 and C++20 compilation warnings. #15516

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
22fc695
coin-lemon: fixed C++17 and C++20 compilation warnings.
rturrado Jan 28, 2023
6e1cafe
coin-lemon: renamed fix_cpp_20_compilation_warnings.patch to fix_cpp_…
rturrado Feb 2, 2023
73c9294
coin-lemon: updated patch to fix gcc 5 compilation (gcc 5 doesn't all…
rturrado Feb 4, 2023
788b028
coin-lemon: fix conandata.yml (by removing an extra blank line).
rturrado Feb 4, 2023
05fbdb8
coin-lemon: fixed C++20 compilation errors for path.h too.
rturrado Feb 9, 2023
a94d7b1
coin-lemon: added patch_source lines to conandata.yml.
rturrado Feb 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions recipes/coin-lemon/all/conandata.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,9 @@ patches:
"1.3.1":
- patch_file: "patches/cmake-add-runtime-destination_1.3.1.patch"
- patch_file: "patches/0001-remove-register-keyword.patch"
- patch_file: "patches/fix_cpp_17_compilation_warnings.patch"
rturrado marked this conversation as resolved.
Show resolved Hide resolved
patch_type: "portability"
patch_description: "Fix C++17 compilation warnings regarding the use of the deprecated std::iterator"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/conan-io/conan-center-index/blob/master/docs/adding_packages/sources_and_patches.md#rules

I am not sure the warnings deserve a patch. These are costly to maintain and we really want ones to make "working software"

Does this fail to build with later cppstd? I would prefer less patches otherwise

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @prince-chrismc ,

I'm more in favour of a 'treatiing warnings as errors' policy. Anyway, yes, some of these warnings cause failures when compiling with C++17 or newer (see #15515 ).

Best regards,
Roberto.

- patch_file: "patches/fix_cpp_20_compilation_errors.patch"
patch_type: "portability"
patch_description: "Fix C++20 compilation errors regarding the use of allocator.construct and allocator.destroy"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the patch_source too with the PR to upstream were you submitted this 🙏 so others can benefit from it too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @prince-chrismc ,

I tried to get in touch with the maintainers of the Lemon Graph library via their website and their user mailing list, without luck. The last version of the library is from 2014.

I've issued a new ticket, and attached the first of the patches (C++17), to: https://lemon.cs.elte.hu/trac/lemon/attachment/ticket/680/fix_cpp_17_compilation_warnings.patch
The second patch (C++20) is actually part of this other attachment to a previous ticket (I only took the patch section for array_map.h because the patch section for path.h wasn't patching correctly): http://lemon.cs.elte.hu/trac/lemon/attachment/ticket/631/93c983122692.patch

I think I should update this second patch with the fixes for path.h as well.

If you reckon, I can add those two links mentioned above as patch_source.

Thanks,
Roberto.

50 changes: 50 additions & 0 deletions recipes/coin-lemon/all/patches/fix_cpp_17_compilation_warnings.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
Subject: [PATCH] Fix C++17 compilation warnings regarding the use of the deprecated std::iterator.
---
Index: lemon/maps.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/lemon/maps.h b/lemon/maps.h
--- a/lemon/maps.h (revision c4284157265976120e57771693a9be7c45d1b617)
+++ b/lemon/maps.h (date 1674925094000)
@@ -1969,13 +1969,17 @@
/// be accessed in the <tt>[beginValue, endValue)</tt> range.
/// They are considered with multiplicity, so each value is
/// traversed for each item it is assigned to.
- class ValueIt
- : public std::iterator<std::forward_iterator_tag, Value> {
+ class ValueIt {
friend class CrossRefMap;
private:
ValueIt(typename Container::const_iterator _it)
: it(_it) {}
public:
+ typedef std::forward_iterator_tag iterator_category;
+ typedef Value value_type;
+ typedef void difference_type;
+ typedef void pointer;
+ typedef void reference;

/// Constructor
ValueIt() {}
@@ -3131,13 +3135,17 @@
/// This iterator is an STL compatible forward
/// iterator on the values of the map. The values can
/// be accessed in the <tt>[beginValue, endValue)</tt> range.
- class ValueIt
- : public std::iterator<std::forward_iterator_tag, Value> {
+ class ValueIt {
friend class IterableValueMap;
private:
ValueIt(typename std::map<Value, Key>::const_iterator _it)
: it(_it) {}
public:
+ typedef std::forward_iterator_tag iterator_category;
+ typedef Value value_type;
+ typedef void difference_type;
+ typedef void pointer;
+ typedef void reference;

/// Constructor
ValueIt() {}
124 changes: 124 additions & 0 deletions recipes/coin-lemon/all/patches/fix_cpp_20_compilation_errors.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
diff --git a/lemon/bits/array_map.h b/lemon/bits/array_map.h
--- a/lemon/bits/array_map.h (revision 2dd2399e37ab7b19f25a382bfef590cafe2f7584)
+++ b/lemon/bits/array_map.h (date 1674605699188)
@@ -75,6 +75,7 @@
typedef typename Notifier::ObserverBase Parent;

typedef std::allocator<Value> Allocator;
+ typedef std::allocator_traits<std::allocator<Value>> AllocatorTraits;

public:

@@ -87,8 +88,8 @@
Notifier* nf = Parent::notifier();
Item it;
for (nf->first(it); it != INVALID; nf->next(it)) {
- int id = nf->id(it);;
- allocator.construct(&(values[id]), Value());
+ int id = nf->id(it);
+ AllocatorTraits::construct(allocator, &(values[id]), Value());
}
}

@@ -101,8 +102,8 @@
Notifier* nf = Parent::notifier();
Item it;
for (nf->first(it); it != INVALID; nf->next(it)) {
- int id = nf->id(it);;
- allocator.construct(&(values[id]), value);
+ int id = nf->id(it);
+ AllocatorTraits::construct(allocator, &(values[id]), value);
}
}

@@ -120,8 +121,8 @@
Notifier* nf = Parent::notifier();
Item it;
for (nf->first(it); it != INVALID; nf->next(it)) {
- int id = nf->id(it);;
- allocator.construct(&(values[id]), copy.values[id]);
+ int id = nf->id(it);
+ AllocatorTraits::construct(allocator, &(values[id]), copy.values[id]);
}
}

@@ -216,17 +217,17 @@
Value* new_values = allocator.allocate(new_capacity);
Item it;
for (nf->first(it); it != INVALID; nf->next(it)) {
- int jd = nf->id(it);;
+ int jd = nf->id(it);
if (id != jd) {
- allocator.construct(&(new_values[jd]), values[jd]);
- allocator.destroy(&(values[jd]));
+ AllocatorTraits::construct(allocator, &(new_values[jd]), values[jd]);
+ AllocatorTraits::destroy(allocator, &(values[jd]));
}
}
if (capacity != 0) allocator.deallocate(values, capacity);
values = new_values;
capacity = new_capacity;
}
- allocator.construct(&(values[id]), Value());
+ AllocatorTraits::construct(allocator, &(values[id]), Value());
}

// \brief Adds more new keys to the map.
@@ -260,8 +261,8 @@
}
}
if (found) continue;
- allocator.construct(&(new_values[id]), values[id]);
- allocator.destroy(&(values[id]));
+ AllocatorTraits::construct(allocator, &(new_values[id]), values[id]);
+ AllocatorTraits::destroy(allocator, &(values[id]));
}
if (capacity != 0) allocator.deallocate(values, capacity);
values = new_values;
@@ -269,7 +270,7 @@
}
for (int i = 0; i < int(keys.size()); ++i) {
int id = nf->id(keys[i]);
- allocator.construct(&(values[id]), Value());
+ AllocatorTraits::construct(allocator, &(values[id]), Value());
}
}

@@ -279,7 +280,7 @@
// and it overrides the erase() member function of the observer base.
virtual void erase(const Key& key) {
int id = Parent::notifier()->id(key);
- allocator.destroy(&(values[id]));
+ AllocatorTraits::destroy(allocator, &(values[id]));
}

// \brief Erase more keys from the map.
@@ -289,7 +290,7 @@
virtual void erase(const std::vector<Key>& keys) {
for (int i = 0; i < int(keys.size()); ++i) {
int id = Parent::notifier()->id(keys[i]);
- allocator.destroy(&(values[id]));
+ AllocatorTraits::destroy(allocator, &(values[id]));
}
}

@@ -302,8 +303,8 @@
allocate_memory();
Item it;
for (nf->first(it); it != INVALID; nf->next(it)) {
- int id = nf->id(it);;
- allocator.construct(&(values[id]), Value());
+ int id = nf->id(it);
+ AllocatorTraits::construct(allocator, &(values[id]), Value());
}
}

@@ -317,7 +318,7 @@
Item it;
for (nf->first(it); it != INVALID; nf->next(it)) {
int id = nf->id(it);
- allocator.destroy(&(values[id]));
+ AllocatorTraits::destroy(allocator, &(values[id]));
}
allocator.deallocate(values, capacity);
capacity = 0;