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

Fix header namespace pollution. #2372

Merged
merged 2 commits into from
Oct 18, 2024
Merged

Conversation

nlutsenko
Copy link
Contributor

This removes using namespace ...; from headers, as such reducing header namespace pollution.
Whilst not critical for MoltenVK itself, when incorporated in a larger project makes sure that symbols/functionality is addressed appropriately and items within mvk namespace aren't considered to be part of the global namespace.

Also enabled the warning within xcodeproj, so it's visible when this is added again.

@CLAassistant
Copy link

CLAassistant commented Oct 9, 2024

CLA assistant check
All committers have signed the CLA.

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 submitting this.

LGTM.

Since you are modifying WARNING_CFLAGS, would you mind also adding the -Wunused-but-set-variable that you identified in PR #2370, please?

This will trigger a warning in the MVKSampler constructor. To fix that, can you also remove the two lines there referencing pSampReductInfo, please? Please leave the empty case statement though, to highlight that option exists and may be needed at a later time.

After your PR #2370, I was going to issue a follow up PR with those changes, but you might as well close the loop here.

@billhollings billhollings merged commit 30332f3 into KhronosGroup:main Oct 18, 2024
6 checks passed
@billhollings
Copy link
Contributor

Since you are modifying WARNING_CFLAGS, would you mind also adding the -Wunused-but-set-variable that you identified in PR #2370, please?

This will trigger a warning in the MVKSampler constructor. To fix that, can you also remove the two lines there referencing pSampReductInfo, please? Please leave the empty case statement though, to highlight that option exists and may be needed at a later time.

I've included these additional items in PR #2382.

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.

3 participants