[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