-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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.
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
Create comparator wrapper classes for MSVC 2019 build issues: #3786
Conversation
src/test/csf/ledgers.h
Outdated
@@ -21,6 +21,7 @@ | |||
|
|||
#include <ripple/basics/UnorderedContainers.h> | |||
#include <ripple/basics/chrono.h> | |||
#include <ripple/basics/comparitors.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is misspelled? "comparators.h"?
Also, the commit message misspells "comparators" I think. That would be good to fix. |
🤦♂️ Will fix. |
I fixed the typo in a |
👍 Looks great! |
src/ripple/basics/comparators.h
Outdated
//------------------------------------------------------------------------------ | ||
/* | ||
This file is part of rippled: https://github.com/ripple/rippled | ||
Copyright (c) 2012, 2013 Ripple Labs Inc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copyright (c) 2021 Ripple Labs Inc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦 Fixed.
src/ripple/basics/comparators.h
Outdated
|
||
} // namespace ripple | ||
|
||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing carriage return
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 LGTM
* The std::less and std::equal_to comparators have [[nodiscard]], which conflicts with boost::bimap validations.
Squashed and rebased onto 1.8.0-b1 |
High Level Overview of Change
Travis CI builds started failing for Visual Studio 2019 builds. Example 1, example 2.
@scottschurr , @miguelportilla , and I tracked the problem down to a recent update to VS 2019. The unaltered current develop branch fails with this version, so it is definitely not due to changes in rippled code.
The underlying cause is that the
std::less
andstd::equal_to
comparators have added the[[nodiscard]]
attribute, which conflicts withboost::bimap
type validations, which call the functions without capturing the result.This fix adds classes to exactly wrap the comparators, removing
[[nodiscard]]
attributes.Type of Change
Future Tasks
The new wrapper classes can be removed when either:
This change is![Reviewable](https://camo.githubusercontent.com/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)