<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jun 4, 2016, at 9:10 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Looks like you just have a variable rename in the unary minus implementation - commit separately or drop that change perhaps?<br class=""></div></div></blockquote>Oh yeah. Good catch. Will do that separately.<br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><br class="">I don't think you need versions that take two rvalue refs (+(&&, &&)), is there? (until +=/-= get smart and have an overload that takes an rvalue ref parameter and uses it to steal the right hand side buffer if it's bigger than the left hand side buffer or something like that?)<br class=""></div></div></blockquote>I had a compile error somewhere in the LLVM codebase without this version. I can’t remember where it is, but a small test (attached to the end of the email if you want to hack on it) which triggers it is:</div><div><br class=""></div><blockquote style="margin: 0 0 0 40px; border: none; padding: 0px;" class=""><div>rvalue.cpp:66:22: error: use of overloaded operator '+' is ambiguous (with operand types 'APInt' and 'APInt')</div><div> APInt d2 = (a * b) + (a * b);</div><div><br class=""></div></blockquote><div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><br class="">& can you pass by value instead of by rvalue ref - does that work/choose the right overloads?<br class=""></div></div></blockquote>Doesn’t seem to. Using the above as an example, if I remove the && from both arguments then I get:</div><div><br class=""></div><blockquote style="margin: 0 0 0 40px; border: none; padding: 0px;" class=""><div>rvalue.cpp:72:22: error: use of overloaded operator '+' is ambiguous (with operand types 'APInt' and 'APInt')</div><div> APInt d2 = (a * b) + (a * b);</div><div> ~~~~~~~ ^ ~~~~~~~</div><div>rvalue.cpp:35:14: note: candidate function</div><div>inline APInt operator+(APInt a, APInt b) {</div><div> ^</div><div>rvalue.cpp:41:14: note: candidate function</div><div>inline APInt operator+(APInt &&a, const APInt &b) {</div><div> ^</div><div>rvalue.cpp:47:14: note: candidate function</div><div>inline APInt operator+(const APInt &a, APInt &&b) {</div><div> ^</div><div>rvalue.cpp:53:14: note: candidate function</div><div>inline APInt operator+(const APInt &a, const APInt &b) {</div><div><br class=""></div></blockquote>Note, removing the && from all the variants doesn’t improve the situation.<br class=""><div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><br class="">inline APInt operator-(APInt RHS, const APInt &LHS) {<br class=""> RHS += LHS;<br class=""> return RHS; // shouldn't need std::move here because you're returning a local<br class="">}</div></div></blockquote>I wondered about this too. I turned on -Wpessimizing-move to see if what I was doing was wrong but it didn’t fire. Interestingly, with this method:</div><div><br class=""></div><blockquote style="margin: 0 0 0 40px; border: none; padding: 0px;" class=""><div>inline APInt operator+(APInt &&a, const APInt &b) {</div><div> printf("APInt::+(&&, &)\n");</div><div> a += b;</div><div> return a;</div><div>}</div><div><br class=""></div></blockquote>and with/without the std::move on the return. The above version will call APInt::APInt(&) but the std::move version will call APInt::APInt(&&). I used printfs to verify this. So looks like there is a difference here, even though I totally agree with you that we’re returning a local so it shouldn’t need the std::move. I’m not sure if this is a bug, or just subtlety in rvalue semantics. Would love to know the answer though.<div class=""><br class=""></div><div class=""><div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><br class="">Then you shouldn't need the op-(const APInt&,const APInt&) version, for example.<br class=""></div></div></div></blockquote>Not sure if its a result of the other &&’s ending up being required, but i’ve tested without a (const APInt&,const APInt&) version and I get ambiguous overload errors. Seems like i’m going to need it.<br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><br class="">Tests?</div></div></div></blockquote>I was wondering about this. I can certainly test all the variants to make sure I get the correct numerical results from APInt and I’ll add what tests are needed for that. I wouldn’t be able to test whether we get a certain number of malloc’s, unless its ok to implement my own malloc/free the APInt unit test?</div><div><br class=""></div><div>Thanks for all the comments so far. Will try get an updated patch tomorrow.</div><div><br class=""></div><div>Cheers,</div><div>Pete</div><div><br class=""></div><div></div></div></body></html>