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

utils/Geometry: add "constexpr" #13918

Merged
merged 1 commit into from Jun 6, 2018

Conversation

@MaxKellermann
Copy link
Contributor

commented May 21, 2018

This allows the compiler to invoke those methods at compile time,
instead of adding runtime code.

@FernetMenta

This comment has been minimized.

Copy link
Member

commented May 23, 2018

@pkerling @notspiff could you have a look please

@pkerling pkerling self-requested a review May 26, 2018
@pkerling pkerling requested a review from notspiff May 26, 2018
Copy link
Member

left a comment

PtInRect could also be made constexpr if I'm not mistaken.

I'm not sure how much practical difference these changes will make, but it's certainly not wrong to have them, so +1.

@@ -98,13 +98,13 @@ template <typename T> class CPointGen
};

template<typename T>
inline bool operator==(const CPointGen<T> &point1, const CPointGen<T> &point2) noexcept
inline constexpr bool operator==(const CPointGen<T> &point1, const CPointGen<T> &point2) noexcept

This comment has been minimized.

Copy link
@pkerling

pkerling May 26, 2018

Member

You can remove the inline

This comment has been minimized.

Copy link
@MaxKellermann

MaxKellermann May 29, 2018

Author Contributor

True, but this is orthogonal to my change. The inline here was never necessary or useful.

This comment has been minimized.

Copy link
@pkerling

pkerling May 29, 2018

Member

True, but this is orthogonal to my change.

I disagree. constexpr implies inline in this case so inline can be removed (no matter whether it was necessary before).

This comment has been minimized.

Copy link
@MaxKellermann

MaxKellermann May 29, 2018

Author Contributor

Oh, you're right. I thought template would also imply inline, but it doesn't - it's just that both turn the function into a weak symbol, and both suppress -Wunused.

{
return (point1.x == point2.x && point1.y == point2.y);
}

template<typename T>
inline bool operator!=(const CPointGen<T> &point1, const CPointGen<T> &point2) noexcept
inline constexpr bool operator!=(const CPointGen<T> &point1, const CPointGen<T> &point2) noexcept

This comment has been minimized.

Copy link
@pkerling

pkerling May 26, 2018

Member

same

This comment has been minimized.

Copy link
@pollito05
{
return (x > h) ? h : ((x < l) ? l : x);
}
};

template<typename T>
inline bool operator==(const CRectGen<T> &rect1, const CRectGen<T> &rect2) noexcept
constexpr inline bool operator==(const CRectGen<T> &rect1, const CRectGen<T> &rect2) noexcept

This comment has been minimized.

Copy link
@pkerling
{
return (rect1.x1 == rect2.x1 && rect1.y1 == rect2.y1 && rect1.x2 == rect2.x2 && rect1.y2 == rect2.y2);
}

template<typename T>
inline bool operator!=(const CRectGen<T> &rect1, const CRectGen<T> &rect2) noexcept
constexpr inline bool operator!=(const CRectGen<T> &rect1, const CRectGen<T> &rect2) noexcept

This comment has been minimized.

Copy link
@pkerling
@MaxKellermann MaxKellermann force-pushed the MaxKellermann:geometry_constexpr branch from 4a3909d to eb059e7 Jun 4, 2018
@MaxKellermann

This comment has been minimized.

Copy link
Contributor Author

commented Jun 4, 2018

Rebased & removed redundant inline.

@pkerling

This comment has been minimized.

Copy link
Member

commented Jun 5, 2018

jenkins build this please

This allows the compiler to invoke those methods at compile time,
instead of adding runtime code.
@pkerling pkerling force-pushed the MaxKellermann:geometry_constexpr branch from eb059e7 to f871e9a Jun 5, 2018
@pkerling

This comment has been minimized.

Copy link
Member

commented Jun 5, 2018

Added PtInRect. Will merge after jenkins is through.

@pkerling

This comment has been minimized.

Copy link
Member

commented Jun 6, 2018

failures unrelated

@pkerling pkerling merged commit 8ebbb17 into xbmc:master Jun 6, 2018
1 check failed
1 check failed
default Sorry, building this PR failed. Please check the logs for the errors.
Details
@pkerling

This comment has been minimized.

Copy link
Member

commented Jun 6, 2018

Thanks!

@pkerling pkerling added this to the Leia 18.0-alpha2 milestone Jun 6, 2018
@MaxKellermann MaxKellermann deleted the MaxKellermann:geometry_constexpr branch Jun 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.