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

[Yitzhak Mandelbaum via cfe-commits]

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/6f887601/attachment.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/6f887601/attachment.bin>