[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP
Dimitry Andric via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 13 06:53:34 PST 2019
dim marked an inline comment as done.
dim added inline comments.
================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:370
+ : nullptr;
+ }
return nullptr;
----------------
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...
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