[PATCH] D14922: [ELF/AArch64] Factor out overflow check into a separate function.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 23 11:45:56 PST 2015


On Mon, Nov 23, 2015 at 11:40 AM, Rafael EspĂ­ndola <
rafael.espindola at gmail.com> wrote:

> On 23 November 2015 at 13:04, Rui Ueyama via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> > ruiu added a comment.
> >
> > I'm not in favor of this change, and I wouldn't do that. The previous
> code was super clear that it errors if isInt check fails. Now you have to
> take a look at the definition of checkAArch64OutOfRange function. You saved
> one line per a function call, but it doesn't seem like a good tradeoff from
> code readability point of view.
>
> How about something like
>
> checkIsInt<N>(Val, Msg);
>
> It should be almost as compact as this change and easy to read.
>

That may make sense. I'm not sure if I'll like that than the original code,
but it's worth a try.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151123/55d31441/attachment.html>


More information about the llvm-commits mailing list