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

Remove 'using namespace std' statement. Fixes #276 #281

Merged
merged 4 commits into from
Nov 28, 2023

Conversation

cguimaraes
Copy link
Member

Fixes #276

@Mallets
Copy link
Member

Mallets commented Nov 17, 2023

LGTM but I'd really like a review from @milyin

@cguimaraes
Copy link
Member Author

I forgot to mention that the compilation as backend for zenoh-c also passes.

@milyin
Copy link
Contributor

milyin commented Nov 21, 2023

@cguimaraes this fix still doesn't work beacuse it leaks out defines

#define atomic_store_explicit std::atomic_store_explicit
#define atomic_fetch_add_explicit std::atomic_fetch_add_explicit
#define atomic_fetch_sub_explicit std::atomic_fetch_sub_explicit
#define memory_order_acquire std::memory_order_acquire
#define memory_order_release std::memory_order_release
#define memory_order_relaxed std::memory_order_relaxed

so the following code fails to compile with zenoh-pico:

    std::memory_order order = std::memory_order_relaxed;

with error:

[build] /home/milyin/ZS/zenoh-cpp/examples/universal/z_get.cxx:28:31: error: no member named 'std' in namespace 'std'; did you mean simply 'std'?
[build]     std::memory_order order = std::memory_order_relaxed;

Copy link
Contributor

@milyin milyin left a comment

Choose a reason for hiding this comment

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

the defines with names same as functions of std library still leaks to global namespace

@milyin
Copy link
Contributor

milyin commented Nov 22, 2023

Looks good now, thank you!

@cguimaraes
Copy link
Member Author

@Mallets @p-avital this is ready to merge. Can you take care of it please?

@p-avital p-avital merged commit 4b54b5e into eclipse-zenoh:master Nov 28, 2023
46 checks passed
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.

remove "using namespace std" statement [Bug]
4 participants