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

Misc Cleanup #2025

Merged
merged 5 commits into from
Sep 28, 2023
Merged

Misc Cleanup #2025

merged 5 commits into from
Sep 28, 2023

Conversation

etang-cw
Copy link
Contributor

Minor things I changed while working on other features
You can check the individual commits for details, the biggest one is a modification of MVKArrayRef so you use MVKArrayRef<const X> instead of const MVKArrayRef<X>, as you can't accidentally un-const the former by copying it into a local variable, (which you can do with ArrayRefs, since they don't own their storage)

- Use relaxed atomics where possible
- Calling operator= on a refcounted object should not reinitialize the refcount
Nothing used it, and you should always be able to `static_cast<MVKBaseObject*>` for any object without a crazy inheritance tree
It doesn't do anything, and we don't want anyone to think it does something
Copy link
Contributor

@billhollings billhollings left a comment

Choose a reason for hiding this comment

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

Thanks for the submission! Nice fixes.

LGTM. Just a couple of syntax nits.

MoltenVK/MoltenVK/Utility/MVKFoundation.h Outdated Show resolved Hide resolved
MoltenVK/MoltenVK/Utility/MVKFoundation.h Outdated Show resolved Hide resolved
Make everything constexpr, remove direct access to members
It's very easy to accidentally un-const a `const MVKArrayRef<T>`, since ArrayRefs are meant to be passed by value
Copy link
Contributor

@billhollings billhollings left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@billhollings billhollings merged commit 568cc3a into KhronosGroup:main Sep 28, 2023
6 checks passed
@etang-cw etang-cw deleted the MiscCleanup branch September 28, 2023 22:09
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