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
