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

BUG: Fixed strict weak ordering for itkIPLFileNameList.cxx descend co… #5126

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

ctaylo41
Copy link
Contributor

@ctaylo41 ctaylo41 commented Jan 8, 2025

…mpare

  • Changed type of ascend and descend compare method from int to bool
  • Fixed subtraction is not comparison for both ascend and descend comparator
  • Fixed strict weak odering for descend comparator which was found using libc++ hardening mode in the debug setting
  • Changed name of ascend and descend comparator from qsort to remove confussion as qsort outputs -1,0,1 vs sort 0,1

@github-actions github-actions bot added type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances area:IO Issues affecting the IO module labels Jan 8, 2025
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you for contributing a pull request! 🙏

Welcome to the ITK community! 🤗👋☀️

We are glad you are here and appreciate your contribution. Please keep in mind our community participation guidelines. 📜
More support and guidance on the contribution process can be found in our contributing guide. 📖

This is an automatic message. Allow for time for the ITK community to be able to read the pull request and comment
on it.

@seanm
Copy link
Contributor

seanm commented Jan 8, 2025

@N-Dekker @dzenanz @thewtex : @ctaylo41 here is our intern this semester, and he's trying this with our code and dependencies: https://libcxx.llvm.org/Hardening.html

@thewtex
Copy link
Member

thewtex commented Jan 8, 2025

@seanm wonderful!!

@ctaylo41 welcome!

This looks good to me. The ghostflow check complaint does not appear valid from the most recent push.

Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Good even as-is.

Modules/IO/IPL/src/itkIPLFileNameList.cxx Outdated Show resolved Hide resolved
@dzenanz
Copy link
Member

dzenanz commented Jan 8, 2025

Overflow from the overly long first line spilling into second line in commit message is what makes ghostflow unhappy. Shortening the first line to be <72 characters seems like an obvious solution.

- Changed type of ascend and descend compare method from int to bool
- Fixed subtraction is not comparison for both ascend and descend comparator
- Fixed strict weak odering for descend comparator which was found using libc++ hardening mode in the debug setting
- Changed name of ascend and descend comparator from qsort to remove confussion as qsort outputs -1,0,1 vs sort 0,1
@dzenanz
Copy link
Member

dzenanz commented Jan 8, 2025

I added a blank line in the commit message via force-push. ghostflow is now green.

@dzenanz dzenanz merged commit 78f14df into InsightSoftwareConsortium:master Jan 9, 2025
11 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:IO Issues affecting the IO module type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants