[PATCH] D60030: [ELF] Merge LinkerScript::addSymbol and declareSymbol

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D60030#1449524 <https://reviews.llvm.org/D60030#1449524>, @ruiu wrote:

> Sometimes, I actually like separating functions even if the separated functions have some amount of common code.


I agree with this point: making duplication if it helps readability. And I noticed that rLLD357373 <https://reviews.llvm.org/rLLD357373> was the recent interpretation of this rule. I did think over before creating this, and i think it slightly improves readability.

IMHO, after merging, the common/different parts are emphasized and it stops a reader from asking "what the two functions have in common and what are their differences?" (I asked myself this question while reading it because the shared parts are not too trivial: `if (!shouldDefineSym(Cmd))`, `uint8_t Visibility = Cmd->Hidden ? STV_HIDDEN : STV_DEFAULT;`, `replaceSymbol<Defined>` and the different choices of placeholder `st_value`).


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60030/new/

https://reviews.llvm.org/D60030