open-mmlab / mmcv

OpenMMLab Computer Vision Foundation
https://mmcv.readthedocs.io/en/latest/
Apache License 2.0
5.73k stars 1.61k forks source link

[Bug] `using namespace` in header file #3088

Open r-barnes opened 2 months ago

r-barnes commented 2 months ago

Prerequisite

Environment

LLVM 15 with -Wheader-hygiene enabled on a project including MMCV.

Reproduces the problem - code sample

I don't have a direct reproducer since I'm on a custom build system, but compiling with -Wheader-hygiene enabled would reproduce the issue.

Reproduces the problem - command or script

N/A

Reproduces the problem - error message

N/A

Additional information

This line puts at in the global namespace:

using namespace at;

this is universally considered a bad practice since it pollutes the namespace for any file including a given header.

In an attempt to reduce this practice in a large codebase, I am enabling -Wheader-hygiene. This breaks for MMCV because of the line above.

If we can remove that line and either use at::Tensor as appropriate or using namespace at; in the implementation (cpp) files, that would solve the problem and align with best practices.