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 #3235

Closed
SunBlack opened this issue Jul 16, 2019 · 7 comments · Fixed by #4203
Closed

Remove using namespace std #3235

SunBlack opened this issue Jul 16, 2019 · 7 comments · Fixed by #4203
Labels
effort: medium Rough estimate of time needed to fix/implement/solve good first issue Skills/areas of expertise needed to tackle the issue kind: todo Type of issue

Comments

@SunBlack
Copy link
Contributor

I suggest to remove using namespace std (only std, but not e.g. std::chrono_literals). Reason: There are some math function which exists as C method and as C++, so it is unclear which the compiler is using.

Example:

if (acos (std::fabs (angle1)) > acos (std::fabs (angle2)))

Here acos is used, which only accepts double values, during std::acos have different overloadings (e.g. for float). Currently you have always to check if there is an using namespace std to get an idea, which version is used. If we disallow usage of using namespace std it is always clear, that acos without std:: isn't the wanted code by us.

@taketwo
Copy link
Member

taketwo commented Jul 16, 2019

I am a proponent of not using using namespace std neither in headers, nor in implementation files. I believe our headers are already clean of this, but cpp files are probably not. If you are up for the challenge of removing these, you have my full support.

@stale
Copy link

stale bot commented May 19, 2020

Marking this as stale due to 30 days of inactivity. It will be closed in 7 days if no further activity occurs.

@stale stale bot added the status: stale label May 19, 2020
@kunaltyagi
Copy link
Member

1 header is still a culprit: cuda/segmentation/include/pcl/cuda/segmentation/mssegmentation.h

Lots of cpp files though (209) including one header file which is using std inside octree namespace: io/include/pcl/compression/color_coding.h

@stale stale bot removed the status: stale label May 20, 2020
@kunaltyagi kunaltyagi added effort: medium Rough estimate of time needed to fix/implement/solve good first issue Skills/areas of expertise needed to tackle the issue kind: todo Type of issue labels May 20, 2020
@gnawme
Copy link
Contributor

gnawme commented Jun 16, 2020

I'm trying to push changes for this issue, but I keep getting:

ERROR: Permission to PointCloudLibrary/pcl.git denied to gnawme.

Do I need to request access first? This happens on two different machines in two different OSes.

@kunaltyagi
Copy link
Member

You need to push to your fork and then open a PR. PointCloudLibrary/pcl is protected

@gnawme
Copy link
Contributor

gnawme commented Jun 16, 2020

I'm trying to push my branch so I can open a PR, like so:

git push -u origin norm.evangelista/PCL-3235-remove-namespace-std

@kunaltyagi
Copy link
Member

Your remote (say origin or fork) needs to point to your fork: https://github.com/gnawme/pcl.git. Once this is done, you just need git push -u fork <branch-name>

@kunaltyagi kunaltyagi linked a pull request Jun 18, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: medium Rough estimate of time needed to fix/implement/solve good first issue Skills/areas of expertise needed to tackle the issue kind: todo Type of issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants