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.

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

[hist] Fix assertion failure (invalid comparator) #17211

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bellenot
Copy link
Member

@bellenot bellenot commented Dec 5, 2024

Fix the following error (visible with the /RTC1 flag on Windows)

libHist: Debug Assertion Failed!
Expression: invalid comparator

Fix the following error (visible with the `/RTC1` flag on Windows)
```
libHist: Debug Assertion Failed!
Expression: invalid comparator
```
@bellenot bellenot requested a review from couet December 5, 2024 14:00
@bellenot bellenot self-assigned this Dec 5, 2024
@bellenot bellenot requested a review from lmoneta as a code owner December 5, 2024 14:00
Copy link
Member

@pcanal pcanal left a comment

Choose a reason for hiding this comment

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

If/Since this changes are needed we also need to update the function documentations and enhance the commit message to point/hint to the part of the standard indicating whether the comparators should strict (existing code) or not (this PR).

@bellenot
Copy link
Member Author

bellenot commented Dec 5, 2024

If/Since this changes are needed we also need to update the function documentations and enhance the commit message to point/hint to the part of the standard indicating whether the comparators should strict (existing code) or not (this PR).

You mean this: https://en.cppreference.com/w/cpp/named_req/Compare

@pcanal
Copy link
Member

pcanal commented Dec 5, 2024

You mean this: en.cppreference.com/w/cpp/named_req/Compare

Yes something like that .. except that I am reading it as asking for strict comparison ... For all a, comp(a, a) == false.

Copy link

github-actions bot commented Dec 5, 2024

Test Results

    18 files      18 suites   3d 23h 53m 13s ⏱️
 2 664 tests  2 660 ✅ 0 💤 4 ❌
46 234 runs  46 230 ✅ 0 💤 4 ❌

For more details on these failures, see this check.

Results for commit 9f6c498.

♻️ This comment has been updated with latest results.

@bellenot
Copy link
Member Author

bellenot commented Dec 6, 2024

You mean this: en.cppreference.com/w/cpp/named_req/Compare

Yes something like that .. except that I am reading it as asking for strict comparison ... For all a, comp(a, a) == false.

Yes, I know, but the original code is from 2003 (see the commit here and was not using std::stable_sort, should I completely rewrite those methods?

@bellenot bellenot closed this Dec 9, 2024
@bellenot bellenot reopened this Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants