<div dir="ltr">*nod* Maybe consistency's enough for now. But yeah, if you can test whether the assertion fires (though for invalid locs, that usually means invalid code - and we don't have nice big repositories of weird invalid code - just the clang regression tests).<br><br><br></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Aug 7, 2018 at 3:27 PM Stephen Kelly <<a href="mailto:steveire@gmail.com">steveire@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<br>
Ok, I can look into adding the assertion and run the tests with it
to see if anything comes up.<br>
<br>
I made a tool which dumps SourceLocations reachable from an AST node
(I intend to integrate it into clang-query), and I noticed the large
amount of mixing of get{Start,End}Loc and getLoc{Start,End} (see
other pending reviews). I reviewed all of them and found that this
method is the only case where a pair of methods of that name pattern
differ in what they return (by eye, at least), so I thought it
should be fixed.<br>
<br>
Thanks,<br>
<br>
Stephen.</div><div text="#000000" bgcolor="#FFFFFF"><br>
<br>
<div class="m_-8599552760943218681moz-cite-prefix">On 07/08/18 23:18, David Blaikie wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">If it never comes up, maybe an assertion would
suffice? (& if the assertion ever does fire - hey, we've
found a test case to use)<br>
<br>
How'd you find this/what motivated you to make the change?<br>
<br>
<div class="gmail_quote">
<div dir="ltr">On Tue, Aug 7, 2018 at 3:11 PM Stephen Kelly
<<a href="mailto:steveire@gmail.com" target="_blank">steveire@gmail.com</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF"> <br>
Hi David, <br>
<br>
I'm happy to add a test case, but I don't know how to
catch this case. It's not obvious to me if any code path
intentionally creates a DeclarationNameInfo with a valid
start loc and an invalid end loc. Can you suggest a test
case?<br>
<br>
Thanks,<br>
<br>
Stephen.</div>
<div text="#000000" bgcolor="#FFFFFF"><br>
<br>
<div class="m_-8599552760943218681m_-4641790861911320017moz-cite-prefix">On
07/08/18 03:23, David Blaikie wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">test case?</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Mon, Jul 30, 2018 at 1:39 PM Stephen
Kelly via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author:
steveire<br>
Date: Mon Jul 30 13:39:14 2018<br>
New Revision: 338301<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=338301&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=338301&view=rev</a><br>
Log:<br>
Avoid returning an invalid end source loc<br>
<br>
Modified:<br>
cfe/trunk/include/clang/AST/DeclarationName.h<br>
cfe/trunk/lib/AST/DeclarationName.cpp<br>
<br>
Modified:
cfe/trunk/include/clang/AST/DeclarationName.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclarationName.h?rev=338301&r1=338300&r2=338301&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclarationName.h?rev=338301&r1=338300&r2=338301&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/AST/DeclarationName.h
(original)<br>
+++ cfe/trunk/include/clang/AST/DeclarationName.h
Mon Jul 30 13:39:14 2018<br>
@@ -558,7 +558,7 @@ public:<br>
SourceLocation getBeginLoc() const { return
NameLoc; }<br>
<br>
/// getEndLoc - Retrieve the location of the last
token.<br>
- SourceLocation getEndLoc() const;<br>
+ SourceLocation getEndLoc() const { return
getLocEnd(); }<br>
<br>
/// getSourceRange - The range of the declaration
name.<br>
SourceRange getSourceRange() const LLVM_READONLY
{<br>
@@ -570,9 +570,11 @@ public:<br>
}<br>
<br>
SourceLocation getLocEnd() const LLVM_READONLY {<br>
- SourceLocation EndLoc = getEndLoc();<br>
+ SourceLocation EndLoc = getEndLocPrivate();<br>
return EndLoc.isValid() ? EndLoc :
getLocStart();<br>
}<br>
+private:<br>
+ SourceLocation getEndLocPrivate() const;<br>
};<br>
<br>
/// Insertion operator for diagnostics. This
allows sending DeclarationName's<br>
<br>
Modified: cfe/trunk/lib/AST/DeclarationName.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclarationName.cpp?rev=338301&r1=338300&r2=338301&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclarationName.cpp?rev=338301&r1=338300&r2=338301&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/AST/DeclarationName.cpp (original)<br>
+++ cfe/trunk/lib/AST/DeclarationName.cpp Mon Jul 30
13:39:14 2018<br>
@@ -689,7 +689,7 @@ void
DeclarationNameInfo::printName(raw_<br>
llvm_unreachable("Unexpected declaration name
kind");<br>
}<br>
<br>
-SourceLocation DeclarationNameInfo::getEndLoc()
const {<br>
+SourceLocation
DeclarationNameInfo::getEndLocPrivate() const {<br>
switch (Name.getNameKind()) {<br>
case DeclarationName::Identifier:<br>
case DeclarationName::CXXDeductionGuideName:<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote>
</div>
</blockquote>
<br>
</div>
</blockquote>
</div>
</div>
</blockquote>
<br>
</div></blockquote></div>