[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.

[Yitzhak Mandelbaum via cfe-commits]

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 <yitzhakm at google.com>
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 <yitzhakm at google.com>
> 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 <ibiryukov at google.com>
>> 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 <petr.penzin at intel.com>
>>> 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:yitzhakm at google.com]
>>>> *Sent:* Wednesday, May 22, 2019 1:37 PM
>>>> *To:* reviews+D61774+public+f458bb6144ae8e78 at reviews.llvm.org
>>>> *Cc:* Ilya Biryukov <ibiryukov at google.com>; Penzin, Petr <
>>>> petr.penzin at intel.com>; llvm-commits at lists.llvm.org; Michał Górny <
>>>> mgorny at gentoo.org>; cfe-commits <cfe-commits at lists.llvm.org>; Theko
>>>> Lekena <mlekena at skidmore.edu>; Nicolas Lesser <blitzrakete at gmail.com>;
>>>> Han Shen <shenhan at google.com>
>>>> *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 <
>>>> reviews at 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<CompoundStmt, getStatementsRange>(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
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190523/ddc3722e/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4847 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190523/ddc3722e/attachment-0001.bin>