[PATCH] D59982: [ELF] Rename SyntheticSection::empty to more appropriate needed() with opposite meaning

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 29 03:05:08 PDT 2019


jhenderson added a comment.

In D59982#1447400 <https://reviews.llvm.org/D59982#1447400>, @grimar wrote:

> In D59982#1447395 <https://reviews.llvm.org/D59982#1447395>, @MaskRay wrote:
>
> > As a non-native English speaker my interpretation may be inaccurate and I am open to other suggestions :)
>
>
> We need a help then :) I added @jhenderson, hopefully, he has an opinion about what can be better here.


Thanks 😆

I think I like `needed()` more than `used()`, but either is probably valid. "needed" implies it is required, and is independent of its usage, which I think is probably the desired semantics (you could of course just use `required` instead, but `needed` is fine). - I could imagine an architecture where it is important to create specific synthetic section regardless of anything else in the ELF (e.g. nothing else might actually use that section within the link). By the way, since you're changing from `empty()`, whose name mirrors a common function name in the standard libraries, to something that isn't, you should probably make it into a verb phrase (i.e. `isNeeded()`), based on they style guide.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D59982





More information about the llvm-commits mailing list