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

Replace .at() calls with custom method showing actual source location on failure #12684

Merged
merged 1 commit into from
Sep 26, 2022

Conversation

Nekotekina
Copy link
Member

Adds a supplement for common STL .at() methods but which also provides source location.

@@ -1056,6 +1056,41 @@ template <typename T, usz Size>
return static_cast<u32>(Size);
}

template <typename T, usz Size, typename U>
[[nodiscard]] constexpr auto& at32(const T (&array)[Size], u32 index,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use std::size.

Copy link
Contributor

@elad335 elad335 Sep 19, 2022

Choose a reason for hiding this comment

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

I mean this overload is better removed in this widely used header, and std::size can help with that. A global concept can be written as std::as_const(x)[0].

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I'll look into it

@Nekotekina Nekotekina changed the title [WIP] Minor debugging Replace .at() calls with custom method showing actual source location on failure Sep 20, 2022
@Nekotekina Nekotekina requested review from elad335, Megamouse and kd-11 and removed request for elad335 September 20, 2022 16:55
{
broadcast_insert(range);
m_data[block_for(range.start)].insert_or_assign(range.start, std::forward<T>(value));
}

inline iterator find(const u32 key)
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the methods find and at have been swapped unintendedly.

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed at and added count to make it compatible with at32 requirements

@Megamouse
Copy link
Contributor

I'd rather have these data types wrapped or something instead of replacing all the calls with some obscure syntax

Copy link
Contributor

@kd-11 kd-11 left a comment

Choose a reason for hiding this comment

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

RSX side lgtm

@Megamouse Megamouse added the Refactoring Refactors or simplifies existing code label Sep 22, 2022
Works like .at() but uses source location for "exception".
@Nekotekina
Copy link
Member Author

I think an alternative (replacing container types) would be too intrusive and not always possible.

@Nekotekina Nekotekina merged commit 6ff6a49 into RPCS3:master Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Refactoring Refactors or simplifies existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants