[PATCH] D53623: Rename InputSectionDescription SectionPattern.
George Rimar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 24 02:14:05 PDT 2018
grimar added inline comments.
================
Comment at: lld/ELF/Arch/ARM.cpp:40
void addPltSymbols(InputSection &IS, uint64_t Off) const override;
- void addPltHeaderSymbols(InputSection &ISD) const override;
+ void addPltHeaderSymbols(InputSection &Pat) const override;
bool needsThunk(RelExpr Expr, RelType Type, const InputFile *File,
----------------
This just should have been named `IS` initially it seems.
================
Comment at: lld/ELF/LinkerScript.h:146
// Also it may be surrounded with SORT() command, so contains sorting rules.
-struct SectionPattern {
- SectionPattern(StringMatcher &&Pat1, StringMatcher &&Pat2)
+struct Wildcard {
+ Wildcard(StringMatcher &&Pat1, StringMatcher &&Pat2)
----------------
`SectionPattern` -> `Wildcard` renaming looks good to me.
It also matches the linker script doc and sounds IMO better than `SectionPattern`:
https://sourceware.org/binutils/docs/ld/Input-Section-Wildcards.html#Input-Section-Wildcards
================
Comment at: lld/ELF/LinkerScript.h:161
+// https://sourceware.org/binutils/docs/ld/Input-Section-Basics.html
+struct SectionPattern : BaseCommand {
+ SectionPattern(StringRef FilePattern)
----------------
Honestly, I do not feel `SectionPattern` is a better name.
As far I remember we named it `InputSectionDescription` because it was named like that in the `ld` documentation:
https://sourceware.org/binutils/docs/ld/Input-Section-Basics.html
And I think we usually tried to follow the documented names in such cases, even if they were not ideal. Though in this case
I see probably see nothing wrong with the naming in the documentation.
What about renaming to `SectionDescription`? It is also short.
https://reviews.llvm.org/D53623
More information about the llvm-commits
mailing list