r338301 - Avoid returning an invalid end source loc

[David Blaikie via cfe-commits]

*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).



On Tue, Aug 7, 2018 at 3:27 PM Stephen Kelly <steveire at gmail.com> wrote:

>
> 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> 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> 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
>>> 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/34cf4ee4/attachment.html>