[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
penzn marked an inline comment as done.
penzn added inline comments.
Comment at: cfe/trunk/lib/Tooling/Refactoring/RangeSelector.cpp:229
+RangeSelector tooling::statements(StringRef ID) {
+ return RelativeSelector(ID);
+}
penzn wrote:
> Sorry for posting here, haven't gotten my bugzilla access yet (requested
> though).
>
> This breaks with Visual Studio 2017 (15.7.6):
>
> RangeSelector.cpp(229): error C2971:
> '`anonymous-namespace'::RelativeSelector': template parameter 'Func':
> 'getStatementsRange': a variable with non-static storage duration cannot be
> used as a non-type argument
Fixed in https://reviews.llvm.org/D62202
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61774/new/
https://reviews.llvm.org/D61774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
Either way looks good.
If we go with function_ref, we should definitely store a comment why
storing function_ref is fine there. Or use a function pointer to make it
even clearer that nothing cheesy is going on there.
On Thu, May 23, 2019 at 5:28 PM Yitzhak Mandelbaum
wrote:
> Actually, someone already committed a fix: https://reviews.llvm.org/D62202
>
> I can still make this change if it seems worthwhile, but its not strictly
> necessary at this point.
>
> On Thu, May 23, 2019 at 11:14 AM Yitzhak Mandelbaum
> wrote:
>
>> Given that we'll need to store the function reference, is
>> llvm::function_ref still the way to go? The comments seem to warn away from
>> storing function_refs.
>>
>> On Thu, May 23, 2019 at 11:06 AM Yitzhak Mandelbaum
>> wrote:
>>
>>> Sounds good. I'll send a fix shortly. Found another bug too (captured a
>>> StringRef in a lambda) -- shall i bundle the fixes?
>>>
>>> On Thu, May 23, 2019 at 9:01 AM Ilya Biryukov
>>> wrote:
>>>
>>>> Maybe go with a runtime parameter (of type llvm::function_ref) instead
>>>> of the template parameter?
>>>> In any non-trivial example, the runtime costs of another function
>>>> pointer should be negligible given that we need to parse the ASTs, run the
>>>> matchers, etc.
>>>>
>>>> On Wed, May 22, 2019 at 10:48 PM Penzin, Petr
>>>> wrote:
>>>>
>>>>> It does not like some part of that instantiation, I am not sure which
>>>>> one yet. Let’s see what I can do about it.
>>>>>
>>>>>
>>>>>
>>>>> -Petr
>>>>>
>>>>>
>>>>>
>>>>> *From:* Yitzhak Mandelbaum [mailto:yitzh...@google.com]
>>>>> *Sent:* Wednesday, May 22, 2019 1:37 PM
>>>>> *To:* reviews+d61774+public+f458bb6144ae8...@reviews.llvm.org
>>>>> *Cc:* Ilya Biryukov ; Penzin, Petr <
>>>>> petr.pen...@intel.com>; llvm-comm...@lists.llvm.org; Michał Górny <
>>>>> mgo...@gentoo.org>; cfe-commits ; Theko
>>>>> Lekena ; Nicolas Lesser ;
>>>>> Han Shen
>>>>> *Subject:* Re: [PATCH] D61774: [LibTooling] Add RangeSelector library
>>>>> for defining source ranges based on bound AST nodes.
>>>>>
>>>>>
>>>>>
>>>>> I'm confused by the error given that getStatementsRange is a function
>>>>> name. I don't have Visual Studio -- can you find a fix and send a patch?
>>>>> I
>>>>> wonder if taking the address explicitly is enough? Or, if you know how to
>>>>> trigger this error in clang or gcc, I can fix it myself.
>>>>>
>>>>>
>>>>>
>>>>> On Wed, May 22, 2019 at 4:31 PM Petr Penzin via Phabricator <
>>>>> revi...@reviews.llvm.org> wrote:
>>>>>
>>>>> penzn added inline comments.
>>>>>
>>>>>
>>>>>
>>>>> Comment at: cfe/trunk/lib/Tooling/Refactoring/RangeSelector.cpp:229
>>>>> +RangeSelector tooling::statements(StringRef ID) {
>>>>> + return RelativeSelector(ID);
>>>>> +}
>>>>>
>>>>> Sorry for posting here, haven't gotten my bugzilla access yet
>>>>> (requested though).
>>>>>
>>>>> This breaks with Visual Studio 2017 (15.7.6):
>>>>>
>>>>> RangeSelector.cpp(229): error C2971:
>>>>> '`anonymous-namespace'::RelativeSelector': template parameter 'Func':
>>>>> 'getStatementsRange': a variable with non-static storage duration cannot
>>>>> be
>>>>> used as a non-type argument
>>>>>
>>>>>
>>>>> Repository:
>>>>> rL LLVM
>>>>>
>>>>> CHANGES SINCE LAST ACTION
>>>>> https://reviews.llvm.org/D61774/new/
>>>>>
>>>>> https://reviews.llvm.org/D61774
>>>>>
>>>>>
>>>>>
>>>>
>>>> --
>>>> Regards,
>>>> Ilya Biryukov
>>>>
>>>
--
Regards,
Ilya Biryukov
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
Actually, someone already committed a fix: https://reviews.llvm.org/D62202
I can still make this change if it seems worthwhile, but its not strictly
necessary at this point.
On Thu, May 23, 2019 at 11:14 AM Yitzhak Mandelbaum
wrote:
> Given that we'll need to store the function reference, is
> llvm::function_ref still the way to go? The comments seem to warn away from
> storing function_refs.
>
> On Thu, May 23, 2019 at 11:06 AM Yitzhak Mandelbaum
> wrote:
>
>> Sounds good. I'll send a fix shortly. Found another bug too (captured a
>> StringRef in a lambda) -- shall i bundle the fixes?
>>
>> On Thu, May 23, 2019 at 9:01 AM Ilya Biryukov
>> wrote:
>>
>>> Maybe go with a runtime parameter (of type llvm::function_ref) instead
>>> of the template parameter?
>>> In any non-trivial example, the runtime costs of another function
>>> pointer should be negligible given that we need to parse the ASTs, run the
>>> matchers, etc.
>>>
>>> On Wed, May 22, 2019 at 10:48 PM Penzin, Petr
>>> wrote:
>>>
>>>> It does not like some part of that instantiation, I am not sure which
>>>> one yet. Let’s see what I can do about it.
>>>>
>>>>
>>>>
>>>> -Petr
>>>>
>>>>
>>>>
>>>> *From:* Yitzhak Mandelbaum [mailto:yitzh...@google.com]
>>>> *Sent:* Wednesday, May 22, 2019 1:37 PM
>>>> *To:* reviews+d61774+public+f458bb6144ae8...@reviews.llvm.org
>>>> *Cc:* Ilya Biryukov ; Penzin, Petr <
>>>> petr.pen...@intel.com>; llvm-comm...@lists.llvm.org; Michał Górny <
>>>> mgo...@gentoo.org>; cfe-commits ; Theko
>>>> Lekena ; Nicolas Lesser ;
>>>> Han Shen
>>>> *Subject:* Re: [PATCH] D61774: [LibTooling] Add RangeSelector library
>>>> for defining source ranges based on bound AST nodes.
>>>>
>>>>
>>>>
>>>> I'm confused by the error given that getStatementsRange is a function
>>>> name. I don't have Visual Studio -- can you find a fix and send a patch? I
>>>> wonder if taking the address explicitly is enough? Or, if you know how to
>>>> trigger this error in clang or gcc, I can fix it myself.
>>>>
>>>>
>>>>
>>>> On Wed, May 22, 2019 at 4:31 PM Petr Penzin via Phabricator <
>>>> revi...@reviews.llvm.org> wrote:
>>>>
>>>> penzn added inline comments.
>>>>
>>>>
>>>>
>>>> Comment at: cfe/trunk/lib/Tooling/Refactoring/RangeSelector.cpp:229
>>>> +RangeSelector tooling::statements(StringRef ID) {
>>>> + return RelativeSelector(ID);
>>>> +}
>>>>
>>>> Sorry for posting here, haven't gotten my bugzilla access yet
>>>> (requested though).
>>>>
>>>> This breaks with Visual Studio 2017 (15.7.6):
>>>>
>>>> RangeSelector.cpp(229): error C2971:
>>>> '`anonymous-namespace'::RelativeSelector': template parameter 'Func':
>>>> 'getStatementsRange': a variable with non-static storage duration cannot be
>>>> used as a non-type argument
>>>>
>>>>
>>>> Repository:
>>>> rL LLVM
>>>>
>>>> CHANGES SINCE LAST ACTION
>>>> https://reviews.llvm.org/D61774/new/
>>>>
>>>> https://reviews.llvm.org/D61774
>>>>
>>>>
>>>>
>>>
>>> --
>>> Regards,
>>> Ilya Biryukov
>>>
>>
smime.p7s
Description: S/MIME Cryptographic Signature
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
Given that we'll need to store the function reference, is
llvm::function_ref still the way to go? The comments seem to warn away from
storing function_refs.
On Thu, May 23, 2019 at 11:06 AM Yitzhak Mandelbaum
wrote:
> Sounds good. I'll send a fix shortly. Found another bug too (captured a
> StringRef in a lambda) -- shall i bundle the fixes?
>
> On Thu, May 23, 2019 at 9:01 AM Ilya Biryukov
> wrote:
>
>> Maybe go with a runtime parameter (of type llvm::function_ref) instead of
>> the template parameter?
>> In any non-trivial example, the runtime costs of another function pointer
>> should be negligible given that we need to parse the ASTs, run the
>> matchers, etc.
>>
>> On Wed, May 22, 2019 at 10:48 PM Penzin, Petr
>> wrote:
>>
>>> It does not like some part of that instantiation, I am not sure which
>>> one yet. Let’s see what I can do about it.
>>>
>>>
>>>
>>> -Petr
>>>
>>>
>>>
>>> *From:* Yitzhak Mandelbaum [mailto:yitzh...@google.com]
>>> *Sent:* Wednesday, May 22, 2019 1:37 PM
>>> *To:* reviews+d61774+public+f458bb6144ae8...@reviews.llvm.org
>>> *Cc:* Ilya Biryukov ; Penzin, Petr <
>>> petr.pen...@intel.com>; llvm-comm...@lists.llvm.org; Michał Górny <
>>> mgo...@gentoo.org>; cfe-commits ; Theko
>>> Lekena ; Nicolas Lesser ;
>>> Han Shen
>>> *Subject:* Re: [PATCH] D61774: [LibTooling] Add RangeSelector library
>>> for defining source ranges based on bound AST nodes.
>>>
>>>
>>>
>>> I'm confused by the error given that getStatementsRange is a function
>>> name. I don't have Visual Studio -- can you find a fix and send a patch? I
>>> wonder if taking the address explicitly is enough? Or, if you know how to
>>> trigger this error in clang or gcc, I can fix it myself.
>>>
>>>
>>>
>>> On Wed, May 22, 2019 at 4:31 PM Petr Penzin via Phabricator <
>>> revi...@reviews.llvm.org> wrote:
>>>
>>> penzn added inline comments.
>>>
>>>
>>>
>>> Comment at: cfe/trunk/lib/Tooling/Refactoring/RangeSelector.cpp:229
>>> +RangeSelector tooling::statements(StringRef ID) {
>>> + return RelativeSelector(ID);
>>> +}
>>>
>>> Sorry for posting here, haven't gotten my bugzilla access yet (requested
>>> though).
>>>
>>> This breaks with Visual Studio 2017 (15.7.6):
>>>
>>> RangeSelector.cpp(229): error C2971:
>>> '`anonymous-namespace'::RelativeSelector': template parameter 'Func':
>>> 'getStatementsRange': a variable with non-static storage duration cannot be
>>> used as a non-type argument
>>>
>>>
>>> Repository:
>>> rL LLVM
>>>
>>> CHANGES SINCE LAST ACTION
>>> https://reviews.llvm.org/D61774/new/
>>>
>>> https://reviews.llvm.org/D61774
>>>
>>>
>>>
>>
>> --
>> Regards,
>> Ilya Biryukov
>>
>
smime.p7s
Description: S/MIME Cryptographic Signature
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
Sounds good. I'll send a fix shortly. Found another bug too (captured a
StringRef in a lambda) -- shall i bundle the fixes?
On Thu, May 23, 2019 at 9:01 AM Ilya Biryukov wrote:
> Maybe go with a runtime parameter (of type llvm::function_ref) instead of
> the template parameter?
> In any non-trivial example, the runtime costs of another function pointer
> should be negligible given that we need to parse the ASTs, run the
> matchers, etc.
>
> On Wed, May 22, 2019 at 10:48 PM Penzin, Petr
> wrote:
>
>> It does not like some part of that instantiation, I am not sure which one
>> yet. Let’s see what I can do about it.
>>
>>
>>
>> -Petr
>>
>>
>>
>> *From:* Yitzhak Mandelbaum [mailto:yitzh...@google.com]
>> *Sent:* Wednesday, May 22, 2019 1:37 PM
>> *To:* reviews+d61774+public+f458bb6144ae8...@reviews.llvm.org
>> *Cc:* Ilya Biryukov ; Penzin, Petr <
>> petr.pen...@intel.com>; llvm-comm...@lists.llvm.org; Michał Górny <
>> mgo...@gentoo.org>; cfe-commits ; Theko
>> Lekena ; Nicolas Lesser ;
>> Han Shen
>> *Subject:* Re: [PATCH] D61774: [LibTooling] Add RangeSelector library
>> for defining source ranges based on bound AST nodes.
>>
>>
>>
>> I'm confused by the error given that getStatementsRange is a function
>> name. I don't have Visual Studio -- can you find a fix and send a patch? I
>> wonder if taking the address explicitly is enough? Or, if you know how to
>> trigger this error in clang or gcc, I can fix it myself.
>>
>>
>>
>> On Wed, May 22, 2019 at 4:31 PM Petr Penzin via Phabricator <
>> revi...@reviews.llvm.org> wrote:
>>
>> penzn added inline comments.
>>
>>
>>
>> Comment at: cfe/trunk/lib/Tooling/Refactoring/RangeSelector.cpp:229
>> +RangeSelector tooling::statements(StringRef ID) {
>> + return RelativeSelector(ID);
>> +}
>>
>> Sorry for posting here, haven't gotten my bugzilla access yet (requested
>> though).
>>
>> This breaks with Visual Studio 2017 (15.7.6):
>>
>> RangeSelector.cpp(229): error C2971:
>> '`anonymous-namespace'::RelativeSelector': template parameter 'Func':
>> 'getStatementsRange': a variable with non-static storage duration cannot be
>> used as a non-type argument
>>
>>
>> Repository:
>> rL LLVM
>>
>> CHANGES SINCE LAST ACTION
>> https://reviews.llvm.org/D61774/new/
>>
>> https://reviews.llvm.org/D61774
>>
>>
>>
>
> --
> Regards,
> Ilya Biryukov
>
smime.p7s
Description: S/MIME Cryptographic Signature
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
Maybe go with a runtime parameter (of type llvm::function_ref) instead of
the template parameter?
In any non-trivial example, the runtime costs of another function pointer
should be negligible given that we need to parse the ASTs, run the
matchers, etc.
On Wed, May 22, 2019 at 10:48 PM Penzin, Petr wrote:
> It does not like some part of that instantiation, I am not sure which one
> yet. Let’s see what I can do about it.
>
>
>
> -Petr
>
>
>
> *From:* Yitzhak Mandelbaum [mailto:yitzh...@google.com]
> *Sent:* Wednesday, May 22, 2019 1:37 PM
> *To:* reviews+d61774+public+f458bb6144ae8...@reviews.llvm.org
> *Cc:* Ilya Biryukov ; Penzin, Petr <
> petr.pen...@intel.com>; llvm-comm...@lists.llvm.org; Michał Górny <
> mgo...@gentoo.org>; cfe-commits ; Theko
> Lekena ; Nicolas Lesser ;
> Han Shen
> *Subject:* Re: [PATCH] D61774: [LibTooling] Add RangeSelector library for
> defining source ranges based on bound AST nodes.
>
>
>
> I'm confused by the error given that getStatementsRange is a function
> name. I don't have Visual Studio -- can you find a fix and send a patch? I
> wonder if taking the address explicitly is enough? Or, if you know how to
> trigger this error in clang or gcc, I can fix it myself.
>
>
>
> On Wed, May 22, 2019 at 4:31 PM Petr Penzin via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
> penzn added inline comments.
>
>
>
> Comment at: cfe/trunk/lib/Tooling/Refactoring/RangeSelector.cpp:229
> +RangeSelector tooling::statements(StringRef ID) {
> + return RelativeSelector(ID);
> +}
>
> Sorry for posting here, haven't gotten my bugzilla access yet (requested
> though).
>
> This breaks with Visual Studio 2017 (15.7.6):
>
> RangeSelector.cpp(229): error C2971:
> '`anonymous-namespace'::RelativeSelector': template parameter 'Func':
> 'getStatementsRange': a variable with non-static storage duration cannot be
> used as a non-type argument
>
>
> Repository:
> rL LLVM
>
> CHANGES SINCE LAST ACTION
> https://reviews.llvm.org/D61774/new/
>
> https://reviews.llvm.org/D61774
>
>
>
--
Regards,
Ilya Biryukov
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
RE: [PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
It does not like some part of that instantiation, I am not sure which one yet.
Let’s see what I can do about it.
-Petr
From: Yitzhak Mandelbaum [mailto:yitzh...@google.com]
Sent: Wednesday, May 22, 2019 1:37 PM
To: reviews+d61774+public+f458bb6144ae8...@reviews.llvm.org
Cc: Ilya Biryukov ; Penzin, Petr ;
llvm-comm...@lists.llvm.org; Michał Górny ; cfe-commits
; Theko Lekena ; Nicolas
Lesser ; Han Shen
Subject: Re: [PATCH] D61774: [LibTooling] Add RangeSelector library for
defining source ranges based on bound AST nodes.
I'm confused by the error given that getStatementsRange is a function name. I
don't have Visual Studio -- can you find a fix and send a patch? I wonder if
taking the address explicitly is enough? Or, if you know how to trigger this
error in clang or gcc, I can fix it myself.
On Wed, May 22, 2019 at 4:31 PM Petr Penzin via Phabricator
mailto:revi...@reviews.llvm.org>> wrote:
penzn added inline comments.
Comment at: cfe/trunk/lib/Tooling/Refactoring/RangeSelector.cpp:229
+RangeSelector tooling::statements(StringRef ID) {
+ return RelativeSelector(ID);
+}
Sorry for posting here, haven't gotten my bugzilla access yet (requested
though).
This breaks with Visual Studio 2017 (15.7.6):
RangeSelector.cpp(229): error C2971: '`anonymous-namespace'::RelativeSelector':
template parameter 'Func': 'getStatementsRange': a variable with non-static
storage duration cannot be used as a non-type argument
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61774/new/
https://reviews.llvm.org/D61774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
I'm confused by the error given that getStatementsRange is a function
name. I don't have Visual Studio -- can you find a fix and send a patch? I
wonder if taking the address explicitly is enough? Or, if you know how to
trigger this error in clang or gcc, I can fix it myself.
On Wed, May 22, 2019 at 4:31 PM Petr Penzin via Phabricator <
revi...@reviews.llvm.org> wrote:
> penzn added inline comments.
>
>
>
> Comment at: cfe/trunk/lib/Tooling/Refactoring/RangeSelector.cpp:229
> +RangeSelector tooling::statements(StringRef ID) {
> + return RelativeSelector(ID);
> +}
>
> Sorry for posting here, haven't gotten my bugzilla access yet (requested
> though).
>
> This breaks with Visual Studio 2017 (15.7.6):
>
> RangeSelector.cpp(229): error C2971:
> '`anonymous-namespace'::RelativeSelector': template parameter 'Func':
> 'getStatementsRange': a variable with non-static storage duration cannot be
> used as a non-type argument
>
>
> Repository:
> rL LLVM
>
> CHANGES SINCE LAST ACTION
> https://reviews.llvm.org/D61774/new/
>
> https://reviews.llvm.org/D61774
>
>
>
>
smime.p7s
Description: S/MIME Cryptographic Signature
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
penzn added inline comments.
Comment at: cfe/trunk/lib/Tooling/Refactoring/RangeSelector.cpp:229
+RangeSelector tooling::statements(StringRef ID) {
+ return RelativeSelector(ID);
+}
Sorry for posting here, haven't gotten my bugzilla access yet (requested
though).
This breaks with Visual Studio 2017 (15.7.6):
RangeSelector.cpp(229): error C2971: '`anonymous-namespace'::RelativeSelector':
template parameter 'Func': 'getStatementsRange': a variable with non-static
storage duration cannot be used as a non-type argument
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61774/new/
https://reviews.llvm.org/D61774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
ymandel added a comment. Fixed with r361160. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61774/new/ https://reviews.llvm.org/D61774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
ymandel added a comment. Test file broke gcc builds. Fix in progress... Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61774/new/ https://reviews.llvm.org/D61774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL361152: [LibTooling] Add RangeSelector library for defining
source ranges based on… (authored by ymandel, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D61774?vs=200262=200265#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61774/new/
https://reviews.llvm.org/D61774
Files:
cfe/trunk/include/clang/Tooling/Refactoring/RangeSelector.h
cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
cfe/trunk/lib/Tooling/Refactoring/RangeSelector.cpp
cfe/trunk/unittests/Tooling/CMakeLists.txt
cfe/trunk/unittests/Tooling/RangeSelectorTest.cpp
Index: cfe/trunk/lib/Tooling/Refactoring/RangeSelector.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/RangeSelector.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/RangeSelector.cpp
@@ -0,0 +1,264 @@
+//===--- RangeSelector.cpp - RangeSelector implementations --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/RangeSelector.h"
+#include "clang/AST/Expr.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/Refactoring/SourceCode.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/Error.h"
+#include
+#include
+#include
+
+using namespace clang;
+using namespace tooling;
+
+using ast_matchers::MatchFinder;
+using ast_type_traits::ASTNodeKind;
+using ast_type_traits::DynTypedNode;
+using llvm::Error;
+using llvm::StringError;
+
+using MatchResult = MatchFinder::MatchResult;
+
+static Error invalidArgumentError(Twine Message) {
+ return llvm::make_error(llvm::errc::invalid_argument, Message);
+}
+
+static Error typeError(StringRef ID, const ASTNodeKind ) {
+ return invalidArgumentError("mismatched type (node id=" + ID +
+ " kind=" + Kind.asStringRef() + ")");
+}
+
+static Error typeError(StringRef ID, const ASTNodeKind ,
+ Twine ExpectedType) {
+ return invalidArgumentError("mismatched type: expected one of " +
+ ExpectedType + " (node id=" + ID +
+ " kind=" + Kind.asStringRef() + ")");
+}
+
+static Error missingPropertyError(StringRef ID, Twine Description,
+ StringRef Property) {
+ return invalidArgumentError(Description + " requires property '" + Property +
+ "' (node id=" + ID + ")");
+}
+
+static Expected getNode(const ast_matchers::BoundNodes ,
+ StringRef ID) {
+ auto = Nodes.getMap();
+ auto It = NodesMap.find(ID);
+ if (It == NodesMap.end())
+return invalidArgumentError("ID not bound: " + ID);
+ return It->second;
+}
+
+// FIXME: handling of macros should be configurable.
+static SourceLocation findPreviousTokenStart(SourceLocation Start,
+ const SourceManager ,
+ const LangOptions ) {
+ if (Start.isInvalid() || Start.isMacroID())
+return SourceLocation();
+
+ SourceLocation BeforeStart = Start.getLocWithOffset(-1);
+ if (BeforeStart.isInvalid() || BeforeStart.isMacroID())
+return SourceLocation();
+
+ return Lexer::GetBeginningOfToken(BeforeStart, SM, LangOpts);
+}
+
+// Finds the start location of the previous token of kind \p TK.
+// FIXME: handling of macros should be configurable.
+static SourceLocation findPreviousTokenKind(SourceLocation Start,
+const SourceManager ,
+const LangOptions ,
+tok::TokenKind TK) {
+ while (true) {
+SourceLocation L = findPreviousTokenStart(Start, SM, LangOpts);
+if (L.isInvalid() || L.isMacroID())
+ return SourceLocation();
+
+Token T;
+if (Lexer::getRawToken(L, T, SM, LangOpts, /*IgnoreWhiteSpace=*/true))
+ return SourceLocation();
+
+if (T.is(TK))
+ return T.getLocation();
+
+Start = L;
+ }
+}
+
+static SourceLocation findOpenParen(const CallExpr , const SourceManager ,
+const LangOptions ) {
+ SourceLocation EndLoc =
+ E.getNumArgs() == 0 ? E.getRParenLoc() : E.getArg(0)->getBeginLoc();
+ return findPreviousTokenKind(EndLoc, SM, LangOpts, tok::TokenKind::l_paren);
+}
+
+RangeSelector tooling::node(StringRef ID) {
+ return [ID](const MatchResult ) -> Expected {
+Expected Node = getNode(Result.Nodes,
[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
ymandel updated this revision to Diff 200262.
ymandel added a comment.
updated some comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61774/new/
https://reviews.llvm.org/D61774
Files:
clang/include/clang/Tooling/Refactoring/RangeSelector.h
clang/lib/Tooling/Refactoring/CMakeLists.txt
clang/lib/Tooling/Refactoring/RangeSelector.cpp
clang/unittests/Tooling/CMakeLists.txt
clang/unittests/Tooling/RangeSelectorTest.cpp
Index: clang/unittests/Tooling/RangeSelectorTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/RangeSelectorTest.cpp
@@ -0,0 +1,496 @@
+//===- unittest/Tooling/RangeSelectorTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/RangeSelector.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/FixIt.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace ast_matchers;
+
+namespace {
+using ::testing::AllOf;
+using ::testing::HasSubstr;
+using MatchResult = MatchFinder::MatchResult;
+using ::llvm::Expected;
+using ::llvm::Failed;
+using ::llvm::HasValue;
+using ::llvm::StringError;
+
+struct TestMatch {
+ // The AST unit from which `result` is built. We bundle it because it backs
+ // the result. Users are not expected to access it.
+ std::unique_ptr ASTUnit;
+ // The result to use in the test. References `ast_unit`.
+ MatchResult Result;
+};
+
+template TestMatch matchCode(StringRef Code, M Matcher) {
+ auto ASTUnit = buildASTFromCode(Code);
+ assert(ASTUnit != nullptr && "AST construction failed");
+
+ ASTContext = ASTUnit->getASTContext();
+ assert(!Context.getDiagnostics().hasErrorOccurred() && "Compilation error");
+
+ auto Matches = ast_matchers::match(Matcher, Context);
+ // We expect a single, exact match.
+ assert(Matches.size() != 0 && "no matches found");
+ assert(Matches.size() == 1 && "too many matches");
+
+ return TestMatch{std::move(ASTUnit), MatchResult(Matches[0], )};
+}
+
+// Applies \p Selector to \p Match and, on success, returns the selected source.
+Expected apply(RangeSelector Selector, const TestMatch ) {
+ Expected Range = Selector(Match.Result);
+ if (!Range)
+return Range.takeError();
+ return fixit::internal::getText(*Range, *Match.Result.Context);
+}
+
+// Applies \p Selector to a trivial match with only a single bound node with id
+// "bound_node_id". For use in testing unbound-node errors.
+Expected applyToTrivial(const RangeSelector ) {
+ // We need to bind the result to something, or the match will fail. Use a
+ // binding that is not used in the unbound node tests.
+ TestMatch Match =
+ matchCode("static int x = 0;", varDecl().bind("bound_node_id"));
+ return Selector(Match.Result);
+}
+
+// Matches the message expected for unbound-node failures.
+testing::Matcher withUnboundNodeMessage() {
+ return testing::Property(
+ ::getMessage,
+ AllOf(HasSubstr("unbound_id"), HasSubstr("not bound")));
+}
+
+// Applies \p Selector to code containing assorted node types, where the match
+// binds each one: a statement ("stmt"), a (non-member) ctor-initializer
+// ("init"), an expression ("expr") and a (nameless) declaration ("decl"). Used
+// to test failures caused by applying selectors to nodes of the wrong type.
+Expected applyToAssorted(RangeSelector Selector) {
+ StringRef Code = R"cc(
+ struct A {};
+ class F : public A {
+ public:
+F(int) {}
+ };
+ void g() { F f(1); }
+)cc";
+
+ auto Matcher =
+ compoundStmt(
+ hasDescendant(
+ cxxConstructExpr(
+ hasDeclaration(
+ decl(hasDescendant(cxxCtorInitializer(isBaseInitializer())
+ .bind("init")))
+ .bind("decl")))
+ .bind("expr")))
+ .bind("stmt");
+
+ return Selector(matchCode(Code, Matcher).Result);
+}
+
+// Matches the message expected for type-error failures.
+testing::Matcher withTypeErrorMessage(StringRef NodeID) {
+ return testing::Property(
+ ::getMessage,
+ AllOf(HasSubstr(NodeID), HasSubstr("mismatched type")));
+}
+
+TEST(RangeSelectorTest, UnboundNode) {
+ EXPECT_THAT_EXPECTED(applyToTrivial(node("unbound_id")),
+ Failed(withUnboundNodeMessage()));
+}
+
+TEST(RangeSelectorTest, RangeOp) {
+ StringRef Code = R"cc(
+int f(int x, int y, int z) { return 3; }
+int g() { return f(/*
[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Thanks! LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61774/new/ https://reviews.llvm.org/D61774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
ymandel updated this revision to Diff 200079.
ymandel marked 2 inline comments as done.
ymandel added a comment.
fixed formatting.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61774/new/
https://reviews.llvm.org/D61774
Files:
clang/include/clang/Tooling/Refactoring/RangeSelector.h
clang/lib/Tooling/Refactoring/CMakeLists.txt
clang/lib/Tooling/Refactoring/RangeSelector.cpp
clang/unittests/Tooling/CMakeLists.txt
clang/unittests/Tooling/RangeSelectorTest.cpp
Index: clang/unittests/Tooling/RangeSelectorTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/RangeSelectorTest.cpp
@@ -0,0 +1,496 @@
+//===- unittest/Tooling/RangeSelectorTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/RangeSelector.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/FixIt.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace ast_matchers;
+
+namespace {
+using ::testing::AllOf;
+using ::testing::HasSubstr;
+using MatchResult = MatchFinder::MatchResult;
+using ::llvm::Expected;
+using ::llvm::Failed;
+using ::llvm::HasValue;
+using ::llvm::StringError;
+
+struct TestMatch {
+ // The AST unit from which `result` is built. We bundle it because it backs
+ // the result. Users are not expected to access it.
+ std::unique_ptr ASTUnit;
+ // The result to use in the test. References `ast_unit`.
+ MatchResult Result;
+};
+
+template TestMatch matchCode(StringRef Code, M Matcher) {
+ auto ASTUnit = buildASTFromCode(Code);
+ assert(ASTUnit != nullptr && "AST construction failed");
+
+ ASTContext = ASTUnit->getASTContext();
+ assert(!Context.getDiagnostics().hasErrorOccurred() && "Compilation error");
+
+ auto Matches = ast_matchers::match(Matcher, Context);
+ // We expect a single, exact match.
+ assert(Matches.size() != 0 && "no matches found");
+ assert(Matches.size() == 1 && "too many matches");
+
+ return TestMatch{std::move(ASTUnit), MatchResult(Matches[0], )};
+}
+
+// Applies \p Selector to \p Match and, on success, returns the selected source.
+Expected apply(RangeSelector Selector, const TestMatch ) {
+ Expected Range = Selector(Match.Result);
+ if (!Range)
+return Range.takeError();
+ return fixit::internal::getText(*Range, *Match.Result.Context);
+}
+
+// Applies \p Selector to a trivial match with only a single bound node with id
+// "bound_node_id". For use in testing unbound-node errors.
+Expected applyToTrivial(const RangeSelector ) {
+ // We need to bind the result to something, or the match will fail. Use a
+ // binding that is not used in the unbound node tests.
+ TestMatch Match =
+ matchCode("static int x = 0;", varDecl().bind("bound_node_id"));
+ return Selector(Match.Result);
+}
+
+// Matches the message expected for unbound-node failures.
+testing::Matcher withUnboundNodeMessage() {
+ return testing::Property(
+ ::getMessage,
+ AllOf(HasSubstr("unbound_id"), HasSubstr("not bound")));
+}
+
+// Applies \p Selector to code containing assorted node types, where the match
+// binds each one: a statement ("stmt"), a (non-member) ctor-initializer
+// ("init"), an expression ("expr") and a (nameless) declaration ("decl"). Used
+// to test failures caused by applying selectors to nodes of the wrong type.
+Expected applyToAssorted(RangeSelector Selector) {
+ StringRef Code = R"cc(
+ struct A {};
+ class F : public A {
+ public:
+F(int) {}
+ };
+ void g() { F f(1); }
+)cc";
+
+ auto Matcher =
+ compoundStmt(
+ hasDescendant(
+ cxxConstructExpr(
+ hasDeclaration(
+ decl(hasDescendant(cxxCtorInitializer(isBaseInitializer())
+ .bind("init")))
+ .bind("decl")))
+ .bind("expr")))
+ .bind("stmt");
+
+ return Selector(matchCode(Code, Matcher).Result);
+}
+
+// Matches the message expected for type-error failures.
+testing::Matcher withTypeErrorMessage(StringRef NodeID) {
+ return testing::Property(
+ ::getMessage,
+ AllOf(HasSubstr(NodeID), HasSubstr("mismatched type")));
+}
+
+TEST(RangeSelectorTest, UnboundNode) {
+ EXPECT_THAT_EXPECTED(applyToTrivial(node("unbound_id")),
+ Failed(withUnboundNodeMessage()));
+}
+
+TEST(RangeSelectorTest, RangeOp) {
+ StringRef Code = R"cc(
+int f(int x, int y, int z) { return
[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
ymandel marked 7 inline comments as done.
ymandel added inline comments.
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:29
+
+namespace range_selector {
+inline RangeSelector charRange(CharSourceRange R) {
ilya-biryukov wrote:
> ymandel wrote:
> > ilya-biryukov wrote:
> > > Maybe try putting it into the tooling namespace directly?
> > > Or are there immediate and unfortunate conflicts with existing names?
> > >
> > >
> > No conflicts. Was just being cautious.
> I can see why a separate namespace would make sense, but since the `tooling`
> namespace is not densely populated I hope we can get away with putting
> things here directly and saving some typing on the use-sites.
>
> Hope that does not backfire in the future, though.
SGTM.
Comment at: clang/unittests/Tooling/RangeSelectorTest.cpp:143
+ // Node-id specific version:
+ test(Matcher, range(Arg0, Arg1), Code, "3, 7");
+ // General version:
ilya-biryukov wrote:
> Consider restructuring the code to place assertions into the test function
> itself.
> This wildly improves locations reported in case of test failures and makes
> tests easier to read.
>
> I.e. having something like
> ```
> auto AST = buildASTAndMatch(Code, Matcher);
> EXPECT_EQ(applySelector(range(Arg0, Arg1), AST), "3, 7");
> ```
> is arguably both easier to read and produces better location information on
> failures than a function that runs the test.
> Even though it's a bit more code.
>
>
> Note that it's a bit more complicated if you need to deal with `Expected<>`
> return types, but still possible:
> ```
> EXPECT_THAT_EXPECTED(applySelector(range(Arg0, Arg1), AST), HasValue("3,
> 7"));
> EXPECT_THAT_EXPECTED(applySelector(range(Arg1, Arg0), AST), Failed());
> ```
Thanks for the suggestion. Thoroughly reworked the tests along these lines. I
think the result is much clearer.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61774/new/
https://reviews.llvm.org/D61774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
ymandel updated this revision to Diff 200061.
ymandel marked 4 inline comments as done.
ymandel added a comment.
Restructured tests to simplify.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61774/new/
https://reviews.llvm.org/D61774
Files:
clang/include/clang/Tooling/Refactoring/RangeSelector.h
clang/lib/Tooling/Refactoring/CMakeLists.txt
clang/lib/Tooling/Refactoring/RangeSelector.cpp
clang/unittests/Tooling/CMakeLists.txt
clang/unittests/Tooling/RangeSelectorTest.cpp
Index: clang/unittests/Tooling/RangeSelectorTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/RangeSelectorTest.cpp
@@ -0,0 +1,497 @@
+//===- unittest/Tooling/RangeSelectorTest.cpp
+//---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/RangeSelector.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/FixIt.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace ast_matchers;
+
+namespace {
+using ::testing::AllOf;
+using ::testing::HasSubstr;
+using MatchResult = MatchFinder::MatchResult;
+using ::llvm::Expected;
+using ::llvm::Failed;
+using ::llvm::HasValue;
+using ::llvm::StringError;
+
+struct TestMatch {
+ // The AST unit from which `result` is built. We bundle it because it backs
+ // the result. Users are not expected to access it.
+ std::unique_ptr ASTUnit;
+ // The result to use in the test. References `ast_unit`.
+ MatchResult Result;
+};
+
+template TestMatch matchCode(StringRef Code, M Matcher) {
+ auto ASTUnit = buildASTFromCode(Code);
+ assert(ASTUnit != nullptr && "AST construction failed");
+
+ ASTContext = ASTUnit->getASTContext();
+ assert(!Context.getDiagnostics().hasErrorOccurred() && "Compilation error");
+
+ auto Matches = ast_matchers::match(Matcher, Context);
+ // We expect a single, exact match.
+ assert(Matches.size() != 0 && "no matches found");
+ assert(Matches.size() == 1 && "too many matches");
+
+ return TestMatch{std::move(ASTUnit), MatchResult(Matches[0], )};
+}
+
+// Applies \p Selector to \p Match and, on success, returns the selected source.
+Expected apply(RangeSelector Selector, const TestMatch ) {
+ Expected Range = Selector(Match.Result);
+ if (!Range)
+return Range.takeError();
+ return fixit::internal::getText(*Range, *Match.Result.Context);
+}
+
+// Applies \p Selector to a trivial match with only a single bound node with id
+// "bound_node_id". For use in testing unbound-node errors.
+Expected applyToTrivial(const RangeSelector ) {
+ // We need to bind the result to something, or the match will fail. Use a
+ // binding that is not used in the unbound node tests.
+ TestMatch Match =
+ matchCode("static int x = 0;", varDecl().bind("bound_node_id"));
+ return Selector(Match.Result);
+}
+
+// Matches the message expected for unbound-node failures.
+testing::Matcher withUnboundNodeMessage() {
+ return testing::Property(
+ ::getMessage,
+ AllOf(HasSubstr("unbound_id"), HasSubstr("not bound")));
+}
+
+// Applies \p Selector to code containing assorted node types, where the match
+// binds each one: a statement ("stmt"), a (non-member) ctor-initializer
+// ("init"), an expression ("expr") and a (nameless) declaration ("decl"). Used
+// to test failures caused by applying selectors to nodes of the wrong type.
+Expected applyToAssorted(RangeSelector Selector) {
+ StringRef Code = R"cc(
+ struct A {};
+ class F : public A {
+ public:
+F(int) {}
+ };
+ void g() { F f(1); }
+)cc";
+
+ auto Matcher =
+ compoundStmt(
+ hasDescendant(
+ cxxConstructExpr(
+ hasDeclaration(
+ decl(hasDescendant(cxxCtorInitializer(isBaseInitializer())
+ .bind("init")))
+ .bind("decl")))
+ .bind("expr")))
+ .bind("stmt");
+
+ return Selector(matchCode(Code, Matcher).Result);
+}
+
+// Matches the message expected for type-error failures.
+testing::Matcher withTypeErrorMessage(StringRef NodeID) {
+ return testing::Property(
+ ::getMessage,
+ AllOf(HasSubstr(NodeID), HasSubstr("mismatched type")));
+}
+
+TEST(RangeSelectorTest, UnboundNode) {
+ EXPECT_THAT_EXPECTED(applyToTrivial(node("unbound_id")),
+ Failed(withUnboundNodeMessage()));
+}
+
+TEST(RangeSelectorTest, RangeOp) {
+ StringRef Code = R"cc(
+int f(int x,
[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:29
+
+namespace range_selector {
+inline RangeSelector charRange(CharSourceRange R) {
ymandel wrote:
> ilya-biryukov wrote:
> > Maybe try putting it into the tooling namespace directly?
> > Or are there immediate and unfortunate conflicts with existing names?
> >
> >
> No conflicts. Was just being cautious.
I can see why a separate namespace would make sense, but since the `tooling`
namespace is not densely populated I hope we can get away with putting things
here directly and saving some typing on the use-sites.
Hope that does not backfire in the future, though.
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:53
+/// token of the relevant name, not including qualifiers.
+RangeSelector name(StringRef Id);
+
ymandel wrote:
> ilya-biryukov wrote:
> > NIT: this is super-specialized, but has a very generic name, therefore
> > might cause confusion. Maybe call it `ctorInitializerName`?
> It works with others as well, particularly NamedDecl. This is one place where
> typed node ids would be nice, b/c that would allow us to make this interface
> typed, like the matchers.
>
> I kept the name but tried to clarify the comments. WDYT?
Ah, sorry, misread the original comment. The name actually makes sense in that
case.
Am I correct to assume it will only select the final identifier of a qualified
name, but not the qualifier?
E.g. for `::foo::bar::baz`, it would only select `baz`, right?
What happens when we also have template args? E.g. for `::foo::bar::baz`,
it would only select `baz`, right?
Maybe add those examples to the documentation? It's something that's very easy
to get wrong.
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:57
+// arguments (all source between the call's parentheses).
+RangeSelector args(StringRef Id);
+
ymandel wrote:
> ilya-biryukov wrote:
> > Same thing, maybe rename to `callExprArgs`?
> > And other nodes too
> i'd like to keep these as lightweight as possible. Compromised on callArgs?
`callArgs` LG, thanks!
Comment at: clang/unittests/Tooling/RangeSelectorTest.cpp:39
+llvm::Optional matchAny(StringRef Code, M Matcher) {
+ auto AstUnit = buildASTFromCode(Code);
+ if (AstUnit == nullptr) {
NIT: use `ASTUnit` to match LLVM naming rules
Comment at: clang/unittests/Tooling/RangeSelectorTest.cpp:143
+ // Node-id specific version:
+ test(Matcher, range(Arg0, Arg1), Code, "3, 7");
+ // General version:
Consider restructuring the code to place assertions into the test function
itself.
This wildly improves locations reported in case of test failures and makes
tests easier to read.
I.e. having something like
```
auto AST = buildASTAndMatch(Code, Matcher);
EXPECT_EQ(applySelector(range(Arg0, Arg1), AST), "3, 7");
```
is arguably both easier to read and produces better location information on
failures than a function that runs the test.
Even though it's a bit more code.
Note that it's a bit more complicated if you need to deal with `Expected<>`
return types, but still possible:
```
EXPECT_THAT_EXPECTED(applySelector(range(Arg0, Arg1), AST), HasValue("3, 7"));
EXPECT_THAT_EXPECTED(applySelector(range(Arg1, Arg0), AST), Failed());
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61774/new/
https://reviews.llvm.org/D61774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
ymandel marked 3 inline comments as done and an inline comment as not done.
ymandel added a comment.
Thanks for the review. Added the tests and undid changes to SourceCode.
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:29
+
+namespace range_selector {
+inline RangeSelector charRange(CharSourceRange R) {
ilya-biryukov wrote:
> Maybe try putting it into the tooling namespace directly?
> Or are there immediate and unfortunate conflicts with existing names?
>
>
No conflicts. Was just being cautious.
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:36
+/// \returns the range corresponding to the identified node.
+RangeSelector node(StringRef Id);
+/// Variant of \c node() that identifies the node as a statement, for purposes
ilya-biryukov wrote:
> NIT: I believe LLVM naming rules would mandate to name it `ID`
here and throughout.
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:53
+/// token of the relevant name, not including qualifiers.
+RangeSelector name(StringRef Id);
+
ilya-biryukov wrote:
> NIT: this is super-specialized, but has a very generic name, therefore might
> cause confusion. Maybe call it `ctorInitializerName`?
It works with others as well, particularly NamedDecl. This is one place where
typed node ids would be nice, b/c that would allow us to make this interface
typed, like the matchers.
I kept the name but tried to clarify the comments. WDYT?
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:57
+// arguments (all source between the call's parentheses).
+RangeSelector args(StringRef Id);
+
ilya-biryukov wrote:
> Same thing, maybe rename to `callExprArgs`?
> And other nodes too
i'd like to keep these as lightweight as possible. Compromised on callArgs?
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:74
+/// `SourceManager::getExpansionRange`.
+RangeSelector contraction(RangeSelector S);
+} // namespace range_selector
ilya-biryukov wrote:
> NIT: maybe call it `expansion`?
>
> `contraction` is a new term, might be confusing to use.
>
>
Sigh. You're right. Its just that the existing terminology is confusing. that
said, anyone using this combinator is already doing advanced work, so
consistency is probably more important than clarity.
Comment at: clang/include/clang/Tooling/Refactoring/SourceCode.h:76
+
+SourceLocation findPreviousTokenStart(SourceLocation Start,
+ const SourceManager ,
ilya-biryukov wrote:
> Not sure this should be a public API. Maybe keep it private to the use-site?
Good point. Originally, we needed these in two different files -- but the range
selectors actually obviate that.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61774/new/
https://reviews.llvm.org/D61774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
ymandel updated this revision to Diff 199896.
ymandel marked 9 inline comments as done.
ymandel added a comment.
comment tweak
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61774/new/
https://reviews.llvm.org/D61774
Files:
clang/include/clang/Tooling/Refactoring/RangeSelector.h
clang/lib/Tooling/Refactoring/CMakeLists.txt
clang/lib/Tooling/Refactoring/RangeSelector.cpp
clang/unittests/Tooling/CMakeLists.txt
clang/unittests/Tooling/RangeSelectorTest.cpp
Index: clang/unittests/Tooling/RangeSelectorTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/RangeSelectorTest.cpp
@@ -0,0 +1,466 @@
+//===- unittest/Tooling/RangeSelectorTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/RangeSelector.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/FixIt.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace ast_matchers;
+
+namespace {
+using ::testing::AllOf;
+using ::testing::Eq;
+using ::testing::HasSubstr;
+using MatchResult = MatchFinder::MatchResult;
+using ::llvm::Expected;
+using ::llvm::Optional;
+
+struct TestMatch {
+ // The AST unit from which `result` is built. We bundle it because it backs
+ // the result. Users are not expected to access it.
+ std::unique_ptr AstUnit;
+ // The result to use in the test. References `ast_unit`.
+ MatchResult Result;
+};
+
+template
+llvm::Optional matchAny(StringRef Code, M Matcher) {
+ auto AstUnit = buildASTFromCode(Code);
+ if (AstUnit == nullptr) {
+ADD_FAILURE() << "AST construction failed";
+return llvm::None;
+ }
+ ASTContext = AstUnit->getASTContext();
+ if (Context.getDiagnostics().hasErrorOccurred()) {
+ADD_FAILURE() << "Compilation error";
+return llvm::None;
+ }
+ auto Matches = ast_matchers::match(Matcher, Context);
+ // We expect a single, exact match.
+ if (Matches.size() != 1) {
+ADD_FAILURE() << "Wrong number of matches: " << Matches.size();
+return llvm::None;
+ }
+ return TestMatch{std::move(AstUnit), MatchResult(Matches[0], )};
+}
+
+template
+void test(M Matcher, RangeSelector Selector, StringRef Code,
+ StringRef Selected) {
+ Optional Match = matchAny(Code, Matcher);
+ ASSERT_TRUE(Match);
+ MatchResult = Match->Result;
+ if (Expected Range = Selector(Result))
+EXPECT_EQ(fixit::internal::getText(*Range, *Result.Context), Selected);
+ else
+ADD_FAILURE() << llvm::toString(Range.takeError());
+}
+
+// Verifies that \c Selector fails when evaluated on \c Code.
+template
+void testError(M Matcher, const RangeSelector , StringRef Code,
+ ::testing::Matcher ErrorMatcher) {
+ Optional Match = matchAny(Code, Matcher);
+ ASSERT_TRUE(Match);
+ auto Range = Selector(Match->Result);
+ if (Range) {
+ADD_FAILURE() << "Expected failure but succeeded";
+return;
+ }
+ auto Err = llvm::handleErrors(Range.takeError(),
+[](const llvm::StringError ) {
+ EXPECT_THAT(Err.getMessage(), ErrorMatcher);
+});
+ if (Err)
+ADD_FAILURE() << "Unhandled error: " << llvm::toString(std::move(Err));
+}
+
+// Tests failures caused by references to unbound nodes. `UnboundID` is the ID
+// that will cause the failure.
+void testUnboundNodeError(const RangeSelector , llvm::StringRef UnboundID) {
+ // We need to bind the result to something, or the match will fail. Create an
+ // ID from UnboundID to ensure they are different.
+ std::string BoundID = (UnboundID + "_").str();
+ testError(varDecl().bind(BoundID), S, "static int x = 0;",
+AllOf(HasSubstr(UnboundID), HasSubstr("not bound")));
+}
+
+// Tests failures caused by operations applied to nodes of the wrong type. For
+// convenience, binds a statement to "stmt", a (non-member) ctor-initializer to
+// "init", an expression to "expr" and a (nameless) declaration to "decl".
+void testTypeError(const RangeSelector , llvm::StringRef NodeID) {
+ StringRef Code = R"cc(
+ struct A {};
+ class F : public A {
+ public:
+F(int) {}
+ };
+ void g() { F f(1); }
+)cc";
+
+ auto Matcher =
+ compoundStmt(
+ hasDescendant(
+ cxxConstructExpr(
+ hasDeclaration(
+ decl(hasDescendant(cxxCtorInitializer(isBaseInitializer())
+ .bind("init")))
+ .bind("decl")))
+
[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
ymandel updated this revision to Diff 199894.
ymandel marked 3 inline comments as done.
ymandel added a comment.
Add tests and back out changes to SourceCode.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61774/new/
https://reviews.llvm.org/D61774
Files:
clang/include/clang/Tooling/Refactoring/RangeSelector.h
clang/lib/Tooling/Refactoring/CMakeLists.txt
clang/lib/Tooling/Refactoring/RangeSelector.cpp
clang/unittests/Tooling/CMakeLists.txt
clang/unittests/Tooling/RangeSelectorTest.cpp
Index: clang/unittests/Tooling/RangeSelectorTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/RangeSelectorTest.cpp
@@ -0,0 +1,466 @@
+//===- unittest/Tooling/RangeSelectorTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/RangeSelector.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/FixIt.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace ast_matchers;
+
+namespace {
+using ::testing::AllOf;
+using ::testing::Eq;
+using ::testing::HasSubstr;
+using MatchResult = MatchFinder::MatchResult;
+using ::llvm::Expected;
+using ::llvm::Optional;
+
+struct TestMatch {
+ // The AST unit from which `result` is built. We bundle it because it backs
+ // the result. Users are not expected to access it.
+ std::unique_ptr AstUnit;
+ // The result to use in the test. References `ast_unit`.
+ MatchResult Result;
+};
+
+template
+llvm::Optional matchAny(StringRef Code, M Matcher) {
+ auto AstUnit = buildASTFromCode(Code);
+ if (AstUnit == nullptr) {
+ADD_FAILURE() << "AST construction failed";
+return llvm::None;
+ }
+ ASTContext = AstUnit->getASTContext();
+ if (Context.getDiagnostics().hasErrorOccurred()) {
+ADD_FAILURE() << "Compilation error";
+return llvm::None;
+ }
+ auto Matches = ast_matchers::match(Matcher, Context);
+ // We expect a single, exact match.
+ if (Matches.size() != 1) {
+ADD_FAILURE() << "Wrong number of matches: " << Matches.size();
+return llvm::None;
+ }
+ return TestMatch{std::move(AstUnit), MatchResult(Matches[0], )};
+}
+
+template
+void test(M Matcher, RangeSelector Selector, StringRef Code,
+ StringRef Selected) {
+ Optional Match = matchAny(Code, Matcher);
+ ASSERT_TRUE(Match);
+ MatchResult = Match->Result;
+ if (Expected Range = Selector(Result))
+EXPECT_EQ(fixit::internal::getText(*Range, *Result.Context), Selected);
+ else
+ADD_FAILURE() << llvm::toString(Range.takeError());
+}
+
+// Verifies that \c Selector fails when evaluated on \c Code.
+template
+void testError(M Matcher, const RangeSelector , StringRef Code,
+ ::testing::Matcher ErrorMatcher) {
+ Optional Match = matchAny(Code, Matcher);
+ ASSERT_TRUE(Match);
+ auto Range = Selector(Match->Result);
+ if (Range) {
+ADD_FAILURE() << "Expected failure but succeeded";
+return;
+ }
+ auto Err = llvm::handleErrors(Range.takeError(),
+[](const llvm::StringError ) {
+ EXPECT_THAT(Err.getMessage(), ErrorMatcher);
+});
+ if (Err)
+ADD_FAILURE() << "Unhandled error: " << llvm::toString(std::move(Err));
+}
+
+// Tests failures caused by references to unbound nodes. `UnboundID` is the ID
+// that will cause the failure.
+void testUnboundNodeError(const RangeSelector , llvm::StringRef UnboundID) {
+ // We need to bind the result to something, or the match will fail. Create an
+ // ID from UnboundID to ensure they are different.
+ std::string BoundID = (UnboundID + "_").str();
+ testError(varDecl().bind(BoundID), S, "static int x = 0;",
+AllOf(HasSubstr(UnboundID), HasSubstr("not bound")));
+}
+
+// Tests failures caused by operations applied to nodes of the wrong type. For
+// convenience, binds a statement to "stmt", a (non-member) ctor-initializer to
+// "init", an expression to "expr" and a (nameless) declaration to "decl".
+void testTypeError(const RangeSelector , llvm::StringRef NodeID) {
+ StringRef Code = R"cc(
+ struct A {};
+ class F : public A {
+ public:
+F(int) {}
+ };
+ void g() { F f(1); }
+)cc";
+
+ auto Matcher =
+ compoundStmt(
+ hasDescendant(
+ cxxConstructExpr(
+ hasDeclaration(
+ decl(hasDescendant(cxxCtorInitializer(isBaseInitializer())
+ .bind("init")))
+
[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
ilya-biryukov added a comment.
Thanks! Mostly NITs from me, the design looks nice! Would you mind adding some
tests before we land this?
The only major thing that I'd argue against is adding helper functions like
`findPreviousToken` to `SourceCode.h`.
It's fine to have them in the `.cpp` files if they make the implementation
simpler, but we want to design them more carefully if we put them in the public
API.
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:29
+
+namespace range_selector {
+inline RangeSelector charRange(CharSourceRange R) {
Maybe try putting it into the tooling namespace directly?
Or are there immediate and unfortunate conflicts with existing names?
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:35
+
+/// \returns the range corresponding to the identified node.
+RangeSelector node(StringRef Id);
NIT: maybe explain what Id is?
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:36
+/// \returns the range corresponding to the identified node.
+RangeSelector node(StringRef Id);
+/// Variant of \c node() that identifies the node as a statement, for purposes
NIT: I believe LLVM naming rules would mandate to name it `ID`
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:37
+RangeSelector node(StringRef Id);
+/// Variant of \c node() that identifies the node as a statement, for purposes
+/// of deciding whether to include any trailing semicolon in the selected
range.
Maybe a shorter description could suffice?
```
Selects a node and a trailing semicolon. Useful for selecting expression
statements.
```
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:43
+/// statement.
+RangeSelector sNode(StringRef Id);
+
NIT: maybe name it `statement`?
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:46
+/// Convenience version of \c range where end points are nodes.
+RangeSelector nodeRange(StringRef BeginId, StringRef EndId);
+
Could this be an overload with the same name?
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:53
+/// token of the relevant name, not including qualifiers.
+RangeSelector name(StringRef Id);
+
NIT: this is super-specialized, but has a very generic name, therefore might
cause confusion. Maybe call it `ctorInitializerName`?
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:57
+// arguments (all source between the call's parentheses).
+RangeSelector args(StringRef Id);
+
Same thing, maybe rename to `callExprArgs`?
And other nodes too
Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:74
+/// `SourceManager::getExpansionRange`.
+RangeSelector contraction(RangeSelector S);
+} // namespace range_selector
NIT: maybe call it `expansion`?
`contraction` is a new term, might be confusing to use.
Comment at: clang/include/clang/Tooling/Refactoring/SourceCode.h:76
+
+SourceLocation findPreviousTokenStart(SourceLocation Start,
+ const SourceManager ,
Not sure this should be a public API. Maybe keep it private to the use-site?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61774/new/
https://reviews.llvm.org/D61774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
ymandel created this revision.
ymandel added a reviewer: ilya-biryukov.
Herald added a subscriber: mgorny.
Herald added a project: clang.
NOTE: This is a preliminary revision for discussion; tests have not yet been
provided.
The RangeSelector library defines a combinator language for specifying source
ranges based on bound id for AST nodes. The combinator approach follows the
design of the AST matchers. The RangeSelectors defined here will be used in
both RewriteRule, for specifying source affected by edit, and in Stencil for
specifying source to use constructively in a replacement.
This revision extends the SourceCode library with utility functions needed by
RangeSelector. Some of them come are copied from clang-tidy/utils/LexUtils,
since clang/Tooling can't depend on clang-tidy libraries.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D61774
Files:
clang/include/clang/Tooling/Refactoring/RangeSelector.h
clang/include/clang/Tooling/Refactoring/SourceCode.h
clang/lib/Tooling/Refactoring/CMakeLists.txt
clang/lib/Tooling/Refactoring/RangeSelector.cpp
clang/lib/Tooling/Refactoring/SourceCode.cpp
Index: clang/lib/Tooling/Refactoring/SourceCode.cpp
===
--- clang/lib/Tooling/Refactoring/SourceCode.cpp
+++ clang/lib/Tooling/Refactoring/SourceCode.cpp
@@ -14,18 +14,58 @@
using namespace clang;
-StringRef clang::tooling::getText(CharSourceRange Range,
- const ASTContext ) {
+StringRef tooling::getText(CharSourceRange Range, const ASTContext ) {
return Lexer::getSourceText(Range, Context.getSourceManager(),
Context.getLangOpts());
}
-CharSourceRange clang::tooling::maybeExtendRange(CharSourceRange Range,
- tok::TokenKind Next,
- ASTContext ) {
+CharSourceRange tooling::maybeExtendRange(CharSourceRange Range,
+ tok::TokenKind Next,
+ ASTContext ) {
Optional Tok = Lexer::findNextToken(
Range.getEnd(), Context.getSourceManager(), Context.getLangOpts());
if (!Tok || !Tok->is(Next))
return Range;
return CharSourceRange::getTokenRange(Range.getBegin(), Tok->getLocation());
}
+
+SourceLocation tooling::findPreviousTokenStart(SourceLocation Start,
+ const SourceManager ,
+ const LangOptions ) {
+ if (Start.isInvalid() || Start.isMacroID())
+return SourceLocation();
+
+ SourceLocation BeforeStart = Start.getLocWithOffset(-1);
+ if (BeforeStart.isInvalid() || BeforeStart.isMacroID())
+return SourceLocation();
+
+ return Lexer::GetBeginningOfToken(BeforeStart, SM, LangOpts);
+}
+
+SourceLocation tooling::findPreviousTokenKind(SourceLocation Start,
+ const SourceManager ,
+ const LangOptions ,
+ tok::TokenKind TK) {
+ while (true) {
+SourceLocation L = findPreviousTokenStart(Start, SM, LangOpts);
+if (L.isInvalid() || L.isMacroID())
+ return SourceLocation();
+
+Token T;
+if (Lexer::getRawToken(L, T, SM, LangOpts, /*IgnoreWhiteSpace=*/true))
+ return SourceLocation();
+
+if (T.is(TK))
+ return T.getLocation();
+
+Start = L;
+ }
+}
+
+SourceLocation tooling::findOpenParen(const CallExpr ,
+ const SourceManager ,
+ const LangOptions ) {
+ SourceLocation EndLoc =
+ E.getNumArgs() == 0 ? E.getRParenLoc() : E.getArg(0)->getBeginLoc();
+ return findPreviousTokenKind(EndLoc, SM, LangOpts, tok::TokenKind::l_paren);
+}
Index: clang/lib/Tooling/Refactoring/RangeSelector.cpp
===
--- /dev/null
+++ clang/lib/Tooling/Refactoring/RangeSelector.cpp
@@ -0,0 +1,219 @@
+//===--- Transformer.cpp - Transformer library implementation ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/RangeSelector.h"
+#include "clang/AST/Expr.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Refactoring/SourceCode.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/Error.h"
+#include
+#include
+#include
+
+using namespace clang;
+using namespace tooling;
+
+using ast_matchers::MatchFinder;
+using ast_type_traits::ASTNodeKind;
+using ast_type_traits::DynTypedNode;
+using
[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.
ymandel added a comment. Ilya, this revision follows up from our discussion on the doc. It adds some more selectors. If this change is to big, I'm happy to split it up (e.g. moving the changes to SourceCode to a separate revision and/or splitting up the RangeSelector changes). Also, I have the diff updating Transformer ready if you want to see that. I can post a preview or created a stacked revision. Let me know... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61774/new/ https://reviews.llvm.org/D61774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
