<div dir="ltr">On Tue, Jun 9, 2020 at 2:33 AM Kadir Çetinkaya <<a href="mailto:kadircet@google.com">kadircet@google.com</a>> wrote:<br>><br>> Hi David,<br>><br>> I was doing that in the past, until someone else sent a message saying that the general convention in LLVM is to rather put a `(void)x;` statement. Unfortunately I can't find the mailing thread right now.<br><br>It'd be handy to have that for reference. (void)x is generally used if the same expression is needed multiple times in one or more assertions, if the value needs to be computed far from the assertion (eg: retrieve the size of a container, then do some algorithm that changes that size, then assert that the size before has some relation to the size after), or the value is computed by something which has necessary side effects ("do the thing" then "assert the thing succeeded" - you can't put the "do the thing" in the assert, or a non-asserts build won't "do the thing").<br><br>> I would be more happy with inlining, as it *might* get rid of some redundant calculations in release builds.<br><br>The other motivation is that the expression/variable won't stick around accidentally if the assertion is removed or moved.<br><br>> Do we have any written consensus around this one to direct people in the future for one way or another?<br><br>It's discussed a little towards the end of <a href="https://llvm.org/docs/CodingStandards.html#assert-liberally">https://llvm.org/docs/CodingStandards.html#assert-liberally</a><br><br><p style="margin:0.8em 0px 0.5em;color:rgb(0,0,0);font-family:"Lucida Grande","Lucida Sans Unicode",Geneva,Verdana,sans-serif;font-size:14px">Another issue is that values used only by assertions will produce an “unused value” warning when assertions are disabled. For example, this code will warn:</p><div class="gmail-highlight-c++ gmail-notranslate" style="color:rgb(0,0,0);font-family:"Lucida Grande","Lucida Sans Unicode",Geneva,Verdana,sans-serif;font-size:14px"><div class="gmail-highlight" style="background:rgb(240,240,240)"><pre style="overflow:auto hidden;font-family:Consolas,"Deja Vu Sans Mono","Bitstream Vera Sans Mono",monospace;font-size:0.95em;line-height:15.96px;padding:0.5em;border:1px solid rgb(204,204,204);background-color:rgb(248,248,248)"><span class="gmail-kt" style="color:rgb(144,32,0)">unsigned</span> <span class="gmail-n">Size</span> <span class="gmail-o" style="color:rgb(102,102,102)">=</span> <span class="gmail-n">V</span><span class="gmail-p">.</span><span class="gmail-n">size</span><span class="gmail-p">();</span>
<span class="gmail-n">assert</span><span class="gmail-p">(</span><span class="gmail-n">Size</span> <span class="gmail-o" style="color:rgb(102,102,102)">></span> <span class="gmail-mi" style="color:rgb(64,160,112)">42</span> <span class="gmail-o" style="color:rgb(102,102,102)">&&</span> <span class="gmail-s" style="color:rgb(64,112,160)">"Vector smaller than it should be"</span><span class="gmail-p">);</span>
<span class="gmail-kt" style="color:rgb(144,32,0)">bool</span> <span class="gmail-n">NewToSet</span> <span class="gmail-o" style="color:rgb(102,102,102)">=</span> <span class="gmail-n">Myset</span><span class="gmail-p">.</span><span class="gmail-n">insert</span><span class="gmail-p">(</span><span class="gmail-n">Value</span><span class="gmail-p">);</span>
<span class="gmail-n">assert</span><span class="gmail-p">(</span><span class="gmail-n">NewToSet</span> <span class="gmail-o" style="color:rgb(102,102,102)">&&</span> <span class="gmail-s" style="color:rgb(64,112,160)">"The value shouldn't be in the set yet"</span><span class="gmail-p">);</span>
</pre></div></div><p style="margin:0.8em 0px 0.5em;color:rgb(0,0,0);font-family:"Lucida Grande","Lucida Sans Unicode",Geneva,Verdana,sans-serif;font-size:14px">These are two interesting different cases. In the first case, the call to <code class="gmail-docutils gmail-literal gmail-notranslate" style="font-family:Consolas,"Deja Vu Sans Mono","Bitstream Vera Sans Mono",monospace;font-size:0.95em"><span class="gmail-pre" style="hyphens: none;">V.size()</span></code> is only useful for the assert, and we don’t want it executed when assertions are disabled. Code like this should move the call into the assert itself. In the second case, the side effects of the call must happen whether the assert is enabled or not. In this case, the value should be cast to void to disable the warning. To be specific, it is preferred to write the code like this:</p><div class="gmail-highlight-c++ gmail-notranslate" style="color:rgb(0,0,0);font-family:"Lucida Grande","Lucida Sans Unicode",Geneva,Verdana,sans-serif;font-size:14px"><div class="gmail-highlight" style="background:rgb(240,240,240)"><pre style="overflow:auto hidden;font-family:Consolas,"Deja Vu Sans Mono","Bitstream Vera Sans Mono",monospace;font-size:0.95em;line-height:15.96px;padding:0.5em;border:1px solid rgb(204,204,204);background-color:rgb(248,248,248)"><span class="gmail-n">assert</span><span class="gmail-p">(</span><span class="gmail-n">V</span><span class="gmail-p">.</span><span class="gmail-n">size</span><span class="gmail-p">()</span> <span class="gmail-o" style="color:rgb(102,102,102)">></span> <span class="gmail-mi" style="color:rgb(64,160,112)">42</span> <span class="gmail-o" style="color:rgb(102,102,102)">&&</span> <span class="gmail-s" style="color:rgb(64,112,160)">"Vector smaller than it should be"</span><span class="gmail-p">);</span>
<span class="gmail-kt" style="color:rgb(144,32,0)">bool</span> <span class="gmail-n">NewToSet</span> <span class="gmail-o" style="color:rgb(102,102,102)">=</span> <span class="gmail-n">Myset</span><span class="gmail-p">.</span><span class="gmail-n">insert</span><span class="gmail-p">(</span><span class="gmail-n">Value</span><span class="gmail-p">);</span> <span class="gmail-p">(</span><span class="gmail-kt" style="color:rgb(144,32,0)">void</span><span class="gmail-p">)</span><span class="gmail-n">NewToSet</span><span class="gmail-p">;</span>
<span class="gmail-n">assert</span><span class="gmail-p">(</span><span class="gmail-n">NewToSet</span> <span class="gmail-o" style="color:rgb(102,102,102)">&&</span> <span class="gmail-s" style="color:rgb(64,112,160)">"The value shouldn't be in the set yet"</span><span class="gmail-p">);</span></pre></div></div><br>- Dave<br><br>><br>> On Tue, Jun 9, 2020 at 4:46 AM David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>>><br>>> If a variable is only used once in the assertion ,could you roll the<br>>> initialization into the assert (folding the named variable away)?<br>>><br>>> On Wed, Jun 3, 2020 at 2:49 AM Kadir Cetinkaya via llvm-commits<br>>> <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>>> ><br>>> ><br>>> > Author: Kadir Cetinkaya<br>>> > Date: 2020-06-03T11:49:01+02:00<br>>> > New Revision: c5468253aa555c2df98bd1f49d1e9d44b0150a2e<br>>> ><br>>> > URL: <a href="https://github.com/llvm/llvm-project/commit/c5468253aa555c2df98bd1f49d1e9d44b0150a2e">https://github.com/llvm/llvm-project/commit/c5468253aa555c2df98bd1f49d1e9d44b0150a2e</a><br>>> > DIFF: <a href="https://github.com/llvm/llvm-project/commit/c5468253aa555c2df98bd1f49d1e9d44b0150a2e.diff">https://github.com/llvm/llvm-project/commit/c5468253aa555c2df98bd1f49d1e9d44b0150a2e.diff</a><br>>> ><br>>> > LOG: [llvm] Fix unused variable warnings<br>>> ><br>>> > Added:<br>>> ><br>>> ><br>>> > Modified:<br>>> > llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>>> ><br>>> > Removed:<br>>> ><br>>> ><br>>> ><br>>> > ################################################################################<br>>> > diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>>> > index 67504f827d63..061330677680 100644<br>>> > --- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>>> > +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>>> > @@ -807,6 +807,7 @@ static void collectCallSiteParameters(const MachineInstr *CallMI,<br>>> > auto Suc = std::next(CallMI->getIterator());<br>>> > // Only one-instruction delay slot is supported.<br>>> > auto BundleEnd = llvm::getBundleEnd(CallMI->getIterator());<br>>> > + (void)BundleEnd;<br>>> > assert(std::next(Suc) == BundleEnd &&<br>>> > "More than one instruction in call delay slot");<br>>> > // Try to interpret value loaded by instruction.<br>>> > @@ -856,7 +857,9 @@ void DwarfDebug::constructCallSiteEntryDIEs(const DISubprogram &SP,<br>>> > return false;<br>>> > auto Suc = std::next(MI.getIterator());<br>>> > auto CallInstrBundle = getBundleStart(MI.getIterator());<br>>> > + (void)CallInstrBundle;<br>>> > auto DelaySlotBundle = getBundleStart(Suc);<br>>> > + (void)DelaySlotBundle;<br>>> > // Ensure that label after call is following delay slot instruction.<br>>> > // Ex. CALL_INSTRUCTION {<br>>> > // DELAY_SLOT_INSTRUCTION }<br>>> > @@ -1893,6 +1896,7 @@ void DwarfDebug::beginInstruction(const MachineInstr *MI) {<br>>> > if (!MI.isBundledWithSucc())<br>>> > return false;<br>>> > auto Suc = std::next(MI.getIterator());<br>>> > + (void)Suc;<br>>> > // Ensure that delay slot instruction is successor of the call instruction.<br>>> > // Ex. CALL_INSTRUCTION {<br>>> > // DELAY_SLOT_INSTRUCTION }<br>>> ><br>>> ><br>>> ><br>>> > _______________________________________________<br>>> > llvm-commits mailing list<br>>> > <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>>> > <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></div>