[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP
Johannes Doerfert via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 13 08:18:13 PST 2019
jdoerfert added inline comments.
================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:370
+ : nullptr;
+ }
return nullptr;
----------------
dim wrote:
> jdoerfert wrote:
> > Consistent style please:
> >
> > ```
> > if (Value *StrLen = emitStrLen(SrcStr, B, DL, TLI)
> > return B.CreateGEP(B.getInt8Ty(), SrcStr, StrLen, "strchr");
> > ```
> Consistent with what? :) In this same file, I see at least the following calls to `emitStrLen`, some of which use the `if(!x) return nullptr` spelling, others which use `return x ? y : nullptr`:
>
> ```
> Value *DstLen = emitStrLen(Dst, B, DL, TLI);
> if (!DstLen)
> return nullptr;
> ```
>
> ```
> if (Dst == Src) { // stpcpy(x,x) -> x+strlen(x)
> Value *StrLen = emitStrLen(Src, B, DL, TLI);
> return StrLen ? B.CreateInBoundsGEP(B.getInt8Ty(), Dst, StrLen) : nullptr;
> }
> ```
>
> ```
> Value *StrLen = emitStrLen(CI->getArgOperand(1), B, DL, TLI);
> if (!StrLen)
> return nullptr;
> ```
>
> ```
> Value *Len = emitStrLen(CI->getArgOperand(2), B, DL, TLI);
> if (!Len)
> return nullptr;
> ```
>
> ```
> Value *StrLen = emitStrLen(Src, B, DL, TLI);
> return StrLen ? B.CreateInBoundsGEP(B.getInt8Ty(), Dst, StrLen) : nullptr;
> ```
>
> But I'm fine with whatever you are suggesting, obviously. It just seems strange to introduce yet another spelling variant, making it less consistent, not more...
Consistent with the two prior lines.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70143/new/
https://reviews.llvm.org/D70143
More information about the llvm-commits
mailing list