On Fri, Oct 31, 2014 at 12:52 PM, Kaelyn Takata <[email protected]> wrote:
> I believe the function_ref could also be changed to std::function now that > it is allowed in LLVM, unless I'm overlooking something? > It could, but that would add memory allocation overhead, etc - function_ref is more efficient in the cases where you only need to use it within a single full expression (as is the case when the code looks like yours "auto Result = TransformTypos(*this, std::move(Filter)).Transform(E);" - the TransformTypos object is destroyed by the end of the full expression) I'm not sure what the right answer is here - the code as-is is correct, just a little subtle (as Twine is). I wouldn't object to std::function, but I don't object to the code as-is either - just mentioning it and wondering aloud if there might be a safer (& possibly as efficient) option. - David > > On Fri, Oct 31, 2014 at 11:15 AM, David Blaikie <[email protected]> > wrote: > >> Looks like the standard idiom is to pass function_ref by value, rather >> than rvalue reference. >> >> factoring out CorrectDelayedTyposInExpr is the sort of thing you can >> commit without pre-commit review (it's not a step backwards in readability, >> etc, for the existing code even if it's never reused for more callers - so >> it doesn't need to wait for the extra use-cases to justify it) & possibly >> leave off the filtering until /that's/ needed. >> >> (the lifetime semantics of that function_ref will be a bit tricky if a >> TransformTypos is ever created as a named variable) >> >> >> >> >> >> >> On Wed, Oct 29, 2014 at 12:49 PM, Kaelyn Takata <[email protected]> wrote: >> >>> >>> One takes an Expr* and the other is a simple wrapper that takes an >>> ExprResult instead, and handles checking whether the ExprResult is >>> invalid. >>> >>> Additionally, allow an optional callback that is run on the full result >>> of the tree transform, for filtering potential corrections based on the >>> characteristics of the resulting expression once all of the typos have >>> been replaced. >>> --- >>> include/clang/Sema/Sema.h | 13 +++++++++++++ >>> lib/Sema/SemaExprCXX.cpp | 44 >>> ++++++++++++++++++++++++++++---------------- >>> 2 files changed, 41 insertions(+), 16 deletions(-) >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> >>> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
