r338301 - Avoid returning an invalid end source loc

Stephen Kelly via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 7 15:27:41 PDT 2018


Ok, I can look into adding the assertion and run the tests with it to 
see if anything comes up.

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.

Thanks,

Stephen.

On 07/08/18 23:18, David Blaikie wrote:
> 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)
>
> How'd you find this/what motivated you to make the change?
>
> On Tue, Aug 7, 2018 at 3:11 PM Stephen Kelly <steveire at gmail.com 
> <mailto:steveire at gmail.com>> wrote:
>
>
>     Hi David,
>
>     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?
>
>     Thanks,
>
>     Stephen.
>
>
>     On 07/08/18 03:23, David Blaikie wrote:
>>     test case?
>>
>>     On Mon, Jul 30, 2018 at 1:39 PM Stephen Kelly via cfe-commits
>>     <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>>
>>     wrote:
>>
>>         Author: steveire
>>         Date: Mon Jul 30 13:39:14 2018
>>         New Revision: 338301
>>
>>         URL: http://llvm.org/viewvc/llvm-project?rev=338301&view=rev
>>         Log:
>>         Avoid returning an invalid end source loc
>>
>>         Modified:
>>             cfe/trunk/include/clang/AST/DeclarationName.h
>>             cfe/trunk/lib/AST/DeclarationName.cpp
>>
>>         Modified: cfe/trunk/include/clang/AST/DeclarationName.h
>>         URL:
>>         http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclarationName.h?rev=338301&r1=338300&r2=338301&view=diff
>>         ==============================================================================
>>         --- cfe/trunk/include/clang/AST/DeclarationName.h (original)
>>         +++ cfe/trunk/include/clang/AST/DeclarationName.h Mon Jul 30
>>         13:39:14 2018
>>         @@ -558,7 +558,7 @@ public:
>>            SourceLocation getBeginLoc() const { return NameLoc; }
>>
>>            /// getEndLoc - Retrieve the location of the last token.
>>         -  SourceLocation getEndLoc() const;
>>         +  SourceLocation getEndLoc() const { return getLocEnd(); }
>>
>>            /// getSourceRange - The range of the declaration name.
>>            SourceRange getSourceRange() const LLVM_READONLY {
>>         @@ -570,9 +570,11 @@ public:
>>            }
>>
>>            SourceLocation getLocEnd() const LLVM_READONLY {
>>         -    SourceLocation EndLoc = getEndLoc();
>>         +    SourceLocation EndLoc = getEndLocPrivate();
>>              return EndLoc.isValid() ? EndLoc : getLocStart();
>>            }
>>         +private:
>>         +  SourceLocation getEndLocPrivate() const;
>>          };
>>
>>          /// Insertion operator for diagnostics.  This allows sending
>>         DeclarationName's
>>
>>         Modified: cfe/trunk/lib/AST/DeclarationName.cpp
>>         URL:
>>         http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclarationName.cpp?rev=338301&r1=338300&r2=338301&view=diff
>>         ==============================================================================
>>         --- cfe/trunk/lib/AST/DeclarationName.cpp (original)
>>         +++ cfe/trunk/lib/AST/DeclarationName.cpp Mon Jul 30 13:39:14
>>         2018
>>         @@ -689,7 +689,7 @@ void DeclarationNameInfo::printName(raw_
>>            llvm_unreachable("Unexpected declaration name kind");
>>          }
>>
>>         -SourceLocation DeclarationNameInfo::getEndLoc() const {
>>         +SourceLocation DeclarationNameInfo::getEndLocPrivate() const {
>>            switch (Name.getNameKind()) {
>>            case DeclarationName::Identifier:
>>            case DeclarationName::CXXDeductionGuideName:
>>
>>
>>         _______________________________________________
>>         cfe-commits mailing list
>>         cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
>>         http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180807/886b1c6a/attachment.html>


More information about the cfe-commits mailing list