Skip to content

Adds missing subscript operator to ArrayIteratorBase - #1963

Merged
kennyweiss merged 3 commits into
developfrom
bugfix/kweiss/iterator-base-subscript
Aug 25, 2026
Merged

Adds missing subscript operator to ArrayIteratorBase#1963
kennyweiss merged 3 commits into
developfrom
bugfix/kweiss/iterator-base-subscript

Conversation

@kennyweiss

@kennyweiss kennyweiss commented Aug 25, 2026

Copy link
Copy Markdown
Member

Summary

  • This PR is a bugfix
  • It adds a missing subscript operator to core's ArrayIteratorBase, which is necessary to satisfy the random access iterator contract
    • This missing function was responsible for the error triggered in a clang-22 build on Mac (Build with clang-22 #1961) due to an apparent library change from a dereference to a subscript operator in the implementation of std::__sift_down, called by std::sort
    • Note: I don't have access to the environment from Build with clang-22 #1961, but the new tests trigger an error for the missing operator[] without the changes to ArrayIteratorBase and are resolved with this fix
  • Fixes Build with clang-22 #1961

@kennyweiss kennyweiss added this to the FY26 August release milestone Aug 25, 2026
@kennyweiss kennyweiss self-assigned this Aug 25, 2026
@kennyweiss kennyweiss added bug Something isn't working Core Issues related to Axom's 'core' component User Request Issues related to user requests labels Aug 25, 2026
@kennyweiss
kennyweiss force-pushed the bugfix/kweiss/iterator-base-subscript branch from 21cb31d to 07d1dbc Compare August 25, 2026 04:25
Comment on lines +52 to +55
/// \brief Returns the value at offset \a n from the current iterator position
AXOM_HOST_DEVICE
ValueType& operator[](IndexType n) const { return m_arrayPtr->flatIndex(BaseType::m_pos + n); }

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is the bugfix

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it make sense to put this in IteratorBase?

@kennyweiss kennyweiss Aug 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good suggestion, but I don't think so since other iterators that are not random access iterators derive from IteratorBase.

E.g. ItemCollection is a forward iterator, and it's advance would give the wrong answer for operator[](-1)
See:

/// Implementation of advance() as required by IteratorBase
void advance(IndexType n)
{
for(int i = 0; i < n; ++i)
{
BaseType::m_pos = m_collection->getNextValidIndex(BaseType::m_pos);
}
}

@Arlie-Capps Arlie-Capps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, Kenny!

@rhornung67 rhornung67 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thank you @kennyweiss for the quick fix

@kennyweiss
kennyweiss merged commit ee1474b into develop Aug 25, 2026
24 checks passed
@kennyweiss
kennyweiss deleted the bugfix/kweiss/iterator-base-subscript branch August 25, 2026 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Core Issues related to Axom's 'core' component User Request Issues related to user requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build with clang-22

4 participants