[PATCH] D38137: [ELF] Simpler scheme for handling common symbols
Rui Ueyama via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 21 10:39:10 PDT 2017
ruiu added a comment.
First of all, could you read https://llvm.org/docs/CodingStandards.html and follow that style? One of the easiest way is to run clang-format-diff on your patch so that everything in your patch is formatted automatically.
================
Comment at: ELF/Driver.cpp:1088
+ if (Config->DefineCommon)
+ lld::elf::commonToBss<ELFT>();
+
----------------
Remove `lld::elf::`. In general, please look around and make your new code looks like the same in style as other code.
================
Comment at: ELF/MarkLive.cpp:67-68
- if (auto *Sym = dyn_cast<DefinedCommon>(&B)) {
- Sym->Live = true;
- return;
- }
+ if (auto *Sym = dyn_cast<DefinedCommon>(&B))
+ llvm_unreachable("unreachable: common are converted to bss");
----------------
Just remove the code.
================
Comment at: ELF/MarkLive.cpp:229-230
}
if (auto *S = dyn_cast_or_null<DefinedCommon>(Sym))
- S->Live = true;
+ llvm_unreachable("unreachable: common are converted to bss");
};
----------------
Remove
================
Comment at: ELF/SyntheticSections.cpp:64
- Sym->Section = make<BssSection>("COMMON");
- size_t Pos = Sym->Section->reserveSpace(Sym->Size, Sym->Alignment);
+ // create a fake section for the common data
+ auto * Section = make<BssSection>("COMMON");
----------------
create -> Create
It's not a fake section. We call it a synthetic section.
Please add a full stop at end.
================
Comment at: ELF/SyntheticSections.cpp:65
+ // create a fake section for the common data
+ auto * Section = make<BssSection>("COMMON");
+ InputSections.push_back(Section);
----------------
`auto * ` -> `auto *`
================
Comment at: ELF/SyntheticSections.cpp:69-70
+ size_t Pos = Section->reserveSpace(Sym->Size, Sym->Alignment);
assert(Pos == 0);
(void)Pos;
+ auto * File = Sym->getFile();
----------------
Remove `Pos` and `assert`.
================
Comment at: ELF/SyntheticSections.cpp:71
(void)Pos;
- Sym->Section->File = Sym->getFile();
- Ret.push_back(Sym->Section);
+ auto * File = Sym->getFile();
+
----------------
We do not use `auto` unless its actual type is obvious from RHS.
================
Comment at: ELF/SyntheticSections.cpp:73
+
+ // change the symbol to be a regular definition
+ Section->File = File;
----------------
change -> Change
Add a full stop.
https://reviews.llvm.org/D38137
More information about the llvm-commits
mailing list