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,
t;>>>>
>>>>>
>>>>>
>>>>> *From:* Yitzhak Mandelbaum [mailto:yitzh...@google.com]
>>>>> *Sent:* Wednesday, May 22, 2019 1:37 PM
>>>>> *To:* reviews+d61774+public+f458bb6144ae8...@reviews.llvm.org
>>>>&
gt;>> *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...
o:* 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 ;
>>>
e8...@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]
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] D6
; 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
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,
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
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
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
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
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:
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
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:
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
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:
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:
>
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 {
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:
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
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
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
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
24 matches
Mail list logo
