<div dir="ltr">I think I'm not familiarized to the idea of using lambda to eliminate break's from switches. Not sure if I'll adopt that, but for now, I'd go with the plain C-like approach.</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 1, 2016 at 1:38 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">Small preference for a lambda, but LGTM without it too if Rui prefers<br>
it that way.<br>
<br>
Cheers,<br>
Rafael<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
On 1 March 2016 at 16:30, Simon Atanasyan <<a href="mailto:simon@atanasyan.com">simon@atanasyan.com</a>> wrote:<br>
> atanasyan added inline comments.<br>
><br>
> ================<br>
> Comment at: ELF/OutputSections.cpp:669-670<br>
> @@ -668,16 +668,4 @@<br>
>  typename EhFrameHeader<ELFT>::uintX_t<br>
>  EhFrameHeader<ELFT>::getFdePc(uintX_t EhVA, const FdeData &F) {<br>
> -  const endianness E = ELFT::TargetEndianness;<br>
> -  assert((F.Enc & 0xF0) != DW_EH_PE_datarel);<br>
> -<br>
> -  uintX_t FdeOff = EhVA + F.Off + 8;<br>
> -  switch (F.Enc & 0xF) {<br>
> -  case DW_EH_PE_udata2:<br>
> -  case DW_EH_PE_sdata2:<br>
> -    return FdeOff + read16<E>(F.PCRel);<br>
> -  case DW_EH_PE_udata4:<br>
> -  case DW_EH_PE_sdata4:<br>
> -    return FdeOff + read32<E>(F.PCRel);<br>
> -  case DW_EH_PE_udata8:<br>
> -  case DW_EH_PE_sdata8:<br>
> -    return FdeOff + read64<E>(F.PCRel);<br>
> +  auto Read = [=](uint8_t Size) -> uint64_t {<br>
> +    const endianness E = ELFT::TargetEndianness;<br>
> ----------------<br>
> ruiu wrote:<br>
>> Do you have to use lambda? It seems you can write without it in a more regular way.<br>
>><br>
>>   const endianness E = ELFT::TargetEndianness;<br>
>><br>
>>   int Size = F.Enc & 0x7;<br>
>>   if (Size == DW_EH_PE_absptr)<br>
>>     Size = sizeof(uintX_t) == 8 ? DW_EH_PE_udata8 : DW_EH_PE_udata4;<br>
>><br>
>>   uintX_t V;<br>
>>   switch (Size) {<br>
>>   case DW_EH_PE_udata2:<br>
>>     V = read16<E>(F.PCRel);<br>
>>   case DW_EH_PE_udata4:<br>
>>     V = read32<E>(F.PCRel);<br>
>>   case DW_EH_PE_udata8:<br>
>>     V = read64<E>(F.PCRel);<br>
>>   default:<br>
>>     fatal("unknown FDE size encoding");<br>
>>   }<br>
>><br>
>>   switch (F.Enc & 0x70) {<br>
>>   case DW_EH_PE_absptr:<br>
>>     return V;<br>
>>   case DW_EH_PE_pcrel:<br>
>>     return V + EhVA + F.Off + 8;<br>
>>   default:<br>
>>     fatal("unknown FDE size relative encoding");<br>
>>   }<br>
>><br>
>> Do you have to use lambda? It seems you can write without it in a more regular way.<br>
><br>
> The lambda is not mandatory. But without it the code is a bit longer because we have to add `break` to each case of the `switch (Size)`.<br>
><br>
><br>
> Repository:<br>
>   rL LLVM<br>
><br>
> <a href="http://reviews.llvm.org/D17733" rel="noreferrer" target="_blank">http://reviews.llvm.org/D17733</a><br>
><br>
><br>
><br>
</div></div></blockquote></div><br></div>