Headers are getting polluted by `using namespace std`
mrexodia opened this issue · 6 comments
The ghidra source code is riddled with using namespace std in various header files. This is bad practice and it would be nice if this could be patched away. It causes all kinds of confusing issues (one notable one being with std::byte and byte from some Windows COM headers).
Ideally these occurrences would be patched out, but obviously the cost is high because you'd have to maintain an extensive patch set...
Thanks for reporting @mrexodia.
Yeah, that's a bit of a pain...
Just off the top of my head, maybe we can have a test file that includes the "super" header libsleigh.hh and then has some code that we expect to fail compilation (perhaps it can refer to an STL container type without the std:: prefix). And then if it compiles without an error, we can assume that one of the Sleigh headers has a using namespace std; that hasn't been patched out and we can fail the test.
I think I could also just submit a Ghidra PR to fix this as it's pretty uncontroversial that these don't belong in headers.
Any thoughts on this @ekilmer?
I think I could also just submit a Ghidra PR to fix this as it's pretty uncontroversial that these don't belong in headers.
This would be ideal. Depending on how big the diff of the patch is, we can try maintaining it in this repo for a while until it makes it upstream 🤞.
I opened #140 and set up some branches on our Ghidra fork to maybe make the patching process less painful (https://github.com/trail-of-forks/ghidra/wiki#sleigh).
It looks like there’s maybe 15 occurrences of using namespace. Unfortunately patching it manually is a lot of work. I gave up after 5 files.
My current idea is to use clangd or maybe write a clang-tidy pass. In this case it’s enough to do it for a few tokens, but in general it’s not so easy…
Thanks a lot! I'll try it on Monday ❤️
Unfortunately I didn't have time to try it yet, but I didn't forget about this...