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 <yitzh...@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 <yitzh...@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 <ibiryu...@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.pen...@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:yitzh...@google.com] >>>> *Sent:* Wednesday, May 22, 2019 1:37 PM >>>> *To:* reviews+d61774+public+f458bb6144ae8...@reviews.llvm.org >>>> *Cc:* Ilya Biryukov <ibiryu...@google.com>; Penzin, Petr < >>>> petr.pen...@intel.com>; llvm-comm...@lists.llvm.org; Michał Górny < >>>> mgo...@gentoo.org>; cfe-commits <cfe-commits@lists.llvm.org>; Theko >>>> Lekena <mlek...@skidmore.edu>; Nicolas Lesser <blitzrak...@gmail.com>; >>>> Han Shen <shen...@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 < >>>> 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<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 >>> >>
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
