[PATCH] D17529: ELF: Implement ICF.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 26 07:53:50 PST 2016


ruiu added inline comments.

================
Comment at: ELF/InputSection.h:175
@@ -161,1 +174,3 @@
+  // Used by ICF.
+  uint64_t GroupId = 0;
 };
----------------
grimar wrote:
> ruiu wrote:
> > grimar wrote:
> > > This comment is very tight. It has no value as anyone who look at this variable can quick search and find that it is used somewhere in ICF.
> > > I suggest to expand or to remove it.
> > I think the comments are valuable for first-time readers.
> Of course. What I meant - for me as for first time reader of this, it is not clear anyways what GroupId do from this comment.
> It says it is used in ICF, that what I can find in few seconds, but question is "for what", right ? Comments should explain the details and not the obvious things.
That is explained in ICF.cpp so we don't want to repeat that in the other file. The point is that you can just ignore this field unless you are working on ICF because it is used (only) by ICF.


http://reviews.llvm.org/D17529





More information about the llvm-commits mailing list