<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;}
p.MsoPlainText, li.MsoPlainText, div.MsoPlainText
{mso-style-priority:99;
mso-style-link:"Nur Text Zchn";
margin:0cm;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;
mso-fareast-language:EN-US;}
span.NurTextZchn
{mso-style-name:"Nur Text Zchn";
mso-style-priority:99;
mso-style-link:"Nur Text";
font-family:"Calibri",sans-serif;}
span.E-MailFormatvorlage22
{mso-style-type:personal-compose;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;
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=MsoPlainText><span lang=EN-GB>Hi,<o:p></o:p></span></p><p class=MsoPlainText><span lang=EN-GB><o:p> </o:p></span></p><p class=MsoPlainText><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<o:p></o:p></span></p><p class=MsoPlainText><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...<o:p></o:p></span></p><p class=MsoPlainText><span lang=EN-GB><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-GB style='mso-fareast-language:EN-US'>>> Expr vs Stmt<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-GB style='mso-fareast-language:EN-US'>Turns out DoStmt is also fun as the conditions is an Expr.<o:p></o:p></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><o:p></o:p></span></p><p class=MsoNormal><span lang=EN-GB style='color:green'>/* Comment is not part of DoStmt,</span><span lang=EN-GB><o:p></o:p></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><span lang=EN-GB><o:p></o:p></span></p><p class=MsoNormal><span lang=EN-GB style='mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoPlainText><span lang=EN-GB>>> In general, associating comments is hard and it seems reasonable to me either way -- including or excluding.<o:p></o:p></span></p><p class=MsoPlainText><span lang=EN-GB>Sure, as long as it’s consistent. The semicolon (and preceding comments) are not.<o:p></o:p></span></p><p class=MsoPlainText><span lang=EN-GB><o:p> </o:p></span></p><p class=MsoPlainText><span lang=EN-GB>>> it infects other statements as well<o:p></o:p></span></p><p class=MsoPlainText><span lang=EN-GB>Yeah. I realized that too late…<o:p></o:p></span></p><p class=MsoPlainText><span lang=EN-GB>I’ve added a makeUnifiedFileCharRange function to LexerUtils which basically simply calls findTokenSkippingComments + some special handling for NullStmt.<o:p></o:p></span></p><p class=MsoPlainText><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:<o:p></o:p></span></p><p class=MsoPlainText><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.<o:p></o:p></span></p><p class=MsoPlainText><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.<o:p></o:p></span></p><p class=MsoPlainText><span lang=EN-GB><o:p> </o:p></span></p><p class=MsoPlainText><span lang=EN-GB>Any advice?<o:p></o:p></span></p><p class=MsoPlainText><span lang=EN-GB><o:p> </o:p></span></p><p class=MsoPlainText><span lang=EN-GB><o:p> </o:p></span></p><p class=MsoNormal style='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><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'><o:p></o:p></span></p><p class=MsoNormal style='line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'><o:p> </o:p></span></p><p class=MsoNormal style='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'>; }<o:p></o:p></span></p><p class=MsoNormal style='line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'><o:p> </o:p></span></p><p class=MsoNormal style='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'>() {<o:p></o:p></span></p><p class=MsoNormal style='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'>)<o:p></o:p></span></p><p class=MsoNormal style='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'>)<o:p></o:p></span></p><p class=MsoNormal style='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'>) {<o:p></o:p></span></p><p class=MsoNormal style='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'>();<o:p></o:p></span></p><p class=MsoNormal style='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><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'><o:p></o:p></span></p><p class=MsoNormal style='line-height:14.25pt'><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'><o:p> </o:p></span></p><p class=MsoNormal style='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;<o:p></o:p></span></p><p class=MsoNormal style='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'>)<o:p></o:p></span></p><p class=MsoNormal style='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'>)<o:p></o:p></span></p><p class=MsoNormal style='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'>{<o:p></o:p></span></p><p class=MsoNormal style='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'>()<o:p></o:p></span></p><p class=MsoNormal style='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><span lang=EN-GB style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'><o:p></o:p></span></p><p class=MsoNormal style='line-height:14.25pt'><span style='font-size:10.5pt;font-family:Consolas;color:#D4D4D4'>}<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-GB style='mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-GB style='mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-GB style='mso-fareast-language:EN-US'>Best regards,<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-GB style='mso-fareast-language:EN-US'>Alexander<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-GB style='mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-GB style='mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><b><span lang=EN-GB>Von:</span></b><span lang=EN-GB> Yitzhak Mandelbaum <yitzhakm@google.com> <br><b>Gesendet:</b> Donnerstag, 27. Februar 2020 16:08<br><b>An:</b> Alexander Lanin <alex@lanin.de><br><b>Cc:</b> cfe-dev@lists.llvm.org<br><b>Betreff:</b> Re: [cfe-dev] Stmt.getEndLoc() vs semicolon<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-GB><o:p> </o:p></span></p><div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'>Sorry, I overlooked your point about the trailing comments. Yes, there are two issues: trailing comments and inclusion of semicolon -- my response was with regards to the semicolon. Since there are a lot issues here, I've tried to break them down below.<o:p></o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'><o:p> </o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'>## Including semicolons (aka Expr vs Stmt)<o:p></o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'>When an expression is used in a context expecting a statement and is trailed by a semicolon, the range will not include the semicolon. If there are any comments between the expression and the semicolon those, too, will be left out. If we had an ExprStmt, then one could imagine the expression range covering just the expression, but the statement range extending to include the comment and semicolon.<o:p></o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'><o:p> </o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'>## Including trailing comments<o:p></o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'>The trailing comments issue seems to be consistent across syntactic forms (at least, the ones you've presented). Add a trailing comment after a decl statement, eg.<o:p></o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'> int x; // foo<o:p></o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'>and the range still ends at the semicolon. Similarly,<o:p></o:p></span></p></div><div><div><div><p class=MsoNormal style='background:#FFFFFE'><span lang=EN-GB style='font-family:"Arial",sans-serif;color:blue'> if</span><span lang=EN-GB style='font-family:"Arial",sans-serif;color:black'> (</span><span lang=EN-GB style='font-family:"Arial",sans-serif;color:blue'>true</span><span lang=EN-GB style='font-family:"Arial",sans-serif;color:black'>) {} </span><span lang=EN-GB style='font-family:"Arial",sans-serif;color:green'>// foo</span><span lang=EN-GB style='font-family:"Arial",sans-serif;color:black'><o:p></o:p></span></p></div></div></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'>the range ends at '}'.<o:p></o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'><o:p> </o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'>In general, associating comments is hard and it seems reasonable to me either way -- including or excluding. I'll admit that it feels arbitrary that leading comments are included why trailing are excluded, but I'd venture a guess that this is caused by the parser. Morever, since it seems to be consistent, I think its ok, but should be documented clearly somewhere.<o:p></o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'><o:p> </o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'># re: Is Expr the only Stmt that doesn’t include „it’s end“?<o:p></o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'>I'd say no, with respect to trailing comments, but yes with respect to semicolons. However, you could argue that it infects other statements as well, e.g.<o:p></o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'> if (...) foo();<o:p></o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'>The range of the _if statement_ will not include the semicolon. So, it too does not include "it's end".<o:p></o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'><o:p> </o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'>## unifyStmtRange()<o:p></o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'>I'm all for this kind of thing and, in fact, think it's really the only good solution (that is, adding a layer atop the AST to compute things like this). I'm happy to review patches. Or, if you wait long enough, I'll probably get to this myself. :)<o:p></o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'><o:p> </o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'>Cheers,<o:p></o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'>Yitzhak<o:p></o:p></span></p></div><div><p class=MsoNormal><span lang=EN-GB style='font-family:"Arial",sans-serif'><o:p> </o:p></span></p></div></div><p class=MsoNormal><span lang=EN-GB><o:p> </o:p></span></p><div><div><p class=MsoNormal><span lang=EN-GB>On Wed, Feb 26, 2020 at 4:53 PM Alexander Lanin <</span><a href="mailto:alex@lanin.de"><span lang=EN-GB>alex@lanin.de</span></a><span lang=EN-GB>> wrote:<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-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>Hi,<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> <o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>thanks, that’s interesting, but I’m not sure how this relates to trailing comments + semicolon?<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>Why does it matter whether Expr is wrapped or not? It can contain comments in general. But doesn’t contain those at the end.<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>At least for such examples I find it strange: </span><a href="http://ce.steveire.com/z/KdXBDg" target="_blank"><span lang=EN-GB>http://ce.steveire.com/z/KdXBDg</span></a><span lang=EN-GB><o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> <o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> <o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>Is Expr the only Stmt that doesn’t include „it’s end“?<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>Would it make sense to have some unifyStmtRange() in SourceCode.h with special handling for Expr which would extend it with (n x comment) + (optional comma or semi) [+ all the special cases I’m not thinking about right now]?<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> <o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>Currently the check mentioned below always subtracts one char and then checks whether it’s a semicolon. Might be nicer to distinguish by isa<Expr>?<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> <o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>Regards,<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>Alex<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> <o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> <o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> <o:p></o:p></span></p><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm'><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><span lang=EN-GB>Von: </span></b><a href="mailto:yitzhakm@google.com" target="_blank"><span lang=EN-GB>Yitzhak Mandelbaum</span></a><span lang=EN-GB><br><b>Gesendet: </b>Mittwoch, 26. Februar 2020 16:40<br><b>An: </b></span><a href="mailto:alex@lanin.de" target="_blank"><span lang=EN-GB>Alexander Lanin</span></a><span lang=EN-GB><br><b>Cc: </b></span><a href="mailto:cfe-dev@lists.llvm.org" target="_blank"><span lang=EN-GB>cfe-dev@lists.llvm.org</span></a><span lang=EN-GB><br><b>Betreff: </b>Re: [cfe-dev] Stmt.getEndLoc() vs semicolon<o:p></o:p></span></p></div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> <o:p></o:p></span></p><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB style='font-family:"Arial",sans-serif'>Alexander,</span><span lang=EN-GB><o:p></o:p></span></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB style='font-family:"Arial",sans-serif'> </span><span lang=EN-GB><o:p></o:p></span></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB style='font-family:"Arial",sans-serif'>I agree that this behavior is rather confusing, especially to first-time users. The cause of the issue is that Expr derives from Stmt so that Exprs can appear directly in, for example, compound statements, rather than being wrapped in an explicit node to represent statements that consist only of an expression. As far as I understand, this choice to avoid an explicit node had some (positive) performance implications for the AST when it was originally designed. While I'd love to see this choice revisited, I think it would be a significant effort and don't expect it to happen.</span><span lang=EN-GB><o:p></o:p></span></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB style='font-family:"Arial",sans-serif'> </span><span lang=EN-GB><o:p></o:p></span></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB style='font-family:"Arial",sans-serif'>Instead, we tend to work around the issue. For getting proper ranges of statements, you might find clang::tooling::getExtendedText/getExtendedRange meet your needs:</span><span lang=EN-GB><o:p></o:p></span></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><a href="https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Tooling/Transformer/SourceCode.h#L56" target="_blank"><span lang=EN-GB style='font-family:"Arial",sans-serif'>https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Tooling/Transformer/SourceCode.h#L56</span></a><span lang=EN-GB><o:p></o:p></span></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB style='font-family:"Arial",sans-serif'> </span><span lang=EN-GB><o:p></o:p></span></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB style='font-family:"Arial",sans-serif'>For declarations, the (just added!) getAssociatedRange might be even better: </span><a href="https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Tooling/Transformer/SourceCode.h#L39" target="_blank"><span lang=EN-GB style='font-family:"Arial",sans-serif'>https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Tooling/Transformer/SourceCode.h#L39</span></a><span lang=EN-GB><o:p></o:p></span></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB style='font-family:"Arial",sans-serif'> </span><span lang=EN-GB><o:p></o:p></span></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB style='font-family:"Arial",sans-serif'>Cheers,</span><span lang=EN-GB><o:p></o:p></span></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB style='font-family:"Arial",sans-serif'>Yitzhak</span><span lang=EN-GB><o:p></o:p></span></p></div></div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> <o:p></o:p></span></p><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>On Tue, Feb 25, 2020 at 5:15 PM Alexander Lanin via cfe-dev <</span><a href="mailto:cfe-dev@lists.llvm.org" target="_blank"><span lang=EN-GB>cfe-dev@lists.llvm.org</span></a><span lang=EN-GB>> wrote:<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-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>Hello,<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> <o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>I’m having trouble with the locations returned by Stmt getEndLoc()/getSourceRange() and am wondering whether this is a bug/flaw:<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>the returned location sometimes includes and sometimes excludes the semicolon in the end.<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>This gets even more interesting when there are (multiple) comments before the semicolon, simply because the difference between the returned values gets even bigger.<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>Here is an example showing how DeclStmt includes the semicolon, while CallExpr and BinaryOperator exclude the semicolon </span><a href="http://ce.steveire.com/z/TW3IAG" target="_blank"><span lang=EN-GB>http://ce.steveire.com/z/TW3IAG</span></a><span lang=EN-GB>.<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> <o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>So Stmt behaves “completely differently” depending on it’s type, which is no way suggested by it’s interface.<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>Wouldn’t it be better for Stmt-Users if it would always be the same?<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>(Not sure whether it should always be included or excluded. It’s not even always a semicolon as in that 3rd foo() in the example)<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><a href="https://en.wikipedia.org/wiki/Liskov_substitution_principle" target="_blank"><span lang=EN-GB>https://en.wikipedia.org/wiki/Liskov_substitution_principle</span></a><span lang=EN-GB><o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> <o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>Here is one of the effects of this complexity for users of Stmt as they struggle to find that semicolon:<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><a href="https://bugs.llvm.org/show_bug.cgi?id=25970" target="_blank"><span lang=EN-GB>https://bugs.llvm.org/show_bug.cgi?id=25970</span></a><span lang=EN-GB> / </span><a href="https://reviews.llvm.org/D16267" target="_blank"><span lang=EN-GB>https://reviews.llvm.org/D16267</span></a><span lang=EN-GB><o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>Of course it’s fixable there, but that would imply working around the issue in multiple places.<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> <o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>Regards,<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB>Alexander Lanin<o:p></o:p></span></p></div></div></blockquote></div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;margin-left:4.8pt'><span lang=EN-GB>_______________________________________________<br>cfe-dev mailing list<br></span><a href="mailto:cfe-dev@lists.llvm.org" target="_blank"><span lang=EN-GB>cfe-dev@lists.llvm.org</span></a><span lang=EN-GB><br></span><a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" target="_blank"><span lang=EN-GB>https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</span></a><span lang=EN-GB><o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-GB> <o:p></o:p></span></p></div></div></blockquote></div></div></body></html>