[PATCH v2 2/6] Create two helpers for running the typo-correction tree transform.

[David Blaikie]

On Fri, Oct 31, 2014 at 12:52 PM, Kaelyn Takata <rikka at google.com> 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 <dblaikie at gmail.com>
> 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 <rikka at google.com> 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
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141031/12d23df6/attachment.html>