[PATCH] D80002: [yaml2obj] - Implement the "SectionHeaderTable" tag.

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 27 08:38:17 PDT 2020


MaskRay accepted this revision.
MaskRay added a comment.

Looks great!



================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:1483
+
+  for (auto& It : Seen)
+    reportError("section header contains undefined section '" + It.getKey() + "'");
----------------
grimar wrote:
> jhenderson wrote:
> > Formatting here looks off to me? Also, why not `const &`?
> > 
> > (I think @MaskRay was suggesting `const auto &It` with a space in the right place)
> Ah.. I understood it differently: there is just a single line where `It` is used and it is obvious it does not change. LLVM uses `auto` without `const` in many places (as well as with the `const`), so I've removed it because assumed that the comment was about that we can omit it for the slightly shorter code.
> 
> I do not have any preference by myself, because I do not like using "auto" in general and so both ways does not look very nice to me. I'll return the "const" back since the initial comment was misunderstood.
`const auto &` is fine. The full type may be too cumbersome to type.


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

https://reviews.llvm.org/D80002





More information about the llvm-commits mailing list