[PATCH] [lld] Define DefinedAtom::sectionSize.

[Rui Ueyama]

On Tue, Mar 3, 2015 at 5:16 AM, Shankar Easwaran <
shankar.kalpathi.easwaran at gmail.com> wrote:

> Other than the above mentioned comments code LGTM
>
>
> ================
> Comment at: include/lld/Core/DefinedAtom.h:239
> @@ +238,3 @@
> +  ///
> +  /// Merge::mergeByLargestSection is defined in terms of section size
> +  /// and not in terms of atom size, so we need this function separate
> ----------------
> Why are we mentioning the purpose if this function here. We could drop
> this comment.
>

It helps reader understand not only "what" this function does but also
"why" we have this here. That's a useful comment.


> ================
> Comment at: lib/ReaderWriter/Native/ReaderNative.cpp:49
> @@ -51,1 +48,3 @@
> +  uint64_t size() const override { return _ivarData->contentSize; }
> +  uint64_t sectionSize() const override { return _ivarData->sectionSize; }
>
> ----------------
> Can you add a new line before this function
>
> http://reviews.llvm.org/D7966
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150303/c437d991/attachment.html>