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

Delete using namespace in command queue interface header #18355

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nhuang-tt
Copy link
Member

@nhuang-tt nhuang-tt commented Feb 26, 2025

Ticket

#18325

Problem description

  • api/command_queue_interface.hpp has a using namespace tt::tt_metal at the top of the file in the global namespace, causing the entire repository's global namespace to be polluted and making conflicting types.

What's changed

  • Delete using namespace tt::tt_metal from the header
  • Fully qualify types in tt::tt_metalin header files.
  • Updated the cpp files

Checklist

@nhuang-tt nhuang-tt requested a review from tt-dma as a code owner February 26, 2025 16:46
Copy link
Contributor

@omilyutin-tt omilyutin-tt left a comment

Choose a reason for hiding this comment

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

Thank you for doing this - this is really great! 🔥

Adding using namespace ... in .cpp makes sense to do the migration. I think ideally, we wouldn't even need that - spelling full types or adding explicit aliases for stuff we need is less invasive and more robust.

@nhuang-tt nhuang-tt force-pushed the nhuang/delete-using-namespace-c branch from 2f6ac22 to 2c71d7d Compare February 26, 2025 17:27
@nhuang-tt nhuang-tt force-pushed the nhuang/delete-using-namespace-c branch from 2c71d7d to ad74270 Compare February 26, 2025 22:06
@nhuang-tt nhuang-tt force-pushed the nhuang/delete-using-namespace-c branch 2 times, most recently from 1b056ee to 0f27aea Compare February 28, 2025 18:11
@nhuang-tt nhuang-tt self-assigned this Feb 28, 2025
@nhuang-tt nhuang-tt force-pushed the nhuang/delete-using-namespace-c branch 3 times, most recently from f1cabde to 45a394b Compare March 1, 2025 02:37
Copy link
Contributor

@jvegaTT jvegaTT left a comment

Choose a reason for hiding this comment

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

Great PR but be mindful this will basically clash with everything. Be super careful about merge conflicts and please make an announcement on slack when it's merged so those with open PRs are aware

@nhuang-tt nhuang-tt force-pushed the nhuang/delete-using-namespace-c branch 4 times, most recently from a4e8fa1 to 815de5e Compare March 1, 2025 05:25
- Removed using namespace from command_queue_interface.hpp
@nhuang-tt nhuang-tt force-pushed the nhuang/delete-using-namespace-c branch from 815de5e to eecaa6e Compare March 1, 2025 06:35
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.