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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to