<div dir="ltr"><div dir="ltr">On Sat, Jun 8, 2019 at 11:12 AM Tim Northover via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Sat, 8 Jun 2019 at 18:18, Mehdi AMINI via llvm-dev<br>
<<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
> The LLVM coding style does not specify anything about the use of signed/unsigned integer, and the codebase is inconsistent (there is a majority of code that is using unsigned index in loops today though).<br>
><br>
> I'd like to suggest that we specify to prefer `int` when possible and use `unsigned` only for bitmask and when you intend to rely on wrapping behavior, see: <a href="https://reviews.llvm.org/D63049" rel="noreferrer" target="_blank">https://reviews.llvm.org/D63049</a><br>
<br>
I'd prefer us to have something neater than static_cast<int> for the<br>
loop problem before we made that change.</blockquote><div><br></div><div>Not sure this is much of a problem in LLVM-land... We already encourage a loop style that avoids the classical problem:</div><div><br></div><div>```</div><div>for (int Index = 0, Size = MyVector.size(); Index < Size; ++Index) {</div><div>  // ...</div><div>}</div><div>```</div><div><br></div><div>Alternatively, LLVM supports a form I like even more:</div><div>```</div><div>for (int Index : llvm::seq<int>(0, MyVector.size())) {</div><div>  // ...</div><div>}</div><div>```</div><div><br></div><div>If passing the explicit type is an issue, we could fix that?</div><div><br></div><div>FWIW, I'd also be fine changing the return value ef `size()` for LLVM containers, and given the variability of standard library container performance, I'm generally a fan of LLVM code consistently using its own containers.</div><div><br></div><div>-Chnadler</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Perhaps add an ssize (or<br>
equivalent) method to all of our internal data structures? They're a<br>
lot more common than std::* containers.<br>
<br>
But it's not something that would keep me up at night.<br>
<br>
Cheers.<br>
<br>
Tim.<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div></div>