<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type content="text/html; charset=utf-8"><meta name=Generator content="Microsoft Word 15 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
span.gmaildefault
{mso-style-name:gmail_default;}
span.E-MailFormatvorlage20
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;
mso-fareast-language:EN-US;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:70.85pt 70.85pt 2.0cm 70.85pt;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--></head><body lang=DE link=blue vlink=purple><div class=WordSection1><p class=MsoNormal><span lang=EN-US>Ok got it!</span><span lang=EN-US><o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US>I thought all Expressions look like that missing ExprStmt.<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US>There are Statements and Expressions and I initially assumed Expressions look like Statements, but that’s just one sub/supertype of Expr.<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US>Looking into the DoWhile topic I found a corresponding commit:<o:p></o:p></span></p><p class=MsoNormal><a href="https://github.com/llvm/llvm-project/commit/503fadc90f397a773ac17f40f72ca3e81cba2c49"><span lang=EN-US>https://github.com/llvm/llvm-project/commit/503fadc90f397a773ac17f40f72ca3e81cba2c49</span></a><span lang=EN-US><o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US>Interestingly the statements touched in that commit have some high overlap with my “strange behavior” list in </span><a href="https://reviews.llvm.org/D75813#change-Bc7Hs8CXH7Z9"><span lang=EN-US>https://reviews.llvm.org/D75813#change-Bc7Hs8CXH7Z9</span></a><span lang=EN-US><o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US>I came up with my list by comparing results of an AST based approach and a token based approach - over llvm codebase.<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US>Apparently there are no gotos in the part which I have looked at ;-)<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US>I think I could change ParseStmt.cpp for DoWhile, GotoStmt, ContinueStmt, BreakStmt, ReturnStmt, AsmStmt and SEHLeaveStmt.<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US>Looks rather trivial (so it probably isn’t).<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US>But I would have to change Stmt.h as well and add an SemiLoc or something to all of them.<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US>For example BreakStmt doesn’t have any secondary SourceLocation, others already store RParenLoc – but that doesn’t help.<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US>Does that make sense? I would still need to keep my getUnifiedEndLoc, but it could be simplified to getStmtLikeEndLocForStmtLikeExpr.<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US>Alex<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><b>Von:</b> Yitzhak Mandelbaum <yitzhakm@google.com> <br><b>Gesendet:</b> Mittwoch, 11. März 2020 18:47<br><b>An:</b> Alexander Lanin <alex@lanin.de><br><b>Cc:</b> Clang Dev <cfe-dev@lists.llvm.org><br><b>Betreff:</b> Re: [cfe-dev] Stmt.getEndLoc() vs semicolon<o:p></o:p></p><p class=MsoNormal><o:p> </o:p></p><div><div><div><p class=MsoNormal><span style='font-family:"Arial",sans-serif'>responses inline...<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><div><div><p class=MsoNormal>On Sat, Feb 29, 2020 at 3:42 PM <<a href="mailto:alex@lanin.de" target="_blank">alex@lanin.de</a>> wrote:<o:p></o:p></p></div><blockquote style='border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm'><div><div><p><span lang=EN-GB>Hi,</span><o:p></o:p></p><p><span lang=EN-GB> </span><o:p></o:p></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</span><o:p></o:p></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><o:p></o:p></p></div></div></blockquote><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal><span style='font-family:"Arial",sans-serif'>The problem is context. Consider expression `f(e)` in two cases:<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-family:"Arial",sans-serif'><o:p> </o:p></span></p></div><div><p class=MsoNormal><span style='font-family:"Arial",sans-serif'>1. `f(e);`<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-family:"Arial",sans-serif'>2. `return f(e) + 3;`<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-family:"Arial",sans-serif'><o:p> </o:p></span></p></div><div><p class=MsoNormal><span style='font-family:"Arial",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.<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-family:"Arial",sans-serif'><o:p> </o:p></span></p></div><div><p class=MsoNormal><span style='font-family:"Arial",sans-serif'>So, both context and intent matter. An ExprStmt would clearly separate the two.<o:p></o:p></span></p></div><blockquote style='border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm'><div><div><p><span lang=EN-GB> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>>> Expr vs Stmt</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>Turns out DoStmt is also fun as the conditions is an Expr.</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><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><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB style='color:green'>/* Comment is not part of DoStmt,</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB style='color:green'>although it's before the semicolon*/</span><span lang=EN-GB style='color:black'>;</span><o:p></o:p></p></div></div></blockquote><div><p class=MsoNormal><span class=gmaildefault><span style='font-family:"Arial",sans-serif'>Good catch!</span></span> <span class=gmaildefault><span style='font-family:"Arial",sans-serif'>But, 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></span><o:p></o:p></p></div><blockquote style='border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm'><div><div><p><span lang=EN-GB>>> In general, associating comments is hard and it seems reasonable to me either way -- including or excluding.</span><o:p></o:p></p><p><span lang=EN-GB>Sure, as long as it’s consistent. The semicolon (and preceding comments) are not.</span><o:p></o:p></p><p><span lang=EN-GB> </span><o:p></o:p></p><p><span lang=EN-GB>>> it infects other statements as well</span><o:p></o:p></p><p><span lang=EN-GB>Yeah. I realized that too late…</span><o:p></o:p></p><p><span lang=EN-GB>I’ve added a makeUnifiedFileCharRange function to LexerUtils which basically simply calls findTokenSkippingComments + some special handling for NullStmt.</span><o:p></o:p></p><p><span lang=EN-GB>I’ve tried token by token, but I cannot get this to work without a “full blown” parser as there is a very close match to consider:</span><o:p></o:p></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.</span><o:p></o:p></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.</span><o:p></o:p></p><p><span lang=EN-GB> </span><o:p></o:p></p><p><span lang=EN-GB>Any advice?</span><o:p></o:p></p></div></div></blockquote><div><div><p class=MsoNormal><span style='font-family:"Arial",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.<o:p></o:p></span></p></div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal> <o:p></o:p></p></div><blockquote style='border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm'><div><div><p><span lang=EN-GB> </span><o:p></o:p></p><p><span lang=EN-GB> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#6A9955'>// RUN: %check_clang_tidy %s readability-braces-around-statements %t</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#569CD6'>int</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> </span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#DCDCAA'>foo</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'>() { </span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#C586C0'>return</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> </span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#B5CEA8'>42</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'>; }</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#569CD6'>void</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> </span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#DCDCAA'>test</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'>() {</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> </span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#C586C0'>if</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> (</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#569CD6'>true</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'>)</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> </span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#C586C0'>if</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> (</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#569CD6'>true</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'>)</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> </span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#C586C0'>while</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> (</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#CE9178'>"ok"</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'>) {</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> </span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#DCDCAA'>foo</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'>();</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> }</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#6A9955'> // end while</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> </span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#569CD6'>int</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> s;</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> </span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#C586C0'>if</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> (</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#569CD6'>true</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'>)</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> </span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#C586C0'>if</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> (</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#569CD6'>true</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'>)</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> s = </span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#569CD6'>int</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'>{</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> </span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#DCDCAA'>foo</span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'>()</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'> }; </span><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#6A9955'>// assign</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:14.25pt'><span style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'>}</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>Best regards,</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>Alexander</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> </span><o:p></o:p></p><div><blockquote style='border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0cm;margin-bottom:5.0pt'><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> </span><o:p></o:p></p></div></div></blockquote></div></div></div></blockquote></div></div></div></body></html>