[PATCH] [ELF] Emit DT_TEXTREL dynamic table flag
Rui Ueyama
ruiu at google.com
Mon May 12 13:41:38 PDT 2014
================
Comment at: lib/ReaderWriter/ELF/SectionChunks.h:959
@@ +958,3 @@
+ bool canModifyReadonlySection() const {
+ for (const auto &rel : _relocs)
+ if ((rel.first->permissions() & DefinedAtom::permRW_) !=
----------------
Simon Atanasyan wrote:
> Rui Ueyama wrote:
> > can you expand "auto"?
> The type name of `rel` is rather long - `std::pair<const DefinedAtom *, const Reference *>`. It is possible to make it shorter using typedefs but I cannot invent a good short name.
I'd probably write like this
for (const auto &rel : _relocs) {
const DefinedAtom *atom = rel.first
...
But I feel it might be a bit verbose. It's up to you.
================
Comment at: lib/ReaderWriter/ELF/SectionChunks.h:960
@@ +959,3 @@
+ for (const auto &rel : _relocs)
+ if ((rel.first->permissions() & DefinedAtom::permRW_) !=
+ DefinedAtom::permRW_)
----------------
Simon Atanasyan wrote:
> Rui Ueyama wrote:
> > nit: fit in one line
> Yeah, the expression does not look good. But how can we fit it into one line?
It looked me that it would fit in 80 chars. It actually isn't. Sorry.
http://reviews.llvm.org/D3716
More information about the llvm-commits
mailing list