This makes it clear ELF/**/*.cpp files define things in the lld::elf
namespace and simplifies elf::foo to foo.
Details
Diff Detail
- Repository
- rLLD LLVM Linker
- Build Status
Buildable 39065 Build 39065: arc lint + arc unit
Event Timeline
I think that somewhere in the Coding Style Guideline, it is recommended to keep namespace {} as narrow as possible. This is why I chose this style in the first place. Has that changed?
I don't know the history but https://llvm.org/docs/CodingStandards.html#id56 gives an example that does this:
namespace llvm { namespace knowledge { ...
Yeah, I'm fine with quoting the entire file with the namespaces, but I'm not entirely happy about adding endian:: to read32. Looks like the problem is that we use the same name for two functions. Can you just rename one to avoid the name collision? Does that make the code look simpler?
Adding endian:: is due to the rule of unqualified name lookup. Renaming either family of functions below will cause the change of hundreds of call sites changes.
llvm::support::endian::write32 and its friends lld::elf::write32 and its friends
llvm::support::endian::* is used in very few files. And as this patch suggests, only Mips.cpp needs endian::*. I think this change clarifies things without making code more complex.
lld::elf::* are more commonly used in lld. This will end up with a lot of qualified write32 read16 calls. I think we should reduce the number of using namespace llvm::support::endian;.
I mean we can replace all occurrences of endian::write32<E>() with write32(). It looks like lld still compiles after I run the following command to replace them.
sed -i 's/write32<.>/write32/g' lld/ELF/Arch/Mips.cpp
This will make the generated code slightly worse because the implementation tests config->endianness. llvm::support::endian::write16(p, v, config->endianness);
Personally I prefer keeping the templated version, though I don't mind changing it if you are strong about this or @atanasyan is fine with lld::elf::* functions.
I originally preferred the template version of write32 for the exact same reason, but I now prefer non-template write32 because it doesn't seem slow at all, perhaps because the branch is highly predictable. Every time it actually branches to the same direction. So maybe the template version is just making the code too verbose?