<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">responses inline...</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Feb 29, 2020 at 3:42 PM <<a href="mailto:alex@lanin.de" target="_blank">alex@lanin.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div lang="DE"><div><p><span lang="EN-GB">Hi,<u></u><u></u></span></p><p><span lang="EN-GB"><u></u> <u></u></span></p><p><span lang="EN-GB">>> When an expression is used in a context expecting a statement and is trailed by a semicolon, the range will not include the semicolon<u></u><u></u></span></p><p><span lang="EN-GB">Assuming there is a reason for doing that, this explains the effects. It certainly sounds easier to extend Expr range instead of wrapping it in ExprStmt...</span></p></div></div></blockquote><div><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">The problem is context. Consider expression `f(e)` in two cases:</div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif"><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">1. `f(e);`</div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">2. `return f(e) + 3;`</div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif"><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">What is the range of `f(e)`? Is it wider in the first case than the second? if so, will this surprise/break code that might assume it invariant?  But, even worse, in case 1, how do you know which range the user wants? If I'm replacing statements, then I'll want through the semicolon, but if I'm updating call expressions, I'll only want the call itself.</div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif"><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">So, both context and intent matter. An ExprStmt would clearly separate the two.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div lang="DE"><div><p><span lang="EN-GB"><u></u><u></u></span></p><p><span lang="EN-GB"><u></u> <u></u></span></p><p class="MsoNormal"><span lang="EN-GB">>> Expr vs Stmt<u></u><u></u></span></p><p class="MsoNormal"><span lang="EN-GB">Turns out DoStmt is also fun as the conditions is an Expr.<u></u><u></u></span></p><p class="MsoNormal"><span lang="EN-GB" style="color:blue">do</span><span lang="EN-GB" style="color:black"> {} </span><span lang="EN-GB" style="color:blue">while</span><span lang="EN-GB" style="color:black">(</span><span lang="EN-GB" style="color:blue">false</span><span lang="EN-GB" style="color:black">)</span><span lang="EN-GB"><u></u><u></u></span></p><p class="MsoNormal"><span lang="EN-GB" style="color:green">/* Comment is not part of DoStmt,</span><span lang="EN-GB"><u></u><u></u></span></p><p class="MsoNormal"><span lang="EN-GB" style="color:green">although it's before the semicolon*/</span><span lang="EN-GB" style="color:black">;</span></p></div></div></blockquote><div><span class="gmail_default" style="font-family:arial,helvetica,sans-serif">Good catch!</span> <span class="gmail_default" style="font-family:arial,helvetica,sans-serif">But, </span><span class="gmail_default" style="font-family:arial,helvetica,sans-serif">I don't think that's the reason, because the condition is clearly delineated by the parentheses. It just seems to be an inconsistency. I can't see any reason that the comment and semi are excluded here while included in the DeclStmt. I think this should be changed.</span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div lang="DE"><div><p><span lang="EN-GB">>> In general, associating comments is hard and it seems reasonable to me either way -- including or excluding.<u></u><u></u></span></p><p><span lang="EN-GB">Sure, as long as it’s consistent. The semicolon (and preceding comments) are not.<u></u><u></u></span></p><p><span lang="EN-GB"><u></u> <u></u></span></p><p><span lang="EN-GB">>> it infects other statements as well<u></u><u></u></span></p><p><span lang="EN-GB">Yeah. I realized that too late…<u></u><u></u></span></p><p><span lang="EN-GB">I’ve added a makeUnifiedFileCharRange function to LexerUtils which basically simply calls findTokenSkippingComments + some special handling for NullStmt.<u></u><u></u></span></p><p><span lang="EN-GB">I’ve tried token by token, but </span><span lang="EN-GB">I cannot get this to work without a “full blown” parser as there is a very close match to consider:<u></u><u></u></span></p><p><span lang="EN-GB">- In the first case “end while” obviously belongs to the while statement and there is no semicolon to search for. I have to take tok::r_brace as an end. It’s also impossible to go for isa<WhileStmt> since it is nested.<u></u><u></u></span></p><p><span lang="EN-GB">- In the second case “assign” obviously belongs to the statement. Here I have to look beyond tok::r_brace and not stop there.<u></u><u></u></span></p><p><span lang="EN-GB"><u></u> <u></u></span></p><p><span lang="EN-GB">Any advice?</span></p></div></div></blockquote><div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">Have you looked at the code I pointed to earlier (getAssociatedRange)? Properly handling those kinds of cases is quite difficult, but I don't think you need to parse the code. It's more about newlines and such.</div><br></div><div><br></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"><div lang="DE"><div><p><span lang="EN-GB"><u></u><u></u></span></p><p><span lang="EN-GB"><u></u> <u></u></span></p><p><span lang="EN-GB"><u></u> <u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(106,153,85)">// RUN: %check_clang_tidy %s readability-braces-around-statements %t</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)"><u></u><u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)"><u></u> <u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(86,156,214)">int</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)"> </span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(220,220,170)">foo</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">() { </span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(197,134,192)">return</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)"> </span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(181,206,168)">42</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">; }<u></u><u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)"><u></u> <u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(86,156,214)">void</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)"> </span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(220,220,170)">test</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">() {<u></u><u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">  </span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(197,134,192)">if</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)"> (</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(86,156,214)">true</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">)<u></u><u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">    </span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(197,134,192)">if</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)"> (</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(86,156,214)">true</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">)<u></u><u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">      </span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(197,134,192)">while</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)"> (</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(206,145,120)">"ok"</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">) {<u></u><u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">        </span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(220,220,170)">foo</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">();<u></u><u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">      }</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(106,153,85)"> // end while</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)"><u></u><u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)"><u></u> <u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">  </span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(86,156,214)">int</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)"> s;<u></u><u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">  </span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(197,134,192)">if</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)"> (</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(86,156,214)">true</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">)<u></u><u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">    </span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(197,134,192)">if</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)"> (</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(86,156,214)">true</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">)<u></u><u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">      s = </span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(86,156,214)">int</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">{<u></u><u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">        </span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(220,220,170)">foo</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">()<u></u><u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">      }; </span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(106,153,85)">// assign</span><span lang="EN-GB" style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)"><u></u><u></u></span></p><p class="MsoNormal" style="line-height:14.25pt"><span style="font-size:10.5pt;font-family:Consolas;color:rgb(212,212,212)">}<u></u><u></u></span></p><p class="MsoNormal"><span lang="EN-GB"><u></u> <u></u></span></p><p class="MsoNormal"><span lang="EN-GB"><u></u> <u></u></span></p><p class="MsoNormal"><span lang="EN-GB">Best regards,<u></u><u></u></span></p><p class="MsoNormal"><span lang="EN-GB">Alexander<u></u><u></u></span></p><p class="MsoNormal"><span lang="EN-GB"><u></u> </span></p><div><blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt solid rgb(204,204,204);padding:0cm 0cm 0cm 6pt;margin:5pt 0cm 5pt 4.8pt"><div><div><p class="MsoNormal"><span lang="EN-GB"> <u></u><u></u></span></p></div></div></blockquote></div></div></div></blockquote></div></div>