Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Fix - core.stdcpp.vector.vector does not implement opEquals #3255

Merged
merged 1 commit into from
Oct 31, 2020

Conversation

n8sh
Copy link
Member

@n8sh n8sh commented Oct 28, 2020

No description provided.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @n8sh!

Bugzilla references

Auto-close Bugzilla Severity Description
21346 normal core.stdcpp.vector.vector does not implement opEquals

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "stable + druntime#3255"

@dlang-bot dlang-bot added the Bug Fix Include reference to corresponding bugzilla issue label Oct 28, 2020
@@ -114,6 +114,17 @@ extern(D):
///
void append(T[] array) { insert(length, array); }

/// Performs elementwise equality check.
bool opEquals(this This, That)(auto ref That rhs)
Copy link
Member Author

Choose a reason for hiding this comment

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

The complicated signature is because opEquals might or might not be callable with a const vector!T for any particular T.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this definitely the best way to express this comparison?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not definitely.


/// Hash to allow `vector`s to be used as keys for built-in associative arrays.
/// **The result will generally not be the same as C++ `std::hash<std::vector<T>>`.**
size_t toHash() const { return .hashOf(as_array); }
Copy link
Member Author

Choose a reason for hiding this comment

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

toHash on the other hand needs to be a non-template const function or else it will be ignored by the compiler for the purpose of determining if a particular struct has a non-default hash code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please leave an obvious TODO that we should use std::hash in the future when it's written?

if (is(immutable That == immutable vector)) { return as_array == rhs.as_array; }

/// Performs lexicographical comparison.
static if (is(typeof((ref T a, ref T b) => a < b)))
Copy link
Member Author

Choose a reason for hiding this comment

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

The static if is so __traits(hasMember, vector!T, "opCmp") will be false for vectors that do not support comparison.

@12345swordy
Copy link
Contributor

@TurkeyMan review this please.

Copy link
Contributor

@TurkeyMan TurkeyMan left a comment

Choose a reason for hiding this comment

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

LGTM

@dlang-bot dlang-bot merged commit 5c31195 into dlang:stable Oct 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Fix Include reference to corresponding bugzilla issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants