<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Oct 25, 2016 at 3:24 PM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 25 October 2016 at 18:20, Rui Ueyama <<a href="mailto:ruiu@google.com">ruiu@google.com</a>> wrote:<br>
> ruiu added a comment.<br>
><br>
> Thank you for doing this. I agree that it should make things easier to create linker-synthesized input sections.<br>
><br>
><br>
><br>
> ================<br>
> Comment at: ELF/InputSection.cpp:36<br>
>    if (!File || Hdr->sh_type == SHT_NOBITS)<br>
> -    return {};<br>
> +    return makeArrayRef<uint8_t>(nullptr, Hdr->sh_size);<br>
>    return check(File->getObj().<wbr>getSectionContents(Hdr));<br>
> ----------------<br>
> What is this change for?<br>
<br>
<br>
</span>The size is now always stored in the Data ArrayRef.<br>
<span class=""><br>
> ================<br>
> Comment at: ELF/InputSection.cpp:842<br>
>    }<br>
> +  ArrayRef<uint8_t> Data = makeArrayRef<uint8_t>(nullptr, Size);<br>
> +  InputSection Ret(SHF_ALLOC | SHF_WRITE, SHT_NOBITS, Alignment, Data);<br>
> ----------------<br>
> Is it a common practice to create an ArrayRef with non-zero size and a nullptr? It seems odd to me.<br>
<br>
</span>It is not too common. The alternative is to store an explicit size and<br>
pointer in InputSectionData and create the ArrayRef in a getter. Would<br>
you prefer that?<br></blockquote><div><br></div><div>I think I prefer that because an ArrayRef that doesn't refer any array feels a weird thing.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
<br>
> ================<br>
> Comment at: ELF/InputSection.h:88<br>
><br>
> +  uintX_t Flags;<br>
> +  uintX_t Entsize;<br>
> ----------------<br>
> Please add a comment to note that they correspond to Elf_Shdr.<br>
<br>
</span>Will do.<br>
<span class=""><br>
><br>
> ================<br>
> Comment at: ELF/InputSection.h:119-123<br>
> +  uintX_t getFlags() const { return Flags; }<br>
> +  uint32_t getType() const { return Type; }<br>
> +  uintX_t getEntsize() const { return Entsize; }<br>
> +  uint32_t getLink() const { return Link; }<br>
> +  uint32_t getInfo() const { return Info; }<br>
> ----------------<br>
> Do you still need these accessors?<br>
<br>
</span>No, but can that be in another patch?</blockquote><div><br></div><div>Sure.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<br>
><br>
> ================<br>
> Comment at: ELF/InputSection.h:280-281<br>
> +  // Common symbols don't belong to any section. But it is easier for us<br>
> +  // to handle them as if they belong to some input section. So we defined<br>
> +  // this class. CommonInputSection is a virtual singleton class that<br>
> +  // "contains" all common symbols.<br>
> ----------------<br>
> Please update the comment becasue "this class" no longer exists.<br>
<br>
</span>Will do.<br>
</blockquote></div><br></div></div>