Re: r338301 - Avoid returning an invalid end source loc
*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 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 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@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=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=338300=338301=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=338300=338301=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@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r338301 - Avoid returning an invalid end source loc
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 > 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
mailto:cfe-commits@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=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=338300=338301=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=338300=338301=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@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r338301 - Avoid returning an invalid end source loc
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 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@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=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=338300=338301=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=338300=338301=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@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r338301 - Avoid returning an invalid end source loc
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
mailto:cfe-commits@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=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=338300=338301=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=338300=338301=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@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r338301 - Avoid returning an invalid end source loc
test case?
On Mon, Jul 30, 2018 at 1:39 PM Stephen Kelly via cfe-commits <
cfe-commits@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=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=338300=338301=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=338300=338301=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@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
